public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 0/4] gdb: (includes PR28413), Suppress some general warnings if built with Clang
@ 2022-09-15  3:10 Tsukasa OI
  2022-09-15  3:10 ` [PATCH 1/4] include: Add macro to ignore -Wuser-defined-warnings Tsukasa OI
                   ` (4 more replies)
  0 siblings, 5 replies; 17+ messages in thread
From: Tsukasa OI @ 2022-09-15  3:10 UTC (permalink / raw)
  To: Tsukasa OI, Pedro Alves, Joel Brobecker, Enze Li; +Cc: gdb-patches, binutils

Hello,

[Common Background: Building GNU Binutils / GDB with Clang 15.0.0]

I'm now testing to build GNU Binutils / GDB with latest Clang (15.0.0) and
found some errors by default (when Binutils / GDB is not configured with
"--disable-werror").

While the best compiler to build GNU Binutils / GDB is GNU GCC, testing
other compilers are helpful to discover underlying problems and modernize
Binutils / GDB, even if building entire Binutils / GDB with the latest Clang
is unrealistic.  To be sure, I'm not going to finish "porting for Clang".
I will take low-hanging fruits and...

1.  make building with Clang easier and/or
2.  fix code issues (or non-issues) discovered as Clang warnings.

I made four patchsets in which, applying them all makes it possible to
build GNU Binutils / GDB with Clang (without help of --disable-werrors) for
many (but not all) targets including i386 and RISC-V with Ubuntu 22.04 LTS
(x86_64) host.  At least, I think they fix all (at minimum, most of) arch-
independent parts which prevents building with the latest version of Clang.

This is the one of them.


[About this Patchset]

This patchset deals with two kinds of warnings but with similar methods.

1.  gdb/unittests: PR28413, suppress custom warnings by Gnulib
    (PATCH 1/4, 3/4)

Gnulib generates a warning ("-Wuser-defined-warnings" on Clang) if the
system version of certain functions are used (to redirect the developer to
use Gnulib version).  However, this behavior is found harmful to
gdb/unittests/string_view-selftests.c, making the build failure.

This issue occurs if:

-   Compiled with Clang
-   -Werror is specified (by default)
-   C++ standard used by Clang is before C++17 (by default as of 15.0.0)
    when this unit test is activated.

This error is identified as PR28413.

Warnings about "free" may be resolved by modifying libiberty part but one of
the warnings generated by Gnulib points at the C++ standard library.

So, directly fixing the location which generates the warning is partially
impossible.  In the past, there were a proposal to fix this issue from the
Gnulib side:
<https://lists.gnu.org/archive/html/bug-gnulib/2021-10/msg00003.html>
but rejected because it ruins the intent of Gnulib warnings.  So, a fix from
the Binutils / GDB side is desired.

2.  gdb: Suppress "unused" variable warning on Clang in Bison-generated code
    (PATCH 2/4, 4/4)

Clang generates a warning on "written but not read thereafter" varibles
("-Wset-but-unused-variable"), making the build failure.

The unique problem here is, the cause is the variable yynerrs in
$(builddir)/gdb/cp-name-parser.c, generated by
$(srcdir)/gdb/cp-name-parser.y.  So, directly fixing the location which
causes the warning is nonfeasible.


In either cases, we can resolve the problem if we use include/diagnostics.h.
This header file defines a set of macros to suppress certain warnings only
when necessary.  In this patchset (PATCH 1-2/4), it adds two "ignore
warning" macros and uses them when necessary (PATCH 3-4/4).

This is mainly a GDB-side fix.  But since it contains changes to shared
part (includes/diagnostics.h), this patchset is also posted to Binutils ML.


Thanks,
Tsukasa




Tsukasa OI (4):
  include: Add macro to ignore -Wuser-defined-warnings
  include: Add macro to ignore -Wunused-but-set-variable
  gdb/unittests: PR28413, suppress warnings generated by Gnulib
  gdb: Suppress "unused" variable warning on Clang

 gdb/cp-name-parser.y                  |  4 ++++
 gdb/unittests/string_view-selftests.c |  5 +++++
 include/diagnostics.h                 | 23 +++++++++++++++++++++++
 3 files changed, 32 insertions(+)


base-commit: fe39ffdc202f04397f31557f17170b40bc42b77a
-- 
2.34.1


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 1/4] include: Add macro to ignore -Wuser-defined-warnings
  2022-09-15  3:10 [PATCH 0/4] gdb: (includes PR28413), Suppress some general warnings if built with Clang Tsukasa OI
@ 2022-09-15  3:10 ` Tsukasa OI
  2022-09-20 16:36   ` Nick Clifton
  2022-09-15  3:10 ` [PATCH 2/4] include: Add macro to ignore -Wunused-but-set-variable Tsukasa OI
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 17+ messages in thread
From: Tsukasa OI @ 2022-09-15  3:10 UTC (permalink / raw)
  To: Tsukasa OI, Pedro Alves, Joel Brobecker, Enze Li; +Cc: gdb-patches, binutils

User-defined warnings (on Clang, "-Wuser-defined-warnings") can be harmful
if we have specified "-Werror" and we have no control to disable the warning
ourself.  The particular example is Gnulib.

Gnulib generates a warning if the system version of certain functions
are used (to redirect the developer to use Gnulib version).  However,
it can be harmful if we cannot easily replace them (e.g. the target is in
the standard C++ library).

The new DIAGNOSTIC_IGNORE_USER_DEFINED_WARNINGS macro can be helpful on such
cases.  A typical use of this macro is to place this macro before including
certain system headers.

include/ChangeLog:

	* diagnostics.h (DIAGNOSTIC_IGNORE_USER_DEFINED_WARNINGS): New.
---
 include/diagnostics.h | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/include/diagnostics.h b/include/diagnostics.h
index 3da88282261..dbe6288d3d6 100644
--- a/include/diagnostics.h
+++ b/include/diagnostics.h
@@ -63,6 +63,11 @@
 # define DIAGNOSTIC_IGNORE_FORMAT_NONLITERAL \
   DIAGNOSTIC_IGNORE ("-Wformat-nonliteral")
 
+# if __has_warning ("-Wuser-defined-warnings")
+#  define DIAGNOSTIC_IGNORE_USER_DEFINED_WARNINGS \
+   DIAGNOSTIC_IGNORE ("-Wuser-defined-warnings")
+# endif
+
 # define DIAGNOSTIC_ERROR_SWITCH \
   DIAGNOSTIC_ERROR ("-Wswitch")
 
@@ -121,6 +126,10 @@
 # define DIAGNOSTIC_IGNORE_FORMAT_NONLITERAL
 #endif
 
+#ifndef DIAGNOSTIC_IGNORE_USER_DEFINED_WARNINGS
+# define DIAGNOSTIC_IGNORE_USER_DEFINED_WARNINGS
+#endif
+
 #ifndef DIAGNOSTIC_ERROR_SWITCH
 # define DIAGNOSTIC_ERROR_SWITCH
 #endif
-- 
2.34.1


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 2/4] include: Add macro to ignore -Wunused-but-set-variable
  2022-09-15  3:10 [PATCH 0/4] gdb: (includes PR28413), Suppress some general warnings if built with Clang Tsukasa OI
  2022-09-15  3:10 ` [PATCH 1/4] include: Add macro to ignore -Wuser-defined-warnings Tsukasa OI
@ 2022-09-15  3:10 ` Tsukasa OI
  2022-09-15  3:10 ` [PATCH 3/4] gdb/unittests: PR28413, suppress warnings generated by Gnulib Tsukasa OI
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 17+ messages in thread
From: Tsukasa OI @ 2022-09-15  3:10 UTC (permalink / raw)
  To: Tsukasa OI, Pedro Alves, Joel Brobecker, Enze Li; +Cc: gdb-patches, binutils

"-Wunused-but-set-variable" warning option can be helpful to track variables
that are written but not read thereafter.  But it can be harmful if some of
the code is auto-generated and we have no ways to deal with it.

The particular example is Bison-generated code.

The new DIAGNOSTIC_IGNORE_UNUSED_BUT_SET_VARIABLE macro can be helpful on
such cases. A typical use of this macro is to place this macro before the
end of user prologues on Bison (.y) files.

include/ChangeLog:

    * diagnostics.h (DIAGNOSTIC_IGNORE_UNUSED_BUT_SET_VARIABLE): New.
---
 include/diagnostics.h | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/include/diagnostics.h b/include/diagnostics.h
index dbe6288d3d6..4161dff6abc 100644
--- a/include/diagnostics.h
+++ b/include/diagnostics.h
@@ -68,6 +68,11 @@
    DIAGNOSTIC_IGNORE ("-Wuser-defined-warnings")
 # endif
 
+# if __has_warning ("-Wunused-but-set-variable")
+#  define DIAGNOSTIC_IGNORE_UNUSED_BUT_SET_VARIABLE \
+   DIAGNOSTIC_IGNORE ("-Wunused-but-set-variable")
+# endif
+
 # define DIAGNOSTIC_ERROR_SWITCH \
   DIAGNOSTIC_ERROR ("-Wswitch")
 
@@ -89,6 +94,11 @@
 # define DIAGNOSTIC_IGNORE_FORMAT_NONLITERAL \
   DIAGNOSTIC_IGNORE ("-Wformat-nonliteral")
 
+# if __GNUC__ >= 5
+#  define DIAGNOSTIC_IGNORE_UNUSED_BUT_SET_VARIABLE \
+   DIAGNOSTIC_IGNORE ("-Wunused-but-set-variable")
+# endif
+
 /* GCC 4.8's "diagnostic push/pop" seems broken when using this, -Wswitch
    remains enabled at the error level even after a pop.  Therefore, don't
    use it for GCC < 5.  */
@@ -130,6 +140,10 @@
 # define DIAGNOSTIC_IGNORE_USER_DEFINED_WARNINGS
 #endif
 
+#ifndef DIAGNOSTIC_IGNORE_UNUSED_BUT_SET_VARIABLE
+# define DIAGNOSTIC_IGNORE_UNUSED_BUT_SET_VARIABLE
+#endif
+
 #ifndef DIAGNOSTIC_ERROR_SWITCH
 # define DIAGNOSTIC_ERROR_SWITCH
 #endif
-- 
2.34.1


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 3/4] gdb/unittests: PR28413, suppress warnings generated by Gnulib
  2022-09-15  3:10 [PATCH 0/4] gdb: (includes PR28413), Suppress some general warnings if built with Clang Tsukasa OI
  2022-09-15  3:10 ` [PATCH 1/4] include: Add macro to ignore -Wuser-defined-warnings Tsukasa OI
  2022-09-15  3:10 ` [PATCH 2/4] include: Add macro to ignore -Wunused-but-set-variable Tsukasa OI
@ 2022-09-15  3:10 ` Tsukasa OI
  2022-09-15  3:10 ` [PATCH 4/4] gdb: Suppress "unused" variable warning on Clang Tsukasa OI
  2022-09-22  8:25 ` [PATCH v2 0/4] gdb: (includes PR28413), Suppress some general warnings if built with Clang Tsukasa OI
  4 siblings, 0 replies; 17+ messages in thread
From: Tsukasa OI @ 2022-09-15  3:10 UTC (permalink / raw)
  To: Tsukasa OI, Pedro Alves, Joel Brobecker, Enze Li; +Cc: gdb-patches, binutils

Gnulib generates a warning if the system version of certain functions
are used (to redirect the developer to use Gnulib version).  It caused a
compiler error when...

-   Compiled with Clang
-   -Werror is specified (by default)
-   C++ standard used by Clang is before C++17 (by default as of 15.0.0)
    when this unit test is activated.

This issue is raised as PR28413.

However, previous proposal to fix this issue (a "fix" to Gnulib):
<https://lists.gnu.org/archive/html/bug-gnulib/2021-10/msg00003.html>
was rejected because it ruins the intent of Gnulib warnings.

So, we need a Binutils/GDB-side solution.

This commit tries to deal with this issue on the GDB side.  We have
"include/diagnostics.h" to disable certain warnings only when necessary.

This commit suppresses the Gnulib warnings by adding
DIAGNOSTIC_IGNORE_USER_DEFINED_WARNINGS before including "defs.h".

gdb/ChangeLog:

	pr 28413
	* unittests/string_view-selftests.c: Suppress Gnulib-generated
	warnings when "defs.h" is included.
---
 gdb/unittests/string_view-selftests.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/gdb/unittests/string_view-selftests.c b/gdb/unittests/string_view-selftests.c
index 2d7261d18d3..c1f7799d94c 100644
--- a/gdb/unittests/string_view-selftests.c
+++ b/gdb/unittests/string_view-selftests.c
@@ -23,7 +23,12 @@
 
 #define GNULIB_NAMESPACE gnulib
 
+#include "diagnostics.h"
+
+DIAGNOSTIC_PUSH
+DIAGNOSTIC_IGNORE_USER_DEFINED_WARNINGS
 #include "defs.h"
+DIAGNOSTIC_POP
 #include "gdbsupport/selftest.h"
 #include "gdbsupport/gdb_string_view.h"
 
-- 
2.34.1


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 4/4] gdb: Suppress "unused" variable warning on Clang
  2022-09-15  3:10 [PATCH 0/4] gdb: (includes PR28413), Suppress some general warnings if built with Clang Tsukasa OI
                   ` (2 preceding siblings ...)
  2022-09-15  3:10 ` [PATCH 3/4] gdb/unittests: PR28413, suppress warnings generated by Gnulib Tsukasa OI
@ 2022-09-15  3:10 ` Tsukasa OI
  2022-09-22  8:25 ` [PATCH v2 0/4] gdb: (includes PR28413), Suppress some general warnings if built with Clang Tsukasa OI
  4 siblings, 0 replies; 17+ messages in thread
From: Tsukasa OI @ 2022-09-15  3:10 UTC (permalink / raw)
  To: Tsukasa OI, Pedro Alves, Joel Brobecker, Enze Li; +Cc: gdb-patches, binutils

Clang generates a warning if there is a variable which is written but not
read thereafter.  By the default configuration (with "-Werror"), it causes a
build failure (unless "--disable-werror" is specified).

Because the cause of this error is in the Bison-generated code
($(binutils)/gdb/cp-name-parser.y -> $(srcdir)/gdb/cp-name-parser.c),
this commit suppresses this warning ("-Wunused-but-set-variable") by placing
the DIAGNOSTIC_IGNORE_UNUSED_BUT_SET_VARIABLE macro at the end of user
prologue on cp-name-parser.y.

gdb/ChangeLog:

	* cp-name-parser.y: Suppress -Wunused-but-set-variable warning on
	the Bison-generated code.
---
 gdb/cp-name-parser.y | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/gdb/cp-name-parser.y b/gdb/cp-name-parser.y
index 34c691ddabb..21ba51679d3 100644
--- a/gdb/cp-name-parser.y
+++ b/gdb/cp-name-parser.y
@@ -248,6 +248,10 @@ cpname_state::make_name (const char *name, int len)
 
 static int yylex (YYSTYPE *, cpname_state *);
 static void yyerror (cpname_state *, const char *);
+
+#include "diagnostics.h"
+DIAGNOSTIC_IGNORE_UNUSED_BUT_SET_VARIABLE
+
 %}
 
 %type <comp> exp exp1 type start start_opt oper colon_name
-- 
2.34.1


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/4] include: Add macro to ignore -Wuser-defined-warnings
  2022-09-15  3:10 ` [PATCH 1/4] include: Add macro to ignore -Wuser-defined-warnings Tsukasa OI
@ 2022-09-20 16:36   ` Nick Clifton
  2022-09-21  6:13     ` Tsukasa OI
  2022-09-22 13:02     ` Enze Li
  0 siblings, 2 replies; 17+ messages in thread
From: Nick Clifton @ 2022-09-20 16:36 UTC (permalink / raw)
  To: Tsukasa OI, Pedro Alves, Joel Brobecker, Enze Li; +Cc: binutils, gdb-patches

Hi Tsukasa,

> +# if __has_warning ("-Wuser-defined-warnings")

I have not seen __has_warning () before.  Is this a new feature of
recent GCCs and Clang, or am I just behind the times ?

Cheers
   Nick


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/4] include: Add macro to ignore -Wuser-defined-warnings
  2022-09-20 16:36   ` Nick Clifton
@ 2022-09-21  6:13     ` Tsukasa OI
  2022-09-22 13:02     ` Enze Li
  1 sibling, 0 replies; 17+ messages in thread
From: Tsukasa OI @ 2022-09-21  6:13 UTC (permalink / raw)
  To: Nick Clifton, Binutils

On 2022/09/21 1:36, Nick Clifton wrote:
> Hi Tsukasa,
> 
>> +# if __has_warning ("-Wuser-defined-warnings")
> 
> I have not seen __has_warning () before.  Is this a new feature of
> recent GCCs and Clang, or am I just behind the times ?

Nick, this is one of the Clang language extensions:
<https://clang.llvm.org/docs/LanguageExtensions.html#include-file-checking-macros>

and already used in Binutils, include/diagnostics.h:58 (latest master)

    # if __has_warning ("-Wenum-compare-switch")

This and my new line with __has_warning are surrounded by "#if defined
(__clang__)" (a Clang-only part of the code) and should not expand to a
non-zero value unless:

1.  __has_warning is implemented as either a Clang extension
    or a custom macro (I will not assume this for obvious reasons).
2.  Builtin __has_warning (as I assumed above) detects given warning
    is implemented.

__has_warning is a Clang extension but similar standard feature testing
macros with arguments are defined in recent C++ versions (not usable as
standard language features in Binutils, though):

-   __has_include (C++17 or later)
-   __has_cpp_attribute (C++20 or later)

Thanks,
Tsukasa

> 
> Cheers
>   Nick
> 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH v2 0/4] gdb: (includes PR28413), Suppress some general warnings if built with Clang
  2022-09-15  3:10 [PATCH 0/4] gdb: (includes PR28413), Suppress some general warnings if built with Clang Tsukasa OI
                   ` (3 preceding siblings ...)
  2022-09-15  3:10 ` [PATCH 4/4] gdb: Suppress "unused" variable warning on Clang Tsukasa OI
@ 2022-09-22  8:25 ` Tsukasa OI
  2022-09-22  8:25   ` [PATCH v2 1/4] include: Add macro to ignore -Wuser-defined-warnings Tsukasa OI
                     ` (3 more replies)
  4 siblings, 4 replies; 17+ messages in thread
From: Tsukasa OI @ 2022-09-22  8:25 UTC (permalink / raw)
  To: Tsukasa OI, Pedro Alves, Joel Brobecker, Enze Li; +Cc: gdb-patches, binutils

Hello,

[Common Background: Building GNU Binutils / GDB with Clang 15.0.0]

I'm now testing to build GNU Binutils / GDB with latest Clang (15.0.0) and
found some errors by default (when Binutils / GDB is not configured with
"--disable-werror").

While the best compiler to build GNU Binutils / GDB is GNU GCC, testing
other compilers are helpful to discover underlying problems and modernize
Binutils / GDB, even if building entire Binutils / GDB with the latest Clang
is unrealistic.  To be sure, I'm not going to finish "porting for Clang".
I will take low-hanging fruits and...

1.  make building with Clang easier and/or
2.  fix code issues (or non-issues) discovered as Clang warnings.

I made four patchsets in which, applying them all makes it possible to
build GNU Binutils / GDB with Clang (without help of --disable-werrors) for
many (but not all) targets including i386 and RISC-V with Ubuntu 22.04 LTS
(x86_64) host.  At least, I think they fix all (at minimum, most of) arch-
independent parts which prevents building with the latest version of Clang.

This is the one of them.


[Changes: v1 -> v2]

-   Fix commit messages
    (commit message of PATCH 4/4 was very wrong)


[About this Patchset]

This patchset deals with two kinds of warnings but with similar methods.

1.  gdb/unittests: PR28413, suppress custom warnings by Gnulib
    (PATCH 1/4, 3/4)

Gnulib generates a warning ("-Wuser-defined-warnings" on Clang) if the
system version of certain functions are used (to redirect the developer to
use Gnulib version).  However, this behavior is found harmful to
gdb/unittests/string_view-selftests.c, making the build failure.

This issue occurs if:

-   Compiled with Clang
-   -Werror is specified (by default)
-   C++ standard used by Clang is before C++17 (by default as of 15.0.0)
    when this unit test is activated.

This error is identified as PR28413.

Warnings about "free" may be resolved by modifying libiberty part but one of
the warnings generated by Gnulib points at the C++ standard library.

So, directly fixing the location which generates the warning is partially
impossible.  In the past, there were a proposal to fix this issue from the
Gnulib side:
<https://lists.gnu.org/archive/html/bug-gnulib/2021-10/msg00003.html>
but rejected because it ruins the intent of Gnulib warnings.  So, a fix from
the Binutils / GDB side is desired.

2.  gdb: Suppress "unused" variable warning on Clang in Bison-generated code
    (PATCH 2/4, 4/4)

Clang generates a warning on "written but not read thereafter" varibles
("-Wset-but-unused-variable"), making the build failure.

The unique problem here is, the cause is the variable yynerrs in
$(builddir)/gdb/cp-name-parser.c, generated by
$(srcdir)/gdb/cp-name-parser.y.  So, directly fixing the location which
causes the warning is nonfeasible.


In either cases, we can resolve the problem if we use include/diagnostics.h.
This header file defines a set of macros to suppress certain warnings only
when necessary.  In this patchset (PATCH 1-2/4), it adds two "ignore
warning" macros and uses them when necessary (PATCH 3-4/4).

This is mainly a GDB-side fix.  But since it contains changes to shared
part (includes/diagnostics.h), this patchset is also posted to Binutils ML.


Thanks,
Tsukasa




Tsukasa OI (4):
  include: Add macro to ignore -Wuser-defined-warnings
  include: Add macro to ignore -Wunused-but-set-variable
  gdb/unittests: PR28413, suppress warnings generated by Gnulib
  gdb: Suppress "unused" variable warning on Clang

 gdb/cp-name-parser.y                  |  4 ++++
 gdb/unittests/string_view-selftests.c |  5 +++++
 include/diagnostics.h                 | 23 +++++++++++++++++++++++
 3 files changed, 32 insertions(+)


base-commit: b59f8a90ba0866a8605106fdb09389833c7fe8ad
-- 
2.34.1


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH v2 1/4] include: Add macro to ignore -Wuser-defined-warnings
  2022-09-22  8:25 ` [PATCH v2 0/4] gdb: (includes PR28413), Suppress some general warnings if built with Clang Tsukasa OI
@ 2022-09-22  8:25   ` Tsukasa OI
  2022-09-22 11:26     ` Nick Clifton
  2022-09-22  8:25   ` [PATCH v2 2/4] include: Add macro to ignore -Wunused-but-set-variable Tsukasa OI
                     ` (2 subsequent siblings)
  3 siblings, 1 reply; 17+ messages in thread
From: Tsukasa OI @ 2022-09-22  8:25 UTC (permalink / raw)
  To: Tsukasa OI, Pedro Alves, Joel Brobecker, Enze Li; +Cc: gdb-patches, binutils

User-defined warnings (on Clang, "-Wuser-defined-warnings") can be harmful
if we have specified "-Werror" and we have no control to disable the warning
ourself.  The particular example is Gnulib.

Gnulib generates a warning if the system version of certain functions
are used (to redirect the developer to use Gnulib version).  However,
it can be harmful if we cannot easily replace them (e.g. the target is in
the standard C++ library).

The new DIAGNOSTIC_IGNORE_USER_DEFINED_WARNINGS macro can be helpful on such
cases.  A typical use of this macro is to place this macro before including
certain system headers.

include/ChangeLog:

	* diagnostics.h (DIAGNOSTIC_IGNORE_USER_DEFINED_WARNINGS): New.
---
 include/diagnostics.h | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/include/diagnostics.h b/include/diagnostics.h
index 3da88282261..dbe6288d3d6 100644
--- a/include/diagnostics.h
+++ b/include/diagnostics.h
@@ -63,6 +63,11 @@
 # define DIAGNOSTIC_IGNORE_FORMAT_NONLITERAL \
   DIAGNOSTIC_IGNORE ("-Wformat-nonliteral")
 
+# if __has_warning ("-Wuser-defined-warnings")
+#  define DIAGNOSTIC_IGNORE_USER_DEFINED_WARNINGS \
+   DIAGNOSTIC_IGNORE ("-Wuser-defined-warnings")
+# endif
+
 # define DIAGNOSTIC_ERROR_SWITCH \
   DIAGNOSTIC_ERROR ("-Wswitch")
 
@@ -121,6 +126,10 @@
 # define DIAGNOSTIC_IGNORE_FORMAT_NONLITERAL
 #endif
 
+#ifndef DIAGNOSTIC_IGNORE_USER_DEFINED_WARNINGS
+# define DIAGNOSTIC_IGNORE_USER_DEFINED_WARNINGS
+#endif
+
 #ifndef DIAGNOSTIC_ERROR_SWITCH
 # define DIAGNOSTIC_ERROR_SWITCH
 #endif
-- 
2.34.1


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH v2 2/4] include: Add macro to ignore -Wunused-but-set-variable
  2022-09-22  8:25 ` [PATCH v2 0/4] gdb: (includes PR28413), Suppress some general warnings if built with Clang Tsukasa OI
  2022-09-22  8:25   ` [PATCH v2 1/4] include: Add macro to ignore -Wuser-defined-warnings Tsukasa OI
@ 2022-09-22  8:25   ` Tsukasa OI
  2022-09-22 11:27     ` Nick Clifton
  2022-09-22  8:25   ` [PATCH v2 3/4] gdb/unittests: PR28413, suppress warnings generated by Gnulib Tsukasa OI
  2022-09-22  8:25   ` [PATCH v2 4/4] gdb: Suppress "unused" variable warning on Clang Tsukasa OI
  3 siblings, 1 reply; 17+ messages in thread
From: Tsukasa OI @ 2022-09-22  8:25 UTC (permalink / raw)
  To: Tsukasa OI, Pedro Alves, Joel Brobecker, Enze Li; +Cc: gdb-patches, binutils

"-Wunused-but-set-variable" warning option can be helpful to track variables
that are written but not read thereafter.  But it can be harmful if some of
the code is auto-generated and we have no ways to deal with it.

The particular example is Bison-generated code.

The new DIAGNOSTIC_IGNORE_UNUSED_BUT_SET_VARIABLE macro can be helpful on
such cases. A typical use of this macro is to place this macro before the
end of user prologues on Bison (.y) files.

include/ChangeLog:

    * diagnostics.h (DIAGNOSTIC_IGNORE_UNUSED_BUT_SET_VARIABLE): New.
---
 include/diagnostics.h | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/include/diagnostics.h b/include/diagnostics.h
index dbe6288d3d6..4161dff6abc 100644
--- a/include/diagnostics.h
+++ b/include/diagnostics.h
@@ -68,6 +68,11 @@
    DIAGNOSTIC_IGNORE ("-Wuser-defined-warnings")
 # endif
 
+# if __has_warning ("-Wunused-but-set-variable")
+#  define DIAGNOSTIC_IGNORE_UNUSED_BUT_SET_VARIABLE \
+   DIAGNOSTIC_IGNORE ("-Wunused-but-set-variable")
+# endif
+
 # define DIAGNOSTIC_ERROR_SWITCH \
   DIAGNOSTIC_ERROR ("-Wswitch")
 
@@ -89,6 +94,11 @@
 # define DIAGNOSTIC_IGNORE_FORMAT_NONLITERAL \
   DIAGNOSTIC_IGNORE ("-Wformat-nonliteral")
 
+# if __GNUC__ >= 5
+#  define DIAGNOSTIC_IGNORE_UNUSED_BUT_SET_VARIABLE \
+   DIAGNOSTIC_IGNORE ("-Wunused-but-set-variable")
+# endif
+
 /* GCC 4.8's "diagnostic push/pop" seems broken when using this, -Wswitch
    remains enabled at the error level even after a pop.  Therefore, don't
    use it for GCC < 5.  */
@@ -130,6 +140,10 @@
 # define DIAGNOSTIC_IGNORE_USER_DEFINED_WARNINGS
 #endif
 
+#ifndef DIAGNOSTIC_IGNORE_UNUSED_BUT_SET_VARIABLE
+# define DIAGNOSTIC_IGNORE_UNUSED_BUT_SET_VARIABLE
+#endif
+
 #ifndef DIAGNOSTIC_ERROR_SWITCH
 # define DIAGNOSTIC_ERROR_SWITCH
 #endif
-- 
2.34.1


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH v2 3/4] gdb/unittests: PR28413, suppress warnings generated by Gnulib
  2022-09-22  8:25 ` [PATCH v2 0/4] gdb: (includes PR28413), Suppress some general warnings if built with Clang Tsukasa OI
  2022-09-22  8:25   ` [PATCH v2 1/4] include: Add macro to ignore -Wuser-defined-warnings Tsukasa OI
  2022-09-22  8:25   ` [PATCH v2 2/4] include: Add macro to ignore -Wunused-but-set-variable Tsukasa OI
@ 2022-09-22  8:25   ` Tsukasa OI
  2022-10-18 13:44     ` Enze Li
  2022-09-22  8:25   ` [PATCH v2 4/4] gdb: Suppress "unused" variable warning on Clang Tsukasa OI
  3 siblings, 1 reply; 17+ messages in thread
From: Tsukasa OI @ 2022-09-22  8:25 UTC (permalink / raw)
  To: Tsukasa OI, Pedro Alves, Joel Brobecker, Enze Li; +Cc: gdb-patches, binutils

Gnulib generates a warning if the system version of certain functions
are used (to redirect the developer to use Gnulib version).  It caused a
compiler error when...

-   Compiled with Clang
-   -Werror is specified (by default)
-   C++ standard used by Clang is before C++17 (by default as of 15.0.0)
    when this unit test is activated.

This issue is raised as PR28413.

However, previous proposal to fix this issue (a "fix" to Gnulib):
<https://lists.gnu.org/archive/html/bug-gnulib/2021-10/msg00003.html>
was rejected because it ruins the intent of Gnulib warnings.

So, we need a Binutils/GDB-side solution.

This commit tries to deal with this issue on the GDB side.  We have
"include/diagnostics.h" to disable certain warnings only when necessary.

This commit suppresses the Gnulib warnings by adding
DIAGNOSTIC_IGNORE_USER_DEFINED_WARNINGS before including "defs.h".

gdb/ChangeLog:

	pr 28413
	* unittests/string_view-selftests.c: Suppress Gnulib-generated
	warnings when "defs.h" is included.
---
 gdb/unittests/string_view-selftests.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/gdb/unittests/string_view-selftests.c b/gdb/unittests/string_view-selftests.c
index 2d7261d18d3..c1f7799d94c 100644
--- a/gdb/unittests/string_view-selftests.c
+++ b/gdb/unittests/string_view-selftests.c
@@ -23,7 +23,12 @@
 
 #define GNULIB_NAMESPACE gnulib
 
+#include "diagnostics.h"
+
+DIAGNOSTIC_PUSH
+DIAGNOSTIC_IGNORE_USER_DEFINED_WARNINGS
 #include "defs.h"
+DIAGNOSTIC_POP
 #include "gdbsupport/selftest.h"
 #include "gdbsupport/gdb_string_view.h"
 
-- 
2.34.1


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH v2 4/4] gdb: Suppress "unused" variable warning on Clang
  2022-09-22  8:25 ` [PATCH v2 0/4] gdb: (includes PR28413), Suppress some general warnings if built with Clang Tsukasa OI
                     ` (2 preceding siblings ...)
  2022-09-22  8:25   ` [PATCH v2 3/4] gdb/unittests: PR28413, suppress warnings generated by Gnulib Tsukasa OI
@ 2022-09-22  8:25   ` Tsukasa OI
  2022-10-12 17:36     ` Simon Marchi
  3 siblings, 1 reply; 17+ messages in thread
From: Tsukasa OI @ 2022-09-22  8:25 UTC (permalink / raw)
  To: Tsukasa OI, Pedro Alves, Joel Brobecker, Enze Li; +Cc: gdb-patches, binutils

Clang generates a warning if there is a variable which is written but not
read thereafter.  By the default configuration (with "-Werror"), it causes a
build failure (unless "--disable-werror" is specified).

Because the cause of this error is in the Bison-generated code
($(srcdir)/gdb/cp-name-parser.y -> $(builddir)/gdb/cp-name-parser.c),
this commit suppresses this warning ("-Wunused-but-set-variable") by placing
the DIAGNOSTIC_IGNORE_UNUSED_BUT_SET_VARIABLE macro at the end of user
prologue of cp-name-parser.y.

gdb/ChangeLog:

	* cp-name-parser.y: Suppress -Wunused-but-set-variable warning on
	the Bison-generated code.
---
 gdb/cp-name-parser.y | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/gdb/cp-name-parser.y b/gdb/cp-name-parser.y
index 34c691ddabb..21ba51679d3 100644
--- a/gdb/cp-name-parser.y
+++ b/gdb/cp-name-parser.y
@@ -248,6 +248,10 @@ cpname_state::make_name (const char *name, int len)
 
 static int yylex (YYSTYPE *, cpname_state *);
 static void yyerror (cpname_state *, const char *);
+
+#include "diagnostics.h"
+DIAGNOSTIC_IGNORE_UNUSED_BUT_SET_VARIABLE
+
 %}
 
 %type <comp> exp exp1 type start start_opt oper colon_name
-- 
2.34.1


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 1/4] include: Add macro to ignore -Wuser-defined-warnings
  2022-09-22  8:25   ` [PATCH v2 1/4] include: Add macro to ignore -Wuser-defined-warnings Tsukasa OI
@ 2022-09-22 11:26     ` Nick Clifton
  0 siblings, 0 replies; 17+ messages in thread
From: Nick Clifton @ 2022-09-22 11:26 UTC (permalink / raw)
  To: Tsukasa OI, Pedro Alves, Joel Brobecker, Enze Li; +Cc: binutils, gdb-patches

Hi Tsukasa,

> include/ChangeLog:
> 
> 	* diagnostics.h (DIAGNOSTIC_IGNORE_USER_DEFINED_WARNINGS): New.

Patch approved - please apply.

Cheers
   Nick



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 2/4] include: Add macro to ignore -Wunused-but-set-variable
  2022-09-22  8:25   ` [PATCH v2 2/4] include: Add macro to ignore -Wunused-but-set-variable Tsukasa OI
@ 2022-09-22 11:27     ` Nick Clifton
  0 siblings, 0 replies; 17+ messages in thread
From: Nick Clifton @ 2022-09-22 11:27 UTC (permalink / raw)
  To: Tsukasa OI, Pedro Alves, Joel Brobecker, Enze Li; +Cc: binutils, gdb-patches

Hi Tsukasa,

> include/ChangeLog:
> 
>      * diagnostics.h (DIAGNOSTIC_IGNORE_UNUSED_BUT_SET_VARIABLE): New.

Patch approved - please apply.

Cheers
   Nick


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/4] include: Add macro to ignore -Wuser-defined-warnings
  2022-09-20 16:36   ` Nick Clifton
  2022-09-21  6:13     ` Tsukasa OI
@ 2022-09-22 13:02     ` Enze Li
  1 sibling, 0 replies; 17+ messages in thread
From: Enze Li @ 2022-09-22 13:02 UTC (permalink / raw)
  To: Nick Clifton
  Cc: Tsukasa OI, Pedro Alves, Joel Brobecker, Enze Li, binutils, gdb-patches

Hi Nick,

On Tue, Sep 20 2022 at 05:36:05 PM +0100, Nick Clifton wrote:

> Hi Tsukasa,
>
>> +# if __has_warning ("-Wuser-defined-warnings")
>
> I have not seen __has_warning () before.  Is this a new feature of
> recent GCCs and Clang, or am I just behind the times ?
>
> Cheers
>   Nick

I happen to know some info about this function-like macro.  It was first
introduced in 2011.  AFAIK, it is only supported by clang.
More detail, see here[1] for docs and here[2] for implement.

[1] https://clang.llvm.org/docs/LanguageExtensions.html#has-warning
[2] https://github.com/llvm/llvm-project/commit/a35d67dfd927191a16cf24931981d79901938381

Thanks,
Enze

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 4/4] gdb: Suppress "unused" variable warning on Clang
  2022-09-22  8:25   ` [PATCH v2 4/4] gdb: Suppress "unused" variable warning on Clang Tsukasa OI
@ 2022-10-12 17:36     ` Simon Marchi
  0 siblings, 0 replies; 17+ messages in thread
From: Simon Marchi @ 2022-10-12 17:36 UTC (permalink / raw)
  To: Tsukasa OI, Pedro Alves, Joel Brobecker, Enze Li; +Cc: binutils, gdb-patches

On 9/22/22 04:25, Tsukasa OI via Gdb-patches wrote:
> Clang generates a warning if there is a variable which is written but not
> read thereafter.  By the default configuration (with "-Werror"), it causes a
> build failure (unless "--disable-werror" is specified).
> 
> Because the cause of this error is in the Bison-generated code
> ($(srcdir)/gdb/cp-name-parser.y -> $(builddir)/gdb/cp-name-parser.c),
> this commit suppresses this warning ("-Wunused-but-set-variable") by placing
> the DIAGNOSTIC_IGNORE_UNUSED_BUT_SET_VARIABLE macro at the end of user
> prologue of cp-name-parser.y.

Hi,

I just sent a patch to fix the same thing as your patch here, but only
saw your patch after.  However, I took a different, more fine-grained
approach:

https://inbox.sourceware.org/gdb-patches/20221012173256.20079-1-simon.marchi@efficios.com/T/#u

Can you let me know what you think?

Simon

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 3/4] gdb/unittests: PR28413, suppress warnings generated by Gnulib
  2022-09-22  8:25   ` [PATCH v2 3/4] gdb/unittests: PR28413, suppress warnings generated by Gnulib Tsukasa OI
@ 2022-10-18 13:44     ` Enze Li
  0 siblings, 0 replies; 17+ messages in thread
From: Enze Li @ 2022-10-18 13:44 UTC (permalink / raw)
  To: Tsukasa OI; +Cc: Pedro Alves, Joel Brobecker, Enze Li, gdb-patches, binutils

Hi Tsukasa,

Thanks for doing this.  I tested this patch on macOS and found one nit.
Please see below.

On Thu, Sep 22 2022 at 08:25:46 AM +0000, Tsukasa OI wrote:

> Gnulib generates a warning if the system version of certain functions
> are used (to redirect the developer to use Gnulib version).  It caused a
> compiler error when...
>
> -   Compiled with Clang
> -   -Werror is specified (by default)
> -   C++ standard used by Clang is before C++17 (by default as of 15.0.0)
>     when this unit test is activated.
>
> This issue is raised as PR28413.
>
> However, previous proposal to fix this issue (a "fix" to Gnulib):
> <https://lists.gnu.org/archive/html/bug-gnulib/2021-10/msg00003.html>
> was rejected because it ruins the intent of Gnulib warnings.
>
> So, we need a Binutils/GDB-side solution.
>
> This commit tries to deal with this issue on the GDB side.  We have
> "include/diagnostics.h" to disable certain warnings only when necessary.
>
> This commit suppresses the Gnulib warnings by adding
> DIAGNOSTIC_IGNORE_USER_DEFINED_WARNINGS before including "defs.h".
>
> gdb/ChangeLog:
>
> 	pr 28413
> 	* unittests/string_view-selftests.c: Suppress Gnulib-generated
> 	warnings when "defs.h" is included.
> ---
>  gdb/unittests/string_view-selftests.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/gdb/unittests/string_view-selftests.c b/gdb/unittests/string_view-selftests.c
> index 2d7261d18d3..c1f7799d94c 100644
> --- a/gdb/unittests/string_view-selftests.c
> +++ b/gdb/unittests/string_view-selftests.c
> @@ -23,7 +23,12 @@
>  
>  #define GNULIB_NAMESPACE gnulib
>  
> +#include "diagnostics.h"
> +
> +DIAGNOSTIC_PUSH
> +DIAGNOSTIC_IGNORE_USER_DEFINED_WARNINGS
>  #include "defs.h"
> +DIAGNOSTIC_POP
>  #include "gdbsupport/selftest.h"
>  #include "gdbsupport/gdb_string_view.h"

With your patch applied on macOS 15.10.7, it still says,

==========================================================================================
  CXX    unittests/string_view-selftests.o
In file included from unittests/string_view-selftests.c:39:
/Library/Developer/CommandLineTools/usr/bin/../include/c++/v1/fstream:579:17: error: The symbol
      ::fdopen refers to the system function. Use gnulib::fdopen instead.
      [-Werror,-Wuser-defined-warnings]
      __file_ = fdopen(__fd, __mdstr);
                ^
./../gnulib/import/stdio.h:826:1: note: from 'diagnose_if' attribute on 'fdopen':
_GL_CXXALIASWARN (fdopen);
^~~~~~~~~~~~~~~~~~~~~~~~~
./../gnulib/import/stdio.h:461:4: note: expanded from macro '_GL_CXXALIASWARN'
   _GL_CXXALIASWARN_1 (func, GNULIB_NAMESPACE)
   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
./../gnulib/import/stdio.h:463:4: note: expanded from macro '_GL_CXXALIASWARN_1'
   _GL_CXXALIASWARN_2 (func, namespace)
   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
./../gnulib/import/stdio.h:468:5: note: expanded from macro '_GL_CXXALIASWARN_2'
    _GL_WARN_ON_USE (func, \
    ^~~~~~~~~~~~~~~~~~~~~~~~
./../gnulib/import/stdio.h:632:19: note: expanded from macro '_GL_WARN_ON_USE'
  __attribute__ ((__diagnose_if__ (1, message, "warning")))
                  ^                ~
1 error generated.
make[2]: *** [unittests/string_view-selftests.o] Error 1
==========================================================================================

I think this can be solved by just adding
DIAGNOSTIC_IGNORE_USER_DEFINED_WARNINGS before including <fstream>
according to your method.

WDYT?
==========================================================================================
diff --git a/gdb/unittests/string_view-selftests.c b/gdb/unittests/string_view-selftests.c
index 2d7261d18d3..b779b9b765a 100644
--- a/gdb/unittests/string_view-selftests.c
+++ b/gdb/unittests/string_view-selftests.c
@@ -23,7 +23,12 @@

 #define GNULIB_NAMESPACE gnulib

+#include "diagnostics.h"
+
+DIAGNOSTIC_PUSH
+DIAGNOSTIC_IGNORE_USER_DEFINED_WARNINGS
 #include "defs.h"
+DIAGNOSTIC_POP
 #include "gdbsupport/selftest.h"
 #include "gdbsupport/gdb_string_view.h"

@@ -31,7 +36,10 @@
    included test files are wrapped in a namespace.  */
 #include <string>
 #include <sstream>
+DIAGNOSTIC_PUSH
+DIAGNOSTIC_IGNORE_USER_DEFINED_WARNINGS
 #include <fstream>
+DIAGNOSTIC_POP
 #include <iostream>

 /* libstdc++'s testsuite uses VERIFY.  */
==========================================================================================

Other than that, looks good to me.

Reviewed-by: Enze Li <enze.li@hotmail.com>

Thanks,
Enze

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2022-10-18 13:44 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-15  3:10 [PATCH 0/4] gdb: (includes PR28413), Suppress some general warnings if built with Clang Tsukasa OI
2022-09-15  3:10 ` [PATCH 1/4] include: Add macro to ignore -Wuser-defined-warnings Tsukasa OI
2022-09-20 16:36   ` Nick Clifton
2022-09-21  6:13     ` Tsukasa OI
2022-09-22 13:02     ` Enze Li
2022-09-15  3:10 ` [PATCH 2/4] include: Add macro to ignore -Wunused-but-set-variable Tsukasa OI
2022-09-15  3:10 ` [PATCH 3/4] gdb/unittests: PR28413, suppress warnings generated by Gnulib Tsukasa OI
2022-09-15  3:10 ` [PATCH 4/4] gdb: Suppress "unused" variable warning on Clang Tsukasa OI
2022-09-22  8:25 ` [PATCH v2 0/4] gdb: (includes PR28413), Suppress some general warnings if built with Clang Tsukasa OI
2022-09-22  8:25   ` [PATCH v2 1/4] include: Add macro to ignore -Wuser-defined-warnings Tsukasa OI
2022-09-22 11:26     ` Nick Clifton
2022-09-22  8:25   ` [PATCH v2 2/4] include: Add macro to ignore -Wunused-but-set-variable Tsukasa OI
2022-09-22 11:27     ` Nick Clifton
2022-09-22  8:25   ` [PATCH v2 3/4] gdb/unittests: PR28413, suppress warnings generated by Gnulib Tsukasa OI
2022-10-18 13:44     ` Enze Li
2022-09-22  8:25   ` [PATCH v2 4/4] gdb: Suppress "unused" variable warning on Clang Tsukasa OI
2022-10-12 17:36     ` Simon Marchi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).