This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] fix powerpc64le bootstrap failure caused by r243661 (PR 78817)
On 12/19/2016 02:42 AM, Jakub Jelinek wrote:
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).
For the string built-ins (though perhaps not for user-defined
nonnull functions), I wonder if it would make sense to have Ubsan
emit an if (p == null) ubsan_handle_nonnull; else strfunc(p); to
let the program continue instead of almost certainly crash right
after the handler returns. That would solve the warning problem
and also achieve the goal of allowing the handler to return and
uncovering subsequent instances of undefined behavior.
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.
I don't expect to have the cycles to do significant work on this
before stage 3 ends. For GCC 7, to avoid the Ubsan warnings,
the late -Wnonnull warnings could simply be suppressed when
-fsanitize=undefined is used. It wouldn't be ideal but it would
be no worse than what GCC does today. In 8 the warning could be
made smarter to avoid the problem in general.
Martin