Disable -Wreturn-type on GCC.

[jhb]

Summary

GCC is more pedantic than clang about warning when a function doesn't
handle undefined enum values (see GCC bug 87950). Clang's warning
gives a more pragmatic coverage and should find any real bugs, so
disable the warning for GCC rather than adding __unreachable
annotations to appease GCC.

[jhb]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87950 is the GCC bug report

Right now on amd64 there are two places (one in tests/sys/fusefs and one in sys/dev/mgb) that have to be patched with __unreachable. If disabling the warning entirely is considered overboard I could just fix the two places instead, but most devs don't compile with GCC so it's likely to break in the future I think. (I finally have amd64 building with GCC 9 with various patches BTW).

[jhb]

Any thoughts on this? The kind of changes we would have to make if we keep this warning are:

commit ac3f64734580fea32de72d40a384289f6968bd9d
Author: John Baldwin <[e-mail]>
Date:   Wed Feb 2 08:36:26 2022 -0800

    tests/sys/fs/fusefs: Add an __unreachable to fuse_op_from_mutator.
    
    Even though the switch handles all of the valid values for the enum,
    GCC raises a -Wreturn-type warning as it believes the function can
    fail to return.

diff --git a/tests/sys/fs/fusefs/last_local_modify.cc b/tests/sys/fs/fusefs/last_local_modify.cc
index 9826296c80c3..dc34648c57d7 100644
--- a/tests/sys/fs/fusefs/last_local_modify.cc
+++ b/tests/sys/fs/fusefs/last_local_modify.cc
@@ -95,6 +95,7 @@ uint32_t fuse_op_from_mutator(enum Mutator mutator) {
        case VOP_WRITE:
                return(FUSE_WRITE);
        }
+       __unreachable();
 }
 
 class LastLocalModify: public FuseTest, public WithParamInterface<const char*> {

commit 32f7826db86eca72d4163305953a0f2ad3bd6220
Author: John Baldwin <[e-mail]>
Date:   Wed Feb 2 10:30:30 2022 -0800

    mgb: Add __unreachable to mgb_fct_control.
    
    Even though the switch handles all of the valid values for the enum,
    GCC raises a -Wreturn-type warning as it believes the function can
    fail to return.

diff --git a/sys/dev/mgb/if_mgb.c b/sys/dev/mgb/if_mgb.c
index cde7d60927bb..888b89361dec 100644
--- a/sys/dev/mgb/if_mgb.c
+++ b/sys/dev/mgb/if_mgb.c
@@ -1429,6 +1429,7 @@ mgb_fct_control(struct mgb_softc *sc, int reg, int channel,
                CSR_WRITE_REG(sc, reg, MGB_FCT_DSBL(channel));
                return (mgb_wait_for_bits(sc, reg, 0, MGB_FCT_ENBL(channel)));
        }
+       __unreachable();
 }
 
 static int

(Those are the only current ones in the tree.)

[emaste]

I think this is fine. I was on the fence between this or annotating the limited # of places, but your point about most developers not compiling with GCC and breaking again pushed me to disabling the warning.

[imp]

I'd not annotate, and just push this warning in. clang will catch this error and the annotations may get in the way if you added new enums that those functions don't handle. The annotations might hide a future legit warning.