[Bug c/16302] New: gcc fails to warn about some common logic errors

[Bug c/16302] New: gcc fails to warn about some common logic errors

[trt at acm dot org]

A "gcc -Wextra -O" misses the 4 bugs in this test program:

int
main (int argc, char *argv[])
{
   if (argc != 1 || argc != 2) return 1;
   if (argc < 0 && argc > 10) return 1;
   if (argc || !argc) return 1;
   if (argc && !argc) return 1;
   return 0;
}

With a tweak to fold-const.c (below), they will be caught:

t.c:4: warning: `or' of collectively exhaustive tests is always 1
t.c:5: warning: `and' of mutually exclusive tests is always 0
t.c:6: warning: `or' of collectively exhaustive tests is always 1
t.c:7: warning: `and' of mutually exclusive tests is always 0

A bootstrap of gcc-3.5-20040627 reports such a warning in fold-const.c itself,
the patch below includes the obvious fix.

====
Comment: The -O is needed due to this code in fold-const.c:

      /* We only do these simplifications if we are optimizing.  */
      if (!optimize)
        return t;

This is probably from when fold-const.c was much simpler,
but I think is no longer appropriate and recommend it be deleted.
====

*** fold-const.c.orig   Sun Jun 27 11:23:44 2004
--- fold-const.c        Wed Jun 30 12:43:39 2004
***************
*** 4377,4379 ****
                                         in_p, low, high))))
!     return or_op ? invert_truthvalue (tem) : tem;
   
--- 4377,4388 ----
                                         in_p, low, high))))
!     {
!       if (TREE_CODE (tem) == INTEGER_CST && extra_warnings)
!         {
!           if (or_op)
!             warning ("`or' of collectively exhaustive tests is always 1");
!           else
!             warning ("`and' of mutually exclusive tests is always 0");
!         }
!       return or_op ? invert_truthvalue (tem) : tem;
!     }
   
***************
*** 9998,10000 ****
          || (TREE_CODE (op0) == REAL_CST
!             && TREE_CODE (op0) != REAL_CST))
        {
--- 10007,10009 ----
          || (TREE_CODE (op0) == REAL_CST
!             && TREE_CODE (op1) != REAL_CST))
        {

-- 
           Summary: gcc fails to warn about some common logic errors
           Product: gcc
           Version: 3.5.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: c
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: trt at acm dot org
                CC: gcc-bugs at gcc dot gnu dot org


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

[Bug c/16302] gcc fails to warn about some common logic errors

[pinskia at gcc dot gnu dot org]

------- Additional Comments From pinskia at gcc dot gnu dot org  2004-07-01 01:23 -------
Confirmed, patches goto gcc-patches@ after reading http://gcc.gnu.org/contribute.html.

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |diagnostic


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

[Bug c/16302] gcc fails to warn about some common logic errors

[trt at acm dot org]

------- Additional Comments From trt at acm dot org  2004-07-01 15:28 -------
It is not practical for gcc outsiders to submit patches,
the requirements are too exacting and complex.  E.g. see
http://gcc.gnu.org/ml/gcc-patches/2004-07/msg00072.html
which demands a testcase (something which is IMO unclear
in the "contributing" URL you gave).
It is unlikely my first (or second) attempt at a testcase
will be flawless.
Similarly, I am asked to document this warning
(which I find odd, since there are lots of undocumented -Wextra warnings).
Do I need to fix the "broken" existing warnings too?  The response was unclear.
The only requirement I can confidently handle is fixing the warning strings.
And are these *all* the requirements?  I doubt it.

On the other hand, such issues are trivial for veterans such as yourself.
Could you please handle this for me?  I would greatly appreciate it.

-- 


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

[Bug c/16302] gcc fails to warn about some common logic errors

[bangerth at dealii dot org]

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |sayle at gcc dot gnu dot org


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

[Bug c/16302] gcc fails to warn about some common logic errors

[jsm at polyomino dot org dot uk]

------- Additional Comments From jsm at polyomino dot org dot uk  2004-07-01 17:45 -------
Subject: Re:  gcc fails to warn about some common logic errors

On Thu, 1 Jul 2004, trt at acm dot org wrote:

> (which I find odd, since there are lots of undocumented -Wextra warnings).

Please report these as separate bugs.  Phil Edwards fixed the known such
problems in <http://gcc.gnu.org/ml/gcc-patches/2003-01/msg01741.html>; any
that have been added since then would be regressions.



-- 


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

[Bug c/16302] gcc fails to warn about some common logic errors

[trt at acm dot org]

------- Additional Comments From trt at acm dot org  2004-07-07 15:36 -------
I have submitted a revised patch in
http://gcc.gnu.org/ml/gcc-patches/2004-07/msg00148.html


-- 


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

[Bug middle-end/16302] gcc fails to warn about some common logic errors

[pinskia at gcc dot gnu dot org]

------- Additional Comments From pinskia at gcc dot gnu dot org  2004-08-03 07:32 -------
Confirmed.

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
          Component|c                           |middle-end
     Ever Confirmed|                            |1
           Keywords|                            |patch
   Last reconfirmed|0000-00-00 00:00:00         |2004-08-03 07:32:55
               date|                            |


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

[Bug middle-end/16302] gcc fails to warn about some common logic errors

[trt at acm dot org]

------- Additional Comments From trt at acm dot org  2004-09-02 18:19 -------
Here are some reasons why I think this patch should be applied.
1.  Inspection of the patch will show that it is "low risk",
    except for the potential risk of false positives.
2.  I have been using this warning for about 4 years now,
    on a 35Mloc source code base involving 100s of developers,
    and it has triggered dozens of times with only a couple false positives.
    (The false positives do not happen with gcc 3.5, I think some changes
     to fold() have caused them to go away.)
    Sometimes, for fun, I use this compiler on other source code.
    E.g. I remember reporting two of these warnings (among others)
    in an early version of "valgrind".
3.  It spots 3 bugs in gcc 3.5.  One of them is fixed by this patch.
    If you want to see the other two, try applying the patch.

As a downside, some versions of gcc/insn-attrtab.c trigger these warnings.
Last I checked, the ia64 version had about 7 of them.
They are technically valid, but probably indicate harmless inefficiencies.
On the other hand, they might indicate something that should be cleaned up.

I realize there is no urgent need for this patch,
but I have seen much less effective (and sometimes counterproductive)
warning messages added to gcc.  Please consider one.

-- 


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

[Bug middle-end/16302] gcc fails to warn about some common logic errors

[trt at acm dot org]

------- Additional Comments From trt at acm dot org  2005-04-29 20:20 -------
The patch given in comment #4 no successfully applies because the warning()
function has new initial parameter.  Also the fold_buildN() cleanup invalidated
(and fixed) the "REAL_CST" part of the patch.
Some "speculative folding" is now causing false positives, for which there is a
simple fix: suppress warnings when doing such folds. For example

*** tree-ssa-loop-niter.c.orig  Wed Apr 27 13:48:21 2005
--- tree-ssa-loop-niter.c       Wed Apr 27 14:28:59 2005
***************
*** 778,780 ****
--- 778,782 ----
    notcond = invert_truthvalue (cond);
+   inhibit_warnings++;
    e = fold_build2 (TRUTH_OR_EXPR, boolean_type_node, notcond, te);
+   inhibit_warnings--;
    if (nonzero_p (e))

People who think warnings belong only in the front-end might balk.  I could
submit an alternate indirect check for this situation in parser_build_binary_op.
 It would be tidy, but would no longer warn about non-C glitches such as:

libjava/gnu/java/security/x509/X500DistinguishedName.java:447:        if (sep !=
'+' || sep != ',')
libjava/java/net/HttpURLConnection.java:555:    if (((code / 100) != 4) ||
((code / 100) != 5))
libjava/javax/swing/plaf/basic/BasicGraphicsUtils.java:401:    if
((underlinedChar >= 0) || (underlinedChar <= 0xffff))
libjava/javax/security/auth/x500/X500Principal.java:380:        if (sep != '+'
|| sep != ',')

Please let me know if there is any interest in a C-only patch.  (Or any interest
in pursuing this PR.)

-- 


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