libcpp: suppress builtin macro redefined warnings for __LINE__

Message ID 20221201043155.9369-1-luolongjuna@gmail.com
State New
Headers
Series libcpp: suppress builtin macro redefined warnings for __LINE__ |

Commit Message

Longjun Luo Dec. 1, 2022, 4:31 a.m. UTC
  As implied in
gcc.gnu.org/legacy-ml/gcc-patches/2008-09/msg00076.html,
gcc provides -Wno-builtin-macro-redefined to suppress warning when
redefining builtin macro. However, at that time, there was no
scenario for __LINE__ macro.

But, when we try to build a live-patch, we compare sections by using
-ffunction-sections. Some same functions are considered changed because
of __LINE__ macro.

At present, to detect such a changed caused by __LINE__ macro, we
have to analyse code and maintain a function list. For example,
in kpatch, check this commit
github.com/dynup/kpatch/commit/0e1b95edeafa36edb7bcf11da6d1c00f76d7e03d.

So, in this scenario, when we try to compared sections, it would
be better to support suppress builtin macro redefined warnings for
__LINE__ macro.

Signed-off-by: Longjun Luo <luolongjuna@gmail.com>
---
 gcc/testsuite/gcc.dg/builtin-redefine.c | 1 -
 libcpp/init.cc                          | 2 +-
 2 files changed, 1 insertion(+), 2 deletions(-)
  

Comments

Joseph Myers Dec. 1, 2022, 5:01 p.m. UTC | #1
On Thu, 1 Dec 2022, Longjun Luo via Gcc-patches wrote:

> diff --git a/gcc/testsuite/gcc.dg/builtin-redefine.c b/gcc/testsuite/gcc.dg/builtin-redefine.c
> index 882b2210992..9d5b42252ee 100644
> --- a/gcc/testsuite/gcc.dg/builtin-redefine.c
> +++ b/gcc/testsuite/gcc.dg/builtin-redefine.c
> @@ -71,7 +71,6 @@
>  /* { dg-bogus "Expected built-in is not defined" "" { target *-*-* } .-1 } */
>  #endif
>  
> -#define __LINE__ 0           /* { dg-warning "-:\"__LINE__\" redef" } */
>  #define __INCLUDE_LEVEL__ 0  /* { dg-warning "-:\"__INCLUDE_LEVEL__\" redef" } */
>  #define __COUNTER__ 0        /* { dg-warning "-:\"__COUNTER__\" redef" } */

Is there some existing test that verifies that this redefinition is still 
diagnosed by default (in the absence of -Wno-builtin-macro-redefined)?
  
Longjun Luo Dec. 1, 2022, 6:23 p.m. UTC | #2
On 12/2/2022 1:01 AM, Joseph Myers wrote:
> On Thu, 1 Dec 2022, Longjun Luo via Gcc-patches wrote:
>
>> diff --git a/gcc/testsuite/gcc.dg/builtin-redefine.c b/gcc/testsuite/gcc.dg/builtin-redefine.c
>> index 882b2210992..9d5b42252ee 100644
>> --- a/gcc/testsuite/gcc.dg/builtin-redefine.c
>> +++ b/gcc/testsuite/gcc.dg/builtin-redefine.c
>> @@ -71,7 +71,6 @@
>>   /* { dg-bogus "Expected built-in is not defined" "" { target *-*-* } .-1 } */
>>   #endif
>>   
>> -#define __LINE__ 0           /* { dg-warning "-:\"__LINE__\" redef" } */
>>   #define __INCLUDE_LEVEL__ 0  /* { dg-warning "-:\"__INCLUDE_LEVEL__\" redef" } */
>>   #define __COUNTER__ 0        /* { dg-warning "-:\"__COUNTER__\" redef" } */
> Is there some existing test that verifies that this redefinition is still
> diagnosed by default (in the absence of -Wno-builtin-macro-redefined)?

I am not sure I have fully understood your meaning. The problem here is 
that if I try to redefine __LINE__ macro in the situation that projects 
use the option '-Werror', the compile will fail.

For example, the following compilation will fail:

/echo "void main(){}" | gcc -D__LINE__=0 -Werror -x c -/


The compilation output is:

<command-line>: error: "__LINE__" redefined [-Werror]
cc1: all warnings being treated as errors


As I know, most projects including Linux kernel enable '-Werror' by 
default. So if I try to redefine __LINE__ macro in this situation, it 
will be impossible.

The reason that I want to redefine __LINE__ macro has been explained in 
the commit.

Thanks for your patience and hope I hit the point.
  
Joseph Myers Dec. 1, 2022, 7:07 p.m. UTC | #3
On Fri, 2 Dec 2022, Longjun Luo via Gcc-patches wrote:

> 
> On 12/2/2022 1:01 AM, Joseph Myers wrote:
> > On Thu, 1 Dec 2022, Longjun Luo via Gcc-patches wrote:
> > 
> > > diff --git a/gcc/testsuite/gcc.dg/builtin-redefine.c
> > > b/gcc/testsuite/gcc.dg/builtin-redefine.c
> > > index 882b2210992..9d5b42252ee 100644
> > > --- a/gcc/testsuite/gcc.dg/builtin-redefine.c
> > > +++ b/gcc/testsuite/gcc.dg/builtin-redefine.c
> > > @@ -71,7 +71,6 @@
> > >   /* { dg-bogus "Expected built-in is not defined" "" { target *-*-* } .-1
> > > } */
> > >   #endif
> > >   -#define __LINE__ 0           /* { dg-warning "-:\"__LINE__\" redef" }
> > > */
> > >   #define __INCLUDE_LEVEL__ 0  /* { dg-warning "-:\"__INCLUDE_LEVEL__\"
> > > redef" } */
> > >   #define __COUNTER__ 0        /* { dg-warning "-:\"__COUNTER__\" redef" }
> > > */
> > Is there some existing test that verifies that this redefinition is still
> > diagnosed by default (in the absence of -Wno-builtin-macro-redefined)?
> 
> I am not sure I have fully understood your meaning. The problem here is that
> if I try to redefine __LINE__ macro in the situation that projects use the
> option '-Werror', the compile will fail.

There are two cases:

(a) Is redefinition of __LINE__ diagnosed *without* 
-Wno-builtin-macro-redefined?

(b) Is redefinition of __LINE__ diagnosed *with* 
-Wno-builtin-macro-redefined?

My understanding is that both (a) and (b) have answer "yes" at present, 
and your patch would change the answer to (b) to "no", without changing 
the answer to (a).

My question is about whether there is a test verifying the answer to (a).  
If not, I think the patch should add one.
  
Longjun Luo Dec. 1, 2022, 7:51 p.m. UTC | #4
On 12/2/2022 3:07 AM, Joseph Myers wrote:
> On Fri, 2 Dec 2022, Longjun Luo via Gcc-patches wrote:
>
>> On 12/2/2022 1:01 AM, Joseph Myers wrote:
>>> On Thu, 1 Dec 2022, Longjun Luo via Gcc-patches wrote:
>>>
>>>> diff --git a/gcc/testsuite/gcc.dg/builtin-redefine.c
>>>> b/gcc/testsuite/gcc.dg/builtin-redefine.c
>>>> index 882b2210992..9d5b42252ee 100644
>>>> --- a/gcc/testsuite/gcc.dg/builtin-redefine.c
>>>> +++ b/gcc/testsuite/gcc.dg/builtin-redefine.c
>>>> @@ -71,7 +71,6 @@
>>>>    /* { dg-bogus "Expected built-in is not defined" "" { target *-*-* } .-1
>>>> } */
>>>>    #endif
>>>>    -#define __LINE__ 0           /* { dg-warning "-:\"__LINE__\" redef" }
>>>> */
>>>>    #define __INCLUDE_LEVEL__ 0  /* { dg-warning "-:\"__INCLUDE_LEVEL__\"
>>>> redef" } */
>>>>    #define __COUNTER__ 0        /* { dg-warning "-:\"__COUNTER__\" redef" }
>>>> */
>>> Is there some existing test that verifies that this redefinition is still
>>> diagnosed by default (in the absence of -Wno-builtin-macro-redefined)?
>> I am not sure I have fully understood your meaning. The problem here is that
>> if I try to redefine __LINE__ macro in the situation that projects use the
>> option '-Werror', the compile will fail.
> There are two cases:
>
> (a) Is redefinition of __LINE__ diagnosed *without*
> -Wno-builtin-macro-redefined?
>
> (b) Is redefinition of __LINE__ diagnosed *with*
> -Wno-builtin-macro-redefined?
>
> My understanding is that both (a) and (b) have answer "yes" at present,
> and your patch would change the answer to (b) to "no", without changing
> the answer to (a).
>
> My question is about whether there is a test verifying the answer to (a).
> If not, I think the patch should add one.


After some check for the source code, two similiar exist test cases for 
the situation (a).

They are ./gcc/testsuite/gcc.dg/cpp/warn-redefined.c and 
./gcc/testsuite/gcc.dg/cpp/warn-redefined-2.c

These two cases redefine the __TIME__ macro when using the option 
'-Wbuiltin-macro-redefined'.

I think I shoud add a test to verify __LINE__ macro in these two cases.

I will write a complete test for situation (a) and situation (b). But I 
need a little time to be familar with the gcc testcases.

So, the patch itself has no problem. What I need do is to rich its test 
cases and update change log, right?
  
Joseph Myers Dec. 1, 2022, 9:10 p.m. UTC | #5
On Fri, 2 Dec 2022, Longjun Luo via Gcc-patches wrote:

> They are ./gcc/testsuite/gcc.dg/cpp/warn-redefined.c and
> ./gcc/testsuite/gcc.dg/cpp/warn-redefined-2.c
> 
> These two cases redefine the __TIME__ macro when using the option
> '-Wbuiltin-macro-redefined'.
> 
> I think I shoud add a test to verify __LINE__ macro in these two cases.

I think it should be a test that doesn't use either 
-Wbuiltin-macro-redefined or -Wno-builtin-macro-redefined - a test of how 
the compiler behaves by default.

> So, the patch itself has no problem. What I need do is to rich its test cases
> and update change log, right?

The patch needs review, but I'm fine with the principle that 
-Wno-builtin-macro-redefined should apply to __LINE__ as it does to 
various other built-in macros.
  
Longjun Luo Jan. 12, 2023, 4:02 p.m. UTC | #6
From 0821df518b264e754d698d399f98be1a62945e32 Mon Sep 17 00:00:00 2001
From: Longjun Luo <luolongjuna@gmail.com>
Date: Thu, 12 Jan 2023 23:59:54 +0800
Subject: [PATCH] libcpp: suppress builtin macro redefined warnings for
  __LINE__

As implied in
gcc.gnu.org/legacy-ml/gcc-patches/2008-09/msg00076.html,
gcc provides -Wno-builtin-macro-redefined to suppress warning when
redefining builtin macro. However, at that time, there was no
scenario for __LINE__ macro.

But, when we try to build a live-patch, we compare sections by using
-ffunction-sections. Some same functions are considered changed because
of __LINE__ macro.

At present, to detect such a changed caused by __LINE__ macro, we
have to analyse code and maintain a function list. For example,
in kpatch, check this commit
github.com/dynup/kpatch/commit/0e1b95edeafa36edb7bcf11da6d1c00f76d7e03d.

So, in this scenario, when we try to compared sections, it would
be better to support suppress builtin macro redefined warnings for
__LINE__ macro.

Signed-off-by: Longjun Luo <luolongjuna@gmail.com>
---
  gcc/testsuite/gcc.dg/builtin-redefine-1.c | 49 +++++++++++++++++++++++
  gcc/testsuite/gcc.dg/builtin-redefine.c   | 24 +++++++++--
  libcpp/init.cc                            |  2 +-
  3 files changed, 70 insertions(+), 5 deletions(-)
  create mode 100755 gcc/testsuite/gcc.dg/builtin-redefine-1.c

diff --git a/gcc/testsuite/gcc.dg/builtin-redefine-1.c 
b/gcc/testsuite/gcc.dg/builtin-redefine-1.c
new file mode 100755
index 00000000000..c1e05b4fc7c
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/builtin-redefine-1.c
@@ -0,0 +1,49 @@
+/* Test default warnings for redefining builtin macros.  */
+
+/* { dg-do compile } */
+/* { dg-options "-D__TIMESTAMP__=x -D__TIME__=x -D__DATE__=x 
-D__FILE__=x -D__FILE_NAME__=x -D__BASE_FILE__=x -D__LINE__=0" } */
+
+/* Check default behavior for builtin macros redefinition.  */
+
+/* { dg-message "\"__TIMESTAMP__\" redefined" "" {target "*-*-*"} 0 } */
+#ifndef __TIMESTAMP__
+#error "__TIMESTAMP__ builtin is not defined"
+/* { dg-bogus "Expected built-in is not defined" "" { target *-*-* } 
.-1 } */
+#endif
+
+/* { dg-message "\"__TIME__\" redefined" "" {target "*-*-*"} 0 } */
+#ifndef __TIME__
+#error "__TIME__ builtin is not defined"
+/* { dg-bogus "Expected built-in is not defined" "" { target *-*-* } 
.-1 } */
+#endif
+
+/* { dg-message "\"__DATE__\" redefined" "" {target "*-*-*"} 0 } */
+#ifndef __DATE__
+#error "__DATE__ builtin is not defined"
+/* { dg-bogus "Expected built-in is not defined" "" { target *-*-* } 
.-1 } */
+#endif
+
+/* { dg-message "\"__FILE__\" redefined" "" {target "*-*-*"} 0 } */
+#ifndef __FILE__
+#error "__FILE__ builtin is not defined"
+/* { dg-bogus "Expected built-in is not defined" "" { target *-*-* } 
.-1 } */
+#endif
+
+/* { dg-message "\"__FILE_NAME__\" redefined" "" {target "*-*-*"} 0 } */
+#ifndef __FILE_NAME__
+#error "__FILE_NAME__ builtin is not defined"
+/* { dg-bogus "Expected built-in is not defined" "" { target *-*-* } 
.-1 } */
+#endif
+
+/* { dg-message "\"__BASE_FILE__\" redefined" "" {target "*-*-*"} 0 } */
+#ifndef __BASE_FILE__
+#error "__BASE_FILE__ builtin is not defined"
+/* { dg-bogus "Expected built-in is not defined" "" { target *-*-* } 
.-1 } */
+#endif
+
+/* { dg-message "\"__LINE__\" redefined" "" {target "*-*-*"} 0 } */
+#ifndef __LINE__
+#error "__LINE__ builtin is not defined"
+/* { dg-bogus "Expected built-in is not defined" "" { target *-*-* } 
.-1 } */
+#endif
+
diff --git a/gcc/testsuite/gcc.dg/builtin-redefine.c 
b/gcc/testsuite/gcc.dg/builtin-redefine.c
index 882b2210992..fa27ee9aefc 100644
--- a/gcc/testsuite/gcc.dg/builtin-redefine.c
+++ b/gcc/testsuite/gcc.dg/builtin-redefine.c
@@ -1,9 +1,9 @@
  /* Test -Wno-builtin-macro-redefined warnings.  */

  /* { dg-do compile } */
-/* { dg-options "-Wno-builtin-macro-redefined -U__DATE__ -D__TIME__=X" } */
+/* { dg-options "-Wno-builtin-macro-redefined -U__DATE__ -D__TIME__=X 
-D__LINE__=0" } */

-/* Check date, time, and datestamp built-ins warnings may be 
suppressed.  */
+/* Check date, time, datestamp and line built-ins warnings may be 
suppressed.  */

  #if defined(__DATE__)
  #error "__DATE__ is defined, but should not be (-U command line error)"
@@ -15,6 +15,11 @@
  /* { dg-bogus "__TIME__ is not defined" "" { target *-*-* } .-1 } */
  #endif

+#if __LINE__ != 0
+#error "__LINE__ is not defined as expected (-D command line error)"
+/* { dg-bogus "__LINE__ is not defined" "" { target *-*-* } .-1 } */
+#endif
+
  #if !defined(__TIMESTAMP__)
  #error "__TIMESTAMP__ is not defined (built-in macro expectation error)"
  /* { dg-bogus "__TIMESTAMP__ is not defined" "" { target *-*-* } .-1 } */
@@ -53,6 +58,18 @@
  #undef __TIMESTAMP__         /* Undefine while defined.  */


+#undef __LINE__              /* Undefine while defined.  */
+#undef __LINE__              /* Undefine while already undefined.  */
+
+#define __LINE__ "1"         /* Define while undefined.  */
+#define __LINE__ "1"         /* Re-define while defined.  */ /* { 
dg-line line_prev } */
+
+#define __LINE__ "2"         /* { dg-warning "-:\"__LINE__\" redefined" 
} */
+/* { dg-message "-:previous definition" "" { target *-*-* } line_prev } */
+
+#undef __LINE__              /* Undefine while defined.  */
+
+
  /* Check other built-ins with warnings that may be suppressed.  */

  #if !defined(__FILE__) || !defined(__BASE_FILE__)
@@ -66,12 +83,11 @@

  /* Check selected built-ins not affected by warning suppression. */

-#if !defined(__LINE__) || !defined(__INCLUDE_LEVEL__) || 
!defined(__COUNTER__)
+#if !defined(__INCLUDE_LEVEL__) || !defined(__COUNTER__)
  #error "Expected built-in is not defined (built-in macro expectation 
error)"
  /* { dg-bogus "Expected built-in is not defined" "" { target *-*-* } 
.-1 } */
  #endif

-#define __LINE__ 0           /* { dg-warning "-:\"__LINE__\" redef" } */
  #define __INCLUDE_LEVEL__ 0  /* { dg-warning "-:\"__INCLUDE_LEVEL__\" 
redef" } */
  #define __COUNTER__ 0        /* { dg-warning "-:\"__COUNTER__\" redef" 
} */

diff --git a/libcpp/init.cc b/libcpp/init.cc
index 5f34e3515d2..2765b9838b7 100644
--- a/libcpp/init.cc
+++ b/libcpp/init.cc
@@ -421,7 +421,7 @@ static const struct builtin_macro builtin_array[] =
    B("__FILE__",		 BT_FILE,          false),
    B("__FILE_NAME__",	 BT_FILE_NAME,     false),
    B("__BASE_FILE__",	 BT_BASE_FILE,     false),
-  B("__LINE__",		 BT_SPECLINE,      true),
+  B("__LINE__",		 BT_SPECLINE,      false),
    B("__INCLUDE_LEVEL__", BT_INCLUDE_LEVEL, true),
    B("__COUNTER__",	 BT_COUNTER,       true),
    /* Make sure to update the list of built-in
  
Longjun Luo Jan. 12, 2023, 4:05 p.m. UTC | #7
On 12/2/2022 5:10 AM, Joseph Myers wrote:
> On Fri, 2 Dec 2022, Longjun Luo via Gcc-patches wrote:
> 
>> They are ./gcc/testsuite/gcc.dg/cpp/warn-redefined.c and
>> ./gcc/testsuite/gcc.dg/cpp/warn-redefined-2.c
>>
>> These two cases redefine the __TIME__ macro when using the option
>> '-Wbuiltin-macro-redefined'.
>>
>> I think I shoud add a test to verify __LINE__ macro in these two cases.
> 
> I think it should be a test that doesn't use either
> -Wbuiltin-macro-redefined or -Wno-builtin-macro-redefined - a test of how
> the compiler behaves by default.
> 
Sorry for the delay. I have added a test case for this default 
situation. And also update another case to fully test the usage of 
builtin macros redefintions.
>> So, the patch itself has no problem. What I need do is to rich its test cases
>> and update change log, right?
> 
> The patch needs review, but I'm fine with the principle that
> -Wno-builtin-macro-redefined should apply to __LINE__ as it does to
> various other built-in macros.
> 
Thanks for your patience.
  

Patch

diff --git a/gcc/testsuite/gcc.dg/builtin-redefine.c b/gcc/testsuite/gcc.dg/builtin-redefine.c
index 882b2210992..9d5b42252ee 100644
--- a/gcc/testsuite/gcc.dg/builtin-redefine.c
+++ b/gcc/testsuite/gcc.dg/builtin-redefine.c
@@ -71,7 +71,6 @@ 
 /* { dg-bogus "Expected built-in is not defined" "" { target *-*-* } .-1 } */
 #endif
 
-#define __LINE__ 0           /* { dg-warning "-:\"__LINE__\" redef" } */
 #define __INCLUDE_LEVEL__ 0  /* { dg-warning "-:\"__INCLUDE_LEVEL__\" redef" } */
 #define __COUNTER__ 0        /* { dg-warning "-:\"__COUNTER__\" redef" } */
 
diff --git a/libcpp/init.cc b/libcpp/init.cc
index 5f34e3515d2..2765b9838b7 100644
--- a/libcpp/init.cc
+++ b/libcpp/init.cc
@@ -421,7 +421,7 @@  static const struct builtin_macro builtin_array[] =
   B("__FILE__",		 BT_FILE,          false),
   B("__FILE_NAME__",	 BT_FILE_NAME,     false),
   B("__BASE_FILE__",	 BT_BASE_FILE,     false),
-  B("__LINE__",		 BT_SPECLINE,      true),
+  B("__LINE__",		 BT_SPECLINE,      false),
   B("__INCLUDE_LEVEL__", BT_INCLUDE_LEVEL, true),
   B("__COUNTER__",	 BT_COUNTER,       true),
   /* Make sure to update the list of built-in