This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH] PR 7776: Warn about if ("abc" < "xyz") ...


The following patch is my proposed solution to PR middle-end/7776
which is an enhancement request that gcc warn about comparisons to
constant string literals.  The relevant sections of the C99 standard
appear to be 6.4.5p6, 6.5.2.5p8, 6.5.2.5p14 and Annex J5.5, which
pretty much allow a conforming compiler to do anything.  i.e. the
condition if ("foo" == "foo") invokes unspecified behaviour and may
even produce different results based upon command line options, such
as (the now deprecated) -fwriteable-strings.

The approach below is to introduce a new command line option (following
current practices), that is enabled by -Wall but off by default.  This
is an idiom allowed by the standards but that may have portability
problems between conforming compilers, and as shown in the PR may also
be the source of logic problems to inexperienced C programmers.  As
mentioned in the PR, this idiom may appear in macros, but (i) these
macros are inherently unsafe/non-portable, and (ii) any system header
that requires the use of GCC, should also be aware of the local variable
extensions that can be used to avoid duplicating a string literal in
these cases.

>From the little research I did, this would appear to be the first use
of a non-zero first argument to warning that was controlled by -Wall,
so I added a total of four test cases to ensure things were working
the way I expected them to.

I propose a similar follow-up patch for the C++ front-end (I can't
fathom why this is marked as a middle-end PR, and the middle-end
shouldn't be emitting these kinds of diagnostics!).  Unfortunately,
the possible complication of operator overloading in C++, meant that
I'd test the waters with the C front-end first.

The following patch has been tested on i686-pc-linux-gnu with a full
"make bootstrap", all default languages, and regression tested with a
top-level "make -k check" with no new failures.

Ok for mainline?



2005-06-05  Roger Sayle  <roger@eyesopen.com>

	PR middle-end/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.

	* 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.


Index: common.opt
===================================================================
RCS file: /cvs/gcc/gcc/gcc/common.opt,v
retrieving revision 1.73
diff -c -3 -p -r1.73 common.opt
*** common.opt	4 Jun 2005 17:07:55 -0000	1.73
--- common.opt	5 Jun 2005 00:08:59 -0000
*************** Wstrict-aliasing=
*** 117,122 ****
--- 117,126 ----
  Common Joined UInteger
  Warn about code which might break strict aliasing rules

+ Wstring-literal-comparison
+ Common Var(warn_string_literal_comparison)
+ Warn about comparisons to constant string literals
+
  Wswitch
  Common Var(warn_switch)
  Warn about enumerated switches, with no default, missing a case
Index: c-opts.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/c-opts.c,v
retrieving revision 1.146
diff -c -3 -p -r1.146 c-opts.c
*** c-opts.c	25 May 2005 03:58:55 -0000	1.146
--- c-opts.c	5 Jun 2005 00:08:59 -0000
*************** c_common_handle_option (size_t scode, co
*** 370,375 ****
--- 370,376 ----
  	warn_sign_compare = value;
        warn_switch = value;
        warn_strict_aliasing = value;
+       warn_string_literal_comparison = value;

        /* Only warn about unknown pragmas that are not in system
  	 headers.  */
Index: c-typeck.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/c-typeck.c,v
retrieving revision 1.448
diff -c -3 -p -r1.448 c-typeck.c
*** c-typeck.c	4 Jun 2005 01:34:44 -0000	1.448
--- c-typeck.c	5 Jun 2005 00:09:01 -0000
*************** parser_build_binary_op (enum tree_code c
*** 2412,2417 ****
--- 2412,2423 ----

      }

+   if (TREE_CODE_CLASS (code) == tcc_comparison
+       && (TREE_CODE (arg1.value) == STRING_CST
+ 	  || TREE_CODE (arg2.value) == STRING_CST))
+     warning (OPT_Wstring_literal_comparison,
+ 	     "comparison with string literal");
+
    unsigned_conversion_warning (result.value, arg1.value);
    unsigned_conversion_warning (result.value, arg2.value);
    overflow_warning (result.value);


/* PR middle-end/7776 */
/* { dg-do compile } */
/* { dg-options "-Wstring-literal-comparison" } */

int foo(char *ptr)
{
  return ptr == "foo";  /* { dg-warning "comparison with string" } */
}


/* PR middle-end/7776 */
/* { dg-do compile } */
/* { dg-options "-Wall" } */

int foo(char *ptr)
{
  return ptr == "foo";  /* { dg-warning "comparison with string" } */
}


/* PR middle-end/7776 */
/* { dg-do compile } */
/* { dg-options "" } */

int foo(char *ptr)
{
  return ptr == "foo";
}


/* PR middle-end/7776 */
/* { dg-do compile } */
/* { dg-options "-Wall -Wno-string-literal-comparison" } */

int foo(char *ptr)
{
  return ptr == "foo";
}


Roger
--


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]