diff mbox

PR preprocessor/64803 - __LINE__ inside macro is not constant

Message ID 86a90w1ikx.fsf@redhat.com
State New
Headers show

Commit Message

Dodji Seketeli Feb. 2, 2015, 2:41 p.m. UTC
Jakub Jelinek <jakub@redhat.com> writes:

> On Fri, Jan 30, 2015 at 10:19:26AM +0100, Dodji Seketeli wrote:
>> [This is a P1 regression for gcc 5]
>> libcpp/ChangeLog:
>> 
>> 	* internal.h (cpp_reader::top_most_macro_node): New data member.
>> 	* macro.c (enter_macro_context): Pass the location of the end of
>> 	the top-most invocation of the function-like macro, or the
>> 	location of the expansion point of the top-most object-like macro.
>> 	(cpp_get_token_1): Store the top-most macro node in the new
>> 	pfile->top_most_macro_node data member.
>
> The thing that worries me a little bit on the patch is that
> the new field is never cleared, only overwritten next time we attempt to
> expand a function-like? toplevel macro.  So outside of that it can be stale,
> point to a dead memory.  But if it is guaranteed it won't be accessed in
> that case, perhaps that is safe.

Yes, that is correct.  I didn't worry too much myself because
cpp_reader::top_most_macro_node has the same validity span as
cpp_reader::invocation.  But then, unlike top_most_macro_node,
cpp_reader::invocation is not a pointer, so it's rather harmless.

More precisely pfile->top_most_macro_node is (for now) only accessed
from within enter_macro_context; and there, normally,
pfile->top_most_macro_node is set.

But then I agree that we'd rather be safe than sorry.  So I have updated
the patch to clear that data member when the context of the top most
macro being expanded is popped.

I have just lightly tested it locally but a proper bootstrap & test is
currently underway.  Below is the patch I am currently bootstrapping.

libcpp/ChangeLog:

	* internal.h (cpp_reader::top_most_macro_node): New data member.
	* macro.c (enter_macro_context): Pass the location of the end of
	the top-most invocation of the function-like macro, or the
	location of the expansion point of the top-most object-like macro.
	(cpp_get_token_1): Store the top-most macro node in the new
	pfile->top_most_macro_node data member.
	(_cpp_pop_context): Clear the new cpp_reader::top_most_macro_node
	data member.

gcc/testsuite/ChangeLog:

	* gcc.dg/cpp/builtin-macro-1.c: New test case.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
---
 gcc/testsuite/gcc.dg/cpp/builtin-macro-1.c | 28 ++++++++++++++++++++++++++
 libcpp/internal.h                          |  5 +++++
 libcpp/macro.c                             | 32 +++++++++++++++++++++++++++---
 3 files changed, 62 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/cpp/builtin-macro-1.c

Comments

Jakub Jelinek Feb. 2, 2015, 8:37 p.m. UTC | #1
On Mon, Feb 02, 2015 at 03:41:50PM +0100, Dodji Seketeli wrote:
> libcpp/ChangeLog:
> 
> 	* internal.h (cpp_reader::top_most_macro_node): New data member.
> 	* macro.c (enter_macro_context): Pass the location of the end of
> 	the top-most invocation of the function-like macro, or the
> 	location of the expansion point of the top-most object-like macro.
> 	(cpp_get_token_1): Store the top-most macro node in the new
> 	pfile->top_most_macro_node data member.
> 	(_cpp_pop_context): Clear the new cpp_reader::top_most_macro_node
> 	data member.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* gcc.dg/cpp/builtin-macro-1.c: New test case.

Ok, thanks.

	Jakub
diff mbox

Patch

diff --git a/gcc/testsuite/gcc.dg/cpp/builtin-macro-1.c b/gcc/testsuite/gcc.dg/cpp/builtin-macro-1.c
new file mode 100644
index 0000000..90c2883
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/cpp/builtin-macro-1.c
@@ -0,0 +1,28 @@ 
+/* Origin PR preprocessor/64803
+
+   This test ensures that the value the __LINE__ macro expands to is
+   constant and corresponds to the line of the closing parenthesis of
+   the top-most function-like macro expansion it's part of.
+
+   { dg-do run }
+   { do-options -no-integrated-cpp }  */
+
+#include <assert.h>
+
+#define C(a, b) a ## b
+#define L(x) C(L, x)
+#define M(a) int L(__LINE__) = __LINE__; assert(L(__LINE__) == __LINE__);
+
+int
+main()
+{
+  M(a
+    );
+
+  assert(L20 == 20);		/* 20 is the line number of the
+				   closing parenthesis of the
+				   invocation of the M macro.  Please
+				   adjust in case the layout of this
+				   file changes.  */
+  return 0;
+}
diff --git a/libcpp/internal.h b/libcpp/internal.h
index 1a74020..96ccc19 100644
--- a/libcpp/internal.h
+++ b/libcpp/internal.h
@@ -421,6 +421,11 @@  struct cpp_reader
      macro invocation.  */
   source_location invocation_location;
 
+  /* This is the node representing the macro being expanded at
+     top-level.  The value of this data member is valid iff
+     in_macro_expansion_p() returns TRUE.  */
+  cpp_hashnode *top_most_macro_node;
+
   /* Nonzero if we are about to expand a macro.  Note that if we are
      really expanding a macro, the function macro_of_context returns
      the macro being expanded and this flag is set to false.  Client
diff --git a/libcpp/macro.c b/libcpp/macro.c
index 9571345..90ed11a 100644
--- a/libcpp/macro.c
+++ b/libcpp/macro.c
@@ -1228,7 +1228,24 @@  enter_macro_context (cpp_reader *pfile, cpp_hashnode *node,
 
   pfile->about_to_expand_macro_p = false;
   /* Handle built-in macros and the _Pragma operator.  */
-  return builtin_macro (pfile, node, location);
+  {
+    source_location loc;
+    if (/* The top-level macro invocation that triggered the expansion
+	   we are looking at is with a standard macro ...*/
+	!(pfile->top_most_macro_node->flags & NODE_BUILTIN)
+	/* ... and it's a function-like macro invocation.  */
+	&& pfile->top_most_macro_node->value.macro->fun_like)
+      /* Then the location of the end of the macro invocation is the
+	 location of the closing parenthesis.  */
+      loc = pfile->cur_token[-1].src_loc;
+    else
+      /* Otherwise, the location of the end of the macro invocation is
+	 the location of the expansion point of that top-level macro
+	 invocation.  */
+      loc = location;
+
+    return builtin_macro (pfile, node, loc);
+  }
 }
 
 /* De-allocate the memory used by BUFF which is an array of instances
@@ -2296,6 +2313,11 @@  _cpp_pop_context (cpp_reader *pfile)
 	     macro expansion.  */
 	  && macro_of_context (context->prev) != macro)
 	macro->flags &= ~NODE_DISABLED;
+
+      if (macro == pfile->top_most_macro_node
+	  && macro_of_context (context->prev) != macro)
+	/* We are poping the top-most macro node.   */
+	pfile->top_most_macro_node = NULL;
     }
 
   if (context->buff)
@@ -2460,9 +2482,13 @@  cpp_get_token_1 (cpp_reader *pfile, source_location *location)
 	{
 	  int ret = 0;
 	  /* If not in a macro context, and we're going to start an
-	     expansion, record the location.  */
+	     expansion, record the location and the top level macro
+	     about to be expanded.  */
 	  if (!in_macro_expansion_p (pfile))
-	    pfile->invocation_location = result->src_loc;
+	    {
+	      pfile->invocation_location = result->src_loc;
+	      pfile->top_most_macro_node = node;
+	    }
 	  if (pfile->state.prevent_expansion)
 	    break;