[01/40] gdb/unittests: PR28413, suppress warnings generated by Gnulib

Message ID 8b99d666e78ba5b9d32c7889a2161c70b3da88df.1666258361.git.research_trasio@irq.a4lg.com
State Superseded
Headers
Series sim+gdb: Suppress warnings if built with Clang (big batch 1) |

Commit Message

Tsukasa OI Oct. 20, 2022, 9:32 a.m. UTC
  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 address 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 surrounding entire #include
block with DIAGNOSTIC_IGNORE_USER_DEFINED_WARNINGS to disable Gnulib-
generated warnings on all standard C++ header files.
---
 gdb/unittests/string_view-selftests.c | 7 +++++++
 1 file changed, 7 insertions(+)
  

Comments

Simon Marchi Nov. 14, 2022, 2:01 p.m. UTC | #1
On 10/20/22 05:32, Tsukasa OI via Gdb-patches 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 address 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 surrounding entire #include
> block with DIAGNOSTIC_IGNORE_USER_DEFINED_WARNINGS to disable Gnulib-
> generated warnings on all standard C++ header files.
> ---
>  gdb/unittests/string_view-selftests.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/gdb/unittests/string_view-selftests.c b/gdb/unittests/string_view-selftests.c
> index 2d7261d18d3..441d533b54e 100644
> --- a/gdb/unittests/string_view-selftests.c
> +++ b/gdb/unittests/string_view-selftests.c
> @@ -23,6 +23,11 @@
>  
>  #define GNULIB_NAMESPACE gnulib
>  
> +#include "diagnostics.h"
> +
> +DIAGNOSTIC_PUSH
> +DIAGNOSTIC_IGNORE_USER_DEFINED_WARNINGS
> +
>  #include "defs.h"
>  #include "gdbsupport/selftest.h"
>  #include "gdbsupport/gdb_string_view.h"
> @@ -34,6 +39,8 @@
>  #include <fstream>
>  #include <iostream>
>  
> +DIAGNOSTIC_POP
> +
>  /* libstdc++'s testsuite uses VERIFY.  */
>  #define VERIFY SELF_CHECK
>  

I think this is fine, so I went ahead and pushed it, after adding a
little comment.

I tried to look at an alternative way of fixing it, by making
include/libiberty.h using GNULIB_NAMESPACE::free instead of free when
GNULIB_NAMESPACE is defined.  However, we then trip on:

      CXX    unittests/string_view-selftests.o
    In file included from /home/smarchi/src/binutils-gdb/gdb/unittests/string_view-selftests.c:26:
    In file included from /home/smarchi/src/binutils-gdb/gdb/defs.h:28:
    In file included from /home/smarchi/src/binutils-gdb/gdb/../gdbsupport/common-defs.h:204:
    In file included from /home/smarchi/src/binutils-gdb/gdb/../gdbsupport/ptid.h:36:
    In file included from /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/string:40:
    In file included from /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/char_traits.h:40:
    In file included from /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/postypes.h:40:
    /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/cwchar:231:12: error: The symbol ::wmemchr refers to the system function. Use gnulib::wmemchr instead. [-Werror,-Wuser-defined-warnings]
      { return wmemchr(const_cast<const wchar_t*>(__p), __c, __n); }
               ^

And we can't fix that the same way...

Simon
  

Patch

diff --git a/gdb/unittests/string_view-selftests.c b/gdb/unittests/string_view-selftests.c
index 2d7261d18d3..441d533b54e 100644
--- a/gdb/unittests/string_view-selftests.c
+++ b/gdb/unittests/string_view-selftests.c
@@ -23,6 +23,11 @@ 
 
 #define GNULIB_NAMESPACE gnulib
 
+#include "diagnostics.h"
+
+DIAGNOSTIC_PUSH
+DIAGNOSTIC_IGNORE_USER_DEFINED_WARNINGS
+
 #include "defs.h"
 #include "gdbsupport/selftest.h"
 #include "gdbsupport/gdb_string_view.h"
@@ -34,6 +39,8 @@ 
 #include <fstream>
 #include <iostream>
 
+DIAGNOSTIC_POP
+
 /* libstdc++'s testsuite uses VERIFY.  */
 #define VERIFY SELF_CHECK