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

[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

[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

[marc.glisse at normalesup dot org]

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

Marc Glisse <marc.glisse at normalesup dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |marc.glisse at normalesup
                   |                            |dot org

--- Comment #9 from Marc Glisse <marc.glisse at normalesup dot org> 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

[manu at gcc dot gnu.org]

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

--- Comment #10 from Manuel López-Ibáñez <manu at gcc dot gnu.org> 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

--- Comment #11 from Marc Glisse <marc.glisse at normalesup dot org> 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 #12 from Manuel López-Ibáñez <manu at gcc dot gnu.org> 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 #13 from Marc Glisse <marc.glisse at normalesup dot org> 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 x<INT_MIN && x>INT_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 #14 from Manuel López-Ibáñez <manu at gcc dot gnu.org> 2012-04-28 12:49:57 UTC ---
(In reply to comment #13)
> 
> Except that this version would warn for x<INT_MIN && x>INT_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 #15 from Marc Glisse <marc.glisse at normalesup dot org> 2012-04-28 12:55:28 UTC ---
(In reply to comment #14)
> (In reply to comment #13)
> > 
> > Except that this version would warn for x<INT_MIN && x>INT_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 #16 from Manuel López-Ibáñez <manu at gcc dot gnu.org> 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 #17 from Marc Glisse <marc.glisse at normalesup dot org> 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&&i<something or i<INT_MIN||i>something are common, but
i<INT_MIN&&i>something seems less likely.

[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&&i<something or i<INT_MIN||i>something are common, but
> i<INT_MIN&&i>something 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 #19 from Marc Glisse <marc.glisse at normalesup dot org> 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<i&&i<=a seems to be quite precisely what this warning is about. Or maybe
a==INT_MAX (adapt depending on the type) should disable the warning, as a
special case?

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

[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
<http://git.savannah.gnu.org/cgit/gnulib.git/tree/lib/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

[manu at gcc dot gnu.org]

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

--- Comment #21 from Manuel López-Ibáñez <manu at gcc dot gnu.org> 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  <manu@gcc.gnu.org>

    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

[manu at gcc dot gnu.org]

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

--- Comment #22 from Manuel López-Ibáñez <manu at gcc dot gnu.org> 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  <manu@gcc.gnu.org>

    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

[P at draigBrady dot com]

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

Pádraig Brady <P at draigBrady dot com> 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 <P at draigBrady dot com> ---
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
>From gcc-bugs-return-453641-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org Wed Jun 11 05:05:03 2014
Return-Path: <gcc-bugs-return-453641-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org>
Delivered-To: listarch-gcc-bugs@gcc.gnu.org
Received: (qmail 714 invoked by alias); 11 Jun 2014 05:05:02 -0000
Mailing-List: contact gcc-bugs-help@gcc.gnu.org; run by ezmlm
Precedence: bulk
List-Id: <gcc-bugs.gcc.gnu.org>
List-Archive: <http://gcc.gnu.org/ml/gcc-bugs/>
List-Post: <mailto:gcc-bugs@gcc.gnu.org>
List-Help: <mailto:gcc-bugs-help@gcc.gnu.org>
Sender: gcc-bugs-owner@gcc.gnu.org
Delivered-To: mailing list gcc-bugs@gcc.gnu.org
Received: (qmail 647 invoked by uid 48); 11 Jun 2014 05:04:55 -0000
From: "billy.baker at cox dot net" <gcc-bugzilla@gcc.gnu.org>
To: gcc-bugs@gcc.gnu.org
Subject: [Bug fortran/45187] ICE with CRAY pointer in module
Date: Wed, 11 Jun 2014 05:05:00 -0000
X-Bugzilla-Reason: CC
X-Bugzilla-Type: changed
X-Bugzilla-Watch-Reason: None
X-Bugzilla-Product: gcc
X-Bugzilla-Component: fortran
X-Bugzilla-Version: unknown
X-Bugzilla-Keywords: ice-on-valid-code, patch
X-Bugzilla-Severity: normal
X-Bugzilla-Who: billy.baker at cox dot net
X-Bugzilla-Status: ASSIGNED
X-Bugzilla-Priority: P3
X-Bugzilla-Assigned-To: fxcoudert at gcc dot gnu.org
X-Bugzilla-Target-Milestone: 4.8.5
X-Bugzilla-Flags:
X-Bugzilla-Changed-Fields:
Message-ID: <bug-45187-4-R2AygwLehS@http.gcc.gnu.org/bugzilla/>
In-Reply-To: <bug-45187-4@http.gcc.gnu.org/bugzilla/>
References: <bug-45187-4@http.gcc.gnu.org/bugzilla/>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
X-Bugzilla-URL: http://gcc.gnu.org/bugzilla/
Auto-Submitted: auto-generated
MIME-Version: 1.0
X-SW-Source: 2014-06/txt/msg00723.txt.bz2
Content-length: 158

https://gcc.gnu.org/bugzilla/show_bug.cgi?idE187

--- Comment #6 from Billy Baker <billy.baker at cox dot net> ---
Thank you. Glad it was a simple change.

[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|0000-00-00 00:00:00         |2010-04-17 11:03:25
               date|                            |


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