We have gdb::handle_eintr, which allows us to rewrite:
...
ssize_t ret;
do
{
errno = 0;
ret = ::write (pipe[1], "+", 1);
}
while (ret == -1 && errno == EINTR);
...
into:
...
ssize_t ret = gdb::handle_eintr (-1, ::write, pipe[1], "+", 1);
...
However, the call to write got a bit mangled, requiring effort to decode it
back to its original form.
Instead, add a new function gdb::write that allows us to write:
...
ssize_t ret = gdb::write (pipe[1], "+", 1);
...
Likewise for waitpid, read and close.
Tested on x86_64-linux.
In gdb/aarch64-linux-tdep.c we find:
...
gdb::byte_vector za_zeroed (za_bytes, 0);
regcache->raw_supply (tdep->sme_za_regnum, za_zeroed);
...
We can't use reg_buffer::raw_supply_zeroed here because only part of the
register is written.
Add raw_supply_part_zeroed, and use it instead.
Likewise elsewhere in AArch64 tdep code.
Tested on aarch64-linux.
Approved-By: Luis Machado <luis.machado@arm.com>
gdb::hash_enum is a workaround for a small oversight in C++11:
std::hash was not defined for enumeration types. This was rectified
in C++14 and so we can remove the local workaround.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
Currently, gdb_abspath uses the current_directory global. However,
background threads need to capture this global to avoid races with the
user using "cd".
This patch changes this function to accept a cwd parameter, in
prepration for this.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31716
While trying to substitute some std::vector type A in the code with a
gdb::array_view:
...
- using A = std::vector<T>
+ using A = gdb::array_view<T>
....
I ran into the problem that the code was using A::iterator while
gdb::array_view doesn't define such a type.
Fix this by:
- adding types gdb::array_view::iterator and gdb::array_view::const_iterator,
- using them in gdb::array_view::(c)begin and gdb::array_view::(c)end, as is
usual, and
- using them explicitly in a unit test.
Tested on aarch64-linux.
Approved-By: Tom Tromey <tom@tromey.com>
There's a plan to replace gdb::array_view with std::span (PR31422), and making
gdb::array_view more like std::span helps with that.
One difference is that std::span has:
...
constexpr iterator begin() const noexcept;
constexpr const_iterator cbegin() const noexcept;
...
while gdb::array_view has:
...
constexpr T *begin () noexcept;
constexpr const T *begin () const noexcept;
...
Fix this by renaming the second variant to cbegin, and making the first
variant const.
Likewise for gdb::array_view::end.
Tested on aarch64-linux.
Approved-By: Tom Tromey <tom@tromey.com>
When building gdb with gcc 12 and -fsanitize=threads while renabling
background dwarf reading by setting dwarf_synchronous to false, I run into:
...
(gdb) file amd64-watchpoint-downgrade
Reading symbols from amd64-watchpoint-downgrade...
(gdb) watch global_var
==================
WARNING: ThreadSanitizer: data race (pid=20124)
Read of size 8 at 0x7b80000500d8 by main thread:
#0 cooked_index_entry::full_name(obstack*, bool) const cooked-index.c:220
#1 cooked_index::get_main_name(obstack*, language*) const cooked-index.c:735
#2 cooked_index_worker::wait(cooked_state, bool) cooked-index.c:559
#3 cooked_index::wait(cooked_state, bool) cooked-index.c:631
#4 cooked_index_functions::wait(objfile*, bool) cooked-index.h:729
#5 cooked_index_functions::compute_main_name(objfile*) cooked-index.h:806
#6 objfile::compute_main_name() symfile-debug.c:461
#7 find_main_name symtab.c:6503
#8 main_language() symtab.c:6608
#9 set_initial_language_callback symfile.c:1634
#10 get_current_language() language.c:96
...
Previous write of size 8 at 0x7b80000500d8 by thread T1:
#0 cooked_index_shard::finalize(parent_map_map const*) \
dwarf2/cooked-index.c:409
#1 operator() cooked-index.c:663
...
...
SUMMARY: ThreadSanitizer: data race cooked-index.c:220 in \
cooked_index_entry::full_name(obstack*, bool) const
==================
Hardware watchpoint 1: global_var
(gdb) PASS: gdb.arch/amd64-watchpoint-downgrade.exp: watch global_var
...
This was also reported in PR31715.
This is due do gcc PR110799 [1], generating wrong code with
-fhoist-adjacent-loads, and causing a false positive for
-fsanitize=threads.
Work around the gcc PR by forcing -fno-hoist-adjacent-loads for gcc <= 13
and -fsanitize=threads.
Tested in that same configuration on x86_64-linux. Remaining ThreadSanitizer
problems are the ones reported in PR31626 (gdb.rust/dwindex.exp) and
PR32247 (gdb.trace/basic-libipa.exp).
PR gdb/31715
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31715
Tested-By: Bernd Edlinger <bernd.edlinger@hotmail.de>
Approved-By: Tom Tromey <tom@tromey.com>
[1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110799
There is a single declaration of set_tdesc_osabi that is shared
between gdbserver/ and gdb/, this declaration takes a 'const char *'
argument which is the string representing an osabi.
Then in gdb/ we have an overload of set_tdesc_osabi which takes an
'enum gdb_osabi'.
In this commit I change the shared set_tdesc_osabi to be the version
which takes an 'enum gdb_osabi', and I remove the version which takes
a 'const char *'. All users of set_tdesc_osabi are updated to pass an
'enum gdb_osabi'.
The features/ code, which is generated from the xml files, requires a
new function to be added to osabi.{c,h} which can return a string
representation of an 'enum gdb_osabi'. With that new function in
place the features/ code is regenerated.
This change is being made to support the next commit. In the next
commit gdbserver will be updated to call set_tdesc_osabi in more
cases. The problem is that gdbserver stores the osabi as a string.
The issue here is that a typo in the gdbserver/ code might go
unnoticed and result in gdbserver sending back an invalid osabi
string.
To fix this we want gdbserver to pass an 'enum gdb_osabi' to the
set_tdesc_osabi function. With that requirement in place it seems to
make sense if all calls to set_tdesc_osabi pass an 'enum gdb_osabi'.
There should be no user visible changes after this commit.
Approved-By: Luis Machado <luis.machado@arm.com>
Approved-By: Simon Marchi <simon.marchi@efficios.com>
In future commits I want to call set_tdesc_osabi from gdbserver/
code. Currently the only version of set_tdesc_osabi available to
gdbserver takes a string representing the osabi.
The problem with this is that, having lots of calls to set_tdesc_osabi
which all take a string is an invite for a typo to slip in. This typo
could potentially go unnoticed until someone tries to debug the wrong
combination of GDB and gdbserver, at which point GDB will fail to find
the correct gdbarch because it doesn't understand the osabi string.
It would be better if the set_tdesc_osabi calls in gdbserver could
take an 'enum gdb_osabi' value and then convert this to the "correct"
string internally. In this way we are guaranteed to always have a
valid, known, osabi string.
This commit splits the osabi related code, which currently lives
entirely on the GDB side, between gdb/ and gdbsupport/. I've moved
the enum definition along with the array of osabi names into
gdbsupport/. Then all the functions that access the names list, and
which convert between names and enum values are also moved.
I've taken the opportunity of this move to add a '.def' file which
contains all the enum names along with the name strings. This '.def'
file is then used to create 'enum gdb_osabi' as well as the array of
osabi name strings. By using a '.def' file we know that the enum
order will always match the name string array.
This commit is just a refactor, there are no user visible changes
after this commit. This commit doesn't change how gdbserver sets the
target description osabi string, that will come in the next commit.
Approved-By: Luis Machado <luis.machado@arm.com>
Approved-By: Simon Marchi <simon.marchi@efficios.com>
GDB deprecated the commands "show/set mpx bound" in GDB 15.1, as Intel
listed Intel(R) Memory Protection Extensions (MPX) as removed in 2019.
MPX is also deprecated in gcc (since v9.1), the linux kernel (since v5.6)
and glibc (since v2.35). Let's now remove MPX support in GDB completely.
This includes the removal of:
- MPX functionality including register support
- deprecated mpx commands
- i386 and amd64 implementation of the hooks report_signal_info and
get_siginfo_type
- tests
- and pretty printer.
We keep MPX register numbers to not break compatibility with old gdbservers.
Approved-By: Felix Willgerodt <felix.willgerodt@intel.com>
Event tracing allows GDB to show information about interesting asynchronous
events when tracing with Intel PT. Subsequent patches will add support for
displaying each type of event.
Enabling event-tracing unconditionally would result in rather noisy output, as
breakpoints themselves result in interrupt events. Which is why this patch adds
a set/show command to allow the user to enable/disable event-tracing before
starting a recording. The event-tracing setting has no effect on an already
active recording. The default setting is off. As event tracing will use the
auxiliary infrastructure added by ptwrite, the user can still disable printing
events, even when event-tracing was enabled, by using the /a switch for the
record instruction-history/function-call-history commands.
Reviewed-By: Eli Zaretskii <eliz@gnu.org>
Approved-By: Markus Metzger <markus.t.metzger@intel.com>
While reviewing "catch (...)" uses I came across:
...
scope_exit (EFP &&f)
try : m_exit_function ((!std::is_lvalue_reference<EFP>::value
&& std::is_nothrow_constructible<EF, EFP>::value)
? std::move (f)
: f)
{
}
catch (...)
{
/* "If the initialization of exit_function throws an exception,
calls f()." */
f ();
}
...
and while looking up the origin of the comment here [1] I saw right after:
...
throws: Nothing, unless the initialization of exit_function throws
...
I think that means that the exception should be rethrown, so fix this by doing
so.
Tested on aarch64-linux.
Approved-By: Tom Tromey <tom@tromey.com>
[1] https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2017/p0052r5.pdf
It occured to me that `intrusive_list<solib>`, as returned by
`solib_ops::current_sos`, for instance, is not very safe. The
current_sos method returns the ownership of the solib objects
(heap-allocated) to its caller, but the `intrusive_list<solib>` type
does not convey it. If a function is building an
`intrusive_list<solib>` and something throws, the solibs won't
automatically be deleted. Introduce owning_intrusive_list to fill this
gap.
Interface
---------
The interface of owning_intrusive_list is mostly equivalent to
intrusive_list, with the following differences:
- When destroyed, owning_intrusive_list deletes all element objects.
The clear method does so as well.
- The erase method destroys the removed object.
- The push_front, push_back and insert methods accept a `unique_ptr<T>`
(compared to `T &` for intrusive_list), taking ownership of the
object.
- owning_intrusive_list has emplace_front, emplace_back and emplace
methods, allowing to allocate and construct an object directly in the
list. This is really just a shorthand over std::make_unique and
insert (or push_back / push_front if you don't care about the return
value), but I think it is nicer to read:
list.emplace (pos, "hello", 2);
rather than
list.insert (pos, std::make_unique<Foo> ("hello", 2));
These methods are not `noexcept`, since the allocation or the
constructor could throw.
- owning_intrusive_list has a release method, allowing to remove an
element without destroying it. The release method returns a
pair-like struct with an iterator to the next element in the list
(like the erase method) and a unique pointer transferring the
ownership of the released element to the caller.
- owning_intrusive_list does not have a clear_and_dispose method, since
that is typically used to manually free items.
Implementation
--------------
owning_intrusive_list privately inherits from intrusive_list, in order
to re-use the linked list machinery. It adds ownership semantics around
it.
Testing
-------
Because of the subtle differences in the behavior in behavior and what
we want to test for each type of intrusive list, I didn't see how to
share the tests for the two implementations. I chose to copy the
intrusive_list tests and adjust them for owning_intrusive_list.
The verify_items function was made common though, and it tries to
dereference the items in the list, to make sure they have not been
deleted by mistake (which would be caught by Valgrind / ASan).
Change-Id: Idbde09c1417b79992a0a9534d6907433e706f760
Co-Authored-By: Pedro Alves <pedro@palves.net>
Reviewed-by: Keith Seitz <keiths@redhat.com>
Some methods of intrusive_list are marked noexcept. But really,
everything in that file could be noexcept. Add it everywhere.
The only one I had a doubt about is clear_and_dispose: what if the
disposer throws? The boost equivalent [1] is noexcept and requires the
disposer not to throw. The rationale is probably the same as for
destructors. What if the disposer throws for an element in the middle
of the list? Do you skip the remaining elements? Do you swallow the
exception and keep calling the disposer for the remaining elements?
It's simpler to say no exceptions allowed.
[1] https://www.boost.org/doc/libs/1_79_0/doc/html/boost/intrusive/list.html#idm33710-bb
Change-Id: I402646cb12c6b7906f4bdc2ad85203d8c8cdf2cc
Reviewed-by: Keith Seitz <keiths@redhat.com>
We already have one overload of the startswith function that takes a
std::string_view for both arguments. A later patch in this series is
going to be improved by having an overload that takes one argument as
a std::string_view and the other argument as a plain 'char *'.
This commit adds the new overload, but doesn't make use of it (yet).
There should be no user visible changes after this commit.
I spotted that the gdb.base/sysroot-debug-lookup.exp test that I added
recently actually had a KPASS when run with the
native-extended-gdbserver board. This was an oversight when adding
the test.
The failures in this test, when using the 'unix' board, are logged as
bug PR gdb/31804. The problem appears to be caused by the use of the
child_path function in find_separate_debug_file.
What happens on the 'unix' board is that the file is specified to GDB
with a target: prefix, however GDB spots that the target filesystem is
local to GDB and so opens the file without a target: prefix. When we
call into find_separate_debug_file the DIR and CANON_DIR arguments,
which are computed from the objfile_name() no longer have a target:
prefix.
However, in this test if the file was opened with a target: prefix,
then the sysroot also has a target: prefix. When child_path is called
it looks for a common prefix between CANON_DIR (from the objfile_name)
and the sysroot. However, the sysroot still has the target: prefix,
which means the child_path() call fails and returns nullptr.
What happens in the native-extended-gdbserver case is that GDB doesn't
see the target filesystem as local. Now the filename retains the
target: prefix, which means that in the child_path() call both the
sysroot and the CANON_DIR have a target: prefix, and so the
child_path() call succeeds. This allows GDB to progress, try some
additional paths, and then find the debug information.
So, this commit changes gdb.base/sysroot-debug-lookup.exp to expect
the test to succeed when using the native-extended-gdbserver protocol.
This leaves one KFAIL when using the native-extended-gdbserver board,
we find the debug information but (apparently) find it in the wrong
file. What's happening is that when GDB builds the filename for the
debug information we end up with a '//' string as a directory
separator, the test regexp only expects a single separator.
Instead of just fixing the test regexp, I've updated the path_join
function in gdbsupport/pathstuff.{cc,h} to allow for absolute paths to
appear in the argument list after the first argument. This means it's
now possible to do this:
auto result = path_join ("/a/b/c", "/d/e/f");
gdb_assert (result == "/a/b/c/d/e/f");
Additionally I've changed path_join so that it avoids adding
unnecessary directory separators. In the above case when the two
paths were joined GDB only added a single separator between 'c' and
'd'. But additionally, if we did this:
auto result = path_join ("/a/b/c/", "/d/e/f");
gdb_assert (result == "/a/b/c/d/e/f");
We'd still only get a single separator.
With these changes to path_join I can now make use of this function in
find_separate_debug_file. With this done I now have no KFAIL when
using the native-extended-gdbserver board.
After this commit we still have 2 KFAIL when not using the
native-gdbserver and unix boards, these will be addressed in the next
commit.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31804
Reviewed-By: Keith Seitz <keiths@redhat.com>
By default GDB will be printing the hex payload of the ptwrite package as
auxiliary information. To customize this, the user can register a ptwrite
filter function in python, that takes the payload and the PC as arguments and
returns a string which will be printed instead. Registering the filter
function is done using a factory pattern to make per-thread filtering easier.
Approved-By: Markus Metzger <markus.t.metzger@intel.com>
This enables gdb and gdbserver to communicate about ptwrite support. If
ptwrite support would be enabled unconditionally, GDBs with older libipt
versions would break.
Approved-By: Markus Metzger <markus.t.metzger@intel.com>
Reviewed-By: Eli Zaretskii <eliz@gnu.org>
This might have been useful during the C -> C++ conversion (not sure),
but it doesn't appear useful today. I don't see when enum-flags.h would
be used in a C context.
Change-Id: I6c7ed655757248a62a1bf6615995f42e8aa2b4bd
REALTIME_LO was only possibly previously defined in config/nm-nto.h,
which is now removed. So we can remove the #ifndef protecting against a
redefinition of REALTIME_LO in gdbsupport/signals.cc.
Change-Id: I021b9518ceaa6223bd480ff1e07e9a978b0b241e
Approved-by: Kevin Buettner <kevinb@redhat.com>
Now that all known uses of VLAs within GDB are removed, remove the
`-Wno-vla-cxx-extension` (which was used to silence clang warnings) and
add `-Wvla`, such that any use of a VLA will trigger a warning.
Change-Id: I69a8d7f93f973743165b0ba46f9c2ea8adb89025
Reviewed-By: Keith Seitz <keiths@redhat.com>
C++ 11 has a built-in attribute for this, no need to use a compat macro.
Change-Id: I90e4220d26e8f3949d91761f8a13cd9c37da3875
Reviewed-by: Lancelot Six <lancelot.six@amd.com>
Like the previous commit, add two overloads of gdb_tilde_expand, one
takes std::string and other takes gdb::unique_xmalloc_ptr<char>. Make
use of these overloads throughout GDB and gdbserver.
There should be no user visible changes after this commit.
Approved-By: Tom Tromey <tom@tromey.com>
Add two overloads of gdb_abspath, one which takes std::string and one
which takes gdb::unique_xmalloc_ptr<char>, then make use of these
overloads throughout GDB and gdbserver.
There should be no user visible changes after this commit.
Approved-By: Tom Tromey <tom@tromey.com>
I think that hashtabs should never be obstack-allocated. In the past
this was convenient sometimes, because any new data structure needed a
corresponding cleanup. However, with the switch to C++, resource
management has become much simpler; for example, a local variable can
simply be of type htab_up rather than hashtab_t, and the problem is
solved.
This patch removes hashtab_obstack_allocate to try to prevent this
anti-pattern from being used again.
Share the definition of I386_LINUX_XSAVE_XCR0_OFFSET between GDB and
gdbserver.
This commit moves the definition into gdbsupport/x86-xstate.h, which
allows the #define to be shared.
There should be no user visible changes after this commit.
Approved-By: Felix Willgerodt <felix.willgerodt@intel.com>
I believe that the get_exec_file function is unnecessary, and the code
can be simplified if we remove it.
Consider for instance when you "run" a program on Linux with native
debugging.
1. run_command_1 obtains the executable file from
`current_program_space->exec_filename ()`
2. it passes it to `run_target->create_inferior()`, which is
`inf_ptrace_target::create_inferior()` in this case, which then
passes it to `fork_inferior()`
3. `fork_inferior()` then has a fallback, where if the passed exec file
is nullptr, it gets its from `get_exec_file()`.
4. `get_exec_file()` returns `current_program_space->exec_filename ()`
- just like the things we started with - or errors out if the
current program space doesn't have a specified executable.
If there's no exec filename passed in step 1, there's not going to be
any in step 4, so it seems pointless to call `get_exec_file()`, we could
just error out when `exec_file` is nullptr. But we can't error out
directly in `fork_inferior()`, since the error is GDB-specific, and that
function is shared with GDBserver.
Speaking of GDBserver, all code paths that lead to `fork_inferior()`
provide a non-nullptr exec file.
Therefore, to simplify things:
- Make `fork_inferior()` assume that the passed exec file is not
nullptr, don't call `get_exec_file()`
- Change some targets (darwin-nat, go32-nat, gnu-nat, inf-ptrace,
nto-procfs, procfs) to error out when the exec file passed to their
create_inferior method is nullptr. Some targets are fine with a
nullptr exec file, so we can't check that in `run_command_1()`.
- Add the `no_executable_specified_error()` function, which re-uses the
error message that `get_exec_file()` had.
- Change some targets (go32-nat, nto-procfs) to not call
`get_exec_file()`, since it's pointless for the same reason as in the
example above, if it returns, it's going the be the same value as the
`exec_file` parameter. Just rely on `exec_file`.
- Remove the final use of `get_exec_file()`, in `load_command()`.
- Remove the `get_exec_file()` implementations in GDB and GDBserver and
remove the shared declaration.
Change-Id: I601c16498e455f7baa1f111a179da2f6c913baa3
Approved-By: Tom Tromey <tom@tromey.com>
Calls of `get_exec_file (0)` are equivalent to just getting the exec
filename from the current program space. I'm looking to remove
`get_exec_file`, so replace these uses with
`current_program_space->exec_filename ()`.
Remove the `err` parameter of `get_exec_wrapper` since all the calls
that remain pass 1, meaning to error out if no executable is specified.
Change-Id: I7729ea4c7f03dbb046211cc5aa3858ab3a551965
Approved-By: Tom Tromey <tom@tromey.com>
Currently, if you configure gdb with explicit --enable-threading, but
then configure detects std::thread does not work, configure silently
disables threading support and continues configuring.
This patch makes that scenario cause a configuration error, like so:
$ /home/pedro/gdb/src/configure --enable-threading && make
...
configure: error: std::thread does not work; disable threading
make[1]: *** [Makefile:11225: configure-gdbsupport] Error 1
make[1]: Leaving directory '/home/pedro/gdb/build-windows-threads'
make: *** [Makefile:1041: all] Error 2
$
Additionally, if you don't explicitly pass --enable-threading, and
std::thread does not work, we will now get a warning (and the build
continues):
$ /home/pedro/gdb/src/configure && make
...
configure: WARNING: std::thread does not work; disabling threading
...
This is similar to how we handle --enable-tui and missing curses. The
code and error/warning messages were borrowed from there.
Change-Id: I73a8b580d1e2a796b23136920c0e181408ae1b22
Approved-By: Tom Tromey <tom@tromey.com>
With a x86_64-pc-mingw32 toolchain there is a build issue
whether or not the --disable-threading option is used.
The problem happens because _WIN32_WINNT is defined to 0x501
before #include <mutex> which makes the compilation abort
due to missing support for __gthread_cond_t in std_mutex.h,
which is conditional on _WIN32_WINNT >= 0x600.
Fix the case when --disable-threading is used, by only
including <mutex> in gdb/complaints.c when STD_CXX_THREAD
is defined.
Additionally make the configure script try to #include <mutex>
to automatically select --disable-threading when the header file
is not able to compile.
Approved-By: Tom Tromey <tom@tromey.com>
In my occasional and continuing campaign against realloc, this patch
changes event-loop.cc to use std::vector to keep track of pollfd
objects. Regression tested on x86-64 Fedora 38.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
Approved-By: John Baldwin <jhb@FreeBSD.org>
There is no reason the callers of these functions need to change the
returned string, so change the `char *` return types to `const char *`.
Update a few callers to also use `const char *`.
Change-Id: I94adff574d5e1b326e8cc688cf1817a15b408b96
Approved-By: Tom Tromey <tom@tromey.com>
When building with clang 18, I see:
CXX aarch64-linux-tdep.o
/home/smarchi/src/binutils-gdb/gdb/aarch64-linux-tdep.c:1299:26: error: variable length arrays in C++ are a Clang extension [-Werror,-Wvla-cxx-extension]
1299 | gdb_byte za_zeroed[za_bytes];
| ^~~~~~~~
/home/smarchi/src/binutils-gdb/gdb/aarch64-linux-tdep.c:1299:26: note: read of non-const variable 'za_bytes' is not allowed in a constant expression
/home/smarchi/src/binutils-gdb/gdb/aarch64-linux-tdep.c:1282:10: note: declared here
1282 | size_t za_bytes = std::pow (sve_vl_from_vg (svg), 2);
| ^
Since we are using VLAs right now, that warning doesn't make sense for
us. add `-Wno-vla-cxx-extension` to the list of warning flags we try to
enable. If we ever choose to disallow VLAs, we can remove that flag.
Change-Id: Ie41feafc50c343f6e75333d4f836ce32fbeb6d8c
Now that defs.h, server.h and common-defs.h are included via the
`-include` option, it is no longer necessary for source files to include
them. Remove all the inclusions of these files I could find. Update
the generation scripts where relevant.
Change-Id: Ia026cff269c1b7ae7386dd3619bc9bb6a5332837
Approved-By: Pedro Alves <pedro@palves.net>
The motivation for this change is for analysis tools and IDEs to be
better at analyzing header files on their own.
There are some definitions and includes we want to occur at the very
beginning of all translation units. The way we currently do that is by
requiring all source files (.c and .cc files) to include one of defs.h
(for gdb), server.h (for gdbserver) of common-defs.h (for gdbsupport and
shared source files). These special header files define and include
everything that needs to be included at the very beginning. Other
header files are written in a way that assume that these special
"prologue" header files have already been included.
My problem with that is that my editor (clangd-based) provides a very
bad experience when editing header files. Since clangd doesn't know
that one of defs.h/server.h/common-defs.h was included already, a lot of
things are flagged as errors. For instance, CORE_ADDR is not known.
It's possible to edit the files in this state, but a lot of the power of
the editor is unavailable.
My proposal to help with this is to include those things we always want
to be there using the compilers' `-include` option. Tom Tromey said
that the current approach might exist because not all compilers used to
have an option like this. But I believe that it's safe to assume they
do today.
With this change, clangd picks up the -include option from the compile
command, and is able to analyze the header file correctly, as it sees
all that stuff included or defined by that -include option. That works
because when editing a header file, clangd tries to get the compilation
flags from a source file that includes said header file.
This change is a bit self-serving, because it addresses one of my
frustrations when editing header files, but it might help others too.
I'd be curious to know if others encounter the same kinds of problems
when editing header files. Also, even if the change is not necessary by
any means, I think the solution of using -include for stuff we always
want to be there is more elegant than the current solution.
Even with this -include flag, many header files currently don't include
what they use, but rather depend on files included before them. This
will still cause errors when editing them, but it should be easily
fixable by adding the appropriate include. There's no rush to do so, as
long as the code still compiles, it's just a convenience thing.
The changes are:
- Add the appropriate `-include` option to the various Makefiles.
- There is one particularity for gdbserver's Makefile: we do not want
to include server.h when building `gdbreplay.o`, as `gdbreplay.cc`
doesn't include it. So we can't simply put the `-include` in
`INTERNAL_CFLAGS`. Add the `-include server.h` option to the
`COMPILE` and `IPAGENT_COMPILE` variables, and added a special rule
to compile `gdbreplay.o` with `-include gdbsupport/common-defs.h`.
- Remove the `-include` option from the `check-headers` rule in
gdb/Makefile.in, since it is already included in `INTERNAL_CFLAGS`.
Change-Id: If3e345d00a9fc42336322f1d8286687d22134340
Approved-By: Pedro Alves <pedro@palves.net>
Reformat some variables definitions. I think it makes them easier to
read, and it also makes diffs clearer.
Change-Id: I82f63ba0e6d0fe268eb1f1ad5ab22c3cd016ab02
Approved-By: Pedro Alves <pedro@palves.net>
This patch makes allocate_on_obstack a little bit safer, by enforcing
the rule that objects allocated on an obstack must have a trivial
destructor.
The static assert is done in a method -- doing it inside the class
itself won't work because the class is incomplete at that point.
I noticed in passing that the include guard in the file
gdbsupport/gdb-checked-static-cast.h was wrong, it includes the word
DYNAMIC when STATIC would be better, fixed in this commit.
There should be no user visible changes after this commit.
