This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: PATCH RFC: -Wstrict-overflow


Mark Mitchell <mark@codesourcery.com> writes:

> To be honest, your examples are making me a little concerned that
> -Wsigned-overflow isn't going to be much of a win.  It seems like many
> programs are going to fall afoul of it, and that it will be hard to
> silence.  So, except for the truly dedicated, willing to go through and
> clean up (or selectively disable warnings) on each occurrence, this is
> going to have a low signal-to-noise ratio.

Yes.  I think the gcc developers in general have been pretty
consistent in saying that all along in this discussion.


> I understand what we're trying to do: give users a heads-up about
> possible situation where overflow might give surprising behavior,
> because we will assume it can't happen.  But, if we can't the ratio up
> to the point where the warning is telling you about something that's
> probably a bug most of the time, it's not clear it's actually going to
> be useful.
> 
> Is there any way to distinguish the:
> 
>   A * x / B
> 
> kinds of cases (which we can presume to be ordinary integer arithmetic
> that just happens to be done on a computer with finite precision) from
> these kinds of things:
> 
>   x + 1 > x
>   abs(x) + 1 < 0
> 
> that (since they are always false for mathematical integers) are more
> likely to be wrap-around tests?  In other words, can we make the warning
> more selective by focusing on comparison operations?

Sure, that is easy.  It's right there in the warning message.  There
are no warning messages like that in the cc1 .i files.  But the code
generates warnings like
    assuming signed overflow is undefined when assuming that (X - c) > X is always false

That is not enough to catch cases like the one which started this
brouhaha:

int
foo ()
{
  int j;
  for (j = 1; 0 < j; j *= 2)
    if (! bigtime_test (j))
      return 1;
  return 0;
}

However, this case is a VRP case.

What do people think about different levels for -Wstrict-overflow?  My
main concern with that would be classifying them in the first place,
and maintaining the classification over time.


For reference, here are the warnings for the cc1 .i files about
comparison operations:

../../trunk/gcc/expmed.c: In function âexpand_mult_highpart_optabâ:
../../trunk/gcc/expmed.c:3485: warning: assuming signed overflow is undefined when changing X +- C1 cmp C2 to X cmp C1 +- C2

size - 1 < BITS_PER_WORD ==> size < BITS_PER_WORD + 1

../../trunk/gcc/expmed.c:3511: warning: assuming signed overflow is undefined when changing X +- C1 cmp C2 to X cmp C1 +- C2
../../trunk/gcc/expmed.c:3538: warning: assuming signed overflow is undefined when changing X +- C1 cmp C2 to X cmp C1 +- C2
../../trunk/gcc/expmed.c: In function âexpand_divmodâ:
../../trunk/gcc/expmed.c:4106: warning: assuming signed overflow is undefined when changing X +- C1 cmp C2 to X cmp C1 +- C2
../../trunk/gcc/expmed.c:4272: warning: assuming signed overflow is undefined when changing X +- C1 cmp C2 to X cmp C1 +- C2
../../trunk/gcc/expmed.c:4307: warning: assuming signed overflow is undefined when changing X +- C1 cmp C2 to X cmp C1 +- C2
../../trunk/gcc/expmed.c:4399: warning: assuming signed overflow is undefined when changing X +- C1 cmp C2 to X cmp C1 +- C2

All basically the same.

../../trunk/gcc/config/i386/i386.md:20245: warning: assuming signed overflow is undefined when changing X +- C1 cmp C2 to X cmp C1 +- C2
../../trunk/gcc/config/i386/i386.md:20245: warning: assuming signed overflow is undefined when changing X +- C1 cmp C2 to X cmp C1 +- C2

  "TARGET_64BIT
   && INTVAL (operands[4]) + SSE_REGPARM_MAX * 16 - 16 < 128
   && INTVAL (operands[4]) + INTVAL (operands[2]) * 16 >= -128"

../../trunk/gcc/local-alloc.c: In function âblock_allocâ:
../../trunk/gcc/local-alloc.c:1674: warning: assuming signed overflow is undefined when changing X +- C1 cmp C2 to X cmp C1 +- C2
../../trunk/gcc/local-alloc.c:1675: warning: assuming signed overflow is undefined when combining constants around a comparison

	  int fake_birth = MAX (0, qty[q].birth - 2 + qty[q].birth % 2);
	  int fake_death = MIN (insn_number * 2 + 1,
				qty[q].death + 2 - qty[q].death % 2);

../../trunk/gcc/real.c: In function âdo_compareâ:
../../trunk/gcc/real.c:949: warning: assuming signed overflow is undefined when combining constants around a comparison
../../trunk/gcc/real.c:951: warning: assuming signed overflow is undefined when combining constants around a comparison
../../trunk/gcc/real.c: In function âdo_fix_truncâ:
../../trunk/gcc/real.c:979: warning: assuming signed overflow is undefined when changing X +- C1 cmp C2 to X cmp C1 +- C2
../../trunk/gcc/real.c:981: warning: assuming signed overflow is undefined when changing X +- C1 cmp C2 to X cmp C1 +- C2
../../trunk/gcc/real.c: In function âreal_identicalâ:
../../trunk/gcc/real.c:1208: warning: assuming signed overflow is undefined when combining constants around a comparison
../../trunk/gcc/real.c: In function âreal_to_integerâ:
../../trunk/gcc/real.c:1293: warning: assuming signed overflow is undefined when changing X +- C1 cmp C2 to X cmp C1 +- C2
../../trunk/gcc/real.c:1299: warning: assuming signed overflow is undefined when changing X +- C1 cmp C2 to X cmp C1 +- C2
../../trunk/gcc/real.c: In function âreal_to_decimalâ:
../../trunk/gcc/real.c:1551: warning: assuming signed overflow is undefined when changing X +- C1 cmp C2 to X cmp C1 +- C2
../../trunk/gcc/real.c:1589: warning: assuming signed overflow is undefined when changing X +- C1 cmp C2 to X cmp C1 +- C2

All comparisons involving REAL_EXP, where REAL_EXP is

#define REAL_EXP(REAL) \
  ((int)((REAL)->uexp ^ (unsigned int)(1 << (EXP_BITS - 1))) \
   - (1 << (EXP_BITS - 1)))

The constant (1 << (EXP_BITS - 1)) is being moved around.

../../trunk/gcc/rtlanal.c: In function ânum_sign_bit_copies1â:
../../trunk/gcc/rtlanal.c:4217: warning: assuming signed overflow is undefined when changing X +- C1 cmp C2 to X cmp C1 +- C2

      result = MAX (1, MIN (num0, num1) - 1);

../../trunk/gcc/stmt.c: In function âcheck_operand_nalternativesâ:
../../trunk/gcc/stmt.c:1144: warning: assuming signed overflow is undefined when changing X +- C1 cmp C2 to X cmp C1 +- C2

      if (nalternatives + 1 > MAX_RECOG_ALTERNATIVES)

../../trunk/gcc/stor-layout.c: In function âget_mode_alignmentâ:
../../trunk/gcc/stor-layout.c:263: warning: assuming signed overflow is undefined when eliminating multiplication in comparison with zero
../../trunk/gcc/stor-layout.c:263: warning: assuming signed overflow is undefined when eliminating multiplication in comparison with zero

  return MIN (BIGGEST_ALIGNMENT, MAX (1, mode_base_align[mode]*BITS_PER_UNIT));

../../trunk/gcc/stor-layout.c: In function âset_min_and_max_values_for_integral_typeâ:
../../trunk/gcc/stor-layout.c:2036: warning: assuming signed overflow is undefined when changing X +- C1 cmp C2 to X cmp C1 +- C2
../../trunk/gcc/stor-layout.c:2039: warning: assuming signed overflow is undefined when changing X +- C1 cmp C2 to X cmp C1 +- C2
../../trunk/gcc/stor-layout.c:2049: warning: assuming signed overflow is undefined when changing X +- C1 cmp C2 to X cmp C1 +- C2
../../trunk/gcc/stor-layout.c:2053: warning: assuming signed overflow is undefined when changing X +- C1 cmp C2 to X cmp C1 +- C2
../../trunk/gcc/stor-layout.c:2058: warning: assuming signed overflow is undefined when changing X +- C1 cmp C2 to X cmp C1 +- C2
../../trunk/gcc/stor-layout.c:2061: warning: assuming signed overflow is undefined when changing X +- C1 cmp C2 to X cmp C1 +- C2

A bunch of explicit comparisons passing different values to
build_int_cst_wide.

../../trunk/gcc/varasm.c: In function âassemble_start_functionâ:
../../trunk/gcc/varasm.c:1450: warning: assuming signed overflow is undefined when changing X +- C1 cmp C2 to X cmp C1 +- C2

      ASM_OUTPUT_MAX_SKIP_ALIGN (asm_out_file,
				 align_functions_log, align_functions - 1);

which expands into

      do { if ((align_functions_log) != 0) { if ((align_functions - 1) == 0) fprintf ((asm_out_file), "\t.p2align %d\n", (align_functions_log)); else fprintf ((asm_out_file), "\t.p2align %d,,%d\n", (align_functions_log), (align_functions - 1)); } } while (0);


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]