sim: add default cases to two switches in sim-options.c

Message ID 20210502141724.442932-1-simon.marchi@polymtl.ca
State New
Headers show
Series
  • sim: add default cases to two switches in sim-options.c
Related show

Commit Message

Weimin Pan via Gdb-patches May 2, 2021, 2:17 p.m.
This is the next compilation error I hit when I build all targets with
Clang:

    /home/simark/src/binutils-gdb/sim/aarch64/../common/sim-options.c:234:12: error: no case matching constant switch condition '0' [-Werror]                                                                    switch (WITH_ENVIRONMENT)
                      ^~~~~~~~~~~~~~~~                                                                                                                                                                 ./config.h:215:26: note: expanded from macro 'WITH_ENVIRONMENT'
    #define WITH_ENVIRONMENT ALL_ENVIRONMENT                                                                                                                                                                                    ^~~~~~~~~~~~~~~
    /home/simark/src/binutils-gdb/sim/aarch64/../common/sim-options.c:276:15: error: no case matching constant switch condition '0' [-Werror]                                                                switch (WITH_ALIGNMENT)
                  ^~~~~~~~~~~~~~                                                                                                                                                                       /home/simark/src/binutils-gdb/sim/aarch64/../common/sim-config.h:220:24: note: expanded from macro 'WITH_ALIGNMENT'
    #define WITH_ALIGNMENT 0
                           ^

This is a little bit special because these are switches on compile-time
value.  But regardless, the idea is that we logically can't reach the
switches if WITH_ENVIRONMENT == 0 or WITH_ALIGNMENT == 0, so the code is
correct.

In addition to getting rid of the compiler warning, adding default cases
to these switches ensure that if we do get in an unexpected situation,
it is caught.  In GDB, I'd use gdb_assert_not_reached, I don't know if
there is something similar in sim so I went with abort.

sim/common/ChangeLog:

	* sim-options.c (standard_option_handler): Add default cases to
	switches.

Change-Id: Ie237d67a201caa6b72de0d17cc815193417156b6
---
 sim/common/sim-options.c | 2 ++
 1 file changed, 2 insertions(+)

-- 
2.30.1

Comments

Weimin Pan via Gdb-patches May 2, 2021, 3:01 p.m. | #1
On 02 May 2021 10:17, Simon Marchi via Gdb-patches wrote:
> This is the next compilation error I hit when I build all targets with

> Clang:

> 

>     /home/simark/src/binutils-gdb/sim/aarch64/../common/sim-options.c:234:12: error: no case matching constant switch condition '0' [-Werror]                                                                    switch (WITH_ENVIRONMENT)

>                       ^~~~~~~~~~~~~~~~                                                                                                                                                                 ./config.h:215:26: note: expanded from macro 'WITH_ENVIRONMENT'

>     #define WITH_ENVIRONMENT ALL_ENVIRONMENT                                                                                                                                                                                    ^~~~~~~~~~~~~~~

>     /home/simark/src/binutils-gdb/sim/aarch64/../common/sim-options.c:276:15: error: no case matching constant switch condition '0' [-Werror]                                                                switch (WITH_ALIGNMENT)

>                   ^~~~~~~~~~~~~~                                                                                                                                                                       /home/simark/src/binutils-gdb/sim/aarch64/../common/sim-config.h:220:24: note: expanded from macro 'WITH_ALIGNMENT'

>     #define WITH_ALIGNMENT 0

>                            ^

> 

> This is a little bit special because these are switches on compile-time

> value.  But regardless, the idea is that we logically can't reach the

> switches if WITH_ENVIRONMENT == 0 or WITH_ALIGNMENT == 0, so the code is

> correct.


lgtm, thanks

> In addition to getting rid of the compiler warning, adding default cases

> to these switches ensure that if we do get in an unexpected situation,

> it is caught.  In GDB, I'd use gdb_assert_not_reached, I don't know if

> there is something similar in sim so I went with abort.


abort() is what we have atm for things like this.  if it were due to a user
error (passing wrong values), we'd do something else, but this is a compile
time assertion.
-mike
Weimin Pan via Gdb-patches May 2, 2021, 3:06 p.m. | #2
On 2021-05-02 11:01 a.m., Mike Frysinger wrote:
> On 02 May 2021 10:17, Simon Marchi via Gdb-patches wrote:

>> This is the next compilation error I hit when I build all targets with

>> Clang:

>>

>>     /home/simark/src/binutils-gdb/sim/aarch64/../common/sim-options.c:234:12: error: no case matching constant switch condition '0' [-Werror]                                                                    switch (WITH_ENVIRONMENT)

>>                       ^~~~~~~~~~~~~~~~                                                                                                                                                                 ./config.h:215:26: note: expanded from macro 'WITH_ENVIRONMENT'

>>     #define WITH_ENVIRONMENT ALL_ENVIRONMENT                                                                                                                                                                                    ^~~~~~~~~~~~~~~

>>     /home/simark/src/binutils-gdb/sim/aarch64/../common/sim-options.c:276:15: error: no case matching constant switch condition '0' [-Werror]                                                                switch (WITH_ALIGNMENT)

>>                   ^~~~~~~~~~~~~~                                                                                                                                                                       /home/simark/src/binutils-gdb/sim/aarch64/../common/sim-config.h:220:24: note: expanded from macro 'WITH_ALIGNMENT'

>>     #define WITH_ALIGNMENT 0

>>                            ^

>>

>> This is a little bit special because these are switches on compile-time

>> value.  But regardless, the idea is that we logically can't reach the

>> switches if WITH_ENVIRONMENT == 0 or WITH_ALIGNMENT == 0, so the code is

>> correct.

> 

> lgtm, thanks

> 

>> In addition to getting rid of the compiler warning, adding default cases

>> to these switches ensure that if we do get in an unexpected situation,

>> it is caught.  In GDB, I'd use gdb_assert_not_reached, I don't know if

>> there is something similar in sim so I went with abort.

> 

> abort() is what we have atm for things like this.  if it were due to a user

> error (passing wrong values), we'd do something else, but this is a compile

> time assertion.


Indeed, I agree.  Pushed, thanks!

Simon

Patch

diff --git a/sim/common/sim-options.c b/sim/common/sim-options.c
index 1522cac9379f..1efb21fc62b8 100644
--- a/sim/common/sim-options.c
+++ b/sim/common/sim-options.c
@@ -236,6 +236,7 @@  standard_option_handler (SIM_DESC sd, sim_cpu *cpu, int opt,
 	    case USER_ENVIRONMENT: type = "user"; break;
 	    case VIRTUAL_ENVIRONMENT: type = "virtual"; break;
 	    case OPERATING_ENVIRONMENT: type = "operating"; break;
+	    default: abort ();
 	    }
 	  sim_io_eprintf (sd, "Simulator compiled for the %s environment only.\n",
 			  type);
@@ -284,6 +285,7 @@  standard_option_handler (SIM_DESC sd, sim_cpu *cpu, int opt,
 	case FORCED_ALIGNMENT:
 	  sim_io_eprintf (sd, "Simulator compiled for forced alignment only.\n");
 	  break;
+	default: abort ();
 	}
       return SIM_RC_FAIL;