[PATCH] Fix -traditional-cpp preprocessing (PR preprocessor/39213)

[PATCH] Fix -traditional-cpp preprocessing (PR preprocessor/39213)

[Jakub Jelinek]

Hi!

When doing normal preprocessing, the _cpp_process_line_notes call
in _cpp_lex_direct is guarded by !pfile->overlaid_buffer, which I guess
is to avoid comparing unrelated pointers - the .pos field is from the
normal buffer, while cur if pfile->overlaid_buffer is from the overlay,
thus depending on whether that overlay buffer has been allocated above or
below the normal buffer in the address space it will either work or can
crash.

This patch adds the same test to traditional preprocessing.  I've
bootstrapped/regtested it on x86_64-linux and i686-linux and tries to
preprocess a couple of large files with -traditional-cpp without any
difference, but I can't reproduce the original problem.  CCing
Eric who has managed to reproduce it on Solaris.

Ok for trunk?

2010-12-17  Jakub Jelinek  <jakub@redhat.com>

	PR preprocessor/39213
	* traditional.c (_cpp_scan_out_logical_line): Don't call
	_cpp_process_line_notes if pfile->overlaid_buffer.

--- libcpp/traditional.c.jj	2009-04-14 16:35:21.000000000 +0200
+++ libcpp/traditional.c	2010-12-17 11:34:18.000000000 +0100
@@ -1,5 +1,5 @@
 /* CPP Library - traditional lexical analysis and macro expansion.
-   Copyright (C) 2002, 2004, 2005, 2007, 2008, 2009
+   Copyright (C) 2002, 2004, 2005, 2007, 2008, 2009, 2010
    Free Software Foundation, Inc.
    Contributed by Neil Booth, May 2002
 
@@ -378,7 +378,8 @@ _cpp_scan_out_logical_line (cpp_reader *
   for (;;)
     {
       if (!context->prev
-	  && cur >= pfile->buffer->notes[pfile->buffer->cur_note].pos)
+	  && cur >= pfile->buffer->notes[pfile->buffer->cur_note].pos
+	  && !pfile->overlaid_buffer)
 	{
 	  pfile->buffer->cur = cur;
 	  _cpp_process_line_notes (pfile, false);

	Jakub

Re: [PATCH] Fix -traditional-cpp preprocessing (PR preprocessor/39213)

[Eric Botcazou]

> This patch adds the same test to traditional preprocessing.  I've
> bootstrapped/regtested it on x86_64-linux and i686-linux and tries to
> preprocess a couple of large files with -traditional-cpp without any
> difference, but I can't reproduce the original problem.  CCing
> Eric who has managed to reproduce it on Solaris.

I replied earlier today under the PR though, the patch doesn't seem to work 
for me.  I'm ready to dig further of course.

-- 
Eric Botcazou

Re: [PATCH] Fix -traditional-cpp preprocessing (PR preprocessor/39213)

[Eric Botcazou]

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

> This patch adds the same test to traditional preprocessing.  I've
> bootstrapped/regtested it on x86_64-linux and i686-linux and tries to
> preprocess a couple of large files with -traditional-cpp without any
> difference, but I can't reproduce the original problem.  CCing
> Eric who has managed to reproduce it on Solaris.

As explained in the audit trail, the patch doesn't work because calls to 
_cpp_overlay_buffer and _cpp_remove_overlay are unbalanced in the presence of 
#pragma redefine_extname, so the attached hunk is also needed.  In fact, it 
is even sufficient to fix the problem, without the traditional.c hunk.

Tested on sparc64-sun-solaris2.9 and i586-suse-linux.  OK for mainline and 
4.5/4.4 branches?  If so, which version?


2010-12-19  Eric Botcazou  <ebotcazou@adacore.com>
            Jakub Jelinek  <jakub@redhat.com>

        PR preprocessor/39213
	* directives.c (end_directive): Call _cpp_remove_overlay for deferred
	pragmas as well in traditional mode.
        * traditional.c (_cpp_scan_out_logical_line): Don't call
        _cpp_process_line_notes if pfile->overlaid_buffer.


-- 
Eric Botcazou

[-- Attachment #2: pr39213.diff --]
[-- Type: text/x-diff, Size: 1587 bytes --]

Index: directives.c
===================================================================
--- directives.c	(revision 167901)
+++ directives.c	(working copy)
@@ -281,16 +281,17 @@ start_directive (cpp_reader *pfile)
 static void
 end_directive (cpp_reader *pfile, int skip_line)
 {
-  if (pfile->state.in_deferred_pragma)
-    ;
-  else if (CPP_OPTION (pfile, traditional))
+  if (CPP_OPTION (pfile, traditional))
     {
       /* Revert change of prepare_directive_trad.  */
-      pfile->state.prevent_expansion--;
+      if (!pfile->state.in_deferred_pragma)
+	pfile->state.prevent_expansion--;
 
       if (pfile->directive != &dtable[T_DEFINE])
 	_cpp_remove_overlay (pfile);
     }
+  else if (pfile->state.in_deferred_pragma)
+    ;
   /* We don't skip for an assembler #.  */
   else if (skip_line)
     {
Index: traditional.c
===================================================================
--- traditional.c	(revision 167901)
+++ traditional.c	(working copy)
@@ -1,5 +1,5 @@
 /* CPP Library - traditional lexical analysis and macro expansion.
-   Copyright (C) 2002, 2004, 2005, 2007, 2008, 2009
+   Copyright (C) 2002, 2004, 2005, 2007, 2008, 2009, 2010
    Free Software Foundation, Inc.
    Contributed by Neil Booth, May 2002
 
@@ -378,7 +378,8 @@ _cpp_scan_out_logical_line (cpp_reader *
   for (;;)
     {
       if (!context->prev
-	  && cur >= pfile->buffer->notes[pfile->buffer->cur_note].pos)
+	  && cur >= pfile->buffer->notes[pfile->buffer->cur_note].pos
+	  && !pfile->overlaid_buffer)
 	{
 	  pfile->buffer->cur = cur;
 	  _cpp_process_line_notes (pfile, false);

Re: [PATCH] Fix -traditional-cpp preprocessing (PR preprocessor/39213)

[Tom Tromey]

>>>>> "Eric" == Eric Botcazou <ebotcazou@adacore.com> writes:

Eric> As explained in the audit trail, the patch doesn't work because
Eric> calls to _cpp_overlay_buffer and _cpp_remove_overlay are
Eric> unbalanced in the presence of #pragma redefine_extname, so the
Eric> attached hunk is also needed.  In fact, it is even sufficient to
Eric> fix the problem, without the traditional.c hunk.

Eric> Tested on sparc64-sun-solaris2.9 and i586-suse-linux.  OK for
Eric> mainline and 4.5/4.4 branches?  If so, which version?

This is ok.  I think it is fine to commit the version without the
traditional.c change.

Tom

Re: [PATCH] Fix -traditional-cpp preprocessing (PR preprocessor/39213)

[Jakub Jelinek]

On Tue, Jan 04, 2011 at 11:17:35AM -0700, Tom Tromey wrote:
> >>>>> "Eric" == Eric Botcazou <ebotcazou@adacore.com> writes:
> 
> Eric> As explained in the audit trail, the patch doesn't work because
> Eric> calls to _cpp_overlay_buffer and _cpp_remove_overlay are
> Eric> unbalanced in the presence of #pragma redefine_extname, so the
> Eric> attached hunk is also needed.  In fact, it is even sufficient to
> Eric> fix the problem, without the traditional.c hunk.
> 
> Eric> Tested on sparc64-sun-solaris2.9 and i586-suse-linux.  OK for
> Eric> mainline and 4.5/4.4 branches?  If so, which version?
> 
> This is ok.  I think it is fine to commit the version without the
> traditional.c change.

Yeah, the traditional.c change was just a shot in the dark.

	Jakub