public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug c/16302] New: gcc fails to warn about some common logic errors
@ 2004-06-30 19:38 trt at acm dot org
2004-07-01 1:23 ` [Bug c/16302] " pinskia at gcc dot gnu dot org
` (7 more replies)
0 siblings, 8 replies; 12+ messages in thread
From: trt at acm dot org @ 2004-06-30 19:38 UTC (permalink / raw)
To: gcc-bugs
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Bug c/16302] gcc fails to warn about some common logic errors
2004-06-30 19:38 [Bug c/16302] New: gcc fails to warn about some common logic errors trt at acm dot org
@ 2004-07-01 1:23 ` pinskia at gcc dot gnu dot org
2004-07-01 15:28 ` trt at acm dot org
` (6 subsequent siblings)
7 siblings, 0 replies; 12+ messages in thread
From: pinskia at gcc dot gnu dot org @ 2004-07-01 1:23 UTC (permalink / raw)
To: gcc-bugs
------- 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
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Bug c/16302] gcc fails to warn about some common logic errors
2004-06-30 19:38 [Bug c/16302] New: gcc fails to warn about some common logic errors trt at acm dot org
2004-07-01 1:23 ` [Bug c/16302] " pinskia at gcc dot gnu dot org
@ 2004-07-01 15:28 ` trt at acm dot org
2004-07-01 15:53 ` bangerth at dealii dot org
` (5 subsequent siblings)
7 siblings, 0 replies; 12+ messages in thread
From: trt at acm dot org @ 2004-07-01 15:28 UTC (permalink / raw)
To: gcc-bugs
------- 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
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Bug c/16302] gcc fails to warn about some common logic errors
2004-06-30 19:38 [Bug c/16302] New: gcc fails to warn about some common logic errors trt at acm dot org
2004-07-01 1:23 ` [Bug c/16302] " pinskia at gcc dot gnu dot org
2004-07-01 15:28 ` trt at acm dot org
@ 2004-07-01 15:53 ` bangerth at dealii dot org
2004-07-01 17:45 ` jsm at polyomino dot org dot uk
` (4 subsequent siblings)
7 siblings, 0 replies; 12+ messages in thread
From: bangerth at dealii dot org @ 2004-07-01 15:53 UTC (permalink / raw)
To: gcc-bugs
--
What |Removed |Added
----------------------------------------------------------------------------
CC| |sayle at gcc dot gnu dot org
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=16302
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Bug c/16302] gcc fails to warn about some common logic errors
2004-06-30 19:38 [Bug c/16302] New: gcc fails to warn about some common logic errors trt at acm dot org
` (2 preceding siblings ...)
2004-07-01 15:53 ` bangerth at dealii dot org
@ 2004-07-01 17:45 ` jsm at polyomino dot org dot uk
2004-07-07 15:36 ` trt at acm dot org
` (3 subsequent siblings)
7 siblings, 0 replies; 12+ messages in thread
From: jsm at polyomino dot org dot uk @ 2004-07-01 17:45 UTC (permalink / raw)
To: gcc-bugs
------- 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
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Bug c/16302] gcc fails to warn about some common logic errors
2004-06-30 19:38 [Bug c/16302] New: gcc fails to warn about some common logic errors trt at acm dot org
` (3 preceding siblings ...)
2004-07-01 17:45 ` jsm at polyomino dot org dot uk
@ 2004-07-07 15:36 ` trt at acm dot org
2004-08-03 7:32 ` [Bug middle-end/16302] " pinskia at gcc dot gnu dot org
` (2 subsequent siblings)
7 siblings, 0 replies; 12+ messages in thread
From: trt at acm dot org @ 2004-07-07 15:36 UTC (permalink / raw)
To: gcc-bugs
------- 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
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Bug middle-end/16302] gcc fails to warn about some common logic errors
2004-06-30 19:38 [Bug c/16302] New: gcc fails to warn about some common logic errors trt at acm dot org
` (4 preceding siblings ...)
2004-07-07 15:36 ` trt at acm dot org
@ 2004-08-03 7:32 ` pinskia at gcc dot gnu dot org
2004-09-02 18:20 ` trt at acm dot org
2005-04-29 20:20 ` trt at acm dot org
7 siblings, 0 replies; 12+ messages in thread
From: pinskia at gcc dot gnu dot org @ 2004-08-03 7:32 UTC (permalink / raw)
To: gcc-bugs
------- 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
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Bug middle-end/16302] gcc fails to warn about some common logic errors
2004-06-30 19:38 [Bug c/16302] New: gcc fails to warn about some common logic errors trt at acm dot org
` (5 preceding siblings ...)
2004-08-03 7:32 ` [Bug middle-end/16302] " pinskia at gcc dot gnu dot org
@ 2004-09-02 18:20 ` trt at acm dot org
2005-04-29 20:20 ` trt at acm dot org
7 siblings, 0 replies; 12+ messages in thread
From: trt at acm dot org @ 2004-09-02 18:20 UTC (permalink / raw)
To: gcc-bugs
------- 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
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Bug middle-end/16302] gcc fails to warn about some common logic errors
2004-06-30 19:38 [Bug c/16302] New: gcc fails to warn about some common logic errors trt at acm dot org
` (6 preceding siblings ...)
2004-09-02 18:20 ` trt at acm dot org
@ 2005-04-29 20:20 ` trt at acm dot org
7 siblings, 0 replies; 12+ messages in thread
From: trt at acm dot org @ 2005-04-29 20:20 UTC (permalink / raw)
To: gcc-bugs
------- 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
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Bug c/16302] gcc fails to warn about some common logic errors
[not found] <bug-16302-4397@http.gcc.gnu.org/bugzilla/>
2007-02-13 16:55 ` [Bug c/16302] " manu at gcc dot gnu dot org
2009-05-15 20:08 ` manu at gcc dot gnu dot org
@ 2009-05-15 20:10 ` manu at gcc dot gnu dot org
2 siblings, 0 replies; 12+ messages in thread
From: manu at gcc dot gnu dot org @ 2009-05-15 20:10 UTC (permalink / raw)
To: gcc-bugs
------- Comment #10 from manu at gcc dot gnu dot org 2009-05-15 20:10 -------
FIXED for GCC 4.5
--
manu at gcc dot gnu dot org changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |RESOLVED
Resolution| |FIXED
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=16302
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Bug c/16302] gcc fails to warn about some common logic errors
[not found] <bug-16302-4397@http.gcc.gnu.org/bugzilla/>
2007-02-13 16:55 ` [Bug c/16302] " manu at gcc dot gnu dot org
@ 2009-05-15 20:08 ` manu at gcc dot gnu dot org
2009-05-15 20:10 ` manu at gcc dot gnu dot org
2 siblings, 0 replies; 12+ messages in thread
From: manu at gcc dot gnu dot org @ 2009-05-15 20:08 UTC (permalink / raw)
To: gcc-bugs
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 1370 bytes --]
------- Comment #9 from manu at gcc dot gnu dot org 2009-05-15 20:08 -------
Subject: Bug 16302
Author: manu
Date: Fri May 15 20:08:21 2009
New Revision: 147596
URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=147596
Log:
2009-05-15 Manuel López-Ibáñez <manu@gcc.gnu.org>
PR 16302
* fold-const.c (make_range,build_range_check,merge_ranges): Move
declaration to...
(merge_ranges): Returns bool.
* tree.h (make_range): .. to here.
(build_range_check): Likewise.
(merge_ranges): Likewise. Renamed from merge_ranges.
* c-typeck.c (parser_build_binary_op): Update calls to
warn_logical_operator.
* c-common.c (warn_logical_operator): Add new warning.
* c-common.h (warn_logical_operator): Update declaration.
cp/
* call.c (build_new_op): Update calls to warn_logical_operator.
testsuite/
* gcc.dg/pr16302.c: New.
* g++.dg/warn/pr16302.C: New.
Added:
trunk/gcc/testsuite/g++.dg/warn/pr16302.C
trunk/gcc/testsuite/gcc.dg/pr16302.c
Modified:
trunk/gcc/ChangeLog
trunk/gcc/c-common.c
trunk/gcc/c-common.h
trunk/gcc/c-typeck.c
trunk/gcc/cp/ChangeLog
trunk/gcc/cp/call.c
trunk/gcc/fold-const.c
trunk/gcc/testsuite/ChangeLog
trunk/gcc/tree.h
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=16302
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Bug c/16302] gcc fails to warn about some common logic errors
[not found] <bug-16302-4397@http.gcc.gnu.org/bugzilla/>
@ 2007-02-13 16:55 ` manu at gcc dot gnu dot org
2009-05-15 20:08 ` manu at gcc dot gnu dot org
2009-05-15 20:10 ` manu at gcc dot gnu dot org
2 siblings, 0 replies; 12+ messages in thread
From: manu at gcc dot gnu dot org @ 2007-02-13 16:55 UTC (permalink / raw)
To: gcc-bugs
------- Comment #8 from manu at gcc dot gnu dot org 2007-02-13 16:55 -------
I wish to answer this because I am also a newbie and I understand your
frustration. On the other hand, I also understand why the current situation is
as it is now.
(In reply to comment #2)
> 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).
The requirements are documented. Unfortunately, the documentation is divided
among many places and not in the best form. It would be superb and very welcome
if someone took the responsibility of summarizing, re-writting and improving
the different pieces, so they can be as clear and simple as possible. (Like a
step by step guide on how to contribute, maybe especialised for different
user-cases).
Sadly, experienced people don't have time (and perhaps are not very well
suited) for this job. Someone brave needs to step up and take the burden,
perhaps seek collaborators among other frustrated newbies.
Another problem is that as soon as a newbie becomes experienced with the
procedure, then the interest in improving the docs fades away :(
> It is unlikely my first (or second) attempt at a testcase
> will be flawless.
It doesn't matter, reviewers should catch any flaw and patches are regularly
submitted many times. It is part of the learning proccess. Flawless patches are
accepted earlier, though. ;-)
> 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.
That is not an excuse. There are things that are historically wrong. New
mistakes cannot be justified based on past mistakes. New warnings should be
documented.
Normally, you don't need to fix anything that is broken already as long as it
is not closely related to your patch.
If the response was unclear, you have to insist. No all mails are neccessarily
read by all people and people are busy busy busy (or on holidays), so sometimes
your mail will get lost. Wait one week and insist again. Wait another week and
keep insisting. Eventually someone will answer. Short and polite emails get
more answers than angry, long rants.
This also applies to patches. If you patch is not reviewed: wait one week and
insist. Repeat until reviewed.
> The only requirement I can confidently handle is fixing the warning strings.
> And are these *all* the requirements? I doubt it.
It is very likely that if a reviewer finds an obvious mistake in your patch,
he/she will not review in deep the rest of it and just move on to the next
patch. You will need to send it again and wait for the answer. At first you
will make a lot of mistakes but that is normal. My first patch was rejected
several times just for the Changelog format. In a few iterations your patches
will be almost perfect.
> 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.
That is a sad misconception. Bootstrapping + testing take time even if there
are no failures at all. Adding testcases takes time. Having patches around
takes thinking time. Keeping track of submitted patches and pinging reviewers
take time. Fixing minor issues in patches takes time. Committing patches takes
also a little amount of time. Of course, the total time could be trivial if
you had nothing else to do. But if the issue is minor and you are busy (or you
are not paid for fixing it and just prefer to invest your time in something
else like your family), then it is already too much time.
Also there is the issue (I guess) that if sloppy patches are regularly fixed by
reviewers, that will advocate more sloppy patches which in turn will make
reviewers more unwilling to review patches.
So please, reconsiderate your stance, and resubmit your patch. I am sure your
contribution will be appreciated.
Thanks in advance.
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=16302
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2009-05-15 20:10 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2004-06-30 19:38 [Bug c/16302] New: gcc fails to warn about some common logic errors trt at acm dot org
2004-07-01 1:23 ` [Bug c/16302] " pinskia at gcc dot gnu dot org
2004-07-01 15:28 ` trt at acm dot org
2004-07-01 15:53 ` bangerth at dealii dot org
2004-07-01 17:45 ` jsm at polyomino dot org dot uk
2004-07-07 15:36 ` trt at acm dot org
2004-08-03 7:32 ` [Bug middle-end/16302] " pinskia at gcc dot gnu dot org
2004-09-02 18:20 ` trt at acm dot org
2005-04-29 20:20 ` trt at acm dot org
[not found] <bug-16302-4397@http.gcc.gnu.org/bugzilla/>
2007-02-13 16:55 ` [Bug c/16302] " manu at gcc dot gnu dot org
2009-05-15 20:08 ` manu at gcc dot gnu dot org
2009-05-15 20:10 ` manu at gcc dot gnu dot org
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).