On Wed, Jul 22, 2015 at 07:26:22PM +0200, Marek Polacek wrote:
> In this testcase we were generating an uninitialized variable when doing
> -fsanitize=shift,bounds sanitization.  The shift instrumentation is done
> first; after that, the IR looks like
> 
>   res[i] = (m > 31) ? __ubsan (... tab[i] ...) ? 0, ... tab[i] ...;
> 
> where tab[i] are identical.  That means that when we instrument the first
> tab[i] (we shouldn't do this I suppose), the second tab[i] is changed as
> well as they're shared.  But that doesn't play well with SAVE_EXPRs, because
> SAVE_EXPR <i> would only be initialized on one path.  Fixed by unsharing
> the operands when constructing the ubsan check.  The .gimple diff is in
> essence just
> 
> +  i.2 = i;
> +  UBSAN_BOUNDS (0B, i.2, 21);
> -  UBSAN_BOUNDS (0B, i.1, 21);
> 
> (Merely not instrumenting __ubsan_* wouldn't help exactly because of the
> sharing.)
> 
> Bootstrapped/regtested on x86_64-linux, ok for trunk?

That is strange.  I'd have expected you'd want to unshare if you want to use
the same operand multiple times in the same function, instead of unsharing
it just in case it is shared with something different.

So isn't the bug instead that the UBSAN_BOUNDS generating code doesn't
unshare?  Of course, these two functions use op0 and/or op1 sometimes
multiple times too and thus they might want to unshare too, but I'd have
expected in a different spot.

        Jakub

Reply via email to