[CPP] 28079 #line range not verified without -pedantic

[CPP] 28079 #line range not verified without -pedantic

[Manuel López-Ibáñez]

[-- Attachment #1: Type: text/plain, Size: 575 bytes --]

Currently CPP will silently convert line numbers from unsigned long to
unsigned int. It will also wrap-around line numbers higher than
ULONG_MAX. This patch adds a warning when any of these events occur.

Boostrapped and regression tested on x86_64-unknown-linux-gnu.

2008-02-27  Manuel Lopez-Ibanez  <manu@gcc.gnu.org>

  PR28079
libcpp/
  * directives.c (strtoul_for_line): Add a parameter 'wrapped'.
  (do_line): Update call to strtoul_for_line. Warn if wrapped or if truncated.
  (do_linemarker): Update call to strtoul_for_line.
testsuite/
  * gcc.dg/cpp/line6.c: New.

[-- Attachment #2: fix-pr28079.diff --]
[-- Type: text/plain, Size: 4624 bytes --]

Index: gcc/testsuite/gcc.dg/cpp/line6.c
===================================================================
--- gcc/testsuite/gcc.dg/cpp/line6.c	(revision 0)
+++ gcc/testsuite/gcc.dg/cpp/line6.c	(revision 0)
@@ -0,0 +1,7 @@
+/* PR 28079 */
+/* { dg-do preprocess } */
+/* { dg-options "" } */
+
+#line 18446744073709551616 /* { dg-warning "line number out of range" } */
+
+#line 12312312312435 /* { dg-warning "line number out of range" "" { target *-*-* } 0 } */
Index: libcpp/directives.c
===================================================================
--- libcpp/directives.c	(revision 132503)
+++ libcpp/directives.c	(working copy)
@@ -99,11 +99,12 @@ static void directive_diagnostics (cpp_r
 static void run_directive (cpp_reader *, int, const char *, size_t);
 static char *glue_header_name (cpp_reader *);
 static const char *parse_include (cpp_reader *, int *, const cpp_token ***);
 static void push_conditional (cpp_reader *, int, int, const cpp_hashnode *);
 static unsigned int read_flag (cpp_reader *, unsigned int);
-static int strtoul_for_line (const uchar *, unsigned int, unsigned long *);
+static int strtoul_for_line (const uchar *, unsigned int, unsigned long *, 
+			     bool *wrapped);
 static void do_diagnostic (cpp_reader *, int, int);
 static cpp_hashnode *lex_macro_node (cpp_reader *, bool);
 static int undefine_macros (cpp_reader *, cpp_hashnode *, void *);
 static void do_include_common (cpp_reader *, enum include_type);
 static struct pragma_entry *lookup_pragma_entry (struct pragma_entry *,
@@ -818,23 +819,30 @@ read_flag (cpp_reader *pfile, unsigned i
   return 0;
 }
 
 /* Subroutine of do_line and do_linemarker.  Convert a number in STR,
    of length LEN, to binary; store it in NUMP, and return 0 if the
-   number was well-formed, 1 if not.  Temporary, hopefully.  */
+   number was well-formed, 1 if not. WRAPPED is set to true if the
+   number did not fit into 'unsigned long'.  Temporary, hopefully.  */
 static int
-strtoul_for_line (const uchar *str, unsigned int len, long unsigned int *nump)
+strtoul_for_line (const uchar *str, unsigned int len, long unsigned int *nump, bool *wrapped)
 {
   unsigned long reg = 0;
+  unsigned long reg_prev = 0;
+
   uchar c;
+  *wrapped = false;
   while (len--)
     {
       c = *str++;
       if (!ISDIGIT (c))
-	return 1;
+	return 2;
       reg *= 10;
       reg += c - '0';
+      if (reg < reg_prev) 
+	*wrapped = true;
+      reg_prev = reg;
     }
   *nump = reg;
   return 0;
 }
 
@@ -855,28 +863,31 @@ do_line (cpp_reader *pfile)
   const char *new_file = map->to_file;
   unsigned long new_lineno;
 
   /* C99 raised the minimum limit on #line numbers.  */
   unsigned int cap = CPP_OPTION (pfile, c99) ? 2147483647 : 32767;
+  bool wrapped;
 
   /* #line commands expand macros.  */
   token = cpp_get_token (pfile);
   if (token->type != CPP_NUMBER
       || strtoul_for_line (token->val.str.text, token->val.str.len,
-			   &new_lineno))
+			   &new_lineno, &wrapped))
     {
       if (token->type == CPP_EOF)
 	cpp_error (pfile, CPP_DL_ERROR, "unexpected end of file after #line");
       else
 	cpp_error (pfile, CPP_DL_ERROR,
 		   "\"%s\" after #line is not a positive integer",
 		   cpp_token_as_text (pfile, token));
       return;
     }
 
-  if (CPP_PEDANTIC (pfile) && (new_lineno == 0 || new_lineno > cap))
+  if (CPP_PEDANTIC (pfile) && (new_lineno == 0 || new_lineno > cap || wrapped))
     cpp_error (pfile, CPP_DL_PEDWARN, "line number out of range");
+  else if (wrapped || new_lineno != (unsigned int) new_lineno)
+    cpp_error (pfile, CPP_DL_WARNING, "line number out of range");
 
   token = cpp_get_token (pfile);
   if (token->type == CPP_STRING)
     {
       cpp_string s = { 0, 0 };
@@ -909,21 +920,22 @@ do_linemarker (cpp_reader *pfile)
   const char *new_file = map->to_file;
   unsigned long new_lineno;
   unsigned int new_sysp = map->sysp;
   enum lc_reason reason = LC_RENAME;
   int flag;
+  bool wrapped;
 
   /* Back up so we can get the number again.  Putting this in
      _cpp_handle_directive risks two calls to _cpp_backup_tokens in
      some circumstances, which can segfault.  */
   _cpp_backup_tokens (pfile, 1);
 
   /* #line commands expand macros.  */
   token = cpp_get_token (pfile);
   if (token->type != CPP_NUMBER
       || strtoul_for_line (token->val.str.text, token->val.str.len,
-			   &new_lineno))
+			   &new_lineno, &wrapped))
     {
       /* Unlike #line, there does not seem to be a way to get an EOF
 	 here.  So, it should be safe to always spell the token.  */
       cpp_error (pfile, CPP_DL_ERROR,
 		 "\"%s\" after # is not a positive integer",

Re: [CPP] 28079 #line range not verified without -pedantic

[Mark Mitchell]

Manuel López-Ibáñez wrote:

>   PR28079
> libcpp/
>   * directives.c (strtoul_for_line): Add a parameter 'wrapped'.
>   (do_line): Update call to strtoul_for_line. Warn if wrapped or if truncated.
>   (do_linemarker): Update call to strtoul_for_line.
> testsuite/
>   * gcc.dg/cpp/line6.c: New.
> 

The patch is OK, with minor change below, unless a preprocessor 
maintainer objects within 24 hours.

> +strtoul_for_line (const uchar *str, unsigned int len, long unsigned int *nump, bool 
...
> -	return 1;
> +	return 2;

You didn't document this new possible return value.  I don't see any 
reason to return 2; do you need this change?  If not, please just take 
it out.

It's a shame we don't make better use of typedefs throughout GCC.  A lot 
of these problems would be less likely to occur if we had something like

typedef unsigned long linenum_type;

and then just used that everywhere...

Thanks,

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713

Re: [CPP] 28079 #line range not verified without -pedantic

[Manuel López-Ibáñez]

On 29/02/2008, Mark Mitchell <mark@codesourcery.com> wrote:
> Manuel López-Ibáñez wrote:
>
>  You didn't document this new possible return value.  I don't see any
>  reason to return 2; do you need this change?  If not, please just take
>  it out.

Good catch! That was a typo indeed. Thanks!

>  It's a shame we don't make better use of typedefs throughout GCC.  A lot
>  of these problems would be less likely to occur if we had something like
>
>  typedef unsigned long linenum_type;
>
>  and then just used that everywhere...

Well, I didn't try that just because I assumed that there was a reason
to to use int as the line number, but, with hindsight, that seems
unlikely. I can try to change at least this part of the preprocessor
to use a typedef if the preprocessor maintainers think that is OK.

Cheers,

Manuel.

Re: [CPP] 28079 #line range not verified without -pedantic

[Mark Mitchell]

Manuel López-Ibáñez wrote:

> Well, I didn't try that just because I assumed that there was a reason
> to to use int as the line number, but, with hindsight, that seems
> unlikely. I can try to change at least this part of the preprocessor
> to use a typedef if the preprocessor maintainers think that is OK.

Sure, I didn't mean that you should do it as part of this patch.  It's 
just that -- in various places throughout GCC -- we've run into this 
kind of int vs. unsigned int. vs long vs. long long vs. HOST_WIDE_INT 
stuff, and consistent use of typedefs naming the thing we're trying to 
represent would be a lot clearer. :-)

Thanks,

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713

Re: [CPP] 28079 #line range not verified without -pedantic

[Tom Tromey]

>>>>> "Manuel" == Manuel López-Ibáñez <lopezibanez@gmail.com> writes:

>> typedef unsigned long linenum_type;

Manuel> Well, I didn't try that just because I assumed that there was a reason
Manuel> to to use int as the line number, but, with hindsight, that seems
Manuel> unlikely. I can try to change at least this part of the preprocessor
Manuel> to use a typedef if the preprocessor maintainers think that is OK.

I think that would be great.

Tom