Move some more of the things that should be seen everywhere by all
compilation units from common-defs.h to gdbsupport.inc.h.
After this change, to the best of my knowledge, common-defs.h, server.h
and defs.h are no longer special, they don't need to be included first
anymore. A lot of source files don't need what is directly defined in
common-defs.h / server.h / defs.h , but rather use things in header
files included transitively. There's no rush to do so, but for these
files, we'll be able to remove the inclusion of common-defs.h / server.h
/ defs.h and make them include what they actually use.
Ultimately, I think we could just get rid of these header files, since
they pretty much just server as big bags of includes. Moving to more
fine-graned includes means less unnecessary includes, meaning less
unnecessary recompilation when some header file is modified.
The things moved to gdbsupport.inc.h are:
- __STDC_CONSTANT_MACROS, __STDC_LIMIT_MACROS, __STDC_FORMAT_MACROS:
according to the comment, they must be defined before including some
standard headers. Are these defines still useful though?
- _FORTIFY_SOURCE: must come before any standard library include.
- _WIN32_WINNT: must come before including the Windows API headers.
- ATTRIBUTE_PRINTF, ATTRIBUTE_NONNULL: we override what ansidecl.h
gives us. So better define it in gdbsupport.inc.h, so that there is
no chance of some source file including ansidecl.h without having the
overrides.
- HAVE_USEFUL_SBRK: this needs to be seen everywhere, in case one does
`#ifdef HAVE_USEFUL_SBRK`.
- Inclusion of poison.h, so that all code everywhere, uses our
overrides of xfree & co. This requires including liberty.h in
poison.h, since it overrides some macros from it.
- Since poison.h uses xfree and xfree uses some traits defined in
poison.h, but gnulib also defines its own (non-templated) version of
xfree, I had some strange error due to some non-compatible
redefinitions. All in all, it becomes simpler to move xfree to
poison.h and get rid of gdb-xfree.h, so do that.
Change-Id: I21ad69e5f38f9fd7a3943d9f96d3782739d4b6df
---
gdbsupport/common-defs.h | 147 -----------------------------------
gdbsupport/common-utils.cc | 1 -
gdbsupport/gdb-xfree.h | 41 ----------
gdbsupport/gdb_unique_ptr.h | 1 -
gdbsupport/gdbsupport.inc.h | 149 ++++++++++++++++++++++++++++++++++++
gdbsupport/poison.h | 20 ++++-
6 files changed, 167 insertions(+), 192 deletions(-)
delete mode 100644 gdbsupport/gdb-xfree.h
@@ -20,60 +20,6 @@
#ifndef COMMON_COMMON_DEFS_H
#define COMMON_COMMON_DEFS_H
-/* From:
- https://www.gnu.org/software/gnulib/manual/html_node/stdint_002eh.html
-
- "On some hosts that predate C++11, when using C++ one must define
- __STDC_CONSTANT_MACROS to make visible the definitions of constant
- macros such as INTMAX_C, and one must define __STDC_LIMIT_MACROS to
- make visible the definitions of limit macros such as INTMAX_MAX.".
-
- And:
- https://www.gnu.org/software/gnulib/manual/html_node/inttypes_002eh.html
-
- "On some hosts that predate C++11, when using C++ one must define
- __STDC_FORMAT_MACROS to make visible the declarations of format
- macros such as PRIdMAX."
-
- Must do this before including any system header, since other system
- headers may include stdint.h/inttypes.h. */
-#define __STDC_CONSTANT_MACROS 1
-#define __STDC_LIMIT_MACROS 1
-#define __STDC_FORMAT_MACROS 1
-
-/* Some distros enable _FORTIFY_SOURCE by default, which on occasion
- has caused build failures with -Wunused-result when a patch is
- developed on a distro that does not enable _FORTIFY_SOURCE. We
- enable it here in order to try to catch these problems earlier;
- plus this seems like a reasonable safety measure. The check for
- optimization is required because _FORTIFY_SOURCE only works when
- optimization is enabled. If _FORTIFY_SOURCE is already defined,
- then we don't do anything. Also, on MinGW, fortify requires
- linking to -lssp, and to avoid the hassle of checking for
- that and linking to it statically, we just don't define
- _FORTIFY_SOURCE there. */
-
-#if (!defined _FORTIFY_SOURCE && defined __OPTIMIZE__ && __OPTIMIZE__ > 0 \
- && !defined(__MINGW32__))
-#define _FORTIFY_SOURCE 2
-#endif
-
-/* We don't support Windows versions before XP, so we define
- _WIN32_WINNT correspondingly to ensure the Windows API headers
- expose the required symbols.
-
- NOTE: this must be kept in sync with common.m4. */
-#if defined (__MINGW32__) || defined (__CYGWIN__)
-# ifdef _WIN32_WINNT
-# if _WIN32_WINNT < 0x0501
-# undef _WIN32_WINNT
-# define _WIN32_WINNT 0x0501
-# endif
-# else
-# define _WIN32_WINNT 0x0501
-# endif
-#endif /* __MINGW32__ || __CYGWIN__ */
-
#include <stdarg.h>
#include <stdio.h>
@@ -93,89 +39,6 @@
#include <alloca.h>
#endif
-#include "ansidecl.h"
-/* This is defined by ansidecl.h, but we prefer gnulib's version. On
- MinGW, gnulib might enable __USE_MINGW_ANSI_STDIO, which may or not
- require use of attribute gnu_printf instead of printf. gnulib
- checks that at configure time. Since _GL_ATTRIBUTE_FORMAT_PRINTF_STANDARD
- is compatible with ATTRIBUTE_PRINTF, simply use it. */
-#undef ATTRIBUTE_PRINTF
-#define ATTRIBUTE_PRINTF _GL_ATTRIBUTE_FORMAT_PRINTF_STANDARD
-
-/* This is defined by ansidecl.h, but we disable the attribute.
-
- Say a developer starts out with:
- ...
- extern void foo (void *ptr) __attribute__((nonnull (1)));
- void foo (void *ptr) {}
- ...
- with the idea in mind to catch:
- ...
- foo (nullptr);
- ...
- at compile time with -Werror=nonnull, and then adds:
- ...
- void foo (void *ptr) {
- + gdb_assert (ptr != nullptr);
- }
- ...
- to catch:
- ...
- foo (variable_with_nullptr_value);
- ...
- at runtime as well.
-
- Said developer then verifies that the assert works (using -O0), and commits
- the code.
-
- Some other developer then checks out the code and accidentally writes some
- variant of:
- ...
- foo (variable_with_nullptr_value);
- ...
- and builds with -O2, and ... the assert doesn't trigger, because it's
- optimized away by gcc.
-
- There's no suppported recipe to prevent the assertion from being optimized
- away (other than: build with -O0, or remove the nonnull attribute). Note
- that -fno-delete-null-pointer-checks does not help. A patch was submitted
- to improve gcc documentation to point this out more clearly (
- https://gcc.gnu.org/pipermail/gcc-patches/2021-July/576218.html ). The
- patch also mentions a possible workaround that obfuscates the pointer
- using:
- ...
- void foo (void *ptr) {
- + asm ("" : "+r"(ptr));
- gdb_assert (ptr != nullptr);
- }
- ...
- but that still requires the developer to manually add this in all cases
- where that's necessary.
-
- A warning was added to detect the situation: -Wnonnull-compare, which does
- help in detecting those cases, but each new gcc release may indicate a new
- batch of locations that needs fixing, which means we've added a maintenance
- burden.
-
- We could try to deal with the problem more proactively by introducing a
- gdb_assert variant like:
- ...
- void gdb_assert_non_null (void *ptr) {
- asm ("" : "+r"(ptr));
- gdb_assert (ptr != nullptr);
- }
- void foo (void *ptr) {
- gdb_assert_nonnull (ptr);
- }
- ...
- and make it a coding style to use it everywhere, but again, maintenance
- burden.
-
- With all these things considered, for now we go with the solution with the
- least maintenance burden: disable the attribute, such that we reliably deal
- with it everywhere. */
-#undef ATTRIBUTE_NONNULL
-#define ATTRIBUTE_NONNULL(m)
#define ATTRIBUTE_UNUSED_RESULT __attribute__ ((__warn_unused_result__))
#define ATTRIBUTE_USED __attribute__ ((__used__))
@@ -193,18 +56,8 @@
#include "common-debug.h"
#include "cleanups.h"
#include "common-exceptions.h"
-#include "gdbsupport/poison.h"
/* Pull in gdb::unique_xmalloc_ptr. */
#include "gdbsupport/gdb_unique_ptr.h"
-/* sbrk on macOS is not useful for our purposes, since sbrk(0) always
- returns the same value. brk/sbrk on macOS is just an emulation
- that always returns a pointer to a 4MB section reserved for
- that. */
-
-#if defined (HAVE_SBRK) && !__APPLE__
-#define HAVE_USEFUL_SBRK 1
-#endif
-
#endif /* COMMON_COMMON_DEFS_H */
@@ -21,7 +21,6 @@
#include "common-utils.h"
#include "host-defs.h"
#include "gdbsupport/gdb-safe-ctype.h"
-#include "gdbsupport/gdb-xfree.h"
void *
xzalloc (size_t size)
deleted file mode 100644
@@ -1,41 +0,0 @@
-/* Copyright (C) 1986-2024 Free Software Foundation, Inc.
-
- This file is part of GDB.
-
- This program is free software; you can redistribute it and/or modify
- it under the terms of the GNU General Public License as published by
- the Free Software Foundation; either version 3 of the License, or
- (at your option) any later version.
-
- This program is distributed in the hope that it will be useful,
- but WITHOUT ANY WARRANTY; without even the implied warranty of
- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- GNU General Public License for more details.
-
- You should have received a copy of the GNU General Public License
- along with this program. If not, see <http://www.gnu.org/licenses/>. */
-
-#ifndef GDBSUPPORT_GDB_XFREE_H
-#define GDBSUPPORT_GDB_XFREE_H
-
-#include "gdbsupport/poison.h"
-
-/* GDB uses this instead of 'free', it detects when it is called on an
- invalid type. */
-
-template <typename T>
-static void
-xfree (T *ptr)
-{
- static_assert (IsFreeable<T>::value, "Trying to use xfree with a non-POD \
-data type. Use operator delete instead.");
-
- if (ptr != NULL)
-#ifdef GNULIB_NAMESPACE
- GNULIB_NAMESPACE::free (ptr); /* ARI: free */
-#else
- free (ptr); /* ARI: free */
-#endif
-}
-
-#endif /* GDBSUPPORT_GDB_XFREE_H */
@@ -22,7 +22,6 @@
#include <memory>
#include <string>
-#include "gdbsupport/gdb-xfree.h"
namespace gdb
{
@@ -33,3 +33,152 @@
#undef PACKAGE_STRING
#undef PACKAGE_TARNAME
#undef PACKAGE_VERSION
+
+/* From:
+ https://www.gnu.org/software/gnulib/manual/html_node/stdint_002eh.html
+
+ "On some hosts that predate C++11, when using C++ one must define
+ __STDC_CONSTANT_MACROS to make visible the definitions of constant
+ macros such as INTMAX_C, and one must define __STDC_LIMIT_MACROS to
+ make visible the definitions of limit macros such as INTMAX_MAX.".
+
+ And:
+ https://www.gnu.org/software/gnulib/manual/html_node/inttypes_002eh.html
+
+ "On some hosts that predate C++11, when using C++ one must define
+ __STDC_FORMAT_MACROS to make visible the declarations of format
+ macros such as PRIdMAX."
+
+ Must do this before including any system header, since other system
+ headers may include stdint.h/inttypes.h. */
+#define __STDC_CONSTANT_MACROS 1
+#define __STDC_LIMIT_MACROS 1
+#define __STDC_FORMAT_MACROS 1
+
+/* Some distros enable _FORTIFY_SOURCE by default, which on occasion
+ has caused build failures with -Wunused-result when a patch is
+ developed on a distro that does not enable _FORTIFY_SOURCE. We
+ enable it here in order to try to catch these problems earlier;
+ plus this seems like a reasonable safety measure. The check for
+ optimization is required because _FORTIFY_SOURCE only works when
+ optimization is enabled. If _FORTIFY_SOURCE is already defined,
+ then we don't do anything. Also, on MinGW, fortify requires
+ linking to -lssp, and to avoid the hassle of checking for
+ that and linking to it statically, we just don't define
+ _FORTIFY_SOURCE there. */
+
+#if (!defined _FORTIFY_SOURCE && defined __OPTIMIZE__ && __OPTIMIZE__ > 0 \
+ && !defined(__MINGW32__))
+#define _FORTIFY_SOURCE 2
+#endif
+
+/* We don't support Windows versions before XP, so we define
+ _WIN32_WINNT correspondingly to ensure the Windows API headers
+ expose the required symbols.
+
+ NOTE: this must be kept in sync with common.m4. */
+#if defined (__MINGW32__) || defined (__CYGWIN__)
+# ifdef _WIN32_WINNT
+# if _WIN32_WINNT < 0x0501
+# undef _WIN32_WINNT
+# define _WIN32_WINNT 0x0501
+# endif
+# else
+# define _WIN32_WINNT 0x0501
+# endif
+#endif /* __MINGW32__ || __CYGWIN__ */
+
+#include "ansidecl.h"
+/* This is defined by ansidecl.h, but we prefer gnulib's version. On
+ MinGW, gnulib might enable __USE_MINGW_ANSI_STDIO, which may or not
+ require use of attribute gnu_printf instead of printf. gnulib
+ checks that at configure time. Since _GL_ATTRIBUTE_FORMAT_PRINTF_STANDARD
+ is compatible with ATTRIBUTE_PRINTF, simply use it. */
+#undef ATTRIBUTE_PRINTF
+#define ATTRIBUTE_PRINTF _GL_ATTRIBUTE_FORMAT_PRINTF_STANDARD
+
+/* This is defined by ansidecl.h, but we disable the attribute.
+
+ Say a developer starts out with:
+ ...
+ extern void foo (void *ptr) __attribute__((nonnull (1)));
+ void foo (void *ptr) {}
+ ...
+ with the idea in mind to catch:
+ ...
+ foo (nullptr);
+ ...
+ at compile time with -Werror=nonnull, and then adds:
+ ...
+ void foo (void *ptr) {
+ + gdb_assert (ptr != nullptr);
+ }
+ ...
+ to catch:
+ ...
+ foo (variable_with_nullptr_value);
+ ...
+ at runtime as well.
+
+ Said developer then verifies that the assert works (using -O0), and commits
+ the code.
+
+ Some other developer then checks out the code and accidentally writes some
+ variant of:
+ ...
+ foo (variable_with_nullptr_value);
+ ...
+ and builds with -O2, and ... the assert doesn't trigger, because it's
+ optimized away by gcc.
+
+ There's no suppported recipe to prevent the assertion from being optimized
+ away (other than: build with -O0, or remove the nonnull attribute). Note
+ that -fno-delete-null-pointer-checks does not help. A patch was submitted
+ to improve gcc documentation to point this out more clearly (
+ https://gcc.gnu.org/pipermail/gcc-patches/2021-July/576218.html ). The
+ patch also mentions a possible workaround that obfuscates the pointer
+ using:
+ ...
+ void foo (void *ptr) {
+ + asm ("" : "+r"(ptr));
+ gdb_assert (ptr != nullptr);
+ }
+ ...
+ but that still requires the developer to manually add this in all cases
+ where that's necessary.
+
+ A warning was added to detect the situation: -Wnonnull-compare, which does
+ help in detecting those cases, but each new gcc release may indicate a new
+ batch of locations that needs fixing, which means we've added a maintenance
+ burden.
+
+ We could try to deal with the problem more proactively by introducing a
+ gdb_assert variant like:
+ ...
+ void gdb_assert_non_null (void *ptr) {
+ asm ("" : "+r"(ptr));
+ gdb_assert (ptr != nullptr);
+ }
+ void foo (void *ptr) {
+ gdb_assert_nonnull (ptr);
+ }
+ ...
+ and make it a coding style to use it everywhere, but again, maintenance
+ burden.
+
+ With all these things considered, for now we go with the solution with the
+ least maintenance burden: disable the attribute, such that we reliably deal
+ with it everywhere. */
+#undef ATTRIBUTE_NONNULL
+#define ATTRIBUTE_NONNULL(m)
+
+#include "gdbsupport/poison.h"
+
+/* sbrk on macOS is not useful for our purposes, since sbrk(0) always
+ returns the same value. brk/sbrk on macOS is just an emulation
+ that always returns a pointer to a 4MB section reserved for
+ that. */
+
+#if defined (HAVE_SBRK) && !__APPLE__
+#define HAVE_USEFUL_SBRK 1
+#endif
@@ -20,6 +20,7 @@
#ifndef COMMON_POISON_H
#define COMMON_POISON_H
+#include "libiberty.h"
#include "traits.h"
#include "obstack.h"
@@ -90,8 +91,23 @@ using IsMallocable = std::is_trivially_constructible<T>;
template<typename T>
using IsFreeable = gdb::Or<std::is_trivially_destructible<T>, std::is_void<T>>;
-template <typename T, typename = gdb::Requires<gdb::Not<IsFreeable<T>>>>
-void free (T *ptr) = delete;
+/* GDB uses this instead of 'free', it detects when it is called on an
+ invalid type. */
+
+template <typename T>
+static void
+xfree (T *ptr)
+{
+ static_assert (IsFreeable<T>::value, "Trying to use xfree with a non-POD \
+data type. Use operator delete instead.");
+
+ if (ptr != NULL)
+#ifdef GNULIB_NAMESPACE
+ GNULIB_NAMESPACE::free (ptr); /* ARI: free */
+#else
+ free (ptr); /* ARI: free */
+#endif
+}
template<typename T>
static T *