On Sat, Dec 17, 2016 at 02:55:15PM -0700, Martin Sebor wrote:
> I agree that these warnings should probably not be issued, though
> it's interesting to see where they come from. The calls are in
> the code emitted by GCC, are reachable, and end up taking place
> with the right Ubsan runtime recovery options. It turns out that
> Ubsan transforms calls to nonnull functions into conditional
> branches testing the argument for null, like so:
>
> if (s == 0)
> __builtin___ubsan_handle_nonnull_arg();
> n = strlen (s);
>
> and GCC then transforms those into
>
> if (s == 0)
> {
> __builtin___ubsan_handle_nonnull_arg();
> n = strlen (NULL);
> }
>
> When the ubsan_handle_nonnull_arg function returns to the caller
> the call to strlen(NULL) is made.
>
> This doesn't happen when -fno-sanitize-recover=undefined is used
> when compiling the file because then Ubsan inserts calls to
> __builtin___ubsan_handle_nonnull_arg_abort instead which is declared
> noreturn.
>
> It would be easy to suppress these warnings when -fsantize=undefined
> is used. Distinguishing the Ubsan-inserted calls from those in the
> source code would be more challenging. Implementing the warning as
> a pass that runs before the Ubsan pass gets around the problem.
The ubsan pass runs before IPA, so not sure how do you want to do that
(and it is needed to run it early).
One question is if we should perform path isolation in this case at all,
since the branches to __builtin___ubsan_handle_nonnull_arg are with
PROB_VERY_UNLIKELY, so the question is if we should be growing the
paths which is really cold. It has some small benefit (removing one
cheap comparison from the hot path), but the grows cold block.
Another thing is that what the compiler does can very well just happen
in some generic function that is called by the function that calls these
strlen/strcpy etc. functions (fns with nonnull attribute).
BTW, in the testcase from the Linux kernel it is also path isolation
for error recovery path, something that ought to be predicted unlikely
(though, probably not very unlikely unlike this case), so the question is
if we want to isolate that or not too.
Jakub
