User account creation filtered due to spam.

Bug 7776 - const char* p = "foo"; if (p == "foo") ... is compiled without warning!
Summary: const char* p = "foo"; if (p == "foo") ... is compiled without warning!
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: c (show other bugs)
Version: 3.0.3
: P3 enhancement
Target Milestone: 4.2.0
Assignee: Not yet assigned to anyone
URL:
Keywords: diagnostic, patch
Depends on:
Blocks:
 
Reported: 2002-08-30 09:06 UTC by udbz
Modified: 2005-12-06 15:39 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2005-09-24 16:09:09


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description udbz 2002-08-30 09:06:00 UTC
The code construct below IMHO never makes sense and
should spill a warning.

It once caused me two months of work to discover this little
bug in the wine sources.

Release:
gcc-3.0.3

How-To-Repeat:
#include <stdlib.h>
#include <stdio.h>

int main(int argc, const char** argv)
{
	const char * p = "Hello";

	if (p == "Hello") {
		fprintf(stderr, "1\n");
	} else {
		fprintf(stderr, "2\n");
	}
	return 0;
}
Comment 1 Andrew Pinski 2002-08-30 13:35:21 UTC
From: Andrew Pinski <pinskia@physics.uc.edu>
To: udbz@rz.uni-karlsruhe.de
Cc: gcc-gnats@gcc.gnu.org
Subject: Re: c/7776: const char* p = "foo"; if (p == "foo") ... is compiled without warning!
Date: Fri, 30 Aug 2002 13:35:21 -0400

 Are you asking for a warning so that the warning would suggest strcmp?
 The code you provide does make sense to check for if the two occurrences
 of "foo" are the same because the C standard does not say if they have
 to be the same.
 
 Thanks,
 Andrew Pinski
 

Comment 2 udbz 2002-08-30 19:57:40 UTC
From: Peter Schlaile <udbz@rz.uni-karlsruhe.de>
To: Andrew Pinski <pinskia@physics.uc.edu>
Cc: gcc-gnats@gcc.gnu.org
Subject: Re: c/7776: const char* p = "foo"; if (p == "foo") ... is compiled
 without warning!
Date: Fri, 30 Aug 2002 19:57:40 +0200 (CEST)

 Hi,
 
 On Fri, 30 Aug 2002, Andrew Pinski wrote:
 
 > Are you asking for a warning so that the warning would suggest strcmp?
 
 That was the intended idea, yes!
 
 > The code you provide does make sense to check for if the two occurrences
 > of "foo" are the same because the C standard does not say if they have
 > to be the same.
 
 Yup!
 
 Greetings,
 Peter
 
 ---
 Peter Schlaile  ***  eMail udbz@rz.uni-karlsruhe.de
 
 So Linus, what are we doing tonight?
           The same thing we do every night Tux. Try to take over the world!
Comment 3 Andrew Pinski 2003-05-24 14:08:34 UTC
confirmed on mainline (20030524).
Should there be a warning on comparing a char* to string constant, suggesting strcmp?
Comment 4 udbz 2003-05-24 18:43:23 UTC
Subject: Re: [Bug c/7776] const char* p = "foo"; if (p == "foo") ... is
 compiled without warning!

Hi,

On 24 May 2003, pinskia@physics.uc.edu wrote:

> ------- Additional Comments From pinskia@physics.uc.edu  2003-05-24 14:08 -------
> confirmed on mainline (20030524).
> Should there be a warning on comparing a char* to string constant,
> suggesting strcmp?

Yes, that was the idea.

Greetings,
Peter

---
Peter Schlaile  ***  eMail udbz@rz.uni-karlsruhe.de

So Linus, what are we doing tonight?
          The same thing we do every night Tux. Try to take over the world!

Comment 5 falk.hueffner 2003-05-25 00:58:10 UTC
Subject: Re: [Bug c/7776] const char* p = "foo"; if (p == "foo") ... is compiled without warning!

"udbz@rz.uni-karlsruhe.de" <gcc-bugzilla@gcc.gnu.org> writes:

> On 24 May 2003, pinskia@physics.uc.edu wrote:
> 
> > confirmed on mainline (20030524).
> > Should there be a warning on comparing a char* to string constant,
> > suggesting strcmp?
> 
> Yes, that was the idea.

Seems like a reasonable idea to me; I don't see any legitimate reasons
for comparisons with string literals. How about something like this?

Index: fold-const.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/fold-const.c,v
retrieving revision 1.252
diff -u -p -r1.252 fold-const.c
--- fold-const.c        23 May 2003 03:46:52 -0000      1.252
+++ fold-const.c        25 May 2003 00:56:19 -0000
@@ -7163,6 +7163,12 @@ fold (expr)
            }
        }
 
+      /* Warn about things like if (str == "foo").  */
+      if ((code == EQ_EXPR || code == NE_EXPR)
+         && TREE_CODE (arg1) == ADDR_EXPR
+         && TREE_CODE (TREE_OPERAND (arg1, 0)) == STRING_CST)
+       warning("comparison with string literal");
+
       /* If this is a comparison of a field, we may be able to simplify it.  */
       if (((TREE_CODE (arg0) == COMPONENT_REF
            && (*lang_hooks.can_use_bit_fields_p) ())

Comment 6 Joseph S. Myers 2003-05-25 01:29:25 UTC
Subject: Re: [Bug c/7776] const char* p = "foo"; if (p == "foo") ... is
 compiled without warning!

On Sun, 25 May 2003, Falk Hueffner wrote:

> Seems like a reasonable idea to me; I don't see any legitimate reasons
> for comparisons with string literals. How about something like this?

E.g., an optimised string function macro that first compares the pointers
(which might be to string constants) before looking at contents only if
necessary.  glibc headers have contained funny expressions that expect 
multiple copies of a string constant argument to be the same, and 
differences such as &"foo"[1] - "foo" to be folded, if not actually 
evaluating "foo" == "foo".  If you know that the implementation merges 
identical string literals, this is a reasonable thing to do.

I don't believe this should be a mandatory warning - in general those (as 
opposed to mandatory pedwarns) should be avoided; a warning within -Wall 
would be the natural place, except that the possibility of this arising in 
conjunction with macros may mean -W is more appropriate.

Comment 7 roger 2005-06-06 15:06:35 UTC
C front-end patch here: http://gcc.gnu.org/ml/gcc-patches/2005-06/msg00369.html
Comment 8 Tom Truscott 2005-06-06 15:48:54 UTC
I recommend a version with fewer false positives.
I've been using a warning like this for years, with zero false positives.
The current gcc-4-ified version is:

-   /* check for comparing string constant with anything besides simple zero */
-   if (TREE_CODE_CLASS (code) == tcc_comparison && extra_warnings
-       && (code1 == STRING_CST) != (code2 == STRING_CST)
-       && !integer_zerop (arg1.value) && !integer_zerop (arg2.value))
-     warning (0, "comparison of pointer with string literal");

An older suggestion is http://gcc.gnu.org/ml/gcc-patches/1999-10n/msg00548.html
Comment 9 roger 2005-06-19 01:49:26 UTC
A revised patch, allowing equality and inequality comparisons against NULL, yet
retaining warnings for things like 'if ("foo" > 0)' and 'if ("foo" == "bar")'
was posted here:  http://gcc.gnu.org/ml/gcc-patches/2005-06/msg01177.html
Comment 10 Pawel Sikora 2005-06-24 13:44:57 UTC
(In reply to comment #9)  
> A revised patch, allowing equality and inequality comparisons against NULL,  
yet  
> retaining warnings for things like 'if ("foo" > 0)' and 'if ("foo" ==  
"bar")'  
> was posted here:  http://gcc.gnu.org/ml/gcc-patches/2005-06/msg01177.html  
>   
  
with this patch I get an ice at amd64:  
  
(..)  
-c ../../gcc/unwind-dw2.c -o libgcc/./unwind-dw2.o  
  
In file included from ../../gcc/unwind-dw2.c:256:  
../../gcc/config/i386/linux-unwind.h:  
In function 'x86_64_fallback_frame_state': 
../../gcc/config/i386/linux-unwind.h:55: warning: dereferencing type-punned 
                                   pointer will break strict-aliasing rules  
../../gcc/unwind.inc: In function '_Unwind_ForcedUnwind': 
../../gcc/unwind.inc:215: internal compiler error: in create_pre_exit, 
                                                   at mode-switching.c:350  
 
Comment 11 Pawel Sikora 2005-06-24 13:50:53 UTC
(In reply to comment #10) 
> (In reply to comment #9)   
> > A revised patch, allowing equality and inequality comparisons against 
NULL,   
> yet   
> > retaining warnings for things like 'if ("foo" > 0)' and 'if ("foo" ==   
> "bar")'   
> > was posted here:  http://gcc.gnu.org/ml/gcc-patches/2005-06/msg01177.html   
> >    
>    
> with this patch I get an ice at amd64:   
>    
> (..)   
> -c ../../gcc/unwind-dw2.c -o libgcc/./unwind-dw2.o   
>    
> In file included from ../../gcc/unwind-dw2.c:256:   
> ../../gcc/config/i386/linux-unwind.h:   
> In function 'x86_64_fallback_frame_state':  
> ../../gcc/config/i386/linux-unwind.h:55: warning: dereferencing type-punned  
>                                    pointer will break strict-aliasing rules   
> ../../gcc/unwind.inc: In function '_Unwind_ForcedUnwind':  
> ../../gcc/unwind.inc:215: internal compiler error: in create_pre_exit,  
>                                                    at mode-switching.c:350   
>   
 
oops, sorry. i've posted above in wrong PR. 
Comment 12 Roger Sayle 2005-12-04 19:56:50 UTC
Subject: Bug 7776

Author: sayle
Date: Sun Dec  4 19:56:47 2005
New Revision: 108018

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=108018
Log:

	PR c/7776
	* common.opt (Wstring-literal-comparison): New command line option.
	* c-opts.c (c_common_handle_option): Set it with -Wall.
	* c-typeck.c (parser_build_binary_op): Issue warning if either
	operand of a comparison operator is a string literal, except for
	testing equality or inequality against NULL.

	* doc/invoke.texi: Document new -Wstring-literal-comparison option.

	* gcc.dg/Wstring-literal-comparison-1.c: New test case.
	* gcc.dg/Wstring-literal-comparison-2.c: Likewise.
	* gcc.dg/Wstring-literal-comparison-3.c: Likewise.
	* gcc.dg/Wstring-literal-comparison-4.c: Likewise.


Added:
    trunk/gcc/testsuite/gcc.dg/Wstring-literal-comparison-1.c
    trunk/gcc/testsuite/gcc.dg/Wstring-literal-comparison-2.c
    trunk/gcc/testsuite/gcc.dg/Wstring-literal-comparison-3.c
    trunk/gcc/testsuite/gcc.dg/Wstring-literal-comparison-4.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/c-opts.c
    trunk/gcc/c-typeck.c
    trunk/gcc/common.opt
    trunk/gcc/testsuite/ChangeLog

Comment 13 Roger Sayle 2005-12-04 19:58:40 UTC
Subject: Bug 7776

Author: sayle
Date: Sun Dec  4 19:58:37 2005
New Revision: 108019

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=108019
Log:

	PR c/7776
	* doc/invoke.texi: Document new -Wstring-literal-comparison option.


Modified:
    trunk/gcc/doc/invoke.texi

Comment 14 roger 2005-12-06 15:39:40 UTC
Fixed for gcc v4.2