Date: Sat, 22 Feb 2020 17:44:04 +0100 From: Mateusz Guzik <mjguzik@gmail.com> To: Kyle Evans <kevans@freebsd.org> Cc: Ian Lepore <ian@freebsd.org>, svn-src-head <svn-src-head@freebsd.org>, svn-src-all <svn-src-all@freebsd.org>, src-committers <src-committers@freebsd.org> Subject: Re: svn commit: r358248 - head/sys/vm Message-ID: <CAGudoHHg5R0zOc7RYge36roz%2B3C_sSRZcsyXC55W0yAyQpuuBA@mail.gmail.com> In-Reply-To: <CACNAnaHZnrqRv9J-B7XRCc7eN7Hkccf1R-7e36LiAXvZR4etVw@mail.gmail.com> References: <202002221620.01MGK46E072303@repo.freebsd.org> <a3b2125de10d214d6e422d183f1fdc7e0e38e014.camel@freebsd.org> <CACNAnaHZnrqRv9J-B7XRCc7eN7Hkccf1R-7e36LiAXvZR4etVw@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On 2/22/20, Kyle Evans <kevans@freebsd.org> wrote: > On Sat, Feb 22, 2020 at 10:25 AM Ian Lepore <ian@freebsd.org> wrote: >> >> On Sat, 2020-02-22 at 16:20 +0000, Kyle Evans wrote: >> > Author: kevans >> > Date: Sat Feb 22 16:20:04 2020 >> > New Revision: 358248 >> > URL: https://svnweb.freebsd.org/changeset/base/358248 >> > >> > Log: >> > vm_radix: prefer __builtin_unreachable() to an unreachable panic() >> > >> > This provides the needed hint to GCC and offers an annotation for >> > readers to >> > observe that it's in-fact impossible to hit this point. We'll get hit >> > with a >> > a -Wswitch error if the enum applicable to the switch above were to >> > get >> > expanded without the new value(s) being handled. >> > >> > Modified: >> > head/sys/vm/vm_radix.c >> > >> > Modified: head/sys/vm/vm_radix.c >> > ============================================================================== >> > --- head/sys/vm/vm_radix.c Sat Feb 22 13:23:27 2020 (r358247) >> > +++ head/sys/vm/vm_radix.c Sat Feb 22 16:20:04 2020 (r358248) >> > @@ -208,8 +208,7 @@ vm_radix_node_load(smrnode_t *p, enum >> > vm_radix_access >> > case SMR: >> > return (smr_entered_load(p, vm_radix_smr)); >> > } >> > - /* This is unreachable, silence gcc. */ >> > - panic("vm_radix_node_get: Unknown access type"); >> > + __unreachable(); >> > } >> > >> > static __inline void >> >> What does __unreachable() do if the code ever becomes reachable? Like >> if a new enum value is added and this switch doesn't get updated? >> > > __unreachable doesn't help here, but the compiler will error out on > the switch() if all enum values aren't addressed and there's no > default: case. > > IMO, compilers could/should become smart enough to error if there's an > explicit __builtin_unreachable() and they can trivially determine that > all paths will terminate before this, independent of -Werror=switch*. > _______________________________________________ I think this is way too iffy, check this program: #include <stdio.h> int main(void) { __builtin_unreachable(); printf("test\n"); } Neither clang nor gcc warn about this and both stop code generation past the statement. Thus I think for production kernels __unreachable can expand to to the builtin, but for debug it should be a panic with func/file/line. This would work fine in terms of analysis since panic is noreturn or so. -- Mateusz Guzik <mjguzik gmail.com>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAGudoHHg5R0zOc7RYge36roz%2B3C_sSRZcsyXC55W0yAyQpuuBA>