| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
|
|
The sim testsuite was reorganized last year, so update the path.
|
|
Move the 64-bit bfd logic out of bfd/configure.ac and into bfd64.m4
under config so it can be shared between all the other subdirs.
This replaces want64 with enable_64_bit_bfd which was already being
declared, but not used directly.
|
|
This commit updates the copyright year range in the script
gdb/testsuite/gdb.arch/powerpc-power10.exp. The update was
performed by running gdb/copyright.py again, to make sure
that the copyright year range will be automatically updated
in years forward.
|
|
The copyright year and holder line is slight malformed, missing
a space after a comma, and this is sufficient for gdb's
copyright.py script to miss this file during its automated
copyright year update.
This commit fixes this.
|
|
Add gdb/syscalls/update-netbsd.sh to the reminder printed
at the end of the execution listing all the files where
a manual update of the copyright header is needed. This
scripts contains some inline code which includes a copyright
header.
|
|
This commit updates the copyright year in some files where
we have a copyright year outside of the copyright year,
and thus are not included in gdb's copyright.py script.
|
|
This commit brings all the changes made by running gdb/copyright.py
as per GDB's Start of New Year Procedure.
For the avoidance of doubt, all changes in this commits were
performed by the script.
|
|
This commit changes the copyright year printed by gdb, gdbserver
and gdbreplay when printing the tool's version.
|
|
avr_io_reg_read_command is an ordinary gdb command, and so should not
be calling reinitialize_more_filter. This patch removes it. I'm
checking this in as obvious. Tested by rebuilding.
|
|
"show" command callbacks, like most ordinary gdb commands, should use
filtered output. I found a few that did not, so this patch changes
them to use the filtered form.
|
|
I happened to notice that one "show" callback was printing to
gdb_stdout rather than to the passed-in ui_file parameter. I went
through all such callbacks and fixed them to consistently use the
ui_file.
Regression tested on x86-64 Fedora 34.
|
|
When MI debugging is enabled, the logging output should be sent to
gdb_stdlog. This is part of PR gdb/7233.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=7233
|
|
This changes index-cache.c to use debug_prefixed_printf_cond_nofunc.
As a side effect, logs are now written to gdb_stdlog. This is part of
PR gdb/7233.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=7233
|
|
This changes minsyms.c to send logging output to gdb_stdlog. This is
part of PR gdb/7233.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=7233
|
|
This changes the separate debug file logging code (spread across two
files) to use gdb_stdlog for its output. This is part of PR gdb/7233.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=7233
|
|
This changes machoread.c to use debug_prefixed_printf_cond_nofunc. As
a side effect, the logs are now written to gdb_stdlog. This is part
of PR gdb/7233.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=7233
|
|
This changes microblaze.c to use the standard logging macro. As a
side effect, logs will now go to gdb_stdlog. This is part of PR gdb/7233.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=7233
|
|
This changes mips-linux-nat.c to send some logging output to
gdb_stdlog, rather than stdout. This is part of PR gdb/7233.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=7233
|
|
This changes arch-utils.c to send some error messages to gdb_stderr.
This is part of PR gdb/7233.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=7233
|
|
The process record code often emits unfiltered output. In some cases,
this output ought to go to gdb_stderr (but see below). In other
cases, the output is guarded by a logging variable and so ought to go
to gdb_stdlog. This patch makes these changes.
Note that in many cases, the output to stderr is followed by a
"return -1", which is how process record indicates an error. It seems
to me that calling error here would be preferable, because, in many
cases, that's all the caller does when it sees a -1. However, I
haven't made this change.
This is part of PR gdb/7233.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=7233
|
|
jit.c writes some error messages to gdb_stdout, but using gdb_stderr
is better. This is part of PR gdb/7233.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=7233
|
|
I noticed yesterday that if gdb output is redirected to a file, the
pager will still be active. This is irritating, because the output
isn't actually visible -- just the pager prompt. Looking in bugzilla,
I found that this had been filed 17 years ago, as PR cli/8798.
This patch fixes the bug. It changes the pagination code to query the
particular ui-file to see if paging is allowable. The ui-file
implementations are changed so that only the stdout implementation and
a tee (where one sub-file is stdout) can page.
Regression tested on x86-64 Fedora 34.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=8798
|
|
gdbtypes.h uses core_addr_eq and core_addr_hash in a weird way: taking
the address of a member and then passing this (as a void*) to these
functions.
It seems better to simply inline the ordinary code here. CORE_ADDR is
a scalar so it can be directly compared, and the identity hash
function seems safe to assume as well.
After this, core_addr_eq and core_addr_hash are unused, so this patch
removes them.
|
|
This commit ensures that the following settings are cloned from one
inferior to the new one when processing the clone-inferior command:
- inferior-tty
- environment variables
- cwd
- args
Some of those parameters can be passed as command line arguments to GDB
(-args and -tty), so one could expect the clone-inferior to respect
those flags. The following debugging session illustrates that:
gdb -nx -quiet -batch \
-ex "show args" \
-ex "show inferior-tty" \
-ex "clone-inferior" \
-ex "inferior 2" \
-ex "show args" \
-ex "show inferior-tty" \
-tty=/some/tty \
-args echo foo bar
Argument list to give program being debugged when it is started is "foo bar".
Terminal for future runs of program being debugged is "/some/tty".
[New inferior 2]
Added inferior 2.
[Switching to inferior 2 [<null>] (/bin/echo)]
Argument list to give program being debugged when it is started is "".
Terminal for future runs of program being debugged is "".
The other properties this commit copies on clone (i.e. CWD and the
environment variables) are included since they are related (in the sense
that they influence the runtime behavior of the program) even if they
cannot be directly set using command line switches.
There is a chance that this patch changes existing user workflow. I
think that this change is mostly harmless. If users want to start a new
inferior based on an existing one, they probably already propagate those
settings to the new inferior in some way.
Tested on x86_64-linux.
Change-Id: I3b1f28b662f246228b37bb24c2ea1481567b363d
|
|
gdb_print_host_address is just a simple wrapper around
fprintf_filtered. However, it is readily replaced in all callers by a
combination of %s and call to host_address_to_string. This also
simplifies the code, so I think it's worthwhile to remove this
function.
Regression tested on x86-64 Fedora 64.
|
|
gdb_bfd.c contains most of gdb's BFD-related utility functions.
However, gdb_bfd_errmsg is in utils.c. It seemed better to me to move
this out of util.[ch] and into the BFD-related file instead.
Tested by rebuilding.
|
|
Set of fixes to resolve some duplicate test names in the gdb.mi/
directory. There should be no real test changes after this set of
fixes, they are all either:
- Adding with_test_prefix type constructs to make test names unique,
or
- Changing the test name to be more descriptive, or better reflect
the test being run.
|
|
Bug PR gdb/28405 reports a regression when using attach with an
extended-remote target. In this case the target is not including a
thread-id in the stop packet it sends back after the attach.
The regression was introduced with this commit:
commit 8f66807b98f7634c43149ea62e454ea8f877691d
Date: Wed Jan 13 20:26:58 2021 -0500
gdb: better handling of 'S' packets
The problem is that when GDB processes the stop packet, it sees that
there is no thread-id and so has to "guess" which thread the stop
should apply to.
In this case the target only has one thread, so really, there's no
guessing needed, but GDB still runs through the same process, this
shouldn't cause us any problems.
However, after the above commit, GDB now expects itself to be more
internally consistent, specifically, only a thread that GDB thinks is
resumed, can be a candidate for having stopped.
It turns out that, when GDB attaches to a process through an
extended-remote target, the threads of the process being attached too,
are not, initially, marked as resumed.
And so, when GDB tries to figure out which thread the stop might apply
too, it finds no threads in the processes marked resumed, and so an
assert triggers.
In extended_remote_target::attach we create a new thread with a call
to add_thread_silent, rather than remote_target::remote_add_thread,
the reason is that calling the latter will result in a call to
'add_thread' rather than 'add_thread_silent'. However,
remote_target::remote_add_thread includes additional
actions (i.e. calling remote_thread_info::set_resumed and set_running)
which are missing from extended_remote_target::attach. These missing
calls are what would serve to mark the new thread as resumed.
In this commit I propose that we add an extra parameter to
remote_target::remote_add_thread. This new parameter will force the
new thread to be added with a call to add_thread_silent. We can now
call remote_add_thread from the ::attach method, the extra
actions (listed above) will now be performed, and the thread will be
left in the correct state.
Additionally, in PR gdb/28405, a segfault is reported. This segfault
triggers when 'set debug remote 1' is used before trying to reproduce
the original assertion failure. The cause of this is in
remote_target::select_thread_for_ambiguous_stop_reply, where we do
this:
remote_debug_printf ("first resumed thread is %s",
pid_to_str (first_resumed_thread->ptid).c_str ());
remote_debug_printf ("is this guess ambiguous? = %d", ambiguous);
gdb_assert (first_resumed_thread != nullptr);
Notice that when debug printing is on we dereference
first_resumed_thread before we assert that the pointer is not
nullptr. This is the cause of the segfault, and is resolved by moving
the assert before the debug printing code.
I've extended an existing test, ext-attach.exp, so that the original
test is run multiple times; we run in the original mode, as normal,
but also, we now run with different packets disabled in gdbserver. In
particular, disabling Tthread would trigger the assertion as it was
reported in the original bug. I also run the test in all-stop and
non-stop modes now for extra coverage, we also run the tests with
target-async enabled, and disabled.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28405
|
|
Fixes PR gdb/28681. It was observed that after using the `finish`
command an incorrect value was displayed in some cases. Specifically,
this behaviour was observed on an x86-64 target.
Consider this test program:
struct A
{
int i;
A ()
{ this->i = 0; }
A (const A& a)
{ this->i = a.i; }
};
A
func (int i)
{
A a;
a.i = i;
return a;
}
int
main ()
{
A a = func (3);
return a.i;
}
And this GDB session:
$ gdb -q ex.x
Reading symbols from ex.x...
(gdb) b func
Breakpoint 1 at 0x401115: file ex.cc, line 14.
(gdb) r
Starting program: /home/andrew/tmp/ex.x
Breakpoint 1, func (i=3) at ex.cc:14
14 A a;
(gdb) finish
Run till exit from #0 func (i=3) at ex.cc:14
main () at ex.cc:23
23 return a.i;
Value returned is $1 = {
i = -19044
}
(gdb) p a
$2 = {
i = 3
}
(gdb)
Notice how after the `finish` the contents of $1 are junk, but, when I
immediately ask for the value of `a`, I get back the correct value.
The problem here is that after the finish command GDB calls the
function amd64_return_value to figure out where the return value can
be found (on x86-64 targets anyway).
This function makes the wrong choice for the struct A in our case, as
sizeof(A) <= 8, then amd64_return_value decides that A will be
returned in a register. GDB then reads the return value register an
interprets the contents as an instance of A.
Unfortunately, A is not trivially copyable (due to its copy
constructor), and the sys-v specification for argument and return
value passing, says that any non-trivial C++ object should have space
allocated for it by the caller, and the address of this space is
passed to the callee as a hidden first argument. The callee should
then return the address of this space as the return value.
And so, the register that GDB is treating as containing an instance of
A, actually contains the address of an instance of A (in this case on
the stack), this is why GDB shows the incorrect result.
The call stack within GDB for where we actually go wrong is this:
amd64_return_value
amd64_classify
amd64_classify_aggregate
And it is in amd64_classify_aggregate that we should be classifying
the type as AMD64_MEMORY, instead of as AMD64_INTEGER as we currently
do (via a call to amd64_classify_aggregate_field).
At the top of amd64_classify_aggregate we already have this logic:
if (TYPE_LENGTH (type) > 16 || amd64_has_unaligned_fields (type))
{
theclass[0] = theclass[1] = AMD64_MEMORY;
return;
}
Which handles some easy cases where we know a struct will be placed
into memory, that is (a) the struct is more than 16-bytes in size,
or (b) the struct has any unaligned fields.
All we need then, is to add a check here to see if the struct is
trivially copyable. If it is not then we know the struct will be
passed in memory.
I originally structured the code like this:
if (TYPE_LENGTH (type) > 16
|| amd64_has_unaligned_fields (type)
|| !language_pass_by_reference (type).trivially_copyable)
{
theclass[0] = theclass[1] = AMD64_MEMORY;
return;
}
This solved the example from the bug, and my small example above. So
then I started adding some more extensive tests to the GDB testsuite,
and I ran into a problem. I hit this error:
gdbtypes.h:676: internal-error: loc_bitpos: Assertion `m_loc_kind == FIELD_LOC_KIND_BITPOS' failed.
This problem is triggered from:
amd64_classify_aggregate
amd64_has_unaligned_fields
field::loc_bitpos
Inside the unaligned field check we try to get the bit position of
each field. Unfortunately, in some cases the field location is not
FIELD_LOC_KIND_BITPOS, but is FIELD_LOC_KIND_DWARF_BLOCK.
An example that shows this bug is:
struct B
{
short j;
};
struct A : virtual public B
{
short i;
A ()
{ this->i = 0; }
A (const A& a)
{ this->i = a.i; }
};
A
func (int i)
{
A a;
a.i = i;
return a;
}
int
main ()
{
A a = func (3);
return a.i;
}
It is the virtual base class, B, that causes the problem. The base
class is represented, within GDB, as a field within A. However, the
location type for this field is a DWARF_BLOCK.
I spent a little time trying to figure out how to convert the
DWARF_BLOCK to a BITPOS, however, I realised that, in this case at
least, conversion is not needed.
The C++ standard says that a class is not trivially copyable if it has
any virtual base classes. And so, in this case, even if I could
figure out the BITPOS for the virtual base class fields, I know for
sure that I would immediately fail the trivially_copyable check. So,
lets just reorder the checks in amd64_classify_aggregate to:
if (TYPE_LENGTH (type) > 16
|| !language_pass_by_reference (type).trivially_copyable
|| amd64_has_unaligned_fields (type))
{
theclass[0] = theclass[1] = AMD64_MEMORY;
return;
}
Now, if we have a class with virtual bases we will fail quicker, and
avoid the unaligned fields check completely.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28681
|
|
While working on another patch relating to how GDB manages threads
executing and resumed state, I spotted the following code in
record-btrace.c:
executing = tp->executing ();
set_executing (proc_target, inferior_ptid, false);
id = null_frame_id;
try
{
id = get_frame_id (get_current_frame ());
}
catch (const gdb_exception &except)
{
/* Restore the previous execution state. */
set_executing (proc_target, inferior_ptid, executing);
throw;
}
/* Restore the previous execution state. */
set_executing (proc_target, inferior_ptid, executing);
return id;
I notice that we only catch the exception so we can call
set_executing, and this is the same call to set_executing that we need
to perform in the non-exception return path.
This would be much cleaner if we could use SCOPE_EXIT to avoid the
try/catch, so lets do that.
While cleaning this up, I also applied a similar patch to
record-full.c, though there's no try/catch in that case, but using
SCOPE_EXIT makes the code safe if, in the future, we do start throwing
exceptions.
There should be no user visible changes after this commit.
|
|
I noticed that the mi-async setting was not referenced from the index
in any way, this commit tries to rectify that a bit.
The @cindex lines I think are not controversial, these same index
entries are used elsewhere in the manual for async related topics (see
@node Background Execution).
The only bit that might be controversial is that I've added a @kindex
entry for 'set mi-async' when the command is documented as '-gdb-set
mi-async' (with a similar difference for the show/-gdb-show).
My reasoning here is that nothing else is indexed under -gdb-set or
-gdb-show, and as -gdb-set/-gdb-show are just the MI equivalent for
set/show anything that is documented under set/show can be adjusted
using -gdb-set/-gdbshow, and so, I've tried to keep the index
consistent for mi-async.
|
|
Convert the 'set debug lin-lwp' command to a boolean. Adds a new
LINUX_NAT_SCOPED_DEBUG_ENTER_EXIT macro, and makes use of it in one
place (linux_nat_target::stop).
The manual entry for 'set debug lin-lwp' is already vague about
exactly what arguments this command takes, and the description talks
about turning debug on and off, so I don't think there's any updates
required there.
I have updated the doc strings shown when the users enters 'help show
debug lin-lwp' or 'help show debug lin-lwp'. The old title lines used
to talk about the 'GNU/Linux lwp module', but this debug flag is now
used for any native linux target debug, so we now talk about
'GNU/Linux native target'. The body string for this setting has been
changed from 'Enables printf debugging output.' to 'When on, print
debug messages relating to the GNU/Linux native target.', the old
value looks like a cut&paste error to me.
|
|
Add new commands:
set debug threads on|off
show debug threads
Prints additional debug information relating to thread creation and
deletion.
GDB already announces when threads are created of course.... most of
the time, but sometimes threads are added silently, in which case this
debug message is the only mechanism to see the thread being added.
Also, though GDB does announce when a thread exits, it doesn't
announce when the thread object is deleted, I've added a debug message
for that.
Additionally, having message printed through the debug system will
cause the messages to be nested to an appropriate depth when other
debug sub-systems are turned on (especially things like `infrun` and
`lin-lwp`).
|
|
During review, it was suggested to change the "params" parameter from a
tuple to a list, for esthetic reasons. The empty ones are still tuples
though, they should probably be changed to be empty lists, for
consistency. It does not change anything in the script result.
Change-Id: If13c6c527aa167a5ee5b45740e5f1bda1e9517e4
|
|
Fix mispelling of PROT_ME to PROT_MTE in the error messages.
|
|
This removes the print_spaces helper function, in favor of using the
"*%s" idiom that's already used in many places in gdb. One spot (in
symmisc.c) is changed to use print_spaces_filtered, because the rest
of that function is using filtered output. (This highlights one way
that the printf idiom is better -- this error is harder to make when
using that.)
Regression tested on x86-64 Fedora 34.
|
|
I noticed that puts_debug isn't used in the tree. git log tells me
that the last use was removed in 2015:
commit 40e0b27177e747600d3ec186458fe0e482a1cf77
Author: Pedro Alves <palves@redhat.com>
Date: Mon Aug 24 15:40:26 2015 +0100
Delete the remaining ROM monitor targets
... and this commit mentions that the code being removed here probably
hadn't worked for 6 years prior to that.
Based on this, I'm removing puts_debug. I don't think it's useful.
Tested by rebuilding.
|
|
n_spaces keeps the spaces in a static buffer. If a caller overwrites
these, it may give an incorrect result to a subsequent caller. So,
make the return type const to help avoid this outcome.
|
|
|
|
This commit reformats a comment in gdb/ada-exp.y to avoid
the leading '*' at the beginning of each line of the comment.
|
|
This commit reformats a comment in gdb/ada-lang.h to avoid
the leading '*' at the beginning of each line of the comment.
|
|
This command adds the "exit" command as an alias for the "quit"
command, as requested in PR gdb/28406.
The documentation is also updated to mention this new command.
Tested on x86_64-linux.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28406
|
|
While working on another patch I ended up in a situation where I had
async mode disabled (with 'maint set target-async off'), but the async
event token got marked anyway.
In this situation GDB was continually calling into
remote_target::wait, however, the async token would never become
unmarked as the unmarking is guarded by target_is_async_p.
We could just unconditionally unmark the token, but that would feel
like just ignoring a bug, so, instead, lets assert that if
!target_is_async_p, then the async token should not be marked.
This assertion would have caught my earlier mistake.
There should be no user visible changes with this commit.
|
|
While working on another patch relating to remote targets, I wanted to
test with 'maint set target-async off' in place. Unfortunately I ran
into some problems. This commit is an attempt to fix one of the
issues I hit.
In my particular case I was actually running with:
maint set target-async off
maint set target-non-stop off
that is, we're telling GDB to force the targets to operate in
non-async mode, and in all-stop mode. Here's my GDB session showing
the problem:
(gdb) maintenance set target-async off
(gdb) maintenance set target-non-stop off
(gdb) target extended-remote :54321
Remote debugging using :54321
(gdb) attach 2365960
Attaching to process 2365960
No unwaited-for children left.
(gdb)
Notice the 'No unwaited-for children left.' error, this is the
problem. There's no reason why GDB should not be able to attach to
the process.
The problem is this:
1. The user runs 'attach PID' and this sends GDB into attach_command
in infcmd.c. From here we call the ::attach method on the attach
target, which will be the extended_remote_target.
2. In extended_remote_target::attach, we attach to the remote target
and get the first reply (which is a stop packet). We put off
processing the stop packet until the end of ::attach. We setup the
inferior and thread to represent the process we attached to, and
download the target description. Finally, we process the initial
stop packet.
If '!target_is_non_stop_p ()' and '!target_can_async_p ()', which is
the case for us given the maintenance commands we used, we cache the
stop packet within the remote_state::buf for later processing.
3. Back in attach_command, if 'target_is_non_stop_p ()' then we
request that the target stops. This will either process any cached
stop replies, or request that the target stops, and process the stop
replies. However, this code is not what we use due to non-stop mode
being disabled. So, we skip to the next step which is to call
validate_exec_file.
4. Calling validate_exec_file can cause packets to be sent to the
remote target, and replies received, the first path I hit is the
call to target_pid_to_exec_file, which calls
remote_target::pid_to_exec_file, which can then try to read the
executable from the remote. Sending an receiving packets will make
use of the remote_state::buf object.
5. The attempt to attach continues, but the damage is already done...
So, the problem is that, in step #2 we cache a stop reply in the
remote_state::buf, and then in step #4 we reuse the remote_state::buf
object, discarding any cached stop reply. As a result, the initial
stop, which is sent when GDB first attaches to the target, is lost.
This problem can clearly be seen, I feel, by looking at the
remote_state::cached_wait_status flag. This flag tells GDB if there
is a wait status cached in remote_state::buf. However, in
remote_target::putpkt_binary and remote_target::getpkt_or_notif_sane_1
this flag is just set back to 0, doing this immediately discards any
cached data.
I don't know if this scheme ever made sense, looking at commit
2d717e4f8a54, where the cached_wait_status flag was added, it appears
that there was nothing between where the stop was cached, and where
the stop was consumed, so, I suspect, there never was a situation
where we ended up in putpkt_binary or getpkt_or_notif_sane_1 and
needed to clear to the flag, maybe the clearing was added "just in
case". Whatever the history, I claim that this clearing this flag is
no longer a good idea.
So, my first step toward fixing this issue was to replace the two
instances of 'rs->cached_wait_status = 0;' in ::putpkt_binary and
::getpkt_or_notif_sane_1 with 'gdb_assert (rs->cached_wait_status ==
0);', this, at least would show me when GDB was doing something
dangerous, and indeed, this assert is now hit in my test case above.
I did play with using some kind of scoped restore to backup, and
restore the remote_state::buf object in all the places within remote.c
that I was hitting where the ::buf was being corrupted. The first
problem with this is that, where the ::cached_wait_status flag is
reset is _not_ where ::buf is corrupted. For the ::putpkt_binary
case, by the time we get to the method the buffer has already been
corrupted in many cases, so we end up needing to add the scoped
save/restore within the callers, which means we need the save/restore
in _lots_ of places.
Plus, using this save/restore model feels like the wrong solution. I
don't think that it's obvious that the buffer might be holding cached
data, and I think it would be too easy for new corruptions of the
buffer to be introduced, which could easily go unnoticed for a long
time.
So, I really wanted a solution that didn't require us to cache data in
the ::buf object.
Luckily, I think we already have such a solution in place, the
remote_state::stop_reply_queue, it seems like this does exactly the
same task, just in a slightly different way. With the
::stop_reply_queue, the stop packets are processed upon receipt and
the stop_reply object is added to the queue. With the ::buf cache
solution, the unprocessed stop reply is cached in the ::buf, and
processed later.
So, finally, in this commit, I propose to remove the
remote_state::cached_wait_status flag and to stop using the ::buf to
cache stop replies. Instead, stop replies will now always be stored
in the ::stop_reply_queue.
There are two places where we use the ::buf to hold a cached stop
reply, the first is in the ::attach method, and the second is in
remote_target::start_remote, however, the second of these cases is far
less problematic, as after caching the stop reply in ::buf we call the
global start_remote function, which does very little work before
calling normal_stop, which processes the cached stop reply. However,
my plan is to switch both users over to using ::stop_reply_queue so
that the old (unsafe) ::cached_wait_status mechanism can be completely
removed.
The next problem is that the ::stop_reply_queue is currently only used
for async-mode, and so, in remote_target::push_stop_reply, where we
push stop_reply objects into the ::stop_reply_queue, we currently also
mark the async event token. I've modified this so we only mark the
async event token if 'target_is_async_p ()' - note, _is_, not _can_
here. The ::push_stop_reply method is called in places where async
mode has been temporarily disabled, but, when async mode is switched
back on (see remote_target::async) we will mark the event token if
there are events in the queue.
Another change of interest is in remote_target::remote_interrupt_as.
Previously this code checked ::cached_wait_status, but didn't check
for events in the ::stop_reply_queue. Now that ::cached_wait_status
has been removed we now check the queue length instead, which should
have the same result.
Finally, in remote_target::wait_as, I've tried to merge the processing
of the ::stop_reply_queue with how we used to handle the
::cached_wait_status flag.
Currently, when processing the ::stop_reply_queue we call
process_stop_reply and immediately return. However, when handling
::cached_wait_status we run through the whole of ::wait_as, and return
at the end of the function.
If we consider a standard stop packet, the two differences I see are:
1. Resetting of the remote_state::waiting_for_stop_reply, flag; this
is not currently done when processing a stop from the
::stop_reply_queue.
2. The final return value has the possibility of being adjusted at
the end of ::wait_as, as well as there being calls to
record_currthread, non of which are done if we process a stop from
the ::stop_reply_queue.
After discussion on the mailing list:
https://sourceware.org/pipermail/gdb-patches/2021-December/184535.html
it was suggested that, when an event is pushed into the
::stop_reply_queue, the ::waiting_for_stop_reply flag is never going
to be set. As a result, we don't need to worry about the first
difference. I have however, added a gdb_assert to validate the
assumption that the flag is never going to be set. If in future the
situation ever changes, then we should find out pretty quickly.
As for the second difference, I have resolved this by having all stop
packets taken from the ::stop_reply_queue, pass through the return
value adjustment code at the end of ::wait_as.
An example of a test that reveals the benefits of this commit is:
make check-gdb \
RUNTESTFLAGS="--target_board=native-extended-gdbserver \
GDBFLAGS='-ex maint\ set\ target-async\ off \
-ex maint\ set\ target-non-stop\ off' \
gdb.base/attach.exp"
For testing I've been running test on x86-64/GNU Linux, and run with
target boards unix, native-gdbserver, and native-extended-gdbserver.
For each board I've run with the default GDBFLAGS, as well as with:
GDBFLAGS='-ex maint\ set\ target-async\ off \
-ex maint\ set\ target-non-stop\ off' \
Though running with the above GDBFLAGS is clearly a lot more unstable
both before and after my patch, I'm not seeing any consistent new
failures with my patch, except, with the native-extended-gdbserver
board, where I am seeing new failures, but only because more tests are
now running. For that configuration alone I see the number of
unresolved go down by 49, the number of passes goes up by 446, and the
number of failures also increases by 144. All of the failures are new
tests as far as I can tell.
|
|
This adds a comment to document how to update gdbarch.
|
|
This patch runs gdbarch.py and removes gdbarch.sh.
|
|
The new gdbarch generator is a Python program. It reads the
"components.py" that was created in the previous patch, and generates
gdbarch.c and gdbarch-gen.h.
This is a relatively straightforward translation of the existing .sh
code. It doesn't try very hard to be idiomatic Python or to be
especially smart.
It is, however, incredibly faster:
$ time ./gdbarch.sh
real 0m8.197s
user 0m5.779s
sys 0m3.384s
$ time ./gdbarch.py
real 0m0.065s
user 0m0.053s
sys 0m0.011s
Co-Authored-By: Tom Tromey <tom@tromey.com>
|
|
The new gdbarch.sh approach will be to edit a Python file, rather than
adding a line to a certain part of gdbarch.sh. We use the existing sh
code, though, to generate the first draft of this .py file.
Documentation on the format will come in a subsequent patch.
Note that some info (like "staticdefault") in the current code is
actually unused, and so is ignored by this new generator.
|
|
This changes gdbarch.sh so that it no longer sorts the fields in
gdbarch_dump. This sorting isn't done anywhere else by gdbarch.sh,
and this simplifies the new generator a little bit.
|
