diff mbox

Patches to enable -ftrack-macro-expansion by default

Message ID m3397k450e.fsf@redhat.com
State New
Headers show

Commit Message

Dodji Seketeli May 1, 2012, 9:36 a.m. UTC 
Uros Bizjak <ubizjak@gmail.com> writes:

> Your patch introduced:
>
> FAIL: gcc.dg/gomp/macro-4.c (internal compiler error)
> FAIL: gcc.dg/gomp/macro-4.c (test for excess errors)
>
> on alphaev68-pc-linux-gnu. The failure is silent on
> x86_64-pc-linux-gnu, but can be uncovered by valgrind:
>
> $ valgrind ~/gcc-build/gcc/cc1 -fopenmp -Wunknown-pragmas macro-4.c
>
> ==2461== Conditional jump or move depends on uninitialised value(s)
> ==2461==    at 0xE2B9D2: _cpp_pop_context (macro.c:2143)
> ==2461==    by 0xE1B1EF: skip_rest_of_line(cpp_reader*) (directives.c:207)
> ==2461==    by 0xE1BBFE: end_directive(cpp_reader*, int)
> (directives.c:298)

I see.  Thank you for raising this, and sorry for the inconvenience.

I have a patchlet that seems to make the Valgrind error go away for me
on x86_64-unknown-linux-gnu.  The bootstrap is still underway.

Could you please test that it address the issue for you on
alphaev68-pc-linux-gnu?

Here is the patch, with its introductory text.  Thanks.

From 3e8a8ff118d6fbf038802348bac51ce1dbd26647 Mon Sep 17 00:00:00 2001
From: Dodji Seketeli <dodji@redhat.com>
Date: Tue, 1 May 2012 11:14:45 +0200
Subject: [PATCH] Properly initialize cpp_context in destringize_and_run

destringize_and_run forgets to initialize all the fields of the
cpp_context that it pushes.  Later _cpp_pop_context then gets confused
when it accesses context->tokens_kind via the call to macro_of_context
on context->prev.

The first hunk of this patch is the real obvious fix.  The second hunk
is just an assert that I am adding to err on the safe side.

Tested on x86_64-unknown-linux-gnu against trunk by running the
test gcc.dg/gomp/macro-4.c under Valgrind.  Bootstrap is still
underway.

libcpp/

	* directives.c (destringize_and_run): Properly initialize the new
	context.
	* macro.c (_cpp_pop_context): Assert that we shouldn't try to pop
	the initial base context, which has the same life time as the
	current instance of cpp_file.
---
 libcpp/directives.c |    4 +---
 libcpp/macro.c      |    4 ++++
 2 files changed, 5 insertions(+), 3 deletions(-)

Comments

Dodji Seketeli May 1, 2012, 10:44 a.m. UTC | #1 
Dodji Seketeli <dodji@redhat.com> writes:

> Tested on x86_64-unknown-linux-gnu against trunk by running the
> test gcc.dg/gomp/macro-4.c under Valgrind.  Bootstrap is still
> underway.

For what it's worth, the patch passes bootstrap on
x86_64-unknown-linux-gnu.

> libcpp/
>
> 	* directives.c (destringize_and_run): Properly initialize the new
> 	context.
> 	* macro.c (_cpp_pop_context): Assert that we shouldn't try to pop
> 	the initial base context, which has the same life time as the
> 	current instance of cpp_file.
Jason Merrill May 1, 2012, 12:52 p.m. UTC | #2 
On 05/01/2012 05:36 AM, Dodji Seketeli wrote:
>     pfile->context = XNEW (cpp_context);
> +  memset (pfile->context, 0, sizeof (cpp_context));

You can use XCNEW instead of XNEW + memset.

OK with that change.

Jason
diff mbox

Patch

diff --git a/libcpp/directives.c b/libcpp/directives.c
index 0510c6e..c1c2415 100644
--- a/libcpp/directives.c
+++ b/libcpp/directives.c
@@ -1742,9 +1742,7 @@  destringize_and_run (cpp_reader *pfile, const cpp_string *in)
   saved_cur_run = pfile->cur_run;
 
   pfile->context = XNEW (cpp_context);
-  pfile->context->c.macro = 0;
-  pfile->context->prev = 0;
-  pfile->context->next = 0;
+  memset (pfile->context, 0, sizeof (cpp_context));
 
   /* Inline run_directive, since we need to delay the _cpp_pop_buffer
      until we've read all of the tokens that we want.  */
diff --git a/libcpp/macro.c b/libcpp/macro.c
index ab3e8f6..c4e2a23 100644
--- a/libcpp/macro.c
+++ b/libcpp/macro.c
@@ -2152,6 +2152,10 @@  _cpp_pop_context (cpp_reader *pfile)
 {
   cpp_context *context = pfile->context;
 
+  /* We should not be popping the base context.  */
+  if (context == &pfile->base_context)
+    abort ();
+
   if (context->c.macro)
     {
       cpp_hashnode *macro;