Re: [PATCH] Don't always instrument shifts (PR sanitizer/58413)

[Marek Polacek <polacek at redhat dot com>]

On Mon, Sep 16, 2013 at 03:59:12PM +0000, Joseph S. Myers wrote:
> On Mon, 16 Sep 2013, Marek Polacek wrote:
> 
> > On Fri, Sep 13, 2013 at 07:18:24PM +0000, Joseph S. Myers wrote:
> > > On Fri, 13 Sep 2013, Marek Polacek wrote:
> > > 
> > > > This is kind of fugly, but don't have anything better at the moment.
> > > > 2013-09-13  Marek Polacek  <polacek@redhat.com>
> > > > 
> > > > 	PR sanitizer/58413
> > > > c-family/
> > > > 	* c-ubsan.c (ubsan_instrument_shift): Don't instrument
> > > > 	an expression if we can prove it is correct.
> > > 
> > > Shouldn't the conditions used here for an expression being proved correct 
> > > match those for instrumentation, i.e. depend on flag_isoc99 and on 
> > > (cxx_dialect == cxx11 || cxx_dialect == cxx1y)?
> > 
> > I don't think so: for the unsigned case we could restrict it to C
> > only, but it doesn't hurt doing it even for C++; in the signed case
> > we care only about C, but we can't restrict it to flag_isoc99 only,
> > since we need to prove the correctnes even for ANSI C.
> 
> I don't understand how this answers my question.

I'm sorry.

Please disregard the original (ugly) patch, the folloing applies to
the new (pretty) patch
<http://gcc.gnu.org/ml/gcc-patches/2013-09/msg01283.html>.

> * The following principle applies: for any command-line options, with 
> ubsan enabled, if an integer operation with particular (non-constant) 
> operands is accepted by the sanitization code at runtime, the same 
> operation with the same operand values (and types) as constants should be 
> accepted at compile time (and at runtime) in contexts where an integer 
> constant expression is required.  Does this patch make the compiler meet 
> this principle, for all the different command-line options that vary what 
> is accepted at runtime?

I believe so.  E.g.

int i = 4, j = 3, k;
k = i << j;

is ok, thus the following is ok as well

case (4 << 3) (for C++/C with various -std=*).
 
> * The following principle also applies: for any command-line options, with 
> ubsan enabled, if an integer operation with particular (non-constant) 
> operands is rejected by the sanitization code at runtime, the same 
> operation with the same operand values (and types) as constants should be 
> rejected at compile time (or at runtime) in contexts where an integer 
> constant expression is required.  Does this patch make the compiler meet 
> this principle, for all the different command-line options that vary what 
> is accepted at runtime?

And I think this applies as well.  At runtime we reject e.g.
int i = 1, j = 120, k;
k = i << j;

and at compile-time we reject

  enum e { 
    red = 0 << 120,
  };

	Marek