[Bug c/43772] Errant -Wlogical-op warning when testing limits

[P at draigBrady dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=43772

Pádraig Brady  changed:

   What|Removed |Added

 Status|NEW |RESOLVED
  Known to work||4.8.2
 Resolution|--- |FIXED
  Known to fail||4.6.3

--- Comment #23 from Pádraig Brady  ---
This is fixed by http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=187194

To see where it was first released we can quickly see the tags at:
  https://github.com/mirrors/gcc/commit/686369e8
Which indicates this was fixed as part of the 4.8.0 release.

I've confirmed it's OK in gcc 4.8.2

[Bug c/43772] Errant -Wlogical-op warning when testing limits

[manu at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43772

--- Comment #22 from Manuel López-Ibáñez  2012-05-05 
11:32:30 UTC ---
Author: manu
Date: Sat May  5 11:32:26 2012
New Revision: 187195

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=187195
Log:
2012-05-05  Manuel López-Ibáñez  

PR c/43772
testsuite/
* c-c++-common/pr43772.c: New.

Added:
trunk/gcc/testsuite/c-c++-common/pr43772.c

[Bug c/43772] Errant -Wlogical-op warning when testing limits

[manu at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43772

--- Comment #21 from Manuel López-Ibáñez  2012-05-05 
11:31:03 UTC ---
Author: manu
Date: Sat May  5 11:30:57 2012
New Revision: 187194

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=187194
Log:
2012-05-05  Manuel López-Ibáñez  

PR c/43772
c-family/
* c-common.c (warn_logical_operator): Do not warn if either side
is already true or false.
testsuite/
* c-c++-common/pr43772.c: New.

Modified:
trunk/gcc/c-family/ChangeLog
trunk/gcc/c-family/c-common.c
trunk/gcc/testsuite/ChangeLog

[Bug c/43772] Errant -Wlogical-op warning when testing limits

[eggert at gnu dot org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43772

--- Comment #20 from eggert at gnu dot org 2012-04-28 22:40:27 UTC ---
(In reply to comment #19)
> >   intmax_t i = (whatever);
> >   if (INT_MAX < i && i <= LONG_MAX)
> Have you actually seen that?

No, I just now invented that example.  It was based on experience I've
had in using macros like those in gnulib's intprops.h

though none of those macros actually have that particular code.

>I would imagine the following to be more common:
>if(i<=INT_MAX)
>  print("i is in 'int'");
>else if(i<=LONG_MAX)

Yes, intprops.h already has written code that way (instead of using &&)
in order to avoid a somewhat-similar bug in the Sun C compiler.
There's a comment to that effect in intprops.h.

> we are not talking of having this warning by default,
> this is an isolated warning not even included in -Wall -Wextra.

We can always disable -Wlogical-op when compiling
any code that has the problem.  It's not a big deal,
though it is an annoyance that will prevent people from benefiting
from -Wlogical-op.  I'm afraid I don't have any magic answers here.

[Bug c/43772] Errant -Wlogical-op warning when testing limits

[marc.glisse at normalesup dot org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43772

--- Comment #19 from Marc Glisse  2012-04-28 
22:16:55 UTC ---
(In reply to comment #18)
> I'm afraid that false positives would still be likely.
> For example, suppose we're on a platform where
> INT_MAX = LONG_MAX < INTMAX_MAX.  Then:
> 
>   intmax_t i = (whatever);
>   if (INT_MAX < i && i <= LONG_MAX)
>  print ("i is in 'long' but not 'int' range");

Have you actually seen that? I would imagine the following to be more common:
if(i<=INT_MAX)
  print("i is in 'int'");
else if(i<=LONG_MAX)
  ...

> This sort of thing is fairly common in portable code,
> and GCC shouldn't warn about it merely because
> we're on a platform where the two tests cannot both
> be true when INT_MAX == LONG_MAX.

Well, can you define a set of circumstances where gcc could / should warn?
a

[Bug c/43772] Errant -Wlogical-op warning when testing limits

[eggert at gnu dot org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43772

--- Comment #18 from eggert at gnu dot org 2012-04-28 21:53:30 UTC ---
(In reply to comment #17)
> I expect the remaining false positives to be very
> rare. i>=INT_MIN&&isomething are common, but
> isomething seems less likely.

I'm afraid that false positives would still be likely.
For example, suppose we're on a platform where
INT_MAX = LONG_MAX < INTMAX_MAX.  Then:

  intmax_t i = (whatever);
  if (INT_MAX < i && i <= LONG_MAX)
 print ("i is in 'long' but not 'int' range");

This sort of thing is fairly common in portable code,
and GCC shouldn't warn about it merely because
we're on a platform where the two tests cannot both
be true when INT_MAX == LONG_MAX.

[Bug c/43772] Errant -Wlogical-op warning when testing limits

[marc.glisse at normalesup dot org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43772

--- Comment #17 from Marc Glisse  2012-04-28 
18:49:49 UTC ---
(In reply to comment #16)
> I understand now, and I think you are right. We don't have a warning for
> "((int)x) < INT_MIN" or ((int)x) > INT_MAX but I think it should go to
> Wtype-limits.

Interestingly, for an int i, we don't warn for x<=INT_MAX, but we do warn for
x<=(long)INT_MAX (adapt if your platform has int and long of the same size).

> Do you think we could test this situation just before the Wlogical-op warning?

It is easy to re-check inside warn_logical_operator if one of the tests is
always true. I have no idea how to pass the information from Wtype-limits that
warn_logical_operator shouldn't be called.

> I can see that some macros may generate x >= INT_MIN but the x < INT_MIN case
> seems less likely to be intented and we should warn (and then return and avoid
> warning with Wlogical-op).

I think < INT_MIN and >= INT_MIN should either both warn of both be quiet. It
is a matter of style whether people write:
if (x in range) do the work;
or
if (x out of range) abort;
do the work;

(In reply to comment #12)
> Do you mean:
> 
> if (or_op && integer_onep(tem)) { warn();}
> else if (!or_op && integer_zerop(tem)) { warn();}

Even smaller would be to replace the current (TREE_CODE (tem) != INTEGER_CST)
with integer_zerop(tem) and pass build_range_check in_p^or_op (or in_p==or_op,
don't know which) instead of just in_p. It would already be an improvement over
the current situation, and I expect the remaining false positives to be very
rare. i>=INT_MIN&&isomething are common, but
isomething seems less likely.

[Bug c/43772] Errant -Wlogical-op warning when testing limits

[manu at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43772

--- Comment #16 from Manuel López-Ibáñez  2012-04-28 
13:07:41 UTC ---
(In reply to comment #15)
> 
> No, there could be a warning that the first test is always false, another one
> that the second one is always false, but adding a third warning that the
> conjunction of the 2 is always false seems bogus. This warning is meant for:
> x<5&&x>10, where each test independently could be true, just not both at the
> same time.

I understand now, and I think you are right. We don't have a warning for
"((int)x) < INT_MIN" or ((int)x) > INT_MAX but I think it should go to
Wtype-limits.

Do you think we could test this situation just before the Wlogical-op warning?
I can see that some macros may generate x >= INT_MIN but the x < INT_MIN case
seems less likely to be intented and we should warn (and then return and avoid
warning with Wlogical-op).

I am sure there must be a way to test for "x < MIN_OF_TYPE_OF(x))" and "x >
MAX_OF_TYPE_OF(x)" I just haven't investigated how.

[Bug c/43772] Errant -Wlogical-op warning when testing limits

[marc.glisse at normalesup dot org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43772

--- Comment #15 from Marc Glisse  2012-04-28 
12:55:28 UTC ---
(In reply to comment #14)
> (In reply to comment #13)
> > 
> > Except that this version would warn for xINT_MAX, whereas this
> > belongs to other warnings. So testing the triviality of the first ranges 
> > seems
> > best.
>
> I don't understand. This warning (whatever its name) should precisely warn for
> that with "logical 'and' of mutually exclusive tests is always false".

No, there could be a warning that the first test is always false, another one
that the second one is always false, but adding a third warning that the
conjunction of the 2 is always false seems bogus. This warning is meant for:
x<5&&x>10, where each test independently could be true, just not both at the
same time.

At least that is my understanding...

[Bug c/43772] Errant -Wlogical-op warning when testing limits

[manu at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43772

--- Comment #14 from Manuel López-Ibáñez  2012-04-28 
12:49:57 UTC ---
(In reply to comment #13)
> 
> Except that this version would warn for xINT_MAX, whereas this
> belongs to other warnings. So testing the triviality of the first ranges seems
> best.

I don't understand. This warning (whatever its name) should precisely warn for
that with "logical 'and' of mutually exclusive tests is always false".

[Bug c/43772] Errant -Wlogical-op warning when testing limits

[marc.glisse at normalesup dot org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43772

--- Comment #13 from Marc Glisse  2012-04-28 
12:40:14 UTC ---
(In reply to comment #10)
> But there is something strange, because it is warning "it is always false",
> which is obviously not true. So I think at some moment it is doing some
> transformation we don't want to do.

It notices that it should warn, and unless one of the first ranges is trivial
(a case it forgot), with an operator &&, the only warning that makes sense is
that it is always false. It never shows that it is false, it is just a bit
hasty in deciding which warning to pick. And indeed the "logical and...always
true" sentence does not exist, because it doesn't make sense.

(In reply to comment #11)
> (In reply to comment #9)
> > It forgets to check first whether the first 2 ranges are trivial.
> Or easier, instead of checking:
>   if (TREE_CODE (tem) != INTEGER_CST)
> it could check integer_onep(tem) or integer_zerop(tem) depending on or_op. Or
> build a tree integer constant from or_op and tree_int_cst_equal it to tem.

Except that this version would warn for xINT_MAX, whereas this
belongs to other warnings. So testing the triviality of the first ranges seems
best.

[Bug c/43772] Errant -Wlogical-op warning when testing limits

[manu at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43772

--- Comment #12 from Manuel López-Ibáñez  2012-04-28 
12:37:06 UTC ---
(In reply to comment #11)
> (In reply to comment #9)
> > It forgets to check first whether the first 2 ranges are trivial.
> 
> Or easier, instead of checking:
>   if (TREE_CODE (tem) != INTEGER_CST)
> it could check integer_onep(tem) or integer_zerop(tem) depending on or_op. 

Do you mean:

if (or_op && integer_onep(tem)) { warn();}
else if (!or_op && integer_zerop(tem)) { warn();}

I think that could work. Would you mind testing it?

[Bug c/43772] Errant -Wlogical-op warning when testing limits

[marc.glisse at normalesup dot org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43772

--- Comment #11 from Marc Glisse  2012-04-28 
12:33:26 UTC ---
(In reply to comment #9)
> It forgets to check first whether the first 2 ranges are trivial.

Or easier, instead of checking:
  if (TREE_CODE (tem) != INTEGER_CST)
it could check integer_onep(tem) or integer_zerop(tem) depending on or_op. Or
build a tree integer constant from or_op and tree_int_cst_equal it to tem.

[Bug c/43772] Errant -Wlogical-op warning when testing limits

[manu at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43772

--- Comment #10 from Manuel López-Ibáñez  2012-04-28 
12:32:49 UTC ---
(In reply to comment #9)
> For : x>=INT_MIN && x<=INT_MAX
> the code creates a range for x>=INT_MIN, another range for x<=INT_MAX, merges
> them into a single range, checks that that range is trivial (empty or full),
> and then warns according to the operator && or ||. It forgets to check first
> whether the first 2 ranges are trivial.

But there is something strange, because it is warning "it is always false",
which is obviously not true. So I think at some moment it is doing some
transformation we don't want to do.

[Bug c/43772] Errant -Wlogical-op warning when testing limits

[marc.glisse at normalesup dot org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43772

Marc Glisse  changed:

   What|Removed |Added

 CC||marc.glisse at normalesup
   ||dot org

--- Comment #9 from Marc Glisse  2012-04-28 
12:19:54 UTC ---
For : x>=INT_MIN && x<=INT_MAX
the code creates a range for x>=INT_MIN, another range for x<=INT_MAX, merges
them into a single range, checks that that range is trivial (empty or full),
and then warns according to the operator && or ||. It forgets to check first
whether the first 2 ranges are trivial.

[Bug c/43772] Errant -Wlogical-op warning when testing limits

[eggert at gnu dot org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43772

--- Comment #8 from eggert at gnu dot org 2011-07-15 03:01:42 UTC ---
The problem persists in GCC 4.6.1 (x86-64):

$ cat >t.c
long long
emacs_lseek (int fd, long long offset, int whence)
{
  return -1-9223372036854775807LL <= offset && offset <= 9223372036854775807LL;
}
$ gcc -S -Wlogical-op t.c
t.c: In function 'emacs_lseek':
t.c:4:3: warning: logical 'and' of mutually exclusive tests is always false
[-Wlogical-op]

[Bug c/43772] Errant -Wlogical-op warning when testing limits

[eggert at gnu dot org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43772

eggert at gnu dot org changed:

   What|Removed |Added

 CC||eggert at gnu dot org

--- Comment #7 from eggert at gnu dot org 2011-06-15 19:18:57 UTC ---
I get the same bug with GCC 4.6.0 (x86-64).  Here's a test case:

long long
emacs_lseek (int fd, long long offset, int whence)
{
  return -1-9223372036854775807LL <= offset && offset <= 9223372036854775807LL;
}

This is abstracted from real EMACS source code: the real code is verifying
that an EMACS_INT value fits within off_t.  Both types happen to be
'long long' here, but they might not be the same types on other platforms.

When compiled with "gcc -S -Wlogical-op t.c", I get:

t.c: In function 'emacs_lseek':
t.c:5:3: warning: logical 'and' of mutually exclusive tests is always false
[-Wlogical-op]

which is obviously bogus.

Using "&" rather than "&&" works around the bug, but that's not a
satisfactory solution in general.

[Bug c/43772] Errant -Wlogical-op warning when testing limits

[manu at gcc dot gnu dot org]

--- Comment #6 from manu at gcc dot gnu dot org  2010-09-23 08:24 ---
I don't get a warning in trunk r159764. I think I fixed a similar bug during
4.6. 


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43772

[Bug c/43772] Errant -Wlogical-op warning when testing limits

[manu at gcc dot gnu dot org]

--- Comment #5 from manu at gcc dot gnu dot org  2010-09-23 08:13 ---
(In reply to comment #4)
> "Me too". This is real code, from xdgmime library (errno doesn't matter)
> 
>   long retval = -1;
>   ...
>   if ((retval < INT_MIN) || (retval > INT_MAX) || (errno != 0))
> return -1;

What is the above testing for when int == long?


-- 

manu at gcc dot gnu dot org changed:

   What|Removed |Added

 CC||manu at gcc dot gnu dot org


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43772

[Bug c/43772] Errant -Wlogical-op warning when testing limits

[muntyan at fastmail dot fm]

--- Comment #4 from muntyan at fastmail dot fm  2010-09-23 06:52 ---
"Me too". This is real code, from xdgmime library (errno doesn't matter)

  long retval = -1;
  ...
  if ((retval < INT_MIN) || (retval > INT_MAX) || (errno != 0))
return -1;


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43772

[Bug c/43772] Errant -Wlogical-op warning when testing limits

[P at draigBrady dot com]

--- Comment #3 from P at draigBrady dot com  2010-04-22 00:37 ---
I've confirmed that this is _not_ an issue with the previous
gcc (GCC) 4.4.1 20090725 (Red Hat 4.4.1-2)


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43772

[Bug c/43772] Errant -Wlogical-op warning when testing limits

[P at draigBrady dot com]

--- Comment #2 from P at draigBrady dot com  2010-04-17 17:40 ---
Well the warning should at least change.

However the logical operation itself is not an issue,
so I think a warning should not be issued at all.
I.E. if TOP and BOT are defined as a narrower range
then we don't get a warning currently. When TOP and BOT
do encompass the whole range then the compiler should
just optimize out the conditional test without warning.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43772

[Bug c/43772] Errant -Wlogical-op warning when testing limits

[rguenth at gcc dot gnu dot org]

--- Comment #1 from rguenth at gcc dot gnu dot org  2010-04-17 11:03 ---
$ gcc-4.5 -Wlogical-op t.c -S -B.
t.c: In function 'main':
t.c:11:4: warning: logical 'and' of mutually exclusive tests is always false

confirmed.  It should warn "... is always true".


-- 

rguenth at gcc dot gnu dot org changed:

   What|Removed |Added

 Status|UNCONFIRMED |NEW
 Ever Confirmed|0   |1
   Last reconfirmed|-00-00 00:00:00 |2010-04-17 11:03:25
   date||


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43772