This is the mail archive of the
mailing list for the GCC project.
Re: Request to merge Undefined Behavior Sanitizer in (take 2)
- From: Marek Polacek <polacek at redhat dot com>
- To: Jason Merrill <jason at redhat dot com>
- Cc: GCC Patches <gcc-patches at gcc dot gnu dot org>, Jakub Jelinek <jakub at redhat dot com>, Jeff Law <law at redhat dot com>, "Joseph S. Myers" <joseph at codesourcery dot com>
- Date: Tue, 6 Aug 2013 16:42:07 +0200
- Subject: Re: Request to merge Undefined Behavior Sanitizer in (take 2)
- References: <20130725153227 dot GC32538 at redhat dot com> <51F186E9 dot 9090404 at redhat dot com> <20130731173321 dot GT17022 at redhat dot com> <51F95CF7 dot 8020605 at redhat dot com>
On Wed, Jul 31, 2013 at 02:52:39PM -0400, Jason Merrill wrote:
> Here, the C++ compiler is wrong to fold away the division by zero,
> but given that bug the folding ought to also eliminate the call to
> the sanitize function. Seems like you should attach the call to the
> questionable expression itself.
Hm, actually, we can't easily fold the call to the sanitize function
away, I'm afraid, if we want to do it for the 'case <something>'
case. When we hit the DIV_EXPR in 'case 0 * (1 / 0)',
the ubsan_instrument_division gets 1 as a first argument and 0 as
a second argument, but due to fold_builds in the
ubsan_instrument_division, we replace the case value with just the call
to the __builtin___ubsan_handle_divrem_overflow. I think, what we
could do, is to tweak verify_constant like this:
@@ -6938,6 +6938,14 @@ static bool
verify_constant (tree t, bool allow_non_constant, bool *non_constant_p,
+ /* This is to handle e.g. the goofy 'case 0 * (1 / 0)' case. */
+ if (flag_sanitize & SANITIZE_UNDEFINED
+ && TREE_CODE (t) == CALL_EXPR
+ && is_ubsan_builtin (t))
+ error ("undefined behavior occured");
+ return *non_constant_p;
if (!*non_constant_p && !reduced_constant_expression_p (t))
The logic behind this is that when we can't easily insert the
ubsan builtin call itself (so that it would get called unconditionally
at the runtime), at least error out; at that point the behavior
is certainly undefined thus we're free to do whatever. I'd say it's better
than to e.g. insert 0 instead of the expression and carry on.
Also, this could be useful in other situations (at places, where
we can't insert the ubsan builtin call, but we *know* that the
behavior is undefined). Yes, the behavior
can be undefined only at runtime, but, this would happen only when
sanitizing and in that case is desirable to report even that.
The is_ubsan_builtin fn is not yet implemented, but you get the idea.