On debian 12, I run into:
...
(gdb) target remote | vgdb --wait=2 --max-invoke-ms=2500 --pid=618591^M
Remote debugging using | vgdb --wait=2 --max-invoke-ms=2500 --pid=618591^M
relaying data between gdb and process 618591^M
warning: remote target does not support file transfer, \
attempting to access files from local filesystem.^M
Reading symbols from /lib/ld-linux-aarch64.so.1...^M
(No debugging symbols found in /lib/ld-linux-aarch64.so.1)^M
0x000000000401a980 in ?? () from /lib/ld-linux-aarch64.so.1^M
(gdb) FAIL: gdb.base/valgrind-infcall.exp: target remote for vgdb
...
The problem is that we're expecting to match either of these regexps:
...
set start_re1 " in \\.?_start "
set start_re2 "\\.?_start \\(\\) at "
...
but there are no dwarf or elf symbols present.
Fix this by also allowing:
...
set start_re3 "$::hex in \\?\\? \\(\\) from "
...
Tested on aarch64-linux.
Approved-By: Tom Tromey <tom@tromey.com>
When the linker sees an input object containing nothing but IR during
rescan, it should ignore it (LTO phase is over). But if the input object
is a fat IR object, which has non-IR code as well, it should be used to
resolve references as if it did not contain any IR at all. This patch
adds lto_type to bfd and linker avoids claiming a fat IR object if no IR
object should be claimed.
bfd/
PR ld/23935
* archive.c (_bfd_compute_and_write_armap): Check bfd_get_lto_type
instead of lto_slim_object.
* elflink.c (elf_link_add_object_symbols): Likewise.
* bfd.c (bfd_lto_object_type): New.
(bfd): Remove lto_slim_object and add lto_type.
(bfd_get_lto_type): New function.
* elf.c (lto_section): Removed.
(_bfd_elf_make_section_from_shdr): Don't set lto_slim_object.
* format.c: (lto_section): New.
(bfd_set_lto_type): New function.
(bfd_check_format_matches): Call bfd_set_lto_type.
* bfd-in2.h: Regenerated.
binutils/
PR ld/23935
* nm.c (display_rel_file): Check bfd_get_lto_type instead of
lto_slim_object.
ld/
PR ld/23935
* ldmain.c (add_archive_element): Don't claim a fat IR object if
no IR object should be claimed.
* testsuite/ld-plugin/lto.exp (pr20103): Adjust fat IR test.
Add PR ld/23935 test.
* testsuite/ld-plugin/pr23935a.c: New file.
* testsuite/ld-plugin/pr23935b.c: Likewise.
This commit:
commit 198ff6ff81
Date: Tue Jan 30 15:37:23 2024 +0000
gdb/gdbserver: share x86/linux tdesc caching
added some functions which are always defined, but their use is
guarded within various #ifdef blocks. As a result we were seeing
errors about defined, but unused, functions.
I've fixed this problem in this commit by wrapping the function
definitions within #ifdef blocks.
I'm a little worried that there might be too many #ifdef blocks within
this file, however, I'm going to commit this fix for now as this will
fix the build, then I'll think about if there's a better way to split
this file so we might avoid some of these #ifdef blocks.
After this commit:
commit 198ff6ff81
Date: Tue Jan 30 15:37:23 2024 +0000
gdb/gdbserver: share x86/linux tdesc caching
a possible use of an uninitialised variable was introduced, the
'tdesc' variable in i386_linux_core_read_description might be read
without being written too if 'xcr0' was 0.
This is fixed in this commit. I've updated the function to follow the
same pattern as amd64_linux_core_read_description, if xcr0 is 0 then
we select a default xcr0 value and use that to select a tdesc.
When building with Clang, I get:
CXX nat/x86-linux-tdesc-ipa.o
clang++: error: treating 'c' input as 'c++' when in C++ mode, this behavior is deprecated [-Werror,-Wdeprecated]
Fix that by adding the missing `-x c++` in the rule building
`gdb/nat/*.c` files for the in-process agent.
Change-Id: Ie53e4b9a8b57bef9669397fdfaf21617107c7180
Approved-By: Tom Tromey <tom@tromey.com>
When building GDB with clang, I see:
/usr/lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/unique_ptr.h:95:2: error: delete called on non-final 'addrmap_mutable' that has virtual functions but non-virtual destructor [-Werror,-Wdelete-non
-abstract-non-virtual-dtor]
95 | delete __ptr;
| ^
/usr/lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/unique_ptr.h:396:4: note: in instantiation of member function 'std::default_delete<addrmap_mutable>::operator()' requested here
396 | get_deleter()(std::move(__ptr));
| ^
/home/smarchi/src/binutils-gdb/gdb/addrmap.c:422:14: note: in instantiation of member function 'std::unique_ptr<addrmap_mutable>::~unique_ptr' requested here
422 | auto map = std::make_unique<struct addrmap_mutable> ();
| ^
Fix that by making `addrmap_mutable` final, and `addrmap_fixed` too
while at it.
Change-Id: I03aa0b0907c8d0e3390ddbedeb77d73b19b2b526
Approved-By: Tom Tromey <tom@tromey.com>
libcollector uses pthread_getspecific() and pthread_setspecific() to access
thread local memory. libcollector uses this memory to check that
interposed functions (like malloc, calloc or free) don't have recursion.
The first time we call calloc(), we call pthread_setspecific() to create
a thread-specific value.
On Ubuntu machine, pthread_setspecific() calls calloc(), and we cannot intercept
such recursion.
gcc supports thread-local storage. For example,
static __thread int reentrance = 0;
I rewrote code using this instead of pthread_setspecific().
gprofng/ChangeLog
2024-03-23 Vladimir Mezentsev <vladimir.mezentsev@oracle.com>
PR gprofng/31460
* libcollector/heaptrace.c: Use the __thread variable to check for
* reentry. Clean up code.
Cygwin/MinGW testing links in a set_unbuffered_mode.o object to all
test programs. When running the testsuite in parallel mode, on
Cygwin, I noticed errors like:
ERROR: remote_download to host of ..../build/set_unbuffered_mode.o to ..../build/set_unbuffered_mode_saved.o: cp: cannot open '..../build/set_unbuffered_mode.o' for reading: No such file or directory
...
ERROR: remote_download to host of ..../build/set_unbuffered_mode.o to ..../build/set_unbuffered_mode_saved.o: cp: cannot stat '..../build/set_unbuffered_mode.o': No such file or directory
...
ERROR: remote_download to host of ..../build/set_unbuffered_mode.o to ..../build/set_unbuffered_mode_saved.o: cp: skipping file '..../build/set_unbuffered_mode.o', as it was replaced while being copied
(Absolute paths elided above.)
The problem is that gdb_compile's unbuffered_mode_obj cache isn't
parallel safe. This is fixed in this commit.
Reviewed-by: Kevin Buettner <kevinb@redhat.com>
Change-Id: I67a289473c14ce0603d4b0beb755b124588f18d2
While working on Windows non-stop mode, I managed to introduce a bug
that led to fake_create_process being called. That then resulted in
GDB crashes later on, because fake_create_process added a thread with
an incorrect ptid for this target. It is putting dwThreadId in the
tid field of the ptid instead of on the lwp field. This is fixed by
this patch.
Change-Id: Iaee5d2deaa57c501f7e6909f8ac242af9b183215
If a BFD user is making use of a function like
bfd_get_section_contents to read a section into a pre-allocated
buffer, then that BFD user might also want to make use of
_bfd_section_size_insane prior to allocating the buffer they intend to
use in order to validate that the buffer size that plan to allocate is
sane.
This commit makes _bfd_section_size_insane public, by renaming it to
bfd_section_size_insane.
I've updated the existing uses within bfd/, I don't believe this
function is used outside of bfd/ currently.
One place that I plan to make use of this function is in
gdb/gdb_bfd.c, in the function gdb_bfd_get_full_section_contents.
This change isn't included in this commit, but will come later if/when
this has been merged into bfd.
There should be no change in behaviour after this commit.
bfd/
* bfd-in2.h (bfd_section_size_insane): Add declaration.
* compress.c (bfd_get_full_section_contents): Update for new name
of _bfd_section_size_insane.
(bfd_init_section_compress_status): Likewise.
* dwarf2.c (read_section): Likewise.
(_bfd_dwarf2_slurp_debug_info): Likewise.
* libbfd.h (_bfd_section_size_insane): Remove declaration.
* section.c (_bfd_section_size_insane): Rename to ...
(bfd_section_size_insane): ... this.
binutils/
* readelf.c (uncompress_section_contents): Update comment to
account for new name of _bfd_section_size_insane.
Move more setup of the readline global state relating to tab
completion into completer.c out of top.c.
Lots of the readline setup is done in init_main (top.c). This commit
moves those bits of initialisation that relate to completion, and
which are only set the one time, into completer.c. This does mean
that readline initialisation is now done in multiple locations, some
in init_main (top.c) and some in completer.c, but I think this is OK.
The work done in init_main is the general readline setup.
I think making static what can be made static, and having it all in
one file, makes things easier to reason about. So I'm OK with having
this split initialisation.
The only completion related thing which is still setup in top.c is
rl_completion_display_matches_hook. I've left this where it is for
now as rl_completion_display_matches_hook is also updated in the tui
code, and the display hook functions are not in completer.c anyway, so
moving this initialisation to completer.c would not allow anything
else to be made static.
There should be no user visible changes after this commit.
I noticed that completion_find_completion_word is only used within
completer.c, so lets make it static.
There should be no user visible changes after this commit.
This commit removes some code which is special casing the filename
completion logic. The code in question relates to finding the
beginning of the completion word and was first introduced, or modified
into its existing form in commit 7830cf6fb9 (from 2001).
The code being removed moved the start of the completion word backward
until a character in gdb_completer_file_name_break_characters was
found, or until we reached the end of the actual command.
However, I doubt that this is needed any more. The filename completer
has a corresponding filename_completer_handle_brkchars function which
provides gdb_completer_file_name_break_characters as the word break
characters to readline, and also sets rl_completer_quote_characters.
As such, I would expect readline to be able to correctly find the
start of the completion word.
There is one change which I've needed to make as a consequence of
removing the above code, and I think this is a bug fix.
In complete_line_internal_normal_command we initialised temporary
variable P to the CMD_ARGS; this is the complete text after the
command name. Meanwhile, complete_line_internal_normal_command also
accepts an argument WORD, which is the completion word that readline
found for us.
In the code I removed P was updated, it was first set to WORD, and
then moved backwards to the "new" start of the completion word.
But notice, the default for P is the complete command argument text,
and only if we are performing filename completion do we modify P to be
the completion word.
We then passed P through to the actual commands completion function.
If we are doing anything other than filename completion then the value
of P passed is the complete argument text.
If we are doing filename completion then the value of P passed is the
completion word.
In filename_completer we get two arguments TEXT and WORD, the TEXT
argument is the value of P which is the "new" completion word, while
WORD is the completion word that readline calculated.
After simplifying complete_line_internal_normal_command, and the
temporary P is removed, we always pass the complete argument text into
TEXT, while WORD remains the completion word that readline found.
Previously in filename_completer we actually tried to generate
completions based on TEXT, which worked fine as TEXT actually
contained the completion word that we found in
complete_line_internal_normal_command. But I believe that we should
be fine to use the completion word that readline found, so I have
updated filename_completer to generate completions based on WORD.
If I'm correct, then I don't expect to see any user visible changes
after this commit.
In completer.c there is some code that is surrounded with '#if 0',
this code:
#if 0
/* There is no way to do this just long enough to affect quote
inserting without also affecting the next completion. This
should be fixed in readline. FIXME. */
/* Ensure that readline does the right thing
with respect to inserting quotes. */
rl_completer_word_break_characters = "";
#endif
This code, in some form, and always defined out, has been around since
the original import of GDB. Though the comment hints at what the
problem might be, it's not really clear what the issue is. And
completion within GDB has moved on a long way since this code was
written ... but not used.
I'm proposing that we just remove this code.
If/when a problem comes up then we can look at how to solve it. Maybe
this code would be the answer ... but also, I suspect, given all the
changes ... maybe not. I'm not sure carrying around this code for
another 20+ years adds much value.
There should be no user visible changes after this commit.
Currently GDB only supports using single quotes for quoting things,
the reason for this, as explained in completer.c (next to the variable
gdb_completer_expression_quote_characters) is that double quoted
strings need to be treated differently by the C expression parser.
But for filenames I don't believe this restriction holds. The file
names as passed to things like the 'file' command are not passing
through the C expression parser, so it seems like we should be fine to
allow double quotes for quoting in this case.
And so, this commit extends GDB to allow double quotes for quoting
filenames. Maybe in future we might be able to allow double quote
quoting in additional places, but this seems enough for now.
The testing has been extended to cover double quotes in addition to
the existing single quote testing.
This change does a number of things:
1. Set rl_completer_quote_characters in filename_completer and
filename_completer_handle_brkchars, this overrides the default which
is set in complete_line_internal_1,
2. In advance_to_completion_word we now take a set of quote
characters as a parameter, the two callers
advance_to_expression_complete_word_point and
advance_to_filename_complete_word_point now pass in the required set
of quote characters,
3. In completion_find_completion_word we now use the currently active
set of quote characters, this means we'll use
gdb_completer_expression_quote_characters or
gdb_completer_file_name_quote_characters depending on what type of
things we are completing.
In gdb_completion_word_break_characters_throw, after calling
complete_line_internal, if the completion function chose to use a
custom word point then we set rl_completer_quote_characters to NULL.
However, nowhere do we set rl_completer_quote_characters back to its
default value, which is setup in init_main (top.c).
An example of something that uses a custom word point for its
completion is 'thread apply all ...'.
An example of something that relies on rl_completer_quote_characters
would be completion of a quoted filename that contains white space.
Consider this shell and GDB session. The <TAB> markers indicate where
I've used tab to trigger completion:
$ mkdir /tmp/aaa\ bbb
$ touch /tmp/aaa\ bbb/xx\ 11
$ touch /tmp/aaa\ bbb/xx\ 22
$ gdb -q
(gdb) file '/tmp/aaa bbb/xx<TAB><TAB>
xx 11 xx 22
(gdb) thread apply all hel<TAB>
(gdb) thread apply all help
(gdb) file '/tmp/aaa bbb/xx<TAB><TAB>
First I create a directory structure which uses white space within
file and directory names. Then within GDB I use the 'file' command
and use a single quote to quote the filename. When I tab complete GDB
correctly offers the two files within the directory '/tmp/aaa bbb/'.
This works because rl_completer_quote_characters contains the single
quote, and so readline knows that it is trying to complete the string
that starts after the single quote: /tmp/aaa bbb/xx
Next I invoke the completer for the 'thread apply all' command, to do
this I type 'thread apply all hel' and hit tab, this expands to the
one completion 'thread apply all help'. We can run this command or
not, it doesn't matter (there are no threads, so we'll get no output).
Now I repeat the original 'file' completion. This time though I don't
get offered any completions.
The reason is that the 'thread apply all' completer set
rl_completer_quote_characters to nullptr. Now, when readline tries to
figure out the word to complete it doesn't see the single quote as the
start of a quoted word, so instead readline falls back to the word
break characters, and in this case spots the white space. As a result
readline tries to complete the string 'bbb/xx' which obviously doesn't
have any completions.
By setting rl_completer_quote_characters each time completion is
invoked this problem is resolved and the second 'file' command
completes as expected.
I've extended gdb.base/filename-completion.exp to also test with
quoted filenames, and added a 'thread apply all' completion at the
start to expose this bug.
As setting of rl_completer_quote_characters is now all done in the
completer.c file the function get_gdb_completer_quote_characters()
could be made static. However, as this function is only used one time
to initialise rl_completer_quote_characters, I've instead just deleted
get_gdb_completer_quote_characters() and used
gdb_completer_quote_characters directly.
The function skip_quoted_chars (completer.c) is only used by
skip_quoted (also completer.c), so could be made static. The function
skip_quoted just calls directly to skip_quoted_chars but fills in some
default arguments.
The function skip_quoted is only used by the Pascal expression parser,
and is only used in one place.
The skip_quoted_chars function skips a single string; it either looks
for a string between matching quotes, or for a string up to a word
break character.
However, given how the Pascal expression parser calls this function,
we know that the first character will always be a single quote, in
which case skip_quoted_chars will looks for a string between matching
single quotes.
The skip_quoted_chars doesn't do any escaped character handling, it
will just stop at the next single quote character.
In this commit I propose to remove skip_quoted and skip_quoted_chars,
and replace these with a smaller function pascal_skip_string which
I've placed in p-exp.y. This new function only skips a string between
matching single quotes, which is exactly the use case that we need.
The benefit of this change is to remove (some) code duplication. It
feels like skip_quoted is similar in some ways to
extract_string_maybe_quoted, however, there are some differences;
skip_quoted uses the quotes and word break characters from the
completion engine which extract_string_maybe_quoted does not.
However, I'm currently working on improving filename completion, one
part of this is that I'm looking at allowing filenames to be quoted
with single or double quotes, while the default string quoting in
GDB (for expressions) can only use single quotes. If I do end up
allowing single and double quotes in some cases, but we retain the
single quotes only for expressions then skip_quoted starts to become a
problem, should it accept both quote types, or only one?
But given how skip_quoted is used, I can avoid worrying about this by
simply removing skip_quoted.
The Pascal tests do still pass. The code that called skip_quoted is
called at least once in the Pascal tests (adding an abort() call
causes gdb.pascal/types.exp to fail), but I doubt the testing is
extensive. Not sure how widely used GDB for Pascal actually is
though.
We now have unwind-on-timeout and unwind-on-terminating-exception, and
then the odd one out unwindonsignal.
I'm not a great fan of these squashed together command names, so in
this commit I propose renaming this to unwind-on-signal.
Obviously I've added the hidden alias unwindonsignal so any existing
GDB scripts will keep working.
There's one test that I've extended to test the alias works, but in
most of the other test scripts I've changed over to use the new name.
The docs are updated to reference the new name.
Reviewed-By: Eli Zaretskii <eliz@gnu.org>
Tested-By: Luis Machado <luis.machado@arm.com>
Tested-By: Keith Seitz <keiths@redhat.com>
Now that inferior function calls can timeout (see the recent
introduction of direct-call-timeout and indirect-call-timeout), this
commit adds a new setting unwind-on-timeout.
This new setting is just like the existing unwindonsignal and
unwind-on-terminating-exception, but the new setting will cause GDB to
unwind the stack if an inferior function call times out.
The existing inferior function call timeout tests have been updated to
cover the new setting.
Reviewed-By: Eli Zaretskii <eliz@gnu.org>
Tested-By: Luis Machado <luis.machado@arm.com>
Tested-By: Keith Seitz <keiths@redhat.com>
In the previous commits I have been working on improving inferior
function call support. One thing that worries me about using inferior
function calls from a conditional breakpoint is: what happens if the
inferior function call fails?
If the failure is obvious, e.g. the thread performing the call
crashes, or hits a breakpoint, then this case is already well handled,
and the error is reported to the user.
But what if the thread performing the inferior call just deadlocks?
If the user made the call from a 'print' or 'call' command, then the
user might have some expectation of when the function call should
complete, and, when this time limit is exceeded, the user
will (hopefully) interrupt GDB and regain control of the debug
session.
But, when the inferior function call is from a breakpoint condition it
is much harder to understand that GDB is deadlocked within an inferior
call. Maybe the breakpoint hasn't been hit yet? Or maybe the
condition was always false? Or maybe GDB is deadlocked in an inferior
call? The only way to know for sure is for the user to periodically
interrupt the inferior, check on the state of all the threads, and
then continue.
Additionally, the focus of the previous commit was inferior function
calls, from a conditional breakpoint, in a multi-threaded inferior.
This opens up a whole new set of potential failure conditions. For
example, what if the function called relies on interaction with some
other thread, and the other thread crashes? Or hits a breakpoint?
Given how inferior function calls work (in a synchronous manner), a
stop event in some other thread is going to be ignored while the
inferior function call is being executed as part of a breakpoint
condition, and this means that GDB could get stuck waiting for the
original condition thread, which will now never complete.
In this commit I propose a solution to this problem. A timeout. For
targets that support async-mode we can install an event-loop timer
before starting the inferior function call. When the timer expires we
will stop the thread performing the inferior function call. With this
mechanism in place a user can be sure that any inferior call they make
will either complete, or timeout eventually.
Adding a timer like this is obviously a change in behaviour for the
more common 'call' and 'print' uses of inferior function calls, so, in
this patch, I propose having two different timers. One I call the
'direct-call-timeout', which is used for 'call' and 'print' commands.
This timeout is by default set to unlimited, which, not surprisingly,
means there is no timeout in place.
A second timer, which I've called 'indirect-call-timeout', is used for
inferior function calls from breakpoint conditions. This timeout has
a default value of 30 seconds. This is a reasonably long time to
wait, and hopefully should be enough in most cases to allow the
inferior call to complete. An inferior call that takes more than 30
seconds, which is installed on a breakpoint condition is really going
to slow down the debug session, so hopefully this is not a common use
case.
The user is, of course, free to reduce, or increase the timeout value,
and can always use Ctrl-c to interrupt an inferior function call, but
this timeout will ensure that GDB will stop at some point.
The new commands added by this commit are:
set direct-call-timeout SECONDS
show direct-call-timeout
set indirect-call-timeout SECONDS
show indirect-call-timeout
These new timeouts do depend on async-mode, so, if async-mode is
disabled (maint set target-async off), or not supported (e.g. target
sim), then the timeout is treated as unlimited (that is, no timeout is
set).
For targets that "fake" non-async mode, e.g. Linux native, where
non-async mode is really just async mode, but then we park the target
in a sissuspend, we could easily fix things so that the timeouts still
work, however, for targets that really are not async aware, like the
simulator, fixing things so that timeouts work correctly would be a
much bigger task - that effort would be better spent just making the
target async-aware. And so, I'm happy for now that this feature will
only work on async targets.
The two new show commands will display slightly different text if the
current target is a non-async target, which should allow users to
understand what's going on.
There's a somewhat random test adjustment needed in gdb.base/help.exp,
the test uses a regexp with the apropos command, and expects to find a
single result. Turns out the new settings I added also matched the
regexp, which broke the test. I've updated the regexp a little to
exclude my new settings.
Reviewed-By: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Reviewed-By: Eli Zaretskii <eliz@gnu.org>
Tested-By: Luis Machado <luis.machado@arm.com>
Tested-By: Keith Seitz <keiths@redhat.com>
This commit fixes bug PR 28942, that is, creating a conditional
breakpoint in a multi-threaded inferior, where the breakpoint
condition includes an inferior function call.
Currently, when a user tries to create such a breakpoint, then GDB
will fail with:
(gdb) break infcall-from-bp-cond-single.c:61 if (return_true ())
Breakpoint 2 at 0x4011fa: file /tmp/build/gdb/testsuite/../../../src/gdb/testsuite/gdb.threads/infcall-from-bp-cond-single.c, line 61.
(gdb) continue
Continuing.
[New Thread 0x7ffff7c5d700 (LWP 2460150)]
[New Thread 0x7ffff745c700 (LWP 2460151)]
[New Thread 0x7ffff6c5b700 (LWP 2460152)]
[New Thread 0x7ffff645a700 (LWP 2460153)]
[New Thread 0x7ffff5c59700 (LWP 2460154)]
Error in testing breakpoint condition:
Couldn't get registers: No such process.
An error occurred while in a function called from GDB.
Evaluation of the expression containing the function
(return_true) will be abandoned.
When the function is done executing, GDB will silently stop.
Selected thread is running.
(gdb)
Or, in some cases, like this:
(gdb) break infcall-from-bp-cond-simple.c:56 if (is_matching_tid (arg, 1))
Breakpoint 2 at 0x401194: file /tmp/build/gdb/testsuite/../../../src/gdb/testsuite/gdb.threads/infcall-from-bp-cond-simple.c, line 56.
(gdb) continue
Continuing.
[New Thread 0x7ffff7c5d700 (LWP 2461106)]
[New Thread 0x7ffff745c700 (LWP 2461107)]
../../src.release/gdb/nat/x86-linux-dregs.c:146: internal-error: x86_linux_update_debug_registers: Assertion `lwp_is_stopped (lwp)' failed.
A problem internal to GDB has been detected,
further debugging may prove unreliable.
The precise error depends on the exact thread state; so there's race
conditions depending on which threads have fully started, and which
have not. But the underlying problem is always the same; when GDB
tries to execute the inferior function call from within the breakpoint
condition, GDB will, incorrectly, try to resume threads that are
already running - GDB doesn't realise that some threads might already
be running.
The solution proposed in this patch requires an additional member
variable thread_info::in_cond_eval. This flag is set to true (in
breakpoint.c) when GDB is evaluating a breakpoint condition.
In user_visible_resume_ptid (infrun.c), when the in_cond_eval flag is
true, then GDB will only try to resume the current thread, that is,
the thread for which the breakpoint condition is being evaluated.
This solves the problem of GDB trying to resume threads that are
already running.
The next problem is that inferior function calls are assumed to be
synchronous, that is, GDB doesn't expect to start an inferior function
call in thread #1, then receive a stop from thread #2 for some other,
unrelated reason. To prevent GDB responding to an event from another
thread, we update fetch_inferior_event and do_target_wait in infrun.c,
so that, when an inferior function call (on behalf of a breakpoint
condition) is in progress, we only wait for events from the current
thread (the one evaluating the condition).
In do_target_wait I had to change the inferior_matches lambda
function, which is used to select which inferior to wait on.
Previously the logic was this:
auto inferior_matches = [&wait_ptid] (inferior *inf)
{
return (inf->process_target () != nullptr
&& ptid_t (inf->pid).matches (wait_ptid));
};
This compares the pid of the inferior against the complete ptid we
want to wait on. Before this commit wait_ptid was only ever
minus_one_ptid (which is special, and means any process), and so every
inferior would match.
After this commit though wait_ptid might represent a specific thread
in a specific inferior. If we compare the pid of the inferior to a
specific ptid then these will not match. The fix is to compare
against the pid extracted from the wait_ptid, not against the complete
wait_ptid itself.
In fetch_inferior_event, after receiving the event, we only want to
stop all the other threads, and call inferior_event_handler with
INF_EXEC_COMPLETE, if we are not evaluating a conditional breakpoint.
If we are, then all the other threads should be left doing whatever
they were before. The inferior_event_handler call will be performed
once the breakpoint condition has finished being evaluated, and GDB
decides to stop or not.
The final problem that needs solving relates to GDB's commit-resume
mechanism, which allows GDB to collect resume requests into a single
packet in order to reduce traffic to a remote target.
The problem is that the commit-resume mechanism will not send any
resume requests for an inferior if there are already events pending on
the GDB side.
Imagine an inferior with two threads. Both threads hit a breakpoint,
maybe the same conditional breakpoint. At this point there are two
pending events, one for each thread.
GDB selects one of the events and spots that this is a conditional
breakpoint, GDB evaluates the condition.
The condition includes an inferior function call, so GDB sets up for
the call and resumes the one thread, the resume request is added to
the commit-resume queue.
When the commit-resume queue is committed GDB sees that there is a
pending event from another thread, and so doesn't send any resume
requests to the actual target, GDB is assuming that when we wait we
will select the event from the other thread.
However, as this is an inferior function call for a condition
evaluation, we will not select the event from the other thread, we
only care about events from the thread that is evaluating the
condition - and the resume for this thread was never sent to the
target.
And so, GDB hangs, waiting for an event from a thread that was never
fully resumed.
To fix this issue I have added the concept of "forcing" the
commit-resume queue. When enabling commit resume, if the force flag
is true, then any resumes will be committed to the target, even if
there are other threads with pending events.
A note on authorship: this patch was based on some work done by
Natalia Saiapova and Tankut Baris Aktemur from Intel[1]. I have made
some changes to their work in this version.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28942
[1] https://sourceware.org/pipermail/gdb-patches/2020-October/172454.html
Co-authored-by: Natalia Saiapova <natalia.saiapova@intel.com>
Co-authored-by: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Reviewed-By: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Tested-By: Luis Machado <luis.machado@arm.com>
Tested-By: Keith Seitz <keiths@redhat.com>
This reverts commit ac0d67ed1d.
There was nothing wrong with the commit which I'm reverting here, but
it removed some functionality that will be needed for a later commit;
that is, the ability for GDB to ask for events from a specific ptid_t
via the do_target_wait function.
In a follow up commit, this functionality will be used to implement
inferior function calls in multi-threaded inferiors.
This is not a straight revert of the above commit. Reverting the
above commit replaces a 'nullptr' with 'NULL', I've gone in and
changed that, preserving the 'nullptr'.
Reviewed-By: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Tested-By: Luis Machado <luis.machado@arm.com>
Tested-By: Keith Seitz <keiths@redhat.com>
This commit builds on the previous series of commits to share the
target description caching code between GDB and gdbserver for
x86/Linux targets.
The objective of this commit is to move the four functions (2 each of)
i386_linux_read_description and amd64_linux_read_description into
gdb/nat/x86-linux-tdesc.c and combine them so we have just a single
copy of each. Then both GDB and gdbserver will link against these
shared functions.
It is worth reading the description of the previous commit to see why
this merging is not as simple as it seems: on the gdbserver side we
actually have two users of these functions, gdbserver itself, and the
in process agent (IPA).
However, the previous commit streamlined the gdbserver code, and so
now it is simple to move the two functions along with all their
support functions from the gdbserver directory into the gdb/nat/
directory, and then GDB is fine to call these functions.
One small curiosity with this patch is the function
x86_linux_post_init_tdesc. On the gdbserver side the two functions
amd64_linux_read_description and i386_linux_read_description have some
functionality that is not present on the GDB side, that is some
additional configuration that is performed as each target description
is created to setup the expedited registers.
To support this I've added the function x86_linux_post_init_tdesc.
This function is called from the two *_linux_read_description
functions, but is implemented separately for GDB and gdbserver.
This does mean adding back some non-shared code when this whole series
has been about sharing code, but now the only non-shared bit is the
single line that is actually different between GDB and gdbserver, all
the rest, which is identical, is now shared.
I did need to add a new rule to the gdbserver Makefile, this is to
allow the nat/x86-linux-tdesc.c file to be compiled for the IPA.
Approved-By: John Baldwin <jhb@FreeBSD.org>
This commit is part of a series which aims to share more of the target
description creation between GDB and gdbserver for x86/Linux.
After some refactoring, the previous commit actually started to share
some code, we added the shared x86_linux_tdesc_for_tid function into
nat/x86-linux-tdesc.c. However, this function still relies on
amd64_linux_read_description and i386_linux_read_description which are
implemented separately for both gdbserver and GDB. Given that at
their core, all these functions to is:
1. take an xcr0 value as input,
2. mask out some feature bits,
3. look for a cached pre-generated target description and return it
if found,
4. if no cached target description is found then call either
amd64_create_target_description or
i386_create_target_description to create a new target
description, which is then added to the cache. Return the newly
created target description.
The inner functions amd64_create_target_description and
i386_create_target_description are already shared between GDB and
gdbserver (in the gdb/arch/ directory), so the only thing that
the *_read_description functions really do is add the caching layer,
and it feels like this really could be shared.
However, we have a small problem.
On the GDB side we create target descriptions using a different set of
cpu features than on the gdbserver side! This means that for the
exact same target, we might get a different target description when
using native GDB vs using gdbserver. This surely feels like a
mistake, I would expect to get the same target description on each.
The table below shows the number of possible different target
descriptions that we can create on the GDB side vs on the gdbserver
side for each target type:
| GDB | gdbserver
------|-----|----------
i386 | 64 | 7
amd64 | 32 | 7
x32 | 16 | 7
So in theory, all I want to do is move the GDB version
of *_read_description into the nat/ directory and have gdbserver use
that, then both GDB and gdbserver would be able to create any of the
possible target descriptions.
Unfortunately it's a little more complex than that due to the in
process agent (IPA).
When the IPA is in use, gdbserver sends a target description index to
the IPA, and the IPA uses this to find the correct target description
to use.
** START OF AN ASIDE **
Back in the day I suspect this approach made perfect sense. However
since this commit:
commit a880623024
Date: Thu Dec 7 17:07:01 2017 +0000
Initialize target description early in IPA
I think passing the index is now more trouble than its worth.
We used to pass the index, and then use that index to lookup which
target description to instantiate and use. However, the above commit
fixed an issue where we can't call malloc() within (certain parts of)
the IPA (apparently), so instead we now pre-compute _every_ possible
target description within the IPA. The index is now only used to
lookup which of the (many) pre-computed target descriptions to use.
It would (I think) have been easier all around if the IPA just
self-inspected, figured out its own xcr0 value, and used that to
create the one target description that is required. So long as the
xcr0 to target description code is shared (at compile time) with
gdbserver, then we can be sure that the IPA will derive the same
target description as gdbserver, and we would avoid all this index
passing business, which has made this commit so very, very painful.
** END OF AN ASIDE **
Currently then for x86/linux, gdbserver sends a number between 0 and 7
to the IPA, and the IPA uses this to create a target description.
However, I am proposing that gdbserver should now create one of (up
to) 64 different target descriptions for i386, so this 0 to 7 index
isn't going to be good enough any more (amd64 and x32 have slightly
fewer possible target descriptions, but still more than 8, so the
problem is the same).
For a while I wondered if I was going to have to try and find some
backward compatible solution for this mess. But after seeing how
lightly the IPA is actually documented, I wonder if it is not the case
that there is a tight coupling between a version of gdbserver and a
version of the IPA? At least I'm hoping so.
In this commit I have thrown out the old IPA target description index
numbering scheme, and switched to a completely new numbering scheme.
Instead of the index that is passed being arbitrary, the index is
instead calculated from the set of cpu features that are present on
the target. Within the IPA we can then reverse this logic to recreate
the xcr0 value based on the index, and from the xcr0 value we can
create the correct target description.
With the gdbserver to IPA numbering scheme issue resolved I have then
update the gdbserver versions of amd64_linux_read_description and
i386_linux_read_description so that they create target descriptions
using the same set of cpu features as GDB itself.
After this gdbserver should now always come up with the same target
description as GDB does on any x86/Linux target.
This commit does not introduce any new code sharing between GDB and
gdbserver as previous commits in this series does. Instead this
commit is all about bringing GDB and gdbserver into alignment
functionally so that the next commit can merge the GDB and gdbserver
versions of these functions.
Approved-By: John Baldwin <jhb@FreeBSD.org>
While trying to merge this commit:
commit 4bb20a6244
Date: Wed Mar 20 04:13:18 2024 -0700
gdbserver: Clear X86_XSTATE_MPX bits in xcr0 on x32
With this patch series of mine:
https://inbox.sourceware.org/gdb-patches/cover.1706801009.git.aburgess@redhat.com
I worried that there could be other paths that could result in an xcr0
value that has X86_XSTATE_MPX set in x32 mode. As everyone eventually
calls amd64_create_target_description to build their target
description, I figured we could assert in here that if X86_XSTATE_MPX
is set then we should not be an x32 target, this should uncover any
other bugs in this area.
I'm not currently able to build/run any x32 binaries, so I have no way
to test this.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31511
This commit is part of a series to share more of the x86 target
description creation code between GDB and gdbserver.
Unlike previous commits which were mostly refactoring, this commit is
the first that makes a real change, though that change should mostly
be for gdbserver; I've largely adopted the "GDB" way of doing things
for gdbserver, and this fixes a real gdbserver bug.
On a x86-64 Linux target, running the test:
gdb.server/connect-with-no-symbol-file.exp
results in two core files being created. Both of these core files are
from the inferior process, created after gdbserver has detached.
In this test a gdbserver process is started and then, after gdbserver
has started, but before GDB attaches, we either delete the inferior
executable, or change its permissions so it can't be read. Only after
doing this do we attempt to connect with GDB.
As GDB connects to gdbserver, gdbserver attempts to figure out the
target description so that it can send the description to GDB, this
involves a call to x86_linux_read_description.
In x86_linux_read_description one of the first things we do is try to
figure out if the process is 32-bit or 64-bit. To do this we look up
the executable via the thread-id, and then attempt to read the
architecture size from the executable. This isn't going to work if
the executable has been deleted, or is no longer readable.
And so, as we can't read the executable, we default to an i386 target
and use an i386 target description.
A consequence of using an i386 target description is that addresses
are assumed to be 32-bits. Here's an example session that shows the
problems this causes. This is run on an x86-64 machine, and the test
binary (xx.x) is a standard 64-bit x86-64 binary:
shell_1$ gdbserver --once localhost :54321 /tmp/xx.x
shell_2$ gdb -q
(gdb) set sysroot
(gdb) shell chmod 000 /tmp/xx.x
(gdb) target remote :54321
Remote debugging using :54321
warning: /tmp/xx.x: Permission denied.
0xf7fd3110 in ?? ()
(gdb) show architecture
The target architecture is set to "auto" (currently "i386").
(gdb) p/x $pc
$1 = 0xf7fd3110
(gdb) info proc mappings
process 2412639
Mapped address spaces:
Start Addr End Addr Size Offset Perms objfile
0x400000 0x401000 0x1000 0x0 r--p /tmp/xx.x
0x401000 0x402000 0x1000 0x1000 r-xp /tmp/xx.x
0x402000 0x403000 0x1000 0x2000 r--p /tmp/xx.x
0x403000 0x405000 0x2000 0x2000 rw-p /tmp/xx.x
0xf7fcb000 0xf7fcf000 0x4000 0x0 r--p [vvar]
0xf7fcf000 0xf7fd1000 0x2000 0x0 r-xp [vdso]
0xf7fd1000 0xf7fd3000 0x2000 0x0 r--p /usr/lib64/ld-2.30.so
0xf7fd3000 0xf7ff3000 0x20000 0x2000 r-xp /usr/lib64/ld-2.30.so
0xf7ff3000 0xf7ffb000 0x8000 0x22000 r--p /usr/lib64/ld-2.30.so
0xf7ffc000 0xf7ffe000 0x2000 0x2a000 rw-p /usr/lib64/ld-2.30.so
0xf7ffe000 0xf7fff000 0x1000 0x0 rw-p
0xfffda000 0xfffff000 0x25000 0x0 rw-p [stack]
0xff600000 0xff601000 0x1000 0x0 r-xp [vsyscall]
(gdb) info inferiors
Num Description Connection Executable
* 1 process 2412639 1 (remote :54321)
(gdb) shell cat /proc/2412639/maps
00400000-00401000 r--p 00000000 fd:03 45907133 /tmp/xx.x
00401000-00402000 r-xp 00001000 fd:03 45907133 /tmp/xx.x
00402000-00403000 r--p 00002000 fd:03 45907133 /tmp/xx.x
00403000-00405000 rw-p 00002000 fd:03 45907133 /tmp/xx.x
7ffff7fcb000-7ffff7fcf000 r--p 00000000 00:00 0 [vvar]
7ffff7fcf000-7ffff7fd1000 r-xp 00000000 00:00 0 [vdso]
7ffff7fd1000-7ffff7fd3000 r--p 00000000 fd:00 143904 /usr/lib64/ld-2.30.so
7ffff7fd3000-7ffff7ff3000 r-xp 00002000 fd:00 143904 /usr/lib64/ld-2.30.so
7ffff7ff3000-7ffff7ffb000 r--p 00022000 fd:00 143904 /usr/lib64/ld-2.30.so
7ffff7ffc000-7ffff7ffe000 rw-p 0002a000 fd:00 143904 /usr/lib64/ld-2.30.so
7ffff7ffe000-7ffff7fff000 rw-p 00000000 00:00 0
7ffffffda000-7ffffffff000 rw-p 00000000 00:00 0 [stack]
ffffffffff600000-ffffffffff601000 r-xp 00000000 00:00 0 [vsyscall]
(gdb)
Notice the difference between the mappings reported via GDB and those
reported directly from the kernel via /proc/PID/maps, the addresses of
every mapping is clamped to 32-bits for GDB, while the kernel reports
real 64-bit addresses.
Notice also that the $pc value is a 32-bit value. It appears to be
within one of the mappings reported by GDB, but is outside any of the
mappings reported from the kernel.
And this is where the problem arises. When gdbserver detaches from
the inferior we pass the inferior the address from which it should
resume. Due to the 32/64 bit confusion we tell the inferior to resume
from the 32-bit $pc value, which is not within any valid mapping, and
so, as soon as the inferior resumes, it segfaults.
If we look at how GDB (not gdbserver) figures out its target
description then we see an interesting difference. GDB doesn't try to
read the executable. Instead GDB uses ptrace to query the thread's
state, and uses this to figure out the if the thread is 32 or 64 bit.
If we update gdbserver to do it the "GDB" way then the above problem
is resolved, gdbserver now sees the process as 64-bit, and when we
detach from the inferior we give it the correct 64-bit address, and
the inferior no longer segfaults.
Now, I could just update the gdbserver code, but better, I think, to
share one copy of the code between GDB and gdbserver in gdb/nat/.
That is what this commit does.
The cores of x86_linux_read_description from gdbserver and
x86_linux_nat_target::read_description from GDB are moved into a new
file gdb/nat/x86-linux-tdesc.c and combined into a single function
x86_linux_tdesc_for_tid which is called from each location.
This new function does things the GDB way, the only changes are to
allow for the sharing; we now have a callback function to call the
first time that the xcr0 state is read, this allows for GDB and
gdbserver to perform their own initialisation as needed, and
additionally, the new function takes a pointer for where to cache the
xcr0 value, this isn't needed for this commit, but will be useful in a
later commit where gdbserver will want to read this cached xcr0
value.
Another thing to note about this commit is how the functions
i386_linux_read_description and amd64_linux_read_description are
handled. For now I've left these function as implemented separately
in GDB and gdbserver. I've moved the declarations of these functions
into gdb/nat/x86-linux-tdesc.h, but the implementations are left as
separate.
A later commit in this series will make these functions shared too,
but doing this is not trivial, so I've left that for a separate
commit. Merging the declarations as I've done here ensures that
everyone implements the function to the same API, and once these
functions are shared (in a later commit) we'll want a shared
declaration anyway.
Approved-By: John Baldwin <jhb@FreeBSD.org>
Share the definition of I386_LINUX_XSAVE_XCR0_OFFSET between GDB and
gdbserver.
This commit is part of a series that aims to share more of the x86
target description creation code between GDB and gdbserver. The
I386_LINUX_XSAVE_XCR0_OFFSET #define is used as part of the target
description creation, and I noticed that this constant is defined
separately for GDB and gdbserver.
This commit moves the definition into gdb/nat/x86-linux.h, which
allows the #define to be shared.
There should be no user visible changes after this commit.
Approved-By: John Baldwin <jhb@FreeBSD.org>
This commit is part of a series that aims to share more of the x86
target description reading/generation code between GDB and gdbserver.
There are a huge number of similarities between the code in
gdbserver's x86_linux_read_description function and GDB's
x86_linux_nat_target::read_description function, and it is this
similarity that I plan, in a later commit, to share between GDB and
gdbserver.
However, one thing that is different in x86_linux_read_description is
the code inside the '!use_xml' block. This is the code that handles
the case where gdbserver is not allowed to send an XML target
description back to GDB. In this case gdbserver uses some predefined,
fixed, target descriptions.
First, it's worth noting that I suspect this code is not tested any
more. I couldn't find anything in the testsuite that tries to disable
XML target description support. And the idea of having a single
"fixed" target description really doesn't work well when we think
about all the various x86 extensions that exist. Part of me would
like to rip out the no-xml support in gdbserver (at least for x86),
and if a GDB connects that doesn't support XML target descriptions,
gdbserver can just give an error and drop the connection. GDB has
supported XML target descriptions for 16 years now, I think it would
be reasonable for our shipped gdbserver to drop support for the old
way of doing things.
Anyway.... this commit doesn't do that.
What I did notice was that, over time, the '!use_xml' block appears to
have "drifted" within the x86_linux_read_description function; it's
now not the first check we do. Instead we make some ptrace calls and
return a target description generated based on the result of these
ptrace calls. Surely it only makes sense to generate variable target
descriptions if we can send these back to GDB?
So in this commit I propose to move the '!use_xml' block earlier in
the x86_linux_read_description function.
The benefit of this is that this leaves the later half of
x86_linux_read_description much more similar to the GDB function
x86_linux_nat_target::read_description and sets us up for potentially
sharing code between GDB and gdbserver in a later commit.
Approved-By: John Baldwin <jhb@FreeBSD.org>
This patch is part of a series that has the aim of making the code
that, for x86, reads the target description for a native process
shared between GDB and gdbserver.
Within GDB part of this process involves reading the cs and ds state
from the 'struct user_regs_struct' using a ptrace call.
This isn't done by gdbserver, which is part of the motivation for this
whole series; the approach gdbserver takes is inferior to the approach
GDB takes.
This commit moves the reading of cs and ds, which is used to figure
out if a thread is 32-bit or 64-bit (or in x32 mode), into the gdb/nat
directory so that the code could be shared with gdbserver, but at this
point I'm not actually using the code in gdbserver, that will come
later.
As such there should be no user visible changes after this commit, GDB
continues to do things as it did before (reading cs/ds), while
gdbserver continues to use its own approach (which doesn't require
reading cs/ds).
Approved-By: John Baldwin <jhb@FreeBSD.org>
Convert the have_ptrace_getregset global within gdbserver to a
tribool. This brings the flag into alignment with the corresponding
flag in GDB.
The gdbserver have_ptrace_getregset variable is already used as a
tribool, it just doesn't have the tribool type.
In a future commit I plan to share more code between GDB and
gdbserver, and having this variable be the same type in both code
bases will make the sharing much easier.
There should be no user visible changes after this commit.
Approved-By: John Baldwin <jhb@FreeBSD.org>
With gcc 13, test-case gdb.ada/tagged-lookup.exp passes for me, but with gcc
12, I get:
...
(gdb) set debug symtab-create 1^M
(gdb) print *the_local_var^M
...
$1 = (n => 2)^M
(gdb) FAIL: gdb.ada/tagged-lookup.exp: only one CU expanded
...
The problem is that this fails:
...
-re -wrap ".* = \\\(n => $decimal\\\)" {
if {$found_pck + $found_pck2 == 1} {
pass $gdb_test_name
} else {
fail $gdb_test_name
}
...
because $found_pck == 0 and $found_pck2 == 0.
Indeed, with gcc 13 we have:
...
$ grep "start_subfile: name = .*/tagged-lookup/" gdb.log | sed 's%.*/%%'
b~foo.adb
b~foo.adb
b~foo.adb
b~foo.ads
pck2.adb
pck2.adb
pck2.ads
pck2.adb
pck2.ads
...
and with gcc 12:
...
$ grep "start_subfile: name = .*/tagged-lookup/" gdb.log | sed 's%.*/%%'
b~foo.adb
b~foo.adb
b~foo.adb
b~foo.ads
...
Fix this by checking for "$found_pck + $found_pck2 <= 1" instead.
Tested on x86_64-linux.
Approved-By: Tom Tromey <tom@tromey.com>
PR testsuite/31514
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31514
Commit 467a34bb9e ("gdb tests: Allow for "LWP" or "process" in thread IDs
from info threads") introduces a new global variable tdlabel_re, but fails to
indicate it's global when used in procs in four test-cases.
Fix this by adding "global tdlabel_re".
Tested on aarch64-linux.
Several tests assume that the first word after a thread ID in 'info
threads' output is "Thread". However, several targets use "LWP"
instead such as the FreeBSD and NetBSD native targets. The Linux
native target also uses "LWP" if libthread_db is not being used.
Targets that do not support threads use "process" as the first word
via normal_pid_to_str.
Add a tdlabel_re global variable as a regular-expression for a thread
label in `info threads' that matches either "process", "Thread", or
"LWP".
Some other tests in the tree don't require a specific word, and
some targets may use other first words (e.g. OpenBSD uses "thread"
and Ravenscar threads use "Ravenscar Thread").
Use gdb_realpath instead of realpath in windows-nat.c:windows_make_so,
so that we don't have to manually call free.
Approved-By: John Baldwin <jhb@FreeBSD.org>
Change-Id: Id3cda7e177ac984c9a5f7c23f354e72bd561edff
There is no need to limit shared library path sizes to
SO_NAME_MAX_PATH_SIZE nowadays. windows_solib::name and
windows_solib::original_name are std::strings nowadays, and so are
solib::so_name and solib::so_original_name in the core solib code.
This commit reworks the code to remove that limit. This also fixes a
leak where we were not releasing 'rname' in the realpath branch if the
'rname' string was larger than SO_NAME_MAX_PATH_SIZE.
Note: I tested the cygwin_conv_path with a manual hack to force that
path, and then stepping through the code. You only get to that path
if Windows doesn't report an absolute path for ntdll.dll, and on my
machine (running Windows 10), it always does.
Approved-By: John Baldwin <jhb@FreeBSD.org>
Change-Id: I79e9862d5a7646eebfef7ab5b05b96318a7ca0c5
There are two separate #ifndef __CYGWIN__/#else/#endif sections in the
windows_make_so function with 3 lines of shared code separating them.
I find this makes the code harder to understand than necessary.
AFAICS, there is no reason those three shared lines need to be after
the first #ifdef block. There is no early return, nor are 'load_addr'
nor 'name' modified.
This commit moves that shared code to the top of the function, and
then combines the two #ifndef sections.
Approved-By: John Baldwin <jhb@FreeBSD.org>
Change-Id: If2678b52836b1c3134a5e9f9fdaee74448d8b7bc
