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

[Simon Marchi <simark@simark.ca>]

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