Re: [PATCH RFA] ubsan: do return check with -fsanitize=unreachable

[Jason Merrill <jason@redhat.com>]

On 6/29/22 13:26, Jakub Jelinek wrote:
> On Wed, Jun 29, 2022 at 12:42:04PM -0400, Jason Merrill wrote:
>>> The usual case is that people just use -fsanitize=undefined
>>> and get both return and unreachable sanitization, for fall through
>>> into end of functions returning non-void done through return sanitization.
>>>
>>> In the rare case people use something different like
>>> -fsanitize=undefined -fno-sanitize=return
>>> or
>>> -fsanitize=unreachable
>>> etc., they presumably don't want the fall through from end of function
>>> diagnosed at runtime.
>>
>> I disagree with this assumption for the second case; it seems much more
>> likely to me that the user just wasn't thinking about needing to also
>> mention return.  Missing return is a logical subset of unreachable; if we
>> sanitize all the other __builtin_unreachable introduced by the compiler, why
>> in the world would we want to leave out this one that is such a frequent
>> error?
> 
> UBSan was initially implemented in LLVM and our -fsanitize= options try to
> match where possible what they do.
> And their behavior is too that return and unreachable are separate
> sanitizers, fallthrough from function return is sanitized only for the
> former, they apparently at -O0 implement something like -funreachable-traps
> (but not at -Og) and emit a trap there (regardless of
> -fsanitize=unreachable), for -O1 and higher they act as if non-sanitized
> __builtin_unreachable () is in there regardless of -fsanitize=unreachable.

Hmm, does clang only sanitize explicit calls to __builtin_unreachable?

> It would be strange to diverge from this without a strong reason.
> The fact that we use __builtin_unreachable for the function ends is just our
> implementation detail and when we'd report that to users, they'd just be
> confused on what's going on.  With -fsanitize=return they are told what
> happens.
> 
>> Full -fsanitize=undefined is much higher overhead than just
>> -fsanitize=unreachable, which introduces no extra checks.  And adding return
>> checking to unreachable is essentially zero overhead.  I can't imagine any
>> reason to want to check unreachable points EXCEPT for missing return.
> 
> -fsanitize=unreachable isn't zero overhead, it will force evaluation of all
> the conditionals guarding it and preparation of arguments for it etc.