On 2/22/20, Kyle Evans <[email protected]> wrote:
> On Sat, Feb 22, 2020 at 10:25 AM Ian Lepore <[email protected]> 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>
_______________________________________________
[email protected] mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "[email protected]"
