Define GNULIB_NAMESPACE in unittests/string_view-selftests.c
Commit Message
> > gdb/ChangeLog:
> >
> > * unittests/string_view-selftests.c: Define GNULIB_NAMESPACE.
>
> I noticed the exact same problem, and came up with the exact same
> solution. Sorry for not reporting right away, I have been swamped
> :-( and also wanted to spend some more time investigating the bigger
> picture; it seems to me like we may be having a huge time bomb
> in our hands?!?
>
> In other words, either we don't define GNULIB_NAMESPACE, but then
> we run into unwanted renames of class method names; or we defined
> GNULIB_NAMESPACE, but that means we must be prefixing all the symbols
> we call that are covered by the imported part of GNULIB with that
> namespace.
>
> What worries me is that I don't see what's preventing us from hitting
> that issue outside of the unittests code? We know we can adjust our
> own classes, but this problem occured with a system class, so we had
> no choice but to use GNULIB_NAMESPACE. I worry that the transition
> from no GNULIB_NAMESPACE to using GNULIB_NAMESPACE in a given unit
> will leave some C system calls that should normally be covered by
> gnulib silently now reverting to the system (buggy) version.
Actually, for the record, here is the patch I came up with.
Similar in nature, but additional comments and also only defining
the GNULIB_NAMESPACE macro on Windows hosts -- as a strategy to
minimize impact.
I think your approach is best, as we'll at least get the same
conditions regardless of the host.
From 711720d2b969d679880383cafe48fb09a1f21262 Mon Sep 17 00:00:00 2001
From: Joel Brobecker <brobecker@adacore.com>
Date: Tue, 24 Apr 2018 13:07:19 -0400
Subject: [PATCH] avoid undefined reference to basic_ofstream::rpl_close durin
GDB link
On Windows hosts, GDB currently fails to link with the following error:
| CXXLD gdb.exe
|unittests/string_view-selftests.o: In function `ZN9selftests11string_view11inserters_26test05Ej':
|/[...]/gdb/unittests/basic_string_view/inserters/char/2.cc:60: undefined reference to `std::basic_ofstream<char, std::char_traits<char> >::rpl_close()'
|collect2.exe: error: ld returned 1 exit status
This is due to gnulib's unistd.h defining "close" as "rpl_close",
which interferes with our use of ofstream::close.
This patch works around the issue by defining GNULIB_NAMESPACE
inside the unit test itself, when building on Windows hosts.
gdb/ChangeLog:
* unittests/string_view-selftests.c [_WIN32]: Define GNULIB_NAMESPACE.
TN: R424-035
---
gdb/unittests/string_view-selftests.c | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
@@ -21,6 +21,33 @@
the "real" version. */
#if __cplusplus < 201703L
+#if defined (_WIN32)
+/* On Windows hosts, gnulib's unistd.h uses a macro to redefine "close"
+ into "rpl_close", because of:
+
+ # if !(defined __cplusplus && defined GNULIB_NAMESPACE)
+ # undef close
+ # define close rpl_close
+ # endif
+
+ This interferes with one of the tests below that calls class
+ std::ofstream's "close" method, which gets renamed into a call
+ to non-existant "rpl_close" method instead, thus leading to
+ an undefined symbol error during the link:
+
+ unittests/basic_string_view/inserters/char/2.cc:60:
+ undefined reference toi
+ `std::basic_ofstream<char, std::char_traits<char> >::rpl_close()'
+
+ The gnulib documentation discusses this exact issue and says
+ we can define GNULIB_NAMESPACE to put all gnulib symbols into
+ the given namespace, but this is at the cost of gnulib no longer
+ replacing possibly faulty system functions. We have no real choice
+ here, and this is about unittest, so define GNULIB_NAMESPACE,
+ solely for this unit (the gnulib manual confirms this is OK). */
+#define GNULIB_NAMESPACE gnulib
+#endif
+
#include "defs.h"
#include "selftest.h"
#include "common/gdb_string_view.h"
--
2.11.0