Re: [PATCH] libcpp: suppress builtin macro redefined warnings for __LINE__
On Sun, Apr 30, 2023 at 4:15 PM Jeff Law via Gcc-patches
wrote:
>
>
>
> On 4/30/23 12:30, Jeff Law wrote:
> >
> >
> > On 1/12/23 09:02, Longjun Luo via Gcc-patches wrote:
> >> From 0821df518b264e754d698d399f98be1a62945e32 Mon Sep 17 00:00:00 2001
> >> From: Longjun Luo
> >> 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
> >> ---
> >> 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
> > Thanks. I added a suitable ChangeLog and pushed this to the trunk.
> This is causing regressions on various targets for a few tests:
>
> lm32-sim: c-c++-common/cpp/pr92296-2.c -Wc++-compat (test for
> warnings, line 41)
> lm32-sim: gcc.dg/cpp/undef2.c (test for warnings, line 9)
> lm32-sim: gcc.dg/cpp/undef2.c (test for excess errors)
>
> I have reverted this patch from the trunk. Please address the
> regressions and resubmit.
>From c-c++-common/cpp/pr92296-2.c (line 40):
#pragma push_macro("__LINE__")
#undef __LINE__ /* { dg-warning "undefining" } */
gcc.dg/cpp/undef2.c (line 9):
#undef __LINE__ /* { dg-warning "undefining \"__LINE__\"" } */
These testcases are specifically testing #undef of __LINE__ ...
Should we be still warning for this case or not?
Thanks,
Andrew Pinski
>
> Thanks,
> Jeff
>
Re: [PATCH] libcpp: suppress builtin macro redefined warnings for __LINE__
On 4/30/23 12:30, Jeff Law wrote: On 1/12/23 09:02, Longjun Luo via Gcc-patches wrote: From 0821df518b264e754d698d399f98be1a62945e32 Mon Sep 17 00:00:00 2001 From: Longjun Luo 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 --- 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 Thanks. I added a suitable ChangeLog and pushed this to the trunk. This is causing regressions on various targets for a few tests: lm32-sim: c-c++-common/cpp/pr92296-2.c -Wc++-compat (test for warnings, line 41) lm32-sim: gcc.dg/cpp/undef2.c (test for warnings, line 9) lm32-sim: gcc.dg/cpp/undef2.c (test for excess errors) I have reverted this patch from the trunk. Please address the regressions and resubmit. Thanks, Jeff
Re: [PATCH] libcpp: suppress builtin macro redefined warnings for __LINE__
On 1/12/23 09:02, Longjun Luo via Gcc-patches wrote: From 0821df518b264e754d698d399f98be1a62945e32 Mon Sep 17 00:00:00 2001 From: Longjun Luo 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 --- 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 Thanks. I added a suitable ChangeLog and pushed this to the trunk. jeff
Re: [PATCH] libcpp: suppress builtin macro redefined warnings for __LINE__
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.
Re: [PATCH] libcpp: suppress builtin macro redefined warnings for __LINE__
From 0821df518b264e754d698d399f98be1a62945e32 Mon Sep 17 00:00:00 2001
From: Longjun Luo
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
---
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 000..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
Re: [PATCH] libcpp: suppress builtin macro redefined warnings for __LINE__
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. -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH] libcpp: suppress builtin macro redefined warnings for __LINE__
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?
Re: [PATCH] libcpp: suppress builtin macro redefined warnings for __LINE__
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.
--
Joseph S. Myers
jos...@codesourcery.com
Re: [PATCH] libcpp: suppress builtin macro redefined warnings for __LINE__
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:
: 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.
Re: [PATCH] libcpp: suppress builtin macro redefined warnings for __LINE__
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)?
--
Joseph S. Myers
jos...@codesourcery.com
