| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
attaching / handling a fork child
When trying to attach to a pthread process on a Linux system with glibc 2.33,
we get:
$ ./gdb -q -nx --data-directory=data-directory -p 1472010
Attaching to process 1472010
[New LWP 1472013]
[New LWP 1472014]
[New LWP 1472015]
Error while reading shared library symbols for /usr/lib/libpthread.so.0:
Cannot find user-level thread for LWP 1472015: generic error
0x00007ffff6d3637f in poll () from /usr/lib/libc.so.6
(gdb)
When attaching to a process (or handling a fork child, an operation very
similar to attaching), GDB reads the shared library list from the
process. For each shared library (if "set auto-solib-add" is on), it
reads its symbols and calls the "new_objfile" observable.
The libthread-db code monitors this observable, and if it sees an
objfile named somewhat like "libpthread.so" go by, it tries to load
libthread_db.so in the GDB process itself. libthread_db knows how to
navigate libpthread's data structures to get information about the
existing threads.
To locate these data structures, libthread_db calls ps_pglobal_lookup
(implemented in proc-service.c), passing in a symbol name and expecting
an address in return.
Before glibc 2.33, libthread_db always asked for symbols found in
libpthread. There was no ordering problem: since we were always trying
to load libthread_db in reaction to processing libpthread (and reading
in its symbols) and libthread_db only asked symbols from libpthread, the
requested symbols could always be found. Starting with glibc 2.33,
libthread_db now asks for a symbol name that can be found in
/lib/ld-linux-x86-64.so.2 (_rtld_global). And the ordering in which GDB
reads the shared libraries from the inferior when attaching is
unfortunate, in that libpthread is processed before ld-linux. So when
loading libthread_db in reaction to processing libpthread, and
libthread_db requests the symbol that is from ld-linux, GDB is not yet
able to supply it.
That problematic symbol lookup happens in the thread_from_lwp function,
when we call td_ta_map_lwp2thr_p, and an exception is thrown at this
point:
#0 0x00007ffff6681012 in __cxxabiv1::__cxa_throw (obj=0x60e000006100, tinfo=0x555560033b50 <typeinfo for gdb_exception_error>, dest=0x55555d9404bc <gdb_exception_error::~gdb_exception_error()>) at /build/gcc/src/gcc/libstdc++-v3/libsupc++/eh_throw.cc:78
#1 0x000055555e5d3734 in throw_it(return_reason, errors, const char *, typedef __va_list_tag __va_list_tag *) (reason=RETURN_ERROR, error=GENERIC_ERROR, fmt=0x55555f0c5360 "Cannot find user-level thread for LWP %ld: %s", ap=0x7fffffffaae0) at /home/simark/src/binutils-gdb/gdbsupport/common-exceptions.cc:200
#2 0x000055555e5d37d4 in throw_verror (error=GENERIC_ERROR, fmt=0x55555f0c5360 "Cannot find user-level thread for LWP %ld: %s", ap=0x7fffffffaae0) at /home/simark/src/binutils-gdb/gdbsupport/common-exceptions.cc:208
#3 0x000055555e0b0ed2 in verror (string=0x55555f0c5360 "Cannot find user-level thread for LWP %ld: %s", args=0x7fffffffaae0) at /home/simark/src/binutils-gdb/gdb/utils.c:171
#4 0x000055555e5e898a in error (fmt=0x55555f0c5360 "Cannot find user-level thread for LWP %ld: %s") at /home/simark/src/binutils-gdb/gdbsupport/errors.cc:43
#5 0x000055555d06b4bc in thread_from_lwp (stopped=0x617000035d80, ptid=...) at /home/simark/src/binutils-gdb/gdb/linux-thread-db.c:418
#6 0x000055555d07040d in try_thread_db_load_1 (info=0x60c000011140) at /home/simark/src/binutils-gdb/gdb/linux-thread-db.c:912
#7 0x000055555d071103 in try_thread_db_load (library=0x55555f0c62a0 "libthread_db.so.1", check_auto_load_safe=false) at /home/simark/src/binutils-gdb/gdb/linux-thread-db.c:1014
#8 0x000055555d072168 in try_thread_db_load_from_sdir () at /home/simark/src/binutils-gdb/gdb/linux-thread-db.c:1091
#9 0x000055555d072d1c in thread_db_load_search () at /home/simark/src/binutils-gdb/gdb/linux-thread-db.c:1146
#10 0x000055555d07365c in thread_db_load () at /home/simark/src/binutils-gdb/gdb/linux-thread-db.c:1203
#11 0x000055555d07373e in check_for_thread_db () at /home/simark/src/binutils-gdb/gdb/linux-thread-db.c:1246
#12 0x000055555d0738ab in thread_db_new_objfile (objfile=0x61300000c0c0) at /home/simark/src/binutils-gdb/gdb/linux-thread-db.c:1275
#13 0x000055555bd10740 in std::__invoke_impl<void, void (*&)(objfile*), objfile*> (__f=@0x616000068d88: 0x55555d073745 <thread_db_new_objfile(objfile*)>) at /usr/include/c++/10.2.0/bits/invoke.h:60
#14 0x000055555bd02096 in std::__invoke_r<void, void (*&)(objfile*), objfile*> (__fn=@0x616000068d88: 0x55555d073745 <thread_db_new_objfile(objfile*)>) at /usr/include/c++/10.2.0/bits/invoke.h:153
#15 0x000055555bce0392 in std::_Function_handler<void (objfile*), void (*)(objfile*)>::_M_invoke(std::_Any_data const&, objfile*&&) (__functor=..., __args#0=@0x7fffffffb4a0: 0x61300000c0c0) at /usr/include/c++/10.2.0/bits/std_function.h:291
#16 0x000055555d3595c0 in std::function<void (objfile*)>::operator()(objfile*) const (this=0x616000068d88, __args#0=0x61300000c0c0) at /usr/include/c++/10.2.0/bits/std_function.h:622
#17 0x000055555d356b7f in gdb::observers::observable<objfile*>::notify (this=0x555566727020 <gdb::observers::new_objfile>, args#0=0x61300000c0c0) at /home/simark/src/binutils-gdb/gdb/../gdbsupport/observable.h:106
#18 0x000055555da3f228 in symbol_file_add_with_addrs (abfd=0x61200001ccc0, name=0x6190000d9090 "/usr/lib/libpthread.so.0", add_flags=..., addrs=0x7fffffffbc10, flags=..., parent=0x0) at /home/simark/src/binutils-gdb/gdb/symfile.c:1131
#19 0x000055555da3f763 in symbol_file_add_from_bfd (abfd=0x61200001ccc0, name=0x6190000d9090 "/usr/lib/libpthread.so.0", add_flags=<error reading variable: Cannot access memory at address 0xffffffffffffffb0>, addrs=0x7fffffffbc10, flags=<error reading variable: Cannot access memory at address 0xffffffffffffffc0>, parent=0x0) at /home/simark/src/binutils-gdb/gdb/symfile.c:1167
#20 0x000055555d95f9fa in solib_read_symbols (so=0x6190000d8e80, flags=...) at /home/simark/src/binutils-gdb/gdb/solib.c:681
#21 0x000055555d96233d in solib_add (pattern=0x0, from_tty=0, readsyms=1) at /home/simark/src/binutils-gdb/gdb/solib.c:987
#22 0x000055555d93646e in enable_break (info=0x608000008f20, from_tty=0) at /home/simark/src/binutils-gdb/gdb/solib-svr4.c:2238
#23 0x000055555d93cfc0 in svr4_solib_create_inferior_hook (from_tty=0) at /home/simark/src/binutils-gdb/gdb/solib-svr4.c:3049
#24 0x000055555d96610d in solib_create_inferior_hook (from_tty=0) at /home/simark/src/binutils-gdb/gdb/solib.c:1195
#25 0x000055555cdee318 in post_create_inferior (from_tty=0) at /home/simark/src/binutils-gdb/gdb/infcmd.c:318
#26 0x000055555ce00e6e in setup_inferior (from_tty=0) at /home/simark/src/binutils-gdb/gdb/infcmd.c:2439
#27 0x000055555ce59c34 in handle_one (event=...) at /home/simark/src/binutils-gdb/gdb/infrun.c:4887
#28 0x000055555ce5cd00 in stop_all_threads () at /home/simark/src/binutils-gdb/gdb/infrun.c:5064
#29 0x000055555ce7f0da in stop_waiting (ecs=0x7fffffffd170) at /home/simark/src/binutils-gdb/gdb/infrun.c:8006
#30 0x000055555ce67f5c in handle_signal_stop (ecs=0x7fffffffd170) at /home/simark/src/binutils-gdb/gdb/infrun.c:6062
#31 0x000055555ce63653 in handle_inferior_event (ecs=0x7fffffffd170) at /home/simark/src/binutils-gdb/gdb/infrun.c:5727
#32 0x000055555ce4f297 in fetch_inferior_event () at /home/simark/src/binutils-gdb/gdb/infrun.c:4105
#33 0x000055555cdbe3bf in inferior_event_handler (event_type=INF_REG_EVENT) at /home/simark/src/binutils-gdb/gdb/inf-loop.c:42
#34 0x000055555d018047 in handle_target_event (error=0, client_data=0x0) at /home/simark/src/binutils-gdb/gdb/linux-nat.c:4060
#35 0x000055555e5ea77e in handle_file_event (file_ptr=0x60600008b1c0, ready_mask=1) at /home/simark/src/binutils-gdb/gdbsupport/event-loop.cc:575
#36 0x000055555e5eb09c in gdb_wait_for_event (block=0) at /home/simark/src/binutils-gdb/gdbsupport/event-loop.cc:701
#37 0x000055555e5e8d19 in gdb_do_one_event () at /home/simark/src/binutils-gdb/gdbsupport/event-loop.cc:212
#38 0x000055555dd6e0d4 in wait_sync_command_done () at /home/simark/src/binutils-gdb/gdb/top.c:528
#39 0x000055555dd6e372 in maybe_wait_sync_command_done (was_sync=0) at /home/simark/src/binutils-gdb/gdb/top.c:545
#40 0x000055555d0ec7c8 in catch_command_errors (command=0x55555ce01bb8 <attach_command(char const*, int)>, arg=0x7fffffffe28d "1472010", from_tty=1, do_bp_actions=false) at /home/simark/src/binutils-gdb/gdb/main.c:452
#41 0x000055555d0f03ad in captured_main_1 (context=0x7fffffffdd10) at /home/simark/src/binutils-gdb/gdb/main.c:1149
#42 0x000055555d0f1239 in captured_main (data=0x7fffffffdd10) at /home/simark/src/binutils-gdb/gdb/main.c:1232
#43 0x000055555d0f1315 in gdb_main (args=0x7fffffffdd10) at /home/simark/src/binutils-gdb/gdb/main.c:1257
#44 0x000055555bb70cf9 in main (argc=7, argv=0x7fffffffde88) at /home/simark/src/binutils-gdb/gdb/gdb.c:32
The exception is caught here:
#0 __cxxabiv1::__cxa_begin_catch (exc_obj_in=0x60e0000060e0) at /build/gcc/src/gcc/libstdc++-v3/libsupc++/eh_catch.cc:84
#1 0x000055555d95fded in solib_read_symbols (so=0x6190000d8e80, flags=...) at /home/simark/src/binutils-gdb/gdb/solib.c:689
#2 0x000055555d96233d in solib_add (pattern=0x0, from_tty=0, readsyms=1) at /home/simark/src/binutils-gdb/gdb/solib.c:987
#3 0x000055555d93646e in enable_break (info=0x608000008f20, from_tty=0) at /home/simark/src/binutils-gdb/gdb/solib-svr4.c:2238
#4 0x000055555d93cfc0 in svr4_solib_create_inferior_hook (from_tty=0) at /home/simark/src/binutils-gdb/gdb/solib-svr4.c:3049
#5 0x000055555d96610d in solib_create_inferior_hook (from_tty=0) at /home/simark/src/binutils-gdb/gdb/solib.c:1195
#6 0x000055555cdee318 in post_create_inferior (from_tty=0) at /home/simark/src/binutils-gdb/gdb/infcmd.c:318
#7 0x000055555ce00e6e in setup_inferior (from_tty=0) at /home/simark/src/binutils-gdb/gdb/infcmd.c:2439
#8 0x000055555ce59c34 in handle_one (event=...) at /home/simark/src/binutils-gdb/gdb/infrun.c:4887
#9 0x000055555ce5cd00 in stop_all_threads () at /home/simark/src/binutils-gdb/gdb/infrun.c:5064
#10 0x000055555ce7f0da in stop_waiting (ecs=0x7fffffffd170) at /home/simark/src/binutils-gdb/gdb/infrun.c:8006
#11 0x000055555ce67f5c in handle_signal_stop (ecs=0x7fffffffd170) at /home/simark/src/binutils-gdb/gdb/infrun.c:6062
#12 0x000055555ce63653 in handle_inferior_event (ecs=0x7fffffffd170) at /home/simark/src/binutils-gdb/gdb/infrun.c:5727
#13 0x000055555ce4f297 in fetch_inferior_event () at /home/simark/src/binutils-gdb/gdb/infrun.c:4105
#14 0x000055555cdbe3bf in inferior_event_handler (event_type=INF_REG_EVENT) at /home/simark/src/binutils-gdb/gdb/inf-loop.c:42
#15 0x000055555d018047 in handle_target_event (error=0, client_data=0x0) at /home/simark/src/binutils-gdb/gdb/linux-nat.c:4060
#16 0x000055555e5ea77e in handle_file_event (file_ptr=0x60600008b1c0, ready_mask=1) at /home/simark/src/binutils-gdb/gdbsupport/event-loop.cc:575
#17 0x000055555e5eb09c in gdb_wait_for_event (block=0) at /home/simark/src/binutils-gdb/gdbsupport/event-loop.cc:701
#18 0x000055555e5e8d19 in gdb_do_one_event () at /home/simark/src/binutils-gdb/gdbsupport/event-loop.cc:212
#19 0x000055555dd6e0d4 in wait_sync_command_done () at /home/simark/src/binutils-gdb/gdb/top.c:528
#20 0x000055555dd6e372 in maybe_wait_sync_command_done (was_sync=0) at /home/simark/src/binutils-gdb/gdb/top.c:545
#21 0x000055555d0ec7c8 in catch_command_errors (command=0x55555ce01bb8 <attach_command(char const*, int)>, arg=0x7fffffffe28d "1472010", from_tty=1, do_bp_actions=false) at /home/simark/src/binutils-gdb/gdb/main.c:452
#22 0x000055555d0f03ad in captured_main_1 (context=0x7fffffffdd10) at /home/simark/src/binutils-gdb/gdb/main.c:1149
#23 0x000055555d0f1239 in captured_main (data=0x7fffffffdd10) at /home/simark/src/binutils-gdb/gdb/main.c:1232
#24 0x000055555d0f1315 in gdb_main (args=0x7fffffffdd10) at /home/simark/src/binutils-gdb/gdb/main.c:1257
#25 0x000055555bb70cf9 in main (argc=7, argv=0x7fffffffde88) at /home/simark/src/binutils-gdb/gdb/gdb.c:32
Catching the exception at this point means that the thread_db_info
object for this inferior will be left in place, despite the failure to
load libthread_db. This means that there won't be further attempts at
loading libthread_db, because thread_db_load will think that
libthread_db is already loaded for this inferior and will always exit
early. To fix this, add a try/catch around calling try_thread_db_load_1
in try_thread_db_load, such that if some exception is thrown while
trying to load libthread_db, we reset / delete the thread_db_info for
that inferior. That alone makes attach work fine again, because
check_for_thread_db is called again in the thread_db_inferior_created
observer (that happens after we learned about all shared libraries and
their symbols), and libthread_db is successfully loaded then.
When attaching, I think that the inferior_created observer is a good
place to try to load libthread_db: it is called once everything has
stabilized, when we learned about all shared libraries.
The only problem then is that when we first try (and fail) to load
libthread_db, in reaction to learning about libpthread, we show this
warning:
warning: Unable to find libthread_db matching inferior's thread library, thread debugging will not be available.
This is misleading, because we do succeed in loading it later. So when
attaching, I think we shouldn't try to load libthread_db in reaction to
the new_objfile events, we should wait until we have learned about all
shared libraries (using the inferior_created observable). To do so, add
an `in_initial_library_scan` flag to struct inferior. This flag is used
to postpone loading libthread_db if we are attaching or handling a fork
child.
When debugging remotely with GDBserver, the same problem happens, except
that the qSymbol mechanism (allowing the remote side to ask GDB for
symbols values) is involved. The fix there is the same idea, we make
GDB wait until all shared libraries and their symbols are known before
sending out a qSymbol packet. This way, we never present the remote
side a state where libpthread.so's symbols are known but ld-linux's
symbols aren't.
gdb/ChangeLog:
* inferior.h (class inferior) <in_initial_library_scan>: New.
* infcmd.c (post_create_inferior): Set in_initial_library_scan.
* infrun.c (follow_fork_inferior): Likewise.
* linux-thread-db.c (try_thread_db_load): Catch exception thrown
by try_thread_db_load_1
(thread_db_load): Return early if in_initial_library_scan is
set.
* remote.c (remote_new_objfile): Return early if
in_initial_library_scan is set.
Change-Id: I7a279836cfbb2b362b4fde11b196b4aab82f5efb
|
|
There is no default method for
gdbarch_displaced_step_restore_all_in_ptid, so calling it
unconditionally for fork events triggered an assertion failure on
platforms that do not support displaced stepping. To fix, only invoke
the method if the gdbarch supports displaced stepping.
Note that not all gdbarches support both displaced stepping and fork
events, so gdbarch validation does not require
gdbarch_displaced_step_restore_all_in_ptid for any gdbarch supporting
displaced stepping. However, the internal assertion in
gdbarch_displaced_step_restore_all_in_ptid should catch any gdbarches
which do support both but fail to provide this method.
gdb/ChangeLog:
* infrun.c (handle_inferior_event): Only call
gdbarch_displaced_step_restore_all_in_ptid if
gdbarch_supports_displaced_stepping is true.
|
|
I spotted some indentation issues where we had some spaces followed by
tabs at beginning of line, that I wanted to fix. So while at it, I did
a quick grep to find and fix all I could find.
gdb/ChangeLog:
* Fix tab after space indentation issues throughout.
Change-Id: I1acb414dd9c593b474ae2b8667496584df4316fd
|
|
Same idea as previous patch, but for add_info_alias.
gdb/ChangeLog:
* command.h (add_info_alias): Accept target as
cmd_list_element. Update callers.
Change-Id: If830d423364bf42d7bea5ac4dd3a81adcfce6f7a
|
|
target_ops::follow_exec
On "exec", some targets need to unpush themselves from the inferior,
and do some bookkeeping, like forgetting the data associated to the
exec'ing inferior.
One such example is the thread-db target. It does so in
a special case in thread_db_target::wait, just before returning the
TARGET_WAITKIND_EXECD event to its caller.
We have another such case in the context of rocm-gdb [1], where the
"rocm" target is pushed on top of the linux-nat target. When an exec
happens, we want to unpush the rocm target from the exec'ing inferior to
close some file descriptors that refer to the pre-exec address space and
forget about that inferior. We then want to push the target on the
inferior in which execution continues, to open the file descriptors for
the post-exec address space.
I think that a good way to address this cleanly is to do all this in the
target_ops::follow_exec implementations. Make the
process_stratum_target::follow_exec implementation have the default
behavior of pushing itself to the new inferior's target stack (if
execution continues in a new inferior) and add the initial thread.
remote_target::follow_exec is an example of process target that wants to
do a bit more than the default behavior. So it calls
process_stratum_target::follow_exec first and does the extra work
second.
linux-thread-db (a non-process target) implements follow_exec to do some
bookeeping (forget about that process' data), before handing down the
event down to the process target (which hits
process_stratum_target::follow_exec).
gdb/ChangeLog:
* target.h (struct target_ops) <follow_exec>: Add ptid_t
parameter.
(target_follow_exec): Likewise.
* target.c (target_follow_exec): Add ptid_t parameter.
* infrun.c (follow_exec): Adjust call to target_follow_exec,
don't push target nor create thread.
* linux-thread-db.c (class thread_db_target) <follow_exec>: New.
(thread_db_target::wait): Just return on TARGET_WAITKIND_EXECD.
(thread_db_target::follow_exec): New.
* remote.c (class remote_target) <follow_exec>: Add ptid_t parameter.
(remote_target::follow_exec): Call
process_stratum_target::follow_exec.
* target-delegates.c: Re-generate.
Change-Id: I3f96d0ba3ea0dde6540b7e1b4d5cdb01635088c8
|
|
target_follow_exec is currently only called in the "follow-exec-mode ==
new" branch of follow_exec, not the "follow-exec-mode == same" branch.
I think it would make sense to call it regardless of the mode to let
targets do some necessary handling.
This is needed in the context of rocm-gdb [1], where a target is pushed
on top of the linux-nat target. On exec, it needs to do some
bookkeeping, close some file descriptors / handles that were related to
the process pre-exec and open some new ones for the process post-exec.
However, by looking at the only in-tree implementation of
target_ops::follow_exec, remote_target::follow_exec, I found that it
would be useful for the extended-remote target too, to align its
behavior with native debugging (although I think that behavior is not
very user-friendly, see PR 27745 [2]).
Using two programs, one (let's call it "execer") that execs the other
(let's call it "execee"), with native:
$ ./gdb -q -nx --data-directory=data-directory ./execer
Reading symbols from ./execer...
(gdb) r
Starting program: /home/simark/build/binutils-gdb/gdb/execer
I am execer
process 1495622 is executing new program: /home/simark/build/binutils-gdb/gdb/execee
I am execee
[Inferior 1 (process 1495622) exited normally]
(gdb) r
Starting program: /home/simark/build/binutils-gdb/gdb/execee
I am execee
[Inferior 1 (process 1495626) exited normally]
And now with gdbserver (some irrelevant output lines removed for brevity):
$ ./gdbserver --once --multi :1234
...
$ ./gdb -q -nx --data-directory=data-directory ./execer -ex "set remote exec-file /home/simark/build/binutils-gdb/gdb/execer" -ex "tar ext :1234"
Reading symbols from ./execer...
Remote debugging using :1234
(gdb) r
Starting program: /home/simark/build/binutils-gdb/gdb/execer
process 1495724 is executing new program: /home/simark/build/binutils-gdb/gdb/execee
[Inferior 1 (process 1495724) exited normally]
(gdb) r
`target:/home/simark/build/binutils-gdb/gdb/execee' has disappeared; keeping its symbols.
Starting program: target:/home/simark/build/binutils-gdb/gdb/execee
warning: Build ID mismatch between current exec-file target:/home/simark/build/binutils-gdb/gdb/execee
and automatically determined exec-file target:/home/simark/build/binutils-gdb/gdb/execer
exec-file-mismatch handling is currently "ask"
Reading /home/simark/build/binutils-gdb/gdb/execer from remote target...
Load new symbol table from "target:/home/simark/build/binutils-gdb/gdb/execer"? (y or n)
When handling the exec, GDB updates the exec-file of the inferior to be
the execee. This means that a subsequent "run" will run the execee, not
the original executable (execer).
remote_target::follow_exec is meant to update the "remote exec-file",
which is the file on the remote system that will be executed if you
"run" the inferior, to the execee as well. However, this is not called
when follow-exec-mode is same, because target_follow_exec is not called
in this branch. As a result, GDB thinks the inferior is executing
execee but the remote side is really executing execer, hence the
mismatch message.
By calling target_follow_exec in the "same" branch of the follow_exec
function, we ensure that everybody agrees, and we get the same behavior
with the extended-remote target as we get with the native target, the
execee is executed on the second run:
$ ./gdbserver --once --multi :1234
...
$ ./gdb -q -nx --data-directory=data-directory ./execer -ex "set remote exec-file /home/simark/build/binutils-gdb/gdb/execer" -ex "tar ext :1234"
Reading symbols from ./execer...
Remote debugging using :1234
(gdb) r
Starting program: /home/simark/build/binutils-gdb/gdb/execer
process 1501445 is executing new program: /home/simark/build/binutils-gdb/gdb/execee
[Inferior 1 (process 1501445) exited normally]
(gdb) r
`target:/home/simark/build/binutils-gdb/gdb/execee' has disappeared; keeping its symbols.
Starting program: target:/home/simark/build/binutils-gdb/gdb/execee
[Inferior 1 (process 1501447) exited normally]
(gdb)
This scenario is tested in gdb.base/foll-exec-mode.exp, and in fact this
patch fixes the test for me when using
--target_board=native-extended-gdbserver.
gdb/ChangeLog:
* infrun.c (follow_exec): Call target_follow_fork when
follow-exec-mode is same.
* target.h (target_follow_fork): Improve doc.
[1] https://github.com/ROCm-Developer-Tools/ROCgdb
[2] https://sourceware.org/bugzilla/show_bug.cgi?id=27745
Change-Id: I4ee84a875e39bf3f8eaf3e6789a4bfe23a2a430e
|
|
These two variables:
struct regcache *regcache = get_thread_regcache (tp);
const address_space *aspace = regcache->aspace ();
are only needed inside the "if". Getting a thread's regcache is a
somewhat expensive operation, so it's good to avoid it if not necessary.
Move the variable declarations and their initialization to the "if"
scope.
gdb/ChangeLog:
* infrun.c (save_waitstatus): Move variables to inner scope.
Change-Id: Ief1463728755b4dcc142c0a0a76896e9d594ae84
|
|
Give a name to each observer, this will help produce more meaningful
debug message.
gdbsupport/ChangeLog:
* observable.h (class observable) <struct observer> <observer>:
Add name parameter.
<name>: New field.
<attach>: Add name parameter, update all callers.
Change-Id: Ie0cc4664925215b8d2b09e026011b7803549fba0
|
|
I noticed that all implementations return false, so
target_ops::follow_fork doesn't really need to return a value. Change
it to return void.
gdb/ChangeLog:
* target.h (struct target_ops) <follow_fork>: Return void.
(target_follow_fork): Likewise.
* target.c (default_follow_fork): Likewise.
(target_follow_fork): Likewise.
* infrun.c (follow_fork_inferior): Adjust.
* fbsd-nat.h (class fbsd_nat_target) <follow_fork>: Return void.
* fbsd-nat.c (fbsd_nat_target:::follow_fork): Likewise.
* linux-nat.h (class linux_nat_target) <follow_fork>: Likewise.
* linux-nat.c (linux_nat_target::follow_fork): Return void.
* obsd-nat.h (class obsd_nat_target) <follow_fork>: Return void.
* obsd-nat.c (obsd_nat_target::follow_fork): Likewise.
* remote.c (class remote_target) <follow_fork>: Likewise.
(remote_target::follow_fork): Likewise.
* target-delegates.c: Re-generate.
Change-Id: If908c2f68b29fa275be2b0b9deb41e4c6a1b7879
|
|
Rationale
---------
Let's say you have multiple threads hitting a conditional breakpoint
at the same time, and all of these are going to evaluate to false.
All these threads will need to be resumed.
Currently, GDB fetches one target event (one SIGTRAP representing the
breakpoint hit) and decides that the thread should be resumed. It
calls resume and commit_resume immediately. It then fetches the
second target event, and does the same, until it went through all
threads.
The result is therefore something like:
- consume event for thread A
- resume thread A
- commit resume (affects thread A)
- consume event for thread B
- resume thread B
- commit resume (affects thread B)
- consume event for thread C
- resume thread C
- commit resume (affects thread C)
For targets where it's beneficial to group resumptions requests (most
likely those that implement target_ops::commit_resume), it would be
much better to have:
- consume event for thread A
- resume thread A
- consume event for thread B
- resume thread B
- consume event for thread C
- resume thread C
- commit resume (affects threads A, B and C)
Implementation details
----------------------
To achieve this, this patch adds another check in
maybe_set_commit_resumed_all_targets to avoid setting the
commit-resumed flag of targets that readily have events to provide to
infrun.
To determine if a target has events readily available to report, this
patch adds an `has_pending_events` target_ops method. The method
returns a simple bool to say whether or not it has pending events to
report.
Testing
=======
To test this, I start GDBserver with a program that spawns multiple
threads:
$ ../gdbserver/gdbserver --once :1234 ~/src/many-threads-stepping-over-breakpoints/many-threads-stepping-over-breakpoints
I then connect with GDB and install a conditional breakpoint that always
evaluates to false (and force the evaluation to be done by GDB):
$ ./gdb -nx --data-directory=data-directory \
/home/simark/src/many-threads-stepping-over-breakpoints/many-threads-stepping-over-breakpoints \
-ex "set breakpoint condition-evaluation host" \
-ex "set pag off" \
-ex "set confirm off" \
-ex "maint set target-non-stop on" \
-ex "tar rem :1234" \
-ex "tb main" \
-ex "b 13 if 0" \
-ex c \
-ex "set debug infrun" \
-ex "set debug remote 1" \
-ex "set debug displaced"
I then do "continue" and look at the log.
The remote target receives a bunch of stop notifications for all
threads that have hit the breakpoint. infrun consumes and processes
one event, decides it should not cause a stop, prepares a displaced
step, after which we should see:
[infrun] maybe_set_commit_resumed_all_process_targets: not requesting commit-resumed for target remote, target has pending events
Same for a second thread (since we have 2 displaced step buffers).
For the following threads, their displaced step is deferred since
there are no more buffers available.
After consuming the last event the remote target has to offer, we get:
[infrun] maybe_set_commit_resumed_all_process_targets: enabling commit-resumed for target remote
[infrun] maybe_call_commit_resumed_all_process_targets: calling commit_resumed for target remote
[remote] Sending packet: $vCont;s:p14d16b.14d1b1;s:p14d16b.14d1b2#55
[remote] Packet received: OK
Without the patch, there would have been one vCont;s just after each
prepared displaced step.
gdb/ChangeLog:
yyyy-mm-dd Simon Marchi <simon.marchi@efficios.com>
Pedro Alves <pedro@palves.net>
* async-event.c (async_event_handler_marked): New.
* async-event.h (async_event_handler_marked): Declare.
* infrun.c (maybe_set_commit_resumed_all_targets): Switch to
inferior before calling target method. Don't commit-resumed if
target_has_pending_events is true.
* remote.c (remote_target::has_pending_events): New.
* target-delegates.c: Regenerate.
* target.c (target_has_pending_events): New.
* target.h (target_ops::has_pending_events): New target method.
(target_has_pending_events): New.
Change-Id: I18112ba19a1ff4986530c660f530d847bb4a1f1d
|
|
pending statuses
The rationale for this patch comes from the ROCm port [1], the goal
being to reduce the number of back and forths between GDB and the
target when doing successive operations. I'll start with explaining
the rationale and then go over the implementation. In the ROCm / GPU
world, the term "wave" is somewhat equivalent to a "thread" in GDB.
So if you read if from a GPU stand point, just s/thread/wave/.
ROCdbgapi, the library used by GDB [2] to communicate with the GPU
target, gives the illusion that it's possible for the debugger to
control (start and stop) individual threads. But in reality, this is
not how it works. Under the hood, all threads of a queue are
controlled as a group. To stop one thread in a group of running ones,
the state of all threads is retrieved from the GPU, all threads are
destroyed, and all threads but the one we want to stop are re-created
from the saved state. The net result, from the point of view of GDB,
is that the library stopped one thread. The same thing goes if we
want to resume one thread while others are running: the state of all
running threads is retrieved from the GPU, they are all destroyed, and
they are all re-created, including the thread we want to resume.
This leads to some inefficiencies when combined with how GDB works,
here are two examples:
- Stopping all threads: because the target operates in non-stop mode,
when the user interface mode is all-stop, GDB must stop all threads
individually when presenting a stop. Let's suppose we have 1000
threads and the user does ^C. GDB asks the target to stop one
thread. Behind the scenes, the library retrieves 1000 thread
states and restores the 999 others still running ones. GDB asks
the target to stop another one. The target retrieves 999 thread
states and restores the 998 remaining ones. That means that to
stop 1000 threads, we did 1000 back and forths with the GPU. It
would have been much better to just retrieve the states once and
stop there.
- Resuming with pending events: suppose the 1000 threads hit a
breakpoint at the same time. The breakpoint is conditional and
evaluates to true for the first thread, to false for all others.
GDB pulls one event (for the first thread) from the target, decides
that it should present a stop, so stops all threads using
stop_all_threads. All these other threads have a breakpoint event
to report, which is saved in `thread_info::suspend::waitstatus` for
later. When the user does "continue", GDB resumes that one thread
that did hit the breakpoint. It then processes the pending events
one by one as if they just arrived. It picks one, evaluates the
condition to false, and resumes the thread. It picks another one,
evaluates the condition to false, and resumes the thread. And so
on. In between each resumption, there is a full state retrieval
and re-creation. It would be much nicer if we could wait a little
bit before sending those threads on the GPU, until it processed all
those pending events.
To address this kind of performance issue, ROCdbgapi has a concept
called "forward progress required", which is a boolean state that
allows its user (i.e. GDB) to say "I'm doing a bunch of operations,
you can hold off putting the threads on the GPU until I'm done" (the
"forward progress not required" state). Turning forward progress back
on indicates to the library that all threads that are supposed to be
running should now be really running on the GPU.
It turns out that GDB has a similar concept, though not as general,
commit_resume. One difference is that commit_resume is not stateful:
the target can't look up "does the core need me to schedule resumed
threads for execution right now". It is also specifically linked to
the resume method, it is not used in other contexts. The target
accumulates resumption requests through target_ops::resume calls, and
then commits those resumptions when target_ops::commit_resume is
called. The target has no way to check if it's ok to leave resumed
threads stopped in other target methods.
To bridge the gap, this patch generalizes the commit_resume concept in
GDB to match the forward progress concept of ROCdbgapi. The current
name (commit_resume) can be interpreted as "commit the previous resume
calls". I renamed the concept to "commit_resumed", as in "commit the
threads that are resumed".
In the new version, we have two things:
- the commit_resumed_state field in process_stratum_target: indicates
whether GDB requires target stacks using this target to have
resumed threads committed to the execution target/device. If
false, an execution target is allowed to leave resumed threads
un-committed at the end of whatever method it is executing.
- the commit_resumed target method: called when commit_resumed_state
transitions from false to true. While commit_resumed_state was
false, the target may have left some resumed threads un-committed.
This method being called tells it that it should commit them back
to the execution device.
Let's take the "Stopping all threads" scenario from above and see how
it would work with the ROCm target with this change. Before stopping
all threads, GDB would set the target's commit_resumed_state field to
false. It would then ask the target to stop the first thread. The
target would retrieve all threads' state from the GPU and mark that
one as stopped. Since commit_resumed_state is false, it leaves all
the other threads (still resumed) stopped. GDB would then proceed to
call target_stop for all the other threads. Since resumed threads are
not committed, this doesn't do any back and forth with the GPU.
To simplify the implementation of targets, this patch makes it so that
when calling certain target methods, the contract between the core and
the targets guarantees that commit_resumed_state is false. This way,
the target doesn't need two paths, one for commit_resumed_state ==
true and one for commit_resumed_state == false. It can just assert
that commit_resumed_state is false and work with that assumption.
This also helps catch places where we forgot to disable
commit_resumed_state before calling the method, which represents a
probable optimization opportunity. The commit adds assertions in the
target method wrappers (target_resume and friends) to have some
confidence that this contract between the core and the targets is
respected.
The scoped_disable_commit_resumed type is used to disable the commit
resumed state of all process targets on construction, and selectively
re-enable it on destruction (see below for criteria). Note that it
only sets the process_stratum_target::commit_resumed_state flag. A
subsequent call to maybe_call_commit_resumed_all_targets is necessary
to call the commit_resumed method on all target stacks with process
targets that got their commit_resumed_state flag turned back on. This
separation is because we don't want to call the commit_resumed methods
in scoped_disable_commit_resumed's destructor, as they may throw.
On destruction, commit-resumed is not re-enabled for a given target
if:
1. this target has no threads resumed, or
2. this target has at least one resumed thread with a pending status
known to the core (saved in thread_info::suspend::waitstatus).
The first point is not technically necessary, because a proper
commit_resumed implementation would be a no-op if the target has no
resumed threads. But since we have a flag do to a quick check, it
shouldn't hurt.
The second point is more important: together with the
scoped_disable_commit_resumed instance added in fetch_inferior_event,
it makes it so the "Resuming with pending events" described above is
handled efficiently. Here's what happens in that case:
1. The user types "continue".
2. Upon destruction, the scoped_disable_commit_resumed in the
`proceed` function does not enable commit-resumed, as it sees some
threads have pending statuses.
3. fetch_inferior_event is called to handle another event, the
breakpoint hit evaluates to false, and that thread is resumed.
Because there are still more threads with pending statuses, the
destructor of scoped_disable_commit_resumed in
fetch_inferior_event still doesn't enable commit-resumed.
4. Rinse and repeat step 3, until the last pending status is handled
by fetch_inferior_event. In that case,
scoped_disable_commit_resumed's destructor sees there are no more
threads with pending statues, so it asks the target to commit
resumed threads.
This allows us to avoid all unnecessary back and forths, there is a
single commit_resumed call once all pending statuses are processed.
This change required remote_target::remote_stop_ns to learn how to
handle stopping threads that were resumed but pending vCont. The
simplest example where that happens is when using the remote target in
all-stop, but with "maint set target-non-stop on", to force it to
operate in non-stop mode under the hood. If two threads hit a
breakpoint at the same time, GDB will receive two stop replies. It
will present the stop for one thread and save the other one in
thread_info::suspend::waitstatus.
Before this patch, when doing "continue", GDB first resumes the thread
without a pending status:
Sending packet: $vCont;c:p172651.172676#f3
It then consumes the pending status in the next fetch_inferior_event
call:
[infrun] do_target_wait_1: Using pending wait status status->kind = stopped, signal = GDB_SIGNAL_TRAP for Thread 1517137.1517137.
[infrun] target_wait (-1.0.0, status) =
[infrun] 1517137.1517137.0 [Thread 1517137.1517137],
[infrun] status->kind = stopped, signal = GDB_SIGNAL_TRAP
It then realizes it needs to stop all threads to present the stop, so
stops the thread it just resumed:
[infrun] stop_all_threads: Thread 1517137.1517137 not executing
[infrun] stop_all_threads: Thread 1517137.1517174 executing, need stop
remote_stop called
Sending packet: $vCont;t:p172651.172676#04
This is an unnecessary resume/stop. With this patch, we don't commit
resumed threads after proceeding, because of the pending status:
[infrun] maybe_commit_resumed_all_process_targets: not requesting commit-resumed for target extended-remote, a thread has a pending waitstatus
When GDB handles the pending status and stop_all_threads runs, we stop a
resumed but pending vCont thread:
remote_stop_ns: Enqueueing phony stop reply for thread pending vCont-resume (1520940, 1520976, 0)
That thread was never actually resumed on the remote stub / gdbserver,
so we shouldn't send a packet to the remote side asking to stop the
thread.
Note that there are paths that resume the target and then do a
synchronous blocking wait, in sort of nested event loop, via
wait_sync_command_done. For example, inferior function calls, or any
run control command issued from a breakpoint command list. We handle
that making wait_sync_command_one a "sync" point -- force forward
progress, or IOW, force-enable commit-resumed state.
gdb/ChangeLog:
yyyy-mm-dd Simon Marchi <simon.marchi@efficios.com>
Pedro Alves <pedro@palves.net>
* infcmd.c (run_command_1, attach_command, detach_command)
(interrupt_target_1): Use scoped_disable_commit_resumed.
* infrun.c (do_target_resume): Remove
target_commit_resume call.
(commit_resume_all_targets): Remove.
(maybe_set_commit_resumed_all_targets): New.
(maybe_call_commit_resumed_all_targets): New.
(enable_commit_resumed): New.
(scoped_disable_commit_resumed::scoped_disable_commit_resumed)
(scoped_disable_commit_resumed::~scoped_disable_commit_resumed)
(scoped_disable_commit_resumed::reset)
(scoped_disable_commit_resumed::reset_and_commit)
(scoped_enable_commit_resumed::scoped_enable_commit_resumed)
(scoped_enable_commit_resumed::~scoped_enable_commit_resumed):
New.
(proceed): Use scoped_disable_commit_resumed and
maybe_call_commit_resumed_all_targets.
(fetch_inferior_event): Use scoped_disable_commit_resumed.
* infrun.h (struct scoped_disable_commit_resumed): New.
(maybe_call_commit_resumed_all_process_targets): New.
(struct scoped_enable_commit_resumed): New.
* mi/mi-main.c (exec_continue): Use scoped_disable_commit_resumed.
* process-stratum-target.h (class process_stratum_target):
<commit_resumed_state>: New.
* record-full.c (record_full_wait_1): Change commit_resumed_state
around calling commit_resumed.
* remote.c (class remote_target) <commit_resume>: Rename to...
<commit_resumed>: ... this.
(struct stop_reply): Move up.
(remote_target::commit_resume): Rename to...
(remote_target::commit_resumed): ... this. Check if there is any
thread pending vCont resume.
(remote_target::remote_stop_ns): Generate stop replies for resumed
but pending vCont threads.
(remote_target::wait_ns): Add gdb_assert.
* target-delegates.c: Regenerate.
* target.c (target_wait, target_resume): Assert that the current
process_stratum target isn't in commit-resumed state.
(defer_target_commit_resume): Remove.
(target_commit_resume): Remove.
(target_commit_resumed): New.
(make_scoped_defer_target_commit_resume): Remove.
(target_stop): Assert that the current process_stratum target
isn't in commit-resumed state.
* target.h (struct target_ops) <commit_resume>: Rename to ...
<commit_resumed>: ... this.
(target_commit_resume): Remove.
(target_commit_resumed): New.
(make_scoped_defer_target_commit_resume): Remove.
* top.c (wait_sync_command_done): Use
scoped_enable_commit_resumed.
[1] https://github.com/ROCm-Developer-Tools/ROCgdb/
[2] https://github.com/ROCm-Developer-Tools/ROCdbgapi
Change-Id: I836135531a29214b21695736deb0a81acf8cf566
|
|
The current_top_target function is a hidden dependency on the current
inferior. Since I'd like to slowly move towards reducing our dependency
on the global current state, remove this function and make callers use
current_inferior ()->top_target ()
There is no expected change in behavior, but this one step towards
making those callers use the inferior from their context, rather than
refer to the global current inferior.
gdb/ChangeLog:
* target.h (current_top_target): Remove, make callers use the
current inferior instead.
* target.c (current_top_target): Remove.
Change-Id: Iccd457036f84466cdaa3865aa3f9339a24ea001d
|
|
The following patch removes the current_top_target function, replacing
uses with `current_inferior ()->top_target ()`. This is a problem for
uses in target.h, because they don't have access to the current_inferior
function and the inferior structure: target.h can't include inferior.h,
otherwise that would make a cyclic inclusion.
Avoid this by moving all implementations of the wrappers that call
target methods with the current target to target.c. Many of them are
changed from a macro to a function, which is an improvement for
readability and debuggability, IMO.
target_shortname and target_longname were not function-like macros, so a
few adjustments are needed.
gdb/ChangeLog:
* target.h (target_shortname): Change to function declaration.
(target_longname): Likewise.
(target_attach_no_wait): Likewise.
(target_post_attach): Likewise.
(target_prepare_to_store): Likewise.
(target_supports_enable_disable_tracepoint): Likewise.
(target_supports_string_tracing): Likewise.
(target_supports_evaluation_of_breakpoint_conditions): Likewise.
(target_supports_dumpcore): Likewise.
(target_dumpcore): Likewise.
(target_can_run_breakpoint_commands): Likewise.
(target_files_info): Likewise.
(target_post_startup_inferior): Likewise.
(target_insert_fork_catchpoint): Likewise.
(target_remove_fork_catchpoint): Likewise.
(target_insert_vfork_catchpoint): Likewise.
(target_remove_vfork_catchpoint): Likewise.
(target_insert_exec_catchpoint): Likewise.
(target_remove_exec_catchpoint): Likewise.
(target_set_syscall_catchpoint): Likewise.
(target_rcmd): Likewise.
(target_can_lock_scheduler): Likewise.
(target_can_async_p): Likewise.
(target_is_async_p): Likewise.
(target_execution_direction): Likewise.
(target_extra_thread_info): Likewise.
(target_pid_to_exec_file): Likewise.
(target_thread_architecture): Likewise.
(target_find_memory_regions): Likewise.
(target_make_corefile_notes): Likewise.
(target_get_bookmark): Likewise.
(target_goto_bookmark): Likewise.
(target_stopped_by_watchpoint): Likewise.
(target_stopped_by_sw_breakpoint): Likewise.
(target_supports_stopped_by_sw_breakpoint): Likewise.
(target_stopped_by_hw_breakpoint): Likewise.
(target_supports_stopped_by_hw_breakpoint): Likewise.
(target_have_steppable_watchpoint): Likewise.
(target_can_use_hardware_watchpoint): Likewise.
(target_region_ok_for_hw_watchpoint): Likewise.
(target_can_do_single_step): Likewise.
(target_insert_watchpoint): Likewise.
(target_remove_watchpoint): Likewise.
(target_insert_hw_breakpoint): Likewise.
(target_remove_hw_breakpoint): Likewise.
(target_can_accel_watchpoint_condition): Likewise.
(target_can_execute_reverse): Likewise.
(target_get_ada_task_ptid): Likewise.
(target_filesystem_is_local): Likewise.
(target_trace_init): Likewise.
(target_download_tracepoint): Likewise.
(target_can_download_tracepoint): Likewise.
(target_download_trace_state_variable): Likewise.
(target_enable_tracepoint): Likewise.
(target_disable_tracepoint): Likewise.
(target_trace_start): Likewise.
(target_trace_set_readonly_regions): Likewise.
(target_get_trace_status): Likewise.
(target_get_tracepoint_status): Likewise.
(target_trace_stop): Likewise.
(target_trace_find): Likewise.
(target_get_trace_state_variable_value): Likewise.
(target_save_trace_data): Likewise.
(target_upload_tracepoints): Likewise.
(target_upload_trace_state_variables): Likewise.
(target_get_raw_trace_data): Likewise.
(target_get_min_fast_tracepoint_insn_len): Likewise.
(target_set_disconnected_tracing): Likewise.
(target_set_circular_trace_buffer): Likewise.
(target_set_trace_buffer_size): Likewise.
(target_set_trace_notes): Likewise.
(target_get_tib_address): Likewise.
(target_set_permissions): Likewise.
(target_static_tracepoint_marker_at): Likewise.
(target_static_tracepoint_markers_by_strid): Likewise.
(target_traceframe_info): Likewise.
(target_use_agent): Likewise.
(target_can_use_agent): Likewise.
(target_augmented_libraries_svr4_read): Likewise.
(target_log_command): Likewise.
* target.c (target_shortname): New.
(target_longname): New.
(target_attach_no_wait): New.
(target_post_attach): New.
(target_prepare_to_store): New.
(target_supports_enable_disable_tracepoint): New.
(target_supports_string_tracing): New.
(target_supports_evaluation_of_breakpoint_conditions): New.
(target_supports_dumpcore): New.
(target_dumpcore): New.
(target_can_run_breakpoint_commands): New.
(target_files_info): New.
(target_post_startup_inferior): New.
(target_insert_fork_catchpoint): New.
(target_remove_fork_catchpoint): New.
(target_insert_vfork_catchpoint): New.
(target_remove_vfork_catchpoint): New.
(target_insert_exec_catchpoint): New.
(target_remove_exec_catchpoint): New.
(target_set_syscall_catchpoint): New.
(target_rcmd): New.
(target_can_lock_scheduler): New.
(target_can_async_p): New.
(target_is_async_p): New.
(target_execution_direction): New.
(target_extra_thread_info): New.
(target_pid_to_exec_file): New.
(target_thread_architecture): New.
(target_find_memory_regions): New.
(target_make_corefile_notes): New.
(target_get_bookmark): New.
(target_goto_bookmark): New.
(target_stopped_by_watchpoint): New.
(target_stopped_by_sw_breakpoint): New.
(target_supports_stopped_by_sw_breakpoint): New.
(target_stopped_by_hw_breakpoint): New.
(target_supports_stopped_by_hw_breakpoint): New.
(target_have_steppable_watchpoint): New.
(target_can_use_hardware_watchpoint): New.
(target_region_ok_for_hw_watchpoint): New.
(target_can_do_single_step): New.
(target_insert_watchpoint): New.
(target_remove_watchpoint): New.
(target_insert_hw_breakpoint): New.
(target_remove_hw_breakpoint): New.
(target_can_accel_watchpoint_condition): New.
(target_can_execute_reverse): New.
(target_get_ada_task_ptid): New.
(target_filesystem_is_local): New.
(target_trace_init): New.
(target_download_tracepoint): New.
(target_can_download_tracepoint): New.
(target_download_trace_state_variable): New.
(target_enable_tracepoint): New.
(target_disable_tracepoint): New.
(target_trace_start): New.
(target_trace_set_readonly_regions): New.
(target_get_trace_status): New.
(target_get_tracepoint_status): New.
(target_trace_stop): New.
(target_trace_find): New.
(target_get_trace_state_variable_value): New.
(target_save_trace_data): New.
(target_upload_tracepoints): New.
(target_upload_trace_state_variables): New.
(target_get_raw_trace_data): New.
(target_get_min_fast_tracepoint_insn_len): New.
(target_set_disconnected_tracing): New.
(target_set_circular_trace_buffer): New.
(target_set_trace_buffer_size): New.
(target_set_trace_notes): New.
(target_get_tib_address): New.
(target_set_permissions): New.
(target_static_tracepoint_marker_at): New.
(target_static_tracepoint_markers_by_strid): New.
(target_traceframe_info): New.
(target_use_agent): New.
(target_can_use_agent): New.
(target_augmented_libraries_svr4_read): New.
(target_log_command): New.
* bfin-tdep.c (bfin_sw_breakpoint_from_kind): Adjust.
* infrun.c (set_schedlock_func): Adjust.
* mi/mi-main.c (exec_reverse_continue): Adjust.
* reverse.c (exec_reverse_once): Adjust.
* sh-tdep.c (sh_sw_breakpoint_from_kind): Adjust.
* tui/tui-stack.c (tui_locator_window::make_status_line): Adjust.
* remote-sim.c (gdbsim_target::detach): Adjust.
(gdbsim_target::files_info): Adjust.
Change-Id: I72ef56e9a25adeb0b91f1ad05e34c89f77ebeaa8
|
|
Same as the previous patch, but for the push_target functions.
The implementation of the move variant is moved to a new overload of
inferior::push_target.
gdb/ChangeLog:
* target.h (push_target): Remove, update callers to use
inferior::push_target.
* target.c (push_target): Remove.
* inferior.h (class inferior) <push_target>: New overload.
Change-Id: I5a95496666278b8f3965e5e8aecb76f54a97c185
|
|
If we reach the modified line, resume_target is necessarily nullptr,
because of the check at the beginning of the function. So we'll
necessarily iterate on all non-exited inferiors (across all targets),
which is what we want. So just remove the unnecessary argument.
gdb/ChangeLog:
* infrun.c (check_multi_target_resumption): Remove argument to
all_non_exited_inferiors.
Change-Id: If95704915dca19599d5f7f4732bbd6ccd20bf6b4
|
|
The `ready` flag of async event handlers is cleared by the async event
handler system right before invoking the associated callback, in
check_async_event_handlers.
This is not ideal with how the infrun subsystem consumes events: all
targets' async event handler callbacks essentially just invoke
`inferior_event_handler`, which eventually calls `fetch_inferior_event`
and `do_target_wait`. `do_target_wait` picks an inferior at random,
and thus a target at random (it could be the target whose `ready` flag
was cleared, or not), and pulls one event from it.
So it's possible that:
- the async event handler for a target A is called
- we end up consuming an event for target B
- all threads of target B are stopped, target_async(0) is called on it,
so its async event handler is cleared (e.g.
record_btrace_target::async)
As a result, target A still has events to report while its async event
handler is left unmarked, so these events are not consumed. To counter
this, at the end of their async event handler callbacks, targets check
if they still have something to report and re-mark their async event
handler (e.g. remote_async_inferior_event_handler).
The linux_nat target does not suffer from this because it doesn't use an
async event handler at the moment. It only uses a pipe registered with
the event loop. It is written to in the SIGCHLD handler (and in other
spots that want to get target wait method called) and read from in
the target's wait method. So if linux_nat happened to be target A in
the example above, the pipe would just stay readable, and the event loop
would wake up again, until linux_nat's wait method is finally called and
consumes the contents of the pipe.
I think it would be nicer if targets using async_event_handler worked in
a similar way, where the flag would stay set until the target's wait
method is actually called. As a first step towards that, this patch
moves the responsibility of clearing the ready flags of async event
handlers to the invoked callback.
All async event handler callbacks are modified to clear their ready flag
before doing anything else. So in practice, nothing changes with this
patch. It's only the responsibility of clearing the flag that is
shifted toward the callee.
gdb/ChangeLog:
* async-event.h (async_event_handler_func): Add documentation.
* async-event.c (check_async_event_handlers): Don't clear
async_event_handler ready flag.
* infrun.c (infrun_async_inferior_event_handler): Clear ready
flag.
* record-btrace.c (record_btrace_handle_async_inferior_event):
Likewise.
* record-full.c (record_full_async_inferior_event_handler):
Likewise.
* remote-notif.c (remote_async_get_pending_events_handler):
Likewise.
* remote.c (remote_async_inferior_event_handler): Likewise.
Change-Id: I179ef8e99580eae642d332846fd13664dbddc0c1
|
|
Moving it to an inner scope makes it clearer where it's used (only while
handling the TARGET_WAITKIND_LOADED event).
gdb/ChangeLog:
* infrun.c (handle_inferior_event): Move stop_soon variable to
inner scope.
Change-Id: Ic57685a21714cfbb38f1487ee96cea1d12b44652
|
|
A following patch will add a testcase that has a number of threads
constantly stepping over a breakpoint, and then has GDB detach the
process, while threads are running. If we have more than one inferior
running, and we detach from just one of the inferiors, we expect that
the remaining inferior continues running. However, in all-stop, if
GDB needs to pause the target for the detach, nothing is re-resuming
the other inferiors after the detach. "info threads" shows the
threads as running, but they really aren't. This fixes it.
gdb/ChangeLog:
* infcmd.c (detach_command): Hold strong reference to target, and
if all-stop on entry, restart threads on exit.
* infrun.c (switch_back_to_stepped_thread): Factor out bits to ...
(restart_stepped_thread): ... this new function. Also handle
trap_expected.
(restart_after_all_stop_detach): New function.
* infrun.h (restart_after_all_stop_detach): Declare.
|
|
A following patch will add a testcase that has a number of threads
constantly stepping over a breakpoint, and then has GDB detach the
process. That testcase exercises both "set displaced-stepping
on/off". Testing with "set displaced-stepping off" reveals that GDB
does not handle the case of the user typing "detach" just while some
thread is in the middle of an in-line step over. If that thread
belongs to the inferior that is being detached, then the step-over
never finishes, and threads of other inferiors are never re-resumed.
This fixes it.
gdb/ChangeLog:
* infrun.c (struct step_over_info): Initialize fields.
(prepare_for_detach): Handle ongoing in-line step over.
|
|
I noticed that "detach" while a program was running sometimes resulted
in the process crashing. I tracked it down to this change to
prepare_for_detach in commit 187b041e ("gdb: move displaced stepping
logic to gdbarch, allow starting concurrent displaced steps"):
/* Is any thread of this process displaced stepping? If not,
there's nothing else to do. */
- if (displaced->step_thread == nullptr)
+ if (displaced_step_in_progress (inf))
return;
The problem above is that the condition was inadvertently flipped. It
should have been:
if (!displaced_step_in_progress (inf))
So I fixed it, and wrote a testcase to exercise it. The testcase has
a number of threads constantly stepping over a breakpoint, and then
GDB detaches the process, while threads are running and stepping over
the breakpoint. And then I was surprised that my testcase would hang
-- GDB would get stuck in an infinite loop in prepare_for_detach,
here:
while (displaced_step_in_progress (inf))
{
...
What is going on is that since we now have two displaced stepping
buffers, as one displaced step finishes, GDB starts another, and
there's another one already in progress, and on and on, so the
displaced_step_in_progress condition never turns false. This happens
because we go via the whole handle_inferior_event, which tries to
start new step overs when one finishes. And also because while we
remove breakpoints from the target before prepare_for_detach is
called, handle_inferior_event ends up calling insert_breakpoints via
e.g. keep_going.
Thinking through all this, I came to the conclusion that going through
the whole handle_inferior_event isn't ideal. A _lot_ is done by that
function, e.g., some thread may get a signal which is passed to the
inferior, and gdb decides to try to get over the signal handler, which
reinstalls breakpoints. Or some process may exit. We can end up
reporting these events via normal_stop while detaching, maybe end up
running some breakpoint commands, or maybe even something runs an
inferior function call. Etc. All this after the user has already
declared they don't want to debug the process anymore, by asking to
detach.
I came to the conclusion that it's better to do the minimal amount of
work possible, in a more controlled fashion, without going through
handle_inferior_event. So in the new approach implemented by this
patch, if there are threads of the inferior that we're detaching in
the middle of a displaced step, stop them, and cancel the displaced
step. This is basically what stop_all_threads already does, via
wait_one and (the now factored out) handle_one, so I'm reusing those.
gdb/ChangeLog:
* infrun.c (struct wait_one_event): Move higher up.
(prepare_for_detach): Abort in-progress displaced steps instead of
letting them complete.
(handle_one): If the inferior is detaching, don't add the thread
back to the global step-over chain.
(restart_threads): Don't restart threads if detaching.
(handle_signal_stop): Remove inferior::detaching reference.
|
|
After detaching from a process, the inf->detaching flag is
inadvertently left set to true. If you afterwards reuse the same
inferior to start a new process, GDB will mishave...
The problem is that prepare_for_detach discards the scoped_restore at
the end, while the intention is for the flag to be set only for the
duration of prepare_for_detach.
This was already a bug in the original commit that added
prepare_for_detach, commit 24291992dac3 ("PR gdb/11321"), by yours
truly. Back then, we still used cleanups, and the function called
discard_cleanups instead of do_cleanups, by mistake.
gdb/ChangeLog:
* infrun.c (prepare_for_detach): Don't release scoped_restore
before returning.
|
|
This moves the code handling an event out of wait_one to a separate
function, to be used in another context in a following patch.
gdb/ChangeLog:
* infrun.c (handle_one): New function, factored out from ...
(stop_all_threads): ... here.
|
|
Attaching in non-stop mode currently misbehaves, like so:
(gdb) attach 1244450
Attaching to process 1244450
[New LWP 1244453]
[New LWP 1244454]
[New LWP 1244455]
[New LWP 1244456]
[New LWP 1244457]
[New LWP 1244458]
[New LWP 1244459]
[New LWP 1244461]
[New LWP 1244462]
[New LWP 1244463]
No unwaited-for children left.
At this point, GDB's stopped/running thread state is out of sync with
the inferior:
(gdb) info threads
Id Target Id Frame
* 1 LWP 1244450 "attach-non-stop" 0xf1b443bf in ?? ()
2 LWP 1244453 "attach-non-stop" (running)
3 LWP 1244454 "attach-non-stop" (running)
4 LWP 1244455 "attach-non-stop" (running)
5 LWP 1244456 "attach-non-stop" (running)
6 LWP 1244457 "attach-non-stop" (running)
7 LWP 1244458 "attach-non-stop" (running)
8 LWP 1244459 "attach-non-stop" (running)
9 LWP 1244461 "attach-non-stop" (running)
10 LWP 1244462 "attach-non-stop" (running)
11 LWP 1244463 "attach-non-stop" (running)
(gdb)
(gdb) interrupt -a
(gdb)
*nothing*
The problem is that attaching installs an inferior continuation,
called when the target reports the initial attach stop, here, in
inf-loop.c:inferior_event_handler:
/* Do all continuations associated with the whole inferior (not
a particular thread). */
if (inferior_ptid != null_ptid)
do_all_inferior_continuations (0);
However, currently in non-stop mode, inferior_ptid is still null_ptid
when we get here.
If you try to do "set debug infrun 1" to debug the problem, however,
then the attach completes correctly, with GDB reporting a stop for
each thread.
The bug is that we're missing a switch_to_thread/context_switch call
when handling the initial stop, here:
if (stop_soon == STOP_QUIETLY_NO_SIGSTOP
&& (ecs->event_thread->suspend.stop_signal == GDB_SIGNAL_STOP
|| ecs->event_thread->suspend.stop_signal == GDB_SIGNAL_TRAP
|| ecs->event_thread->suspend.stop_signal == GDB_SIGNAL_0))
{
stop_print_frame = true;
stop_waiting (ecs);
ecs->event_thread->suspend.stop_signal = GDB_SIGNAL_0;
return;
}
Note how the STOP_QUIETLY / STOP_QUIETLY_REMOTE case above that does
call context_switch.
And the reason "set debug infrun 1" "fixes" it, is that the debug path
has a switch_to_thread call.
This patch fixes it by moving the main context_switch call earlier.
It also removes the:
if (ecs->ptid != inferior_ptid)
check at the same time because:
#1 - that is half of what context_switch already does
#2 - deprecated_context_hook is only used in Insight, and all it does
is set an int. It won't care if we call it when the current
thread hasn't actually changed.
A testcase exercising this will be added in a following patch.
gdb/ChangeLog:
PR gdb/27055
* infrun.c (handle_signal_stop): Move main context_switch call
earlier, before STOP_QUIETLY_NO_SIGSTOP.
|
|
Consider the test-case small.c:
...
$ cat -n small.c
1 __attribute__ ((noinline, noclone))
2 int foo (char *c)
3 {
4 asm volatile ("" : : "r" (c) : "memory");
5 return 1;
6 }
7
8 int main ()
9 {
10 char tpl1[20] = "/tmp/test.XXX";
11 char tpl2[20] = "/tmp/test.XXX";
12 int fd1 = foo (tpl1);
13 int fd2 = foo (tpl2);
14 if (fd1 == -1) {
15 return 1;
16 }
17
18 return 0;
19 }
...
Compiled with gcc-8 and optimization:
...
$ gcc-8 -O2 -g small.c
...
We step through the calls to foo, but fail to visit line 13:
...
12 int fd1 = foo (tpl1);
(gdb) step
foo (c=c@entry=0x7fffffffdea0 "/tmp/test.XXX") at small.c:5
5 return 1;
(gdb) step
foo (c=c@entry=0x7fffffffdec0 "/tmp/test.XXX") at small.c:5
5 return 1;
(gdb) step
main () at small.c:14
14 if (fd1 == -1) {
(gdb)
...
This is caused by the following. The calls to foo are implemented by these
insns:
....
4003df: 0f 29 04 24 movaps %xmm0,(%rsp)
4003e3: 0f 29 44 24 20 movaps %xmm0,0x20(%rsp)
4003e8: e8 03 01 00 00 callq 4004f0 <foo>
4003ed: 48 8d 7c 24 20 lea 0x20(%rsp),%rdi
4003f2: 89 c2 mov %eax,%edx
4003f4: e8 f7 00 00 00 callq 4004f0 <foo>
4003f9: 31 c0 xor %eax,%eax
...
with corresponding line table entries:
...
INDEX LINE ADDRESS IS-STMT
8 12 0x00000000004003df Y
9 10 0x00000000004003df
10 11 0x00000000004003e3
11 12 0x00000000004003e8
12 13 0x00000000004003ed
13 12 0x00000000004003f2
14 13 0x00000000004003f4 Y
15 13 0x00000000004003f4
16 14 0x00000000004003f9 Y
17 14 0x00000000004003f9
...
Once we step out of the call to foo at 4003e8, we land at 4003ed, and gdb
enters process_event_stop_test to figure out what to do.
That entry has is-stmt=n, so it's not the start of a line, so we don't stop
there. However, we do update ecs->event_thread->current_line to line 13,
because the frame has changed (because we stepped out of the function).
Next we land at 4003f2. Again the entry has is-stmt=n, so it's not the start
of a line, so we don't stop there. However, because the frame hasn't changed,
we don't update update ecs->event_thread->current_line, so it stays 13.
Next we land at 4003f4. Now is-stmt=y, so it's the start of a line, and we'd
like to stop here.
But we don't stop because this test fails:
...
if ((ecs->event_thread->suspend.stop_pc == stop_pc_sal.pc)
&& (ecs->event_thread->current_line != stop_pc_sal.line
|| ecs->event_thread->current_symtab != stop_pc_sal.symtab))
{
...
because ecs->event_thread->current_line == 13 and stop_pc_sal.line == 13.
Fix this by resetting ecs->event_thread->current_line to 0 if is-stmt=n and
the frame has changed, such that we have:
...
12 int fd1 = foo (tpl1);
(gdb) step
foo (c=c@entry=0x7fffffffdbc0 "/tmp/test.XXX") at small.c:5
5 return 1;
(gdb) step
main () at small.c:13
13 int fd2 = foo (tpl2);
(gdb)
...
Tested on x86_64-linux, with gcc-7 and gcc-8.
gdb/ChangeLog:
2021-01-29 Tom de Vries <tdevries@suse.de>
PR breakpoints/26063
* infrun.c (process_event_stop_test): Reset
ecs->event_thread->current_line to 0 if is-stmt=n and frame has
changed.
gdb/testsuite/ChangeLog:
2021-01-29 Tom de Vries <tdevries@suse.de>
PR breakpoints/26063
* gdb.dwarf2/dw2-step-out-of-function-no-stmt.c: New test.
* gdb.dwarf2/dw2-step-out-of-function-no-stmt.exp: New file.
|
|
I'm trying to enable clang's -Wmissing-variable-declarations warning.
This patch fixes all the obvious spots where we can simply add "static"
(at least, found when building on x86-64 Linux).
gdb/ChangeLog:
* aarch64-linux-tdep.c (aarch64_linux_record_tdep): Make static.
* aarch64-tdep.c (tdesc_aarch64_list, aarch64_prologue_unwind,
aarch64_stub_unwind, aarch64_normal_base, ): Make static.
* arm-linux-tdep.c (arm_prologue_unwind): Make static.
* arm-tdep.c (struct frame_unwind): Make static.
* auto-load.c (auto_load_safe_path_vec): Make static.
* csky-tdep.c (csky_stub_unwind): Make static.
* gdbarch.c (gdbarch_data_registry): Make static.
* gnu-v2-abi.c (gnu_v2_abi_ops): Make static.
* i386-netbsd-tdep.c (i386nbsd_mc_reg_offset): Make static.
* i386-tdep.c (i386_frame_setup_skip_insns,
i386_tramp_chain_in_reg_insns, i386_tramp_chain_on_stack_insns):
Make static.
* infrun.c (observer_mode): Make static.
* linux-nat.c (sigchld_action): Make static.
* linux-thread-db.c (thread_db_list): Make static.
* maint-test-options.c (maintenance_test_options_list):
* mep-tdep.c (mep_csr_registers): Make static.
* mi/mi-cmds.c (struct mi_cmd_stats): Remove struct type name.
(stats): Make static.
* nat/linux-osdata.c (struct osdata_type): Make static.
* ppc-netbsd-tdep.c (ppcnbsd_reg_offsets): Make static.
* progspace.c (last_program_space_num): Make static.
* python/py-param.c (struct parm_constant): Remove struct type
name.
(parm_constants): Make static.
* python/py-record-btrace.c (btpy_list_methods): Make static.
* python/py-record.c (recpy_gap_type): Make static.
* record.c (record_goto_cmdlist): Make static.
* regcache.c (regcache_descr_handle): Make static.
* registry.h (DEFINE_REGISTRY): Make definition static.
* symmisc.c (std_in, std_out, std_err): Make static.
* top.c (previous_saved_command_line): Make static.
* tracepoint.c (trace_user, trace_notes, trace_stop_notes): Make
static.
* unittests/command-def-selftests.c (nr_duplicates,
nr_invalid_prefixcmd, lists): Make static.
* unittests/observable-selftests.c (test_notification): Make
static.
* unittests/optional/assignment/1.cc (counter): Make static.
* unittests/optional/assignment/2.cc (counter): Make static.
* unittests/optional/assignment/3.cc (counter): Make static.
* unittests/optional/assignment/4.cc (counter): Make static.
* unittests/optional/assignment/5.cc (counter): Make static.
* unittests/optional/assignment/6.cc (counter): Make static.
gdbserver/ChangeLog:
* ax.cc (bytecode_address_table): Make static.
* debug.cc (debug_file): Make static.
* linux-low.cc (stopping_threads): Make static.
(step_over_bkpt): Make static.
* linux-x86-low.cc (amd64_emit_ops, i386_emit_ops): Make static.
* tracepoint.cc (stop_tracing_bkpt, flush_trace_buffer_bkpt,
alloced_trace_state_variables, trace_buffer_ctrl,
tracing_start_time, tracing_stop_time, tracing_user_name,
tracing_notes, tracing_stop_note): Make static.
Change-Id: Ic1d8034723b7802502bda23770893be2338ab020
|
|
gdb/ChangeLog:
* infrun.c (normal_stop): Fix indentation.
Change-Id: Icbae5272188f6ddb464b585a9194abd611f5ad27
|
|
I spent a lot of time reading infrun debug logs recently, and I think
they could be made much more readable by being indented, to clearly see
what operation is done as part of what other operation. In the current
format, there are no visual cues to tell where things start and end,
it's just a big flat list. It's also difficult to understand what
caused a given operation (e.g. a call to resume_1) to be done.
To help with this, I propose to add the new scoped_debug_start_end
structure, along with a bunch of macros to make it convenient to use.
The idea of scoped_debug_start_end is simply to print a start and end
message at construction and destruction. It also increments/decrements
a depth counter in order to make debug statements printed during this
range use some indentation. Some care is taken to handle the fact that
debug can be turned on or off in the middle of such a range. For
example, a "set debug foo 1" command in a breakpoint command, or a
superior GDB manually changing the debug_foo variable.
Two macros are added in gdbsupport/common-debug.h, which are helpers to
define module-specific macros:
- scoped_debug_start_end: takes a message that is printed both at
construction / destruction, with "start: " and "end: " prefixes.
- scoped_debug_enter_exit: prints hard-coded "enter" and "exit"
messages, to denote the entry and exit of a function.
I added some examples in the infrun module to give an idea of how it can
be used and what the result looks like. The macros are in capital
letters (INFRUN_SCOPED_DEBUG_START_END and
INFRUN_SCOPED_DEBUG_ENTER_EXIT) to mimic the existing SCOPE_EXIT, but
that can be changed if you prefer something else.
Here's an excerpt of the debug
statements printed when doing "continue", where a displaced step is
started:
[infrun] proceed: enter
[infrun] proceed: addr=0xffffffffffffffff, signal=GDB_SIGNAL_DEFAULT
[infrun] global_thread_step_over_chain_enqueue: enqueueing thread Thread 0x7ffff75a5640 (LWP 2289301) in global step over chain
[infrun] start_step_over: enter
[infrun] start_step_over: stealing global queue of threads to step, length = 1
[infrun] start_step_over: resuming [Thread 0x7ffff75a5640 (LWP 2289301)] for step-over
[infrun] resume_1: step=1, signal=GDB_SIGNAL_0, trap_expected=1, current thread [Thread 0x7ffff75a5640 (LWP 2289301)] at 0x5555555551bd
[displaced] displaced_step_prepare_throw: displaced-stepping Thread 0x7ffff75a5640 (LWP 2289301) now
[displaced] prepare: selected buffer at 0x5555555550c2
[displaced] prepare: saved 0x5555555550c2: 1e fa 31 ed 49 89 d1 5e 48 89 e2 48 83 e4 f0 50
[displaced] amd64_displaced_step_copy_insn: copy 0x5555555551bd->0x5555555550c2: c7 45 fc 00 00 00 00 eb 13 8b 05 d4 2e 00 00 83
[displaced] displaced_step_prepare_throw: prepared successfully thread=Thread 0x7ffff75a5640 (LWP 2289301), original_pc=0x5555555551bd, displaced_pc=0x5555555550c2
[displaced] resume_1: run 0x5555555550c2: c7 45 fc 00
[infrun] infrun_async: enable=1
[infrun] prepare_to_wait: prepare_to_wait
[infrun] start_step_over: [Thread 0x7ffff75a5640 (LWP 2289301)] was resumed.
[infrun] operator(): step-over queue now empty
[infrun] start_step_over: exit
[infrun] proceed: start: resuming threads, all-stop-on-top-of-non-stop
[infrun] proceed: resuming Thread 0x7ffff7da7740 (LWP 2289296)
[infrun] resume_1: step=0, signal=GDB_SIGNAL_0, trap_expected=0, current thread [Thread 0x7ffff7da7740 (LWP 2289296)] at 0x7ffff7f7d9b7
[infrun] prepare_to_wait: prepare_to_wait
[infrun] proceed: resuming Thread 0x7ffff7da6640 (LWP 2289300)
[infrun] resume_1: thread Thread 0x7ffff7da6640 (LWP 2289300) has pending wait status status->kind = stopped, signal = GDB_SIGNAL_TRAP (currently_stepping=0).
[infrun] prepare_to_wait: prepare_to_wait
[infrun] proceed: [Thread 0x7ffff75a5640 (LWP 2289301)] resumed
[infrun] proceed: resuming Thread 0x7ffff6da4640 (LWP 2289302)
[infrun] resume_1: thread Thread 0x7ffff6da4640 (LWP 2289302) has pending wait status status->kind = stopped, signal = GDB_SIGNAL_TRAP (currently_stepping=0).
[infrun] prepare_to_wait: prepare_to_wait
[infrun] proceed: end: resuming threads, all-stop-on-top-of-non-stop
[infrun] proceed: exit
We can easily see where the call to `proceed` starts and end. We can
also see why there are a bunch of resume_1 calls, it's because we are
resuming threads, emulating all-stop on top of a non-stop target.
We also see that debug statements nest well with other modules that have
been migrated to use the "new" debug statement helpers (because they all
use debug_prefixed_vprintf in the end. I think this is desirable, for
example we could see the debug statements about reading the DWARF info
of a library nested under the debug statements about loading that
library.
Of course, modules that haven't been migrated to use the "new" helpers
will still print without indentations. This will be one good reason to
migrate them.
I think the runtime cost (when debug statements are disabled) of this is
reasonable, given the improvement in readability. There is the cost of
the conditionals (like standard debug statements), one more condition
(if (m_must_decrement_print_depth)) and the cost of constructing a stack
object, which means copying a fews pointers.
Adding the print in fetch_inferior_event breaks some tests that use "set
debug infrun", because it prints a debug statement after the prompt. I
adapted these tests to cope with it, by using the "-prompt" switch of
gdb_test_multiple to as if this debug statement is part of the expected
prompt. It's unfortunate that we have to do this, but I think the debug
print is useful, and I don't want a few tests to get in the way of
adding good debug output.
gdbsupport/ChangeLog:
* common-debug.h (debug_print_depth): New.
(struct scoped_debug_start_end): New.
(scoped_debug_start_end): New.
(scoped_debug_enter_exit): New.
* common-debug.cc (debug_prefixed_vprintf): Print indentation.
gdb/ChangeLog:
* debug.c (debug_print_depth): New.
* infrun.h (INFRUN_SCOPED_DEBUG_START_END): New.
(INFRUN_SCOPED_DEBUG_ENTER_EXIT): New.
* infrun.c (start_step_over): Use
INFRUN_SCOPED_DEBUG_ENTER_EXIT.
(proceed): Use INFRUN_SCOPED_DEBUG_ENTER_EXIT and
INFRUN_SCOPED_DEBUG_START_END.
(fetch_inferior_event): Use INFRUN_SCOPED_DEBUG_ENTER_EXIT.
gdbserver/ChangeLog:
* debug.cc (debug_print_depth): New.
gdb/testsuite/ChangeLog:
* gdb.base/ui-redirect.exp: Expect infrun debug print after
prompt.
* gdb.threads/ia64-sigill.exp: Likewise.
* gdb.threads/watchthreads-reorder.exp: Likewise.
Change-Id: I7c3805e6487807aa63a1bae318876a0c69dce949
|
|
The code in print_target_wait_results uses a single call to debug_printf
in order to make sure a single timestamp is emitted, despite printing
multiple lines. The result is:
941502.043284 [infrun] target_wait (-1.0.0, status) =
[infrun] 649832.649832.0 [process 649832],
[infrun] status->kind = stopped, signal = GDB_SIGNAL_TRAP
I find this decision a bit counter productive, because it messes up the
alignment of the three lines. We don't care that three (slightly
different) timestamps are printed.
I suggest to change this function to use infrun_debug_printf, with this
result:
941601.425771 [infrun] print_target_wait_results: target_wait (-1.0.0 [process -1], status) =
941601.425824 [infrun] print_target_wait_results: 651481.651481.0 [process 651481],
941601.425867 [infrun] print_target_wait_results: status->kind = stopped, signal = GDB_SIGNAL_TRAP
Note that the current code only prints the waiton_ptid as a string
between square brackets if pid != -1. I don't think this complexity is
needed in a debug print. I made it so it's always printed, which I
think results in a much simpler function.
gdb/ChangeLog:
* infrun.c (print_target_wait_results): Use infrun_debug_printf.
Change-Id: I817bd10286b8e641a6c751ac3a1bd1ddf9b18ce0
|
|
This commits the result of running gdb/copyright.py as per our Start
of New Year procedure...
gdb/ChangeLog
Update copyright year range in copyright header of all GDB files.
|
|
gdb/ChangeLog:
* infrun.h (debug_infrun): Make a bool.
* infrun.c (debug_infrun): Make a bool.
(_initialize_infrun): Use add_setshow_boolean_cmd to define "set
debug infrun".
Change-Id: If934106a6d3f879b93d265855eb705b1d606339a
|
|
This changes stop_context to use a thread_info_ref, removing some
manual reference counting.
gdb/ChangeLog
2020-12-11 Tom Tromey <tom@tromey.com>
* infrun.c (struct stop_context) <thread>: Now a thread_info_ref.
(stop_context::stop_context): Update.
(stop_context::~stop_context): Remove.
|
|
displaced steps
Today, GDB only allows a single displaced stepping operation to happen
per inferior at a time. There is a single displaced stepping buffer per
inferior, whose address is fixed (obtained with
gdbarch_displaced_step_location), managed by infrun.c.
In the case of the AMD ROCm target [1] (in the context of which this
work has been done), it is typical to have thousands of threads (or
waves, in SMT terminology) executing the same code, hitting the same
breakpoint (possibly conditional) and needing to to displaced step it at
the same time. The limitation of only one displaced step executing at a
any given time becomes a real bottleneck.
To fix this bottleneck, we want to make it possible for threads of a
same inferior to execute multiple displaced steps in parallel. This
patch builds the foundation for that.
In essence, this patch moves the task of preparing a displaced step and
cleaning up after to gdbarch functions. This allows using different
schemes for allocating and managing displaced stepping buffers for
different platforms. The gdbarch decides how to assign a buffer to a
thread that needs to execute a displaced step.
On the ROCm target, we are able to allocate one displaced stepping
buffer per thread, so a thread will never have to wait to execute a
displaced step.
On Linux, the entry point of the executable if used as the displaced
stepping buffer, since we assume that this code won't get used after
startup. From what I saw (I checked with a binary generated against
glibc and musl), on AMD64 we have enough space there to fit two
displaced stepping buffers. A subsequent patch makes AMD64/Linux use
two buffers.
In addition to having multiple displaced stepping buffers, there is also
the idea of sharing displaced stepping buffers between threads. Two
threads doing displaced steps for the same PC could use the same buffer
at the same time. Two threads stepping over the same instruction (same
opcode) at two different PCs may also be able to share a displaced
stepping buffer. This is an idea for future patches, but the
architecture built by this patch is made to allow this.
Now, the implementation details. The main part of this patch is moving
the responsibility of preparing and finishing a displaced step to the
gdbarch. Before this patch, preparing a displaced step is driven by the
displaced_step_prepare_throw function. It does some calls to the
gdbarch to do some low-level operations, but the high-level logic is
there. The steps are roughly:
- Ask the gdbarch for the displaced step buffer location
- Save the existing bytes in the displaced step buffer
- Ask the gdbarch to copy the instruction into the displaced step buffer
- Set the pc of the thread to the beginning of the displaced step buffer
Similarly, the "fixup" phase, executed after the instruction was
successfully single-stepped, is driven by the infrun code (function
displaced_step_finish). The steps are roughly:
- Restore the original bytes in the displaced stepping buffer
- Ask the gdbarch to fixup the instruction result (adjust the target's
registers or memory to do as if the instruction had been executed in
its original location)
The displaced_step_inferior_state::step_thread field indicates which
thread (if any) is currently using the displaced stepping buffer, so it
is used by displaced_step_prepare_throw to check if the displaced
stepping buffer is free to use or not.
This patch defers the whole task of preparing and cleaning up after a
displaced step to the gdbarch. Two new main gdbarch methods are added,
with the following semantics:
- gdbarch_displaced_step_prepare: Prepare for the given thread to
execute a displaced step of the instruction located at its current PC.
Upon return, everything should be ready for GDB to resume the thread
(with either a single step or continue, as indicated by
gdbarch_displaced_step_hw_singlestep) to make it displaced step the
instruction.
- gdbarch_displaced_step_finish: Called when the thread stopped after
having started a displaced step. Verify if the instruction was
executed, if so apply any fixup required to compensate for the fact
that the instruction was executed at a different place than its
original pc. Release any resources that were allocated for this
displaced step. Upon return, everything should be ready for GDB to
resume the thread in its "normal" code path.
The displaced_step_prepare_throw function now pretty much just offloads
to gdbarch_displaced_step_prepare and the displaced_step_finish function
offloads to gdbarch_displaced_step_finish.
The gdbarch_displaced_step_location method is now unnecessary, so is
removed. Indeed, the core of GDB doesn't know how many displaced step
buffers there are nor where they are.
To keep the existing behavior for existing architectures, the logic that
was previously implemented in infrun.c for preparing and finishing a
displaced step is moved to displaced-stepping.c, to the
displaced_step_buffer class. Architectures are modified to implement
the new gdbarch methods using this class. The behavior is not expected
to change.
The other important change (which arises from the above) is that the
core of GDB no longer prevents concurrent displaced steps. Before this
patch, start_step_over walks the global step over chain and tries to
initiate a step over (whether it is in-line or displaced). It follows
these rules:
- if an in-line step is in progress (in any inferior), don't start any
other step over
- if a displaced step is in progress for an inferior, don't start
another displaced step for that inferior
After starting a displaced step for a given inferior, it won't start
another displaced step for that inferior.
In the new code, start_step_over simply tries to initiate step overs for
all the threads in the list. But because threads may be added back to
the global list as it iterates the global list, trying to initiate step
overs, start_step_over now starts by stealing the global queue into a
local queue and iterates on the local queue. In the typical case, each
thread will either:
- have initiated a displaced step and be resumed
- have been added back by the global step over queue by
displaced_step_prepare_throw, because the gdbarch will have returned
that there aren't enough resources (i.e. buffers) to initiate a
displaced step for that thread
Lastly, if start_step_over initiates an in-line step, it stops
iterating, and moves back whatever remaining threads it had in its local
step over queue to the global step over queue.
Two other gdbarch methods are added, to handle some slightly annoying
corner cases. They feel awkwardly specific to these cases, but I don't
see any way around them:
- gdbarch_displaced_step_copy_insn_closure_by_addr: in
arm_pc_is_thumb, arm-tdep.c wants to get the closure for a given
buffer address.
- gdbarch_displaced_step_restore_all_in_ptid: when a process forks
(at least on Linux), the address space is copied. If some displaced
step buffers were in use at the time of the fork, we need to restore
the original bytes in the child's address space.
These two adjustments are also made in infrun.c:
- prepare_for_detach: there may be multiple threads doing displaced
steps when we detach, so wait until all of them are done
- handle_inferior_event: when we handle a fork event for a given
thread, it's possible that other threads are doing a displaced step at
the same time. Make sure to restore the displaced step buffer
contents in the child for them.
[1] https://github.com/ROCm-Developer-Tools/ROCgdb
gdb/ChangeLog:
* displaced-stepping.h (struct
displaced_step_copy_insn_closure): Adjust comments.
(struct displaced_step_inferior_state) <step_thread,
step_gdbarch, step_closure, step_original, step_copy,
step_saved_copy>: Remove fields.
(struct displaced_step_thread_state): New.
(struct displaced_step_buffer): New.
* displaced-stepping.c (displaced_step_buffer::prepare): New.
(write_memory_ptid): Move from infrun.c.
(displaced_step_instruction_executed_successfully): New,
factored out of displaced_step_finish.
(displaced_step_buffer::finish): New.
(displaced_step_buffer::copy_insn_closure_by_addr): New.
(displaced_step_buffer::restore_in_ptid): New.
* gdbarch.sh (displaced_step_location): Remove.
(displaced_step_prepare, displaced_step_finish,
displaced_step_copy_insn_closure_by_addr,
displaced_step_restore_all_in_ptid): New.
* gdbarch.c: Re-generate.
* gdbarch.h: Re-generate.
* gdbthread.h (class thread_info) <displaced_step_state>: New
field.
(thread_step_over_chain_remove): New declaration.
(thread_step_over_chain_next): New declaration.
(thread_step_over_chain_length): New declaration.
* thread.c (thread_step_over_chain_remove): Make non-static.
(thread_step_over_chain_next): New.
(global_thread_step_over_chain_next): Use
thread_step_over_chain_next.
(thread_step_over_chain_length): New.
(global_thread_step_over_chain_enqueue): Add debug print.
(global_thread_step_over_chain_remove): Add debug print.
* infrun.h (get_displaced_step_copy_insn_closure_by_addr):
Remove.
* infrun.c (get_displaced_stepping_state): New.
(displaced_step_in_progress_any_inferior): Remove.
(displaced_step_in_progress_thread): Adjust.
(displaced_step_in_progress): Adjust.
(displaced_step_in_progress_any_thread): New.
(get_displaced_step_copy_insn_closure_by_addr): Remove.
(gdbarch_supports_displaced_stepping): Use
gdbarch_displaced_step_prepare_p.
(displaced_step_reset): Change parameter from inferior to
thread.
(displaced_step_prepare_throw): Implement using
gdbarch_displaced_step_prepare.
(write_memory_ptid): Move to displaced-step.c.
(displaced_step_restore): Remove.
(displaced_step_finish): Implement using
gdbarch_displaced_step_finish.
(start_step_over): Allow starting more than one displaced step.
(prepare_for_detach): Handle possibly multiple threads doing
displaced steps.
(handle_inferior_event): Handle possibility that fork event
happens while another thread displaced steps.
* linux-tdep.h (linux_displaced_step_prepare): New.
(linux_displaced_step_finish): New.
(linux_displaced_step_copy_insn_closure_by_addr): New.
(linux_displaced_step_restore_all_in_ptid): New.
(linux_init_abi): Add supports_displaced_step parameter.
* linux-tdep.c (struct linux_info) <disp_step_buf>: New field.
(linux_displaced_step_prepare): New.
(linux_displaced_step_finish): New.
(linux_displaced_step_copy_insn_closure_by_addr): New.
(linux_displaced_step_restore_all_in_ptid): New.
(linux_init_abi): Add supports_displaced_step parameter,
register displaced step methods if true.
(_initialize_linux_tdep): Register inferior_execd observer.
* amd64-linux-tdep.c (amd64_linux_init_abi_common): Add
supports_displaced_step parameter, adjust call to
linux_init_abi. Remove call to
set_gdbarch_displaced_step_location.
(amd64_linux_init_abi): Adjust call to
amd64_linux_init_abi_common.
(amd64_x32_linux_init_abi): Likewise.
* aarch64-linux-tdep.c (aarch64_linux_init_abi): Adjust call to
linux_init_abi. Remove call to
set_gdbarch_displaced_step_location.
* arm-linux-tdep.c (arm_linux_init_abi): Likewise.
* i386-linux-tdep.c (i386_linux_init_abi): Likewise.
* alpha-linux-tdep.c (alpha_linux_init_abi): Adjust call to
linux_init_abi.
* arc-linux-tdep.c (arc_linux_init_osabi): Likewise.
* bfin-linux-tdep.c (bfin_linux_init_abi): Likewise.
* cris-linux-tdep.c (cris_linux_init_abi): Likewise.
* csky-linux-tdep.c (csky_linux_init_abi): Likewise.
* frv-linux-tdep.c (frv_linux_init_abi): Likewise.
* hppa-linux-tdep.c (hppa_linux_init_abi): Likewise.
* ia64-linux-tdep.c (ia64_linux_init_abi): Likewise.
* m32r-linux-tdep.c (m32r_linux_init_abi): Likewise.
* m68k-linux-tdep.c (m68k_linux_init_abi): Likewise.
* microblaze-linux-tdep.c (microblaze_linux_init_abi): Likewise.
* mips-linux-tdep.c (mips_linux_init_abi): Likewise.
* mn10300-linux-tdep.c (am33_linux_init_osabi): Likewise.
* nios2-linux-tdep.c (nios2_linux_init_abi): Likewise.
* or1k-linux-tdep.c (or1k_linux_init_abi): Likewise.
* riscv-linux-tdep.c (riscv_linux_init_abi): Likewise.
* s390-linux-tdep.c (s390_linux_init_abi_any): Likewise.
* sh-linux-tdep.c (sh_linux_init_abi): Likewise.
* sparc-linux-tdep.c (sparc32_linux_init_abi): Likewise.
* sparc64-linux-tdep.c (sparc64_linux_init_abi): Likewise.
* tic6x-linux-tdep.c (tic6x_uclinux_init_abi): Likewise.
* tilegx-linux-tdep.c (tilegx_linux_init_abi): Likewise.
* xtensa-linux-tdep.c (xtensa_linux_init_abi): Likewise.
* ppc-linux-tdep.c (ppc_linux_init_abi): Adjust call to
linux_init_abi. Remove call to
set_gdbarch_displaced_step_location.
* arm-tdep.c (arm_pc_is_thumb): Call
gdbarch_displaced_step_copy_insn_closure_by_addr instead of
get_displaced_step_copy_insn_closure_by_addr.
* rs6000-aix-tdep.c (rs6000_aix_init_osabi): Adjust calls to
clear gdbarch methods.
* rs6000-tdep.c (struct ppc_inferior_data): New structure.
(get_ppc_per_inferior): New function.
(ppc_displaced_step_prepare): New function.
(ppc_displaced_step_finish): New function.
(ppc_displaced_step_restore_all_in_ptid): New function.
(rs6000_gdbarch_init): Register new gdbarch methods.
* s390-tdep.c (s390_gdbarch_init): Don't call
set_gdbarch_displaced_step_location, set new gdbarch methods.
gdb/testsuite/ChangeLog:
* gdb.arch/amd64-disp-step-avx.exp: Adjust pattern.
* gdb.threads/forking-threads-plus-breakpoint.exp: Likewise.
* gdb.threads/non-stop-fair-events.exp: Likewise.
Change-Id: I387cd235a442d0620ec43608fd3dc0097fcbf8c8
|
|
Move displaced-stepping related stuff unchanged to displaced-stepping.h
and displaced-stepping.c. This helps make the following patch a bit
smaller and easier to read.
gdb/ChangeLog:
* Makefile.in (COMMON_SFILES): Add displaced-stepping.c.
* aarch64-tdep.h: Include displaced-stepping.h.
* displaced-stepping.h (struct displaced_step_copy_insn_closure):
Move here.
(displaced_step_copy_insn_closure_up): Move here.
(struct buf_displaced_step_copy_insn_closure): Move here.
(struct displaced_step_inferior_state): Move here.
(debug_displaced): Move here.
(displaced_debug_printf_1): Move here.
(displaced_debug_printf): Move here.
* displaced-stepping.c: New file.
* gdbarch.sh: Include displaced-stepping.h in gdbarch.h.
* gdbarch.h: Re-generate.
* inferior.h: Include displaced-stepping.h.
* infrun.h (debug_displaced): Move to displaced-stepping.h.
(displaced_debug_printf_1): Likewise.
(displaced_debug_printf): Likewise.
(struct displaced_step_copy_insn_closure): Likewise.
(displaced_step_copy_insn_closure_up): Likewise.
(struct buf_displaced_step_copy_insn_closure): Likewise.
(struct displaced_step_inferior_state): Likewise.
* infrun.c (show_debug_displaced): Move to displaced-stepping.c.
(displaced_debug_printf_1): Likewise.
(displaced_step_copy_insn_closure::~displaced_step_copy_insn_closure):
Likewise.
(_initialize_infrun): Don't register "set/show debug displaced".
Change-Id: I29935f5959b80425370630a45148fc06cd4227ca
|
|
This is a preparatory patch to reduce the size of the diff of the
upcoming main patch. It introduces enum types for the return values of
displaced step "prepare" and "finish" operations. I find that this
expresses better the intention of the code, rather than returning
arbitrary integer values (-1, 0 and 1) which are difficult to remember.
That makes the code easier to read.
I put the new enum types in a new displaced-stepping.h file, because I
introduce that file in a later patch anyway. Putting it there avoids
having to move it later.
There is one change in behavior for displaced_step_finish: it currently
returns 0 if the thread wasn't doing a displaced step and 1 if the
thread was doing a displaced step which was executed successfully. It
turns out that this distinction is not needed by any caller, so I've
merged these two cases into "_OK", rather than adding an extra
enumerator.
gdb/ChangeLog:
* infrun.c (displaced_step_prepare_throw): Change return type to
displaced_step_prepare_status.
(displaced_step_prepare): Likewise.
(displaced_step_finish): Change return type to
displaced_step_finish_status.
(resume_1): Adjust.
(stop_all_threads): Adjust.
* displaced-stepping.h: New file.
Change-Id: I5c8fe07212cd398d5b486b5936d9d0807acd3788
|
|
This is a preparatory patch to reduce a little bit the diff size of the
main patch later in this series. It renames the displaced_step_fixup
function in infrun.c to displaced_step_finish.
The rationale is to better differentiate the low and high level
operations.
We first have the low level operation of writing an instruction to a
displaced buffer, called "copy_insn". The mirror low level operation to
fix up the state after having executed the instruction is "fixup". The
high level operation of preparing a thread for a displaced step (which
includes doing the "copy_insn" and some more bookkeeping) is called
"prepare" (as in displaced_step_prepare). The mirror high level
operation to cleaning up after a displaced step (which includes doing
the "fixup" and some more bookkeeping) is currently also called "fixup"
(as in displaced_step_fixup), just like the low level operation.
I think that choosing a different name for the low and high level
cleanup operation makes it clearer, hence "finish".
gdb/ChangeLog:
* infrun.c (displaced_step_fixup): Rename to...
(displaced_step_finish): ... this, update all callers.
Change-Id: Id32f48c1e2091d09854c77fcedcc14d2519957a2
|
|
Since we're going to introduce other "displaced step" functions and
another kind of displaced step closure, make it clear that this is the
return type of the gdbarch_displaced_step_copy_insn function.
gdb/ChangeLog:
* infrun.h (get_displaced_step_closure_by_addr): Rename to...
(get_displaced_step_copy_insn_closure_by_addr): ... this.
Update all users.
(displaced_step_closure): Rename to...
(displaced_step_copy_insn_closure): ... this. Update all users.
(displaced_step_closure_up): Rename to...
(displaced_step_copy_insn_closure_up). ... this. Update all
users.
(buf_displaced_step_closure): Rename to...
(buf_displaced_step_copy_insn_closure): ... this. Update all
users.
* infrun.c (get_displaced_step_closure_by_addr): Rename to...
(get_displaced_step_copy_insn_closure_by_addr): ... this.
Update all users.
* aarch64-tdep.c (aarch64_displaced_step_closure): Rename to...
(aarch64_displaced_step_copy_insn_closure): ... this. Update
all users.
* amd64-tdep.c (amd64_displaced_step_closure): Rename to...
(amd64_displaced_step_copy_insn_closure): ... this. Update all
users.
* arm-tdep.h (arm_displaced_step_closure): Rename to...
(arm_displaced_step_copy_insn_closure): ... this. Update all
users.
* i386-tdep.h (i386_displaced_step_closure): Rename to...
(i386_displaced_step_copy_insn_closure): ... this. Update all
users.
* rs6000-tdep.c (ppc_displaced_step_closure): Rename to...
(ppc_displaced_step_copy_insn_closure): ... this. Update all
users.
* s390-tdep.c (s390_displaced_step_closure): Rename to...
(s390_displaced_step_copy_insn_closure): ... this. Update all
users.
* gdbarch.h: Re-generate.
* gdbarch.c: Re-generate.
Change-Id: I11f56dbcd4c3532fb195a08ba93bccf1d12a03c8
|
|
Rename step_over_queue_head to global_thread_step_over_chain_head, to
make it more obvious when reading code that we are touching the global
queue. Rename all functions that operate on it to have "global" in
their name, to make it clear on which chain they operate on. Also, in a
subsequent patch, we'll need both global and non-global versions of
these functions, so it will be easier to do the distinction if they are
named properly.
Normalize the naming to use "chain" everywhere instead of sometimes
"queue", sometimes "chain".
I also reworded a few comments in gdbthread.h. They implied that the
step over chain is per-inferior, when in reality there is only one
global chain, not one per inferior, as far as I understand.
gdb/ChangeLog:
* gdbthread.h (thread_step_over_chain_enqueue): Rename to...
(global_thread_step_over_chain_enqueue): ... this. Update all
users.
(thread_step_over_chain_remove): Rename to...
(global_thread_step_over_chain_remove): ... this. Update all
users.
(thread_step_over_chain_next): Rename to...
(global_thread_step_over_chain_next): ... this. Update all
users.
* infrun.h (step_over_queue_head): Rename to...
(global_thread_step_over_chain_head): ... this. Update all
users.
* infrun.c (step_over_queue_head): Rename to...
(global_thread_step_over_chain_head): ... this. Update all
users.
* thread.c (step_over_chain_remove): Rename to...
(thread_step_over_chain_remove): ... this. Update all users.
(thread_step_over_chain_next): Rename to...
(global_thread_step_over_chain_next): ... this. Update all
users.
(thread_step_over_chain_enqueue): Rename to...
(global_thread_step_over_chain_enqueue): ... this. Update all
users.
(thread_step_over_chain_remove): Rename to...
(global_thread_step_over_chain_remove): ... this. Update all
users.
Change-Id: Iabbf57d83c01321ca199d83fadb57f5b04e4d6d9
|
|
Remove function get_displaced_stepping_state. When it was introduced,
inferiors' displaced stepping state was kept in a linked list in
infrun.c, so it was handy. Nowadays, the state is kept inside struct
inferior directly, so we can just access it directly instead.
gdb/ChangeLog:
* infrun.c (get_displaced_stepping_state): Remove, change
callers to access the field directly.
Change-Id: I9a733e32e29c7ebf856ab0befe1076bbb8c7af69
|
|
In handle_inferior_event, where we handle forks, we make sure to restore
the bytes of the displaced stepping buffer in the child's address
space. However, we only do it when the forking thread was the one
doing a displaced step. It could happen that a thread forks while
another one is doing a displaced step. In this case, we also need to
restore the bytes in the child.
Move the byte-restoring code outside of the condition that checks
whether the event thread was displaced stepping.
gdb/ChangeLog:
* infrun.c (handle_inferior_event): Restore displaced step
buffer bytes in child process when handling fork, even if fork
happened in another thread than the displaced-stepping one.
Change-Id: Ibb0daaeb123aba03f4fb4b4d820754eb2436bc69
|
|
When a process does an exec, all its program space is replaced with the
newly loaded executable. All non-main threads disappear and the main
thread starts executing at the entry point of the new executable.
Things can go wrong if a displaced step operation is in progress while
we process the exec event.
If the main thread is the one executing the displaced step: when that
thread (now executing in the new executable) stops somewhere (say, at a
breakpoint), displaced_step_fixup will run and clear up the state. We
will execute the "fixup" phase for the instruction we single-stepped in
the old program space. We are now in a completely different context,
so doing the fixup may corrupt the state.
If it is a non-main thread that is doing the displaced step: while
handling the exec event, GDB deletes the thread_info representing that
thread (since the thread doesn't exist in the inferior after the exec).
But inferior::displaced_step_state::step_thread will still point to it.
When handling events later, this condition, in displaced_step_fixup,
will likely never be true:
/* Was this event for the thread we displaced? */
if (displaced->step_thread != event_thread)
return 0;
... since displaced->step_thread points to a deleted thread (unless that
storage gets re-used for a new thread_info, but that wouldn't be good
either). This effectively makes the displaced stepping buffer occupied
for ever. When a thread in the new program space will want to do a
displaced step, it will wait for ever.
I think we simply need to reset the displaced stepping state of the
inferior on exec. Everything execution-related that existed before the
exec is now gone.
Similarly, if a thread does an in-line step over an exec syscall
instruction, nothing clears the in-line step over info when the event is
handled. So it the in-line step over info stays there indefinitely, and
things hang because we can never start another step over. To fix this,
I added a call to clear_step_over_info in infrun_inferior_execd.
Add a test with a program with two threads that does an exec. The test
includes the following axes:
- whether it's the leader thread or the other thread that does the exec.
- whether the exec'r and exec'd program have different text segment
addresses. This is to hopefully catch cases where the displaced
stepping info doesn't get reset, and GDB later tries to restore bytes
of the old address space in the new address space. If the mapped
addresses are different, we should get some memory error. This
happens without the patch applied:
$ ./gdb -q -nx --data-directory=data-directory testsuite/outputs/gdb.threads/step-over-exec/step-over-exec-execr-thread-leader-diff-text-segs-true -ex "b main" -ex r -ex "b my_execve_syscall if 0" -ex "set displaced-stepping on"
...
Breakpoint 1, main (argc=1, argv=0x7fffffffde38) at /home/simark/src/binutils-gdb/gdb/testsuite/gdb.threads/step-over-exec.c:69
69 argv0 = argv[0];
Breakpoint 2 at 0x60133a: file /home/simark/src/binutils-gdb/gdb/testsuite/lib/my-syscalls.S, line 34.
(gdb) c
Continuing.
[New Thread 0x7ffff7c62640 (LWP 1455423)]
Leader going in exec.
Exec-ing /home/simark/build/binutils-gdb/gdb/testsuite/outputs/gdb.threads/step-over-exec/step-over-exec-execr-thread-leader-diff-text-segs-true-execd
[Thread 0x7ffff7c62640 (LWP 1455423) exited]
process 1455418 is executing new program: /home/simark/build/binutils-gdb/gdb/testsuite/outputs/gdb.threads/step-over-exec/step-over-exec-execr-thread-leader-diff-text-segs-true-execd
Error in re-setting breakpoint 2: Function "my_execve_syscall" not defined.
No unwaited-for children left.
(gdb) n
Single stepping until exit from function _start,
which has no line number information.
Cannot access memory at address 0x6010d2
(gdb)
- Whether displaced stepping is allowed or not, so that we end up
testing both displaced stepping and in-line stepping on arches that do
support displaced stepping (otherwise, it just tests in-line stepping
twice I suppose)
To be able to precisely put a breakpoint on the syscall instruction, I
added a small assembly file (lib/my-syscalls.S) that contains minimal
Linux syscall wrappers. I prefer that to the strategy used in
gdb.base/step-over-syscall.exp, which is to stepi into the glibc wrapper
until we find something that looks like a syscall instruction, I find
that more predictable.
gdb/ChangeLog:
* infrun.c (infrun_inferior_execd): New function.
(_initialize_infrun): Attach inferior_execd observer.
gdb/testsuite/ChangeLog:
* gdb.threads/step-over-exec.exp: New.
* gdb.threads/step-over-exec.c: New.
* gdb.threads/step-over-exec-execd.c: New.
* lib/my-syscalls.S: New.
* lib/my-syscalls.h: New.
Change-Id: I1bbc8538e683f53af5b980091849086f4fec5ff9
|
|
I want to add another action (clearing displaced stepping state) that
happens when an inferior execs. I think it would be cleaner to have an
observer for this event, rather than have infrun know about each other
sub-component.
Replace the calls to solib_create_inferior_hook and
jit_inferior_created_hook in follow_exec by observers.
gdb/ChangeLog:
* observable.h (inferior_execd): Declare new observable.
* observable.c (inferior_execd): Declare new observable.
* infrun.c (follow_exec): Notify inferior_execd observer.
* jit.c (jit_inferior_created_hook): Make static.
(_initialize_jit): Register inferior_execd observer.
* jit.h (jit_inferior_created_hook): Remove declaration.
* solib.c (_initialize_solib): Register inferior_execd observer.
Change-Id: I000cce00094e23baa67df693d912646b6ae38e44
|
|
A while back I noticed that fetch_inferior_event used "int" for
should_stop, whereas it can be bool. The method it is assigned from:
should_stop = thread_fsm->should_stop (thr);
... already returns bool.
Tested by rebuilding.
gdb/ChangeLog
2020-11-14 Tom Tromey <tom@tromey.com>
* infrun.c (fetch_inferior_event): Use "bool" for should_stop.
|
|
Many spots incorrectly use only spaces for indentation (for example,
there are a lot of spots in ada-lang.c). I've always found it awkward
when I needed to edit one of these spots: do I keep the original wrong
indentation, or do I fix it? What if the lines around it are also
wrong, do I fix them too? I probably don't want to fix them in the same
patch, to avoid adding noise to my patch.
So I propose to fix as much as possible once and for all (hopefully).
One typical counter argument for this is that it makes code archeology
more difficult, because git-blame will show this commit as the last
change for these lines. My counter counter argument is: when
git-blaming, you often need to do "blame the file at the parent commit"
anyway, to go past some other refactor that touched the line you are
interested in, but is not the change you are looking for. So you
already need a somewhat efficient way to do this.
Using some interactive tool, rather than plain git-blame, makes this
trivial. For example, I use "tig blame <file>", where going back past
the commit that changed the currently selected line is one keystroke.
It looks like Magit in Emacs does it too (though I've never used it).
Web viewers of Github and Gitlab do it too. My point is that it won't
really make archeology more difficult.
The other typical counter argument is that it will cause conflicts with
existing patches. That's true... but it's a one time cost, and those
are not conflicts that are difficult to resolve. I have also tried "git
rebase --ignore-whitespace", it seems to work well. Although that will
re-introduce the faulty indentation, so one needs to take care of fixing
the indentation in the patch after that (which is easy).
gdb/ChangeLog:
* aarch64-linux-tdep.c: Fix indentation.
* aarch64-ravenscar-thread.c: Fix indentation.
* aarch64-tdep.c: Fix indentation.
* aarch64-tdep.h: Fix indentation.
* ada-lang.c: Fix indentation.
* ada-lang.h: Fix indentation.
* ada-tasks.c: Fix indentation.
* ada-typeprint.c: Fix indentation.
* ada-valprint.c: Fix indentation.
* ada-varobj.c: Fix indentation.
* addrmap.c: Fix indentation.
* addrmap.h: Fix indentation.
* agent.c: Fix indentation.
* aix-thread.c: Fix indentation.
* alpha-bsd-nat.c: Fix indentation.
* alpha-linux-tdep.c: Fix indentation.
* alpha-mdebug-tdep.c: Fix indentation.
* alpha-nbsd-tdep.c: Fix indentation.
* alpha-obsd-tdep.c: Fix indentation.
* alpha-tdep.c: Fix indentation.
* amd64-bsd-nat.c: Fix indentation.
* amd64-darwin-tdep.c: Fix indentation.
* amd64-linux-nat.c: Fix indentation.
* amd64-linux-tdep.c: Fix indentation.
* amd64-nat.c: Fix indentation.
* amd64-obsd-tdep.c: Fix indentation.
* amd64-tdep.c: Fix indentation.
* amd64-windows-tdep.c: Fix indentation.
* annotate.c: Fix indentation.
* arc-tdep.c: Fix indentation.
* arch-utils.c: Fix indentation.
* arch/arm-get-next-pcs.c: Fix indentation.
* arch/arm.c: Fix indentation.
* arm-linux-nat.c: Fix indentation.
* arm-linux-tdep.c: Fix indentation.
* arm-nbsd-tdep.c: Fix indentation.
* arm-pikeos-tdep.c: Fix indentation.
* arm-tdep.c: Fix indentation.
* arm-tdep.h: Fix indentation.
* arm-wince-tdep.c: Fix indentation.
* auto-load.c: Fix indentation.
* auxv.c: Fix indentation.
* avr-tdep.c: Fix indentation.
* ax-gdb.c: Fix indentation.
* ax-general.c: Fix indentation.
* bfin-linux-tdep.c: Fix indentation.
* block.c: Fix indentation.
* block.h: Fix indentation.
* blockframe.c: Fix indentation.
* bpf-tdep.c: Fix indentation.
* break-catch-sig.c: Fix indentation.
* break-catch-syscall.c: Fix indentation.
* break-catch-throw.c: Fix indentation.
* breakpoint.c: Fix indentation.
* breakpoint.h: Fix indentation.
* bsd-uthread.c: Fix indentation.
* btrace.c: Fix indentation.
* build-id.c: Fix indentation.
* buildsym-legacy.h: Fix indentation.
* buildsym.c: Fix indentation.
* c-typeprint.c: Fix indentation.
* c-valprint.c: Fix indentation.
* c-varobj.c: Fix indentation.
* charset.c: Fix indentation.
* cli/cli-cmds.c: Fix indentation.
* cli/cli-decode.c: Fix indentation.
* cli/cli-decode.h: Fix indentation.
* cli/cli-script.c: Fix indentation.
* cli/cli-setshow.c: Fix indentation.
* coff-pe-read.c: Fix indentation.
* coffread.c: Fix indentation.
* compile/compile-cplus-types.c: Fix indentation.
* compile/compile-object-load.c: Fix indentation.
* compile/compile-object-run.c: Fix indentation.
* completer.c: Fix indentation.
* corefile.c: Fix indentation.
* corelow.c: Fix indentation.
* cp-abi.h: Fix indentation.
* cp-namespace.c: Fix indentation.
* cp-support.c: Fix indentation.
* cp-valprint.c: Fix indentation.
* cris-linux-tdep.c: Fix indentation.
* cris-tdep.c: Fix indentation.
* darwin-nat-info.c: Fix indentation.
* darwin-nat.c: Fix indentation.
* darwin-nat.h: Fix indentation.
* dbxread.c: Fix indentation.
* dcache.c: Fix indentation.
* disasm.c: Fix indentation.
* dtrace-probe.c: Fix indentation.
* dwarf2/abbrev.c: Fix indentation.
* dwarf2/attribute.c: Fix indentation.
* dwarf2/expr.c: Fix indentation.
* dwarf2/frame.c: Fix indentation.
* dwarf2/index-cache.c: Fix indentation.
* dwarf2/index-write.c: Fix indentation.
* dwarf2/line-header.c: Fix indentation.
* dwarf2/loc.c: Fix indentation.
* dwarf2/macro.c: Fix indentation.
* dwarf2/read.c: Fix indentation.
* dwarf2/read.h: Fix indentation.
* elfread.c: Fix indentation.
* eval.c: Fix indentation.
* event-top.c: Fix indentation.
* exec.c: Fix indentation.
* exec.h: Fix indentation.
* expprint.c: Fix indentation.
* f-lang.c: Fix indentation.
* f-typeprint.c: Fix indentation.
* f-valprint.c: Fix indentation.
* fbsd-nat.c: Fix indentation.
* fbsd-tdep.c: Fix indentation.
* findvar.c: Fix indentation.
* fork-child.c: Fix indentation.
* frame-unwind.c: Fix indentation.
* frame-unwind.h: Fix indentation.
* frame.c: Fix indentation.
* frv-linux-tdep.c: Fix indentation.
* frv-tdep.c: Fix indentation.
* frv-tdep.h: Fix indentation.
* ft32-tdep.c: Fix indentation.
* gcore.c: Fix indentation.
* gdb_bfd.c: Fix indentation.
* gdbarch.sh: Fix indentation.
* gdbarch.c: Re-generate
* gdbarch.h: Re-generate.
* gdbcore.h: Fix indentation.
* gdbthread.h: Fix indentation.
* gdbtypes.c: Fix indentation.
* gdbtypes.h: Fix indentation.
* glibc-tdep.c: Fix indentation.
* gnu-nat.c: Fix indentation.
* gnu-nat.h: Fix indentation.
* gnu-v2-abi.c: Fix indentation.
* gnu-v3-abi.c: Fix indentation.
* go32-nat.c: Fix indentation.
* guile/guile-internal.h: Fix indentation.
* guile/scm-cmd.c: Fix indentation.
* guile/scm-frame.c: Fix indentation.
* guile/scm-iterator.c: Fix indentation.
* guile/scm-math.c: Fix indentation.
* guile/scm-ports.c: Fix indentation.
* guile/scm-pretty-print.c: Fix indentation.
* guile/scm-value.c: Fix indentation.
* h8300-tdep.c: Fix indentation.
* hppa-linux-nat.c: Fix indentation.
* hppa-linux-tdep.c: Fix indentation.
* hppa-nbsd-nat.c: Fix indentation.
* hppa-nbsd-tdep.c: Fix indentation.
* hppa-obsd-nat.c: Fix indentation.
* hppa-tdep.c: Fix indentation.
* hppa-tdep.h: Fix indentation.
* i386-bsd-nat.c: Fix indentation.
* i386-darwin-nat.c: Fix indentation.
* i386-darwin-tdep.c: Fix indentation.
* i386-dicos-tdep.c: Fix indentation.
* i386-gnu-nat.c: Fix indentation.
* i386-linux-nat.c: Fix indentation.
* i386-linux-tdep.c: Fix indentation.
* i386-nto-tdep.c: Fix indentation.
* i386-obsd-tdep.c: Fix indentation.
* i386-sol2-nat.c: Fix indentation.
* i386-tdep.c: Fix indentation.
* i386-tdep.h: Fix indentation.
* i386-windows-tdep.c: Fix indentation.
* i387-tdep.c: Fix indentation.
* i387-tdep.h: Fix indentation.
* ia64-libunwind-tdep.c: Fix indentation.
* ia64-libunwind-tdep.h: Fix indentation.
* ia64-linux-nat.c: Fix indentation.
* ia64-linux-tdep.c: Fix indentation.
* ia64-tdep.c: Fix indentation.
* ia64-tdep.h: Fix indentation.
* ia64-vms-tdep.c: Fix indentation.
* infcall.c: Fix indentation.
* infcmd.c: Fix indentation.
* inferior.c: Fix indentation.
* infrun.c: Fix indentation.
* iq2000-tdep.c: Fix indentation.
* language.c: Fix indentation.
* linespec.c: Fix indentation.
* linux-fork.c: Fix indentation.
* linux-nat.c: Fix indentation.
* linux-tdep.c: Fix indentation.
* linux-thread-db.c: Fix indentation.
* lm32-tdep.c: Fix indentation.
* m2-lang.c: Fix indentation.
* m2-typeprint.c: Fix indentation.
* m2-valprint.c: Fix indentation.
* m32c-tdep.c: Fix indentation.
* m32r-linux-tdep.c: Fix indentation.
* m32r-tdep.c: Fix indentation.
* m68hc11-tdep.c: Fix indentation.
* m68k-bsd-nat.c: Fix indentation.
* m68k-linux-nat.c: Fix indentation.
* m68k-linux-tdep.c: Fix indentation.
* m68k-tdep.c: Fix indentation.
* machoread.c: Fix indentation.
* macrocmd.c: Fix indentation.
* macroexp.c: Fix indentation.
* macroscope.c: Fix indentation.
* macrotab.c: Fix indentation.
* macrotab.h: Fix indentation.
* main.c: Fix indentation.
* mdebugread.c: Fix indentation.
* mep-tdep.c: Fix indentation.
* mi/mi-cmd-catch.c: Fix indentation.
* mi/mi-cmd-disas.c: Fix indentation.
* mi/mi-cmd-env.c: Fix indentation.
* mi/mi-cmd-stack.c: Fix indentation.
* mi/mi-cmd-var.c: Fix indentation.
* mi/mi-cmds.c: Fix indentation.
* mi/mi-main.c: Fix indentation.
* mi/mi-parse.c: Fix indentation.
* microblaze-tdep.c: Fix indentation.
* minidebug.c: Fix indentation.
* minsyms.c: Fix indentation.
* mips-linux-nat.c: Fix indentation.
* mips-linux-tdep.c: Fix indentation.
* mips-nbsd-tdep.c: Fix indentation.
* mips-tdep.c: Fix indentation.
* mn10300-linux-tdep.c: Fix indentation.
* mn10300-tdep.c: Fix indentation.
* moxie-tdep.c: Fix indentation.
* msp430-tdep.c: Fix indentation.
* namespace.h: Fix indentation.
* nat/fork-inferior.c: Fix indentation.
* nat/gdb_ptrace.h: Fix indentation.
* nat/linux-namespaces.c: Fix indentation.
* nat/linux-osdata.c: Fix indentation.
* nat/netbsd-nat.c: Fix indentation.
* nat/x86-dregs.c: Fix indentation.
* nbsd-nat.c: Fix indentation.
* nbsd-tdep.c: Fix indentation.
* nios2-linux-tdep.c: Fix indentation.
* nios2-tdep.c: Fix indentation.
* nto-procfs.c: Fix indentation.
* nto-tdep.c: Fix indentation.
* objfiles.c: Fix indentation.
* objfiles.h: Fix indentation.
* opencl-lang.c: Fix indentation.
* or1k-tdep.c: Fix indentation.
* osabi.c: Fix indentation.
* osabi.h: Fix indentation.
* osdata.c: Fix indentation.
* p-lang.c: Fix indentation.
* p-typeprint.c: Fix indentation.
* p-valprint.c: Fix indentation.
* parse.c: Fix indentation.
* ppc-linux-nat.c: Fix indentation.
* ppc-linux-tdep.c: Fix indentation.
* ppc-nbsd-nat.c: Fix indentation.
* ppc-nbsd-tdep.c: Fix indentation.
* ppc-obsd-nat.c: Fix indentation.
* ppc-ravenscar-thread.c: Fix indentation.
* ppc-sysv-tdep.c: Fix indentation.
* ppc64-tdep.c: Fix indentation.
* printcmd.c: Fix indentation.
* proc-api.c: Fix indentation.
* producer.c: Fix indentation.
* producer.h: Fix indentation.
* prologue-value.c: Fix indentation.
* prologue-value.h: Fix indentation.
* psymtab.c: Fix indentation.
* python/py-arch.c: Fix indentation.
* python/py-bpevent.c: Fix indentation.
* python/py-event.c: Fix indentation.
* python/py-event.h: Fix indentation.
* python/py-finishbreakpoint.c: Fix indentation.
* python/py-frame.c: Fix indentation.
* python/py-framefilter.c: Fix indentation.
* python/py-inferior.c: Fix indentation.
* python/py-infthread.c: Fix indentation.
* python/py-objfile.c: Fix indentation.
* python/py-prettyprint.c: Fix indentation.
* python/py-registers.c: Fix indentation.
* python/py-signalevent.c: Fix indentation.
* python/py-stopevent.c: Fix indentation.
* python/py-stopevent.h: Fix indentation.
* python/py-threadevent.c: Fix indentation.
* python/py-tui.c: Fix indentation.
* python/py-unwind.c: Fix indentation.
* python/py-value.c: Fix indentation.
* python/py-xmethods.c: Fix indentation.
* python/python-internal.h: Fix indentation.
* python/python.c: Fix indentation.
* ravenscar-thread.c: Fix indentation.
* record-btrace.c: Fix indentation.
* record-full.c: Fix indentation.
* record.c: Fix indentation.
* reggroups.c: Fix indentation.
* regset.h: Fix indentation.
* remote-fileio.c: Fix indentation.
* remote.c: Fix indentation.
* reverse.c: Fix indentation.
* riscv-linux-tdep.c: Fix indentation.
* riscv-ravenscar-thread.c: Fix indentation.
* riscv-tdep.c: Fix indentation.
* rl78-tdep.c: Fix indentation.
* rs6000-aix-tdep.c: Fix indentation.
* rs6000-lynx178-tdep.c: Fix indentation.
* rs6000-nat.c: Fix indentation.
* rs6000-tdep.c: Fix indentation.
* rust-lang.c: Fix indentation.
* rx-tdep.c: Fix indentation.
* s12z-tdep.c: Fix indentation.
* s390-linux-tdep.c: Fix indentation.
* score-tdep.c: Fix indentation.
* ser-base.c: Fix indentation.
* ser-mingw.c: Fix indentation.
* ser-uds.c: Fix indentation.
* ser-unix.c: Fix indentation.
* serial.c: Fix indentation.
* sh-linux-tdep.c: Fix indentation.
* sh-nbsd-tdep.c: Fix indentation.
* sh-tdep.c: Fix indentation.
* skip.c: Fix indentation.
* sol-thread.c: Fix indentation.
* solib-aix.c: Fix indentation.
* solib-darwin.c: Fix indentation.
* solib-frv.c: Fix indentation.
* solib-svr4.c: Fix indentation.
* solib.c: Fix indentation.
* source.c: Fix indentation.
* sparc-linux-tdep.c: Fix indentation.
* sparc-nbsd-tdep.c: Fix indentation.
* sparc-obsd-tdep.c: Fix indentation.
* sparc-ravenscar-thread.c: Fix indentation.
* sparc-tdep.c: Fix indentation.
* sparc64-linux-tdep.c: Fix indentation.
* sparc64-nbsd-tdep.c: Fix indentation.
* sparc64-obsd-tdep.c: Fix indentation.
* sparc64-tdep.c: Fix indentation.
* stabsread.c: Fix indentation.
* stack.c: Fix indentation.
* stap-probe.c: Fix indentation.
* stubs/ia64vms-stub.c: Fix indentation.
* stubs/m32r-stub.c: Fix indentation.
* stubs/m68k-stub.c: Fix indentation.
* stubs/sh-stub.c: Fix indentation.
* stubs/sparc-stub.c: Fix indentation.
* symfile-mem.c: Fix indentation.
* symfile.c: Fix indentation.
* symfile.h: Fix indentation.
* symmisc.c: Fix indentation.
* symtab.c: Fix indentation.
* symtab.h: Fix indentation.
* target-float.c: Fix indentation.
* target.c: Fix indentation.
* target.h: Fix indentation.
* tic6x-tdep.c: Fix indentation.
* tilegx-linux-tdep.c: Fix indentation.
* tilegx-tdep.c: Fix indentation.
* top.c: Fix indentation.
* tracefile-tfile.c: Fix indentation.
* tracepoint.c: Fix indentation.
* tui/tui-disasm.c: Fix indentation.
* tui/tui-io.c: Fix indentation.
* tui/tui-regs.c: Fix indentation.
* tui/tui-stack.c: Fix indentation.
* tui/tui-win.c: Fix indentation.
* tui/tui-winsource.c: Fix indentation.
* tui/tui.c: Fix indentation.
* typeprint.c: Fix indentation.
* ui-out.h: Fix indentation.
* unittests/copy_bitwise-selftests.c: Fix indentation.
* unittests/memory-map-selftests.c: Fix indentation.
* utils.c: Fix indentation.
* v850-tdep.c: Fix indentation.
* valarith.c: Fix indentation.
* valops.c: Fix indentation.
* valprint.c: Fix indentation.
* valprint.h: Fix indentation.
* value.c: Fix indentation.
* value.h: Fix indentation.
* varobj.c: Fix indentation.
* vax-tdep.c: Fix indentation.
* windows-nat.c: Fix indentation.
* windows-tdep.c: Fix indentation.
* xcoffread.c: Fix indentation.
* xml-syscall.c: Fix indentation.
* xml-tdesc.c: Fix indentation.
* xstormy16-tdep.c: Fix indentation.
* xtensa-config.c: Fix indentation.
* xtensa-linux-nat.c: Fix indentation.
* xtensa-linux-tdep.c: Fix indentation.
* xtensa-tdep.c: Fix indentation.
gdbserver/ChangeLog:
* ax.cc: Fix indentation.
* dll.cc: Fix indentation.
* inferiors.h: Fix indentation.
* linux-low.cc: Fix indentation.
* linux-nios2-low.cc: Fix indentation.
* linux-ppc-ipa.cc: Fix indentation.
* linux-ppc-low.cc: Fix indentation.
* linux-x86-low.cc: Fix indentation.
* linux-xtensa-low.cc: Fix indentation.
* regcache.cc: Fix indentation.
* server.cc: Fix indentation.
* tracepoint.cc: Fix indentation.
gdbsupport/ChangeLog:
* common-exceptions.h: Fix indentation.
* event-loop.cc: Fix indentation.
* fileio.cc: Fix indentation.
* filestuff.cc: Fix indentation.
* gdb-dlfcn.cc: Fix indentation.
* gdb_string_view.h: Fix indentation.
* job-control.cc: Fix indentation.
* signals.cc: Fix indentation.
Change-Id: I4bad7ae6be0fbe14168b8ebafb98ffe14964a695
|
|
The *_debug_print_1 functions are all very similar, the only difference
being the subsystem name. Remove them all and make the logging macros
use a new debug_prefixed_printf function directly.
gdb/ChangeLog:
* infrun.c (infrun_debug_printf_1): Remove.
(displaced_debug_printf_1): Remove.
(stop_all_threads): Use debug_prefixed_printf.
* infrun.h (infrun_debug_printf_1): Remove.
(infrun_debug_printf): Use debug_prefixed_printf.
(displaced_debug_printf_1): Remove.
(displaced_debug_printf): Use debug_prefixed_printf.
* linux-nat.c (linux_nat_debug_printf_1): Remove.
(linux_nat_debug_printf): Use debug_prefixed_printf.
gdbsupport/ChangeLog:
* common-debug.cc (debug_prefixed_printf): New.
* common-debug.h (debug_prefixed_printf): New declaration.
* event-loop.cc (event_loop_debug_printf_1): Remove.
* event-loop.h (event_loop_debug_printf_1): Remove.
(event_loop_debug_printf): Use debug_prefixed_printf.
(event_loop_ui_debug_printf): Use debug_prefixed_printf.
Change-Id: Ib323087c7257f0060121d302055c41eb64aa60c6
|
|
Move all debug prints of the "displaced" category to use a new
displaced_debug_printf macro, like what was done for infrun and others
earlier.
The debug output for one displaced step one amd64 looks like:
[displaced] displaced_step_prepare_throw: stepping process 3367044 now
[displaced] displaced_step_prepare_throw: saved 0x555555555042: 1e fa 31 ed 49 89 d1 5e 48 89 e2 48 83 e4 f0 50
[displaced] amd64_displaced_step_copy_insn: copy 0x555555555131->0x555555555042: b8 00 00 00 00 5d c3 0f 1f 84 00 00 00 00 00 f3
[displaced] displaced_step_prepare_throw: displaced pc to 0x555555555042
[displaced] resume_1: run 0x555555555042: b8 00 00 00
[displaced] displaced_step_restore: restored process 3367044 0x555555555042
[displaced] amd64_displaced_step_fixup: fixup (0x555555555131, 0x555555555042), insn = 0xb8 0x00 ...
[displaced] amd64_displaced_step_fixup: relocated %rip from 0x555555555047 to 0x555555555136
On test case needed to be updated because it relied on the specific
formatting of the message.
gdb/ChangeLog:
* infrun.h (displaced_debug_printf): New macro. Replace
displaced debug prints throughout to use it.
(displaced_debug_printf_1): New declaration.
(displaced_step_dump_bytes): Return string, remove ui_file
parameter, update all callers.
* infrun.c (displaced_debug_printf_1): New function.
(displaced_step_dump_bytes): Return string, remove ui_file
parameter
gdb/testsuite/ChangeLog:
* gdb.arch/amd64-disp-step-avx.exp: Update displaced step debug
expected output.
Change-Id: Ie78837f56431f6f98378790ba1e6051337bf6533
|
|
Having pagination enabled when handling an inferior event gives the
user an option to quit, which causes early exit in GDB's flow and may
lead to half-baked state. For instance, here is a case where we quit
in the middle of handling an inferior exit:
$ gdb ./a.out
Reading symbols from ./a.out...
(gdb) set height 2
(gdb) run
Starting program: ./a.out
--Type <RET> for more, q to quit, c to continue without paging--q
Quit
Couldn't get registers: No such process.
(gdb) set height unlimited
Couldn't get registers: No such process.
(gdb) info threads
Id Target Id Frame
* 1 process 27098 Couldn't get registers: No such process.
Couldn't get registers: No such process.
(gdb)
Or suppose having a multi-threaded program like below:
static void *
fun (void *dummy)
{
int a = 1; /* break-here */
return NULL;
}
int
main (void)
{
pthread_t thread;
pthread_create (&thread, NULL, fun, NULL);
pthread_join (thread, NULL);
return 0;
}
If we define a breakpoint at line "break-here", we expect only Thread
2 to hit it.
$ gdb ./a.out
Reading symbols from ./a.out...
(gdb) break 7
Breakpoint 1 at 0x1182: file mt.c, line 7.
(gdb) set height 2
(gdb) run
Starting program: ./a.out
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
[New Thread 0x7ffff77c4700 (LWP 23048)]
--Type <RET> for more, q to quit, c to continue without paging--q
Quit
(gdb) set height unlimited
(gdb) info thread
Id Target Id Frame
* 1 Thread 0x7ffff7fe3740 (LWP 23044) "a.out" 0x00007ffff7bbed2d in ...
2 Thread 0x7ffff77c4700 (LWP 23048) "a.out" fun (dummy=0x0) at mt.c:7
(gdb)
The prompt for continuation was triggered because Thread 2 hit the
breakpoint. (If we had hit 'c', we were going to see that stop event,
but we didn't.) The context did not switch to Thread 2. GDB also did
not execute several other things it would normally do in
infrun.c:normal_stop after outputting "[Switching to Thread ...]" (but
it seems harmless in this case). If we 'continue' at this state, both
threads run until termination, and we don't see the breakpoint hit
event ever.
Here is another related and more complicated scenario that leads to a
GDB crash. Create two inferiors, one sitting on top of a native
target, and the other on a remote target, so that we have a
multi-target setting, like so:
(gdb) i inferiors
Num Description Connection Executable
1 process 13786 1 (native) a.out
* 2 process 13806 2 (remote ...) target:a.out
Next, resume both inferiors to run until termination:
(gdb) set schedule-multiple on
(gdb) set height 2
(gdb) continue
Continuing.
--Type <RET> for more, q to quit, c to continue without paging--[Inferior 2 (process 13806) exited normally]
terminate called after throwing an instance of 'gdb_exception_error'
Aborted
Here, GDB first received a termination event from Inferior 1. GDB
attempted to print this event, triggering a "prompt for continue", and
GDB started polling for events, hoping to get an input from the user.
However, the exit event from Inferior 2 was received instead. So, GDB
started processing an exit event while being in the middle of
processing another exit event. It was not ready for this situation
and eventually crashed.
To address these cases, temporarily disable pagination in
fetch_inferior_event. This doesn't affect commands like 'info
threads', 'backtrace', or 'thread apply'.
Regression-tested on X86_64 Linux.
gdb/ChangeLog:
2020-10-30 Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
* infrun.c (fetch_inferior_event): Temporarily disable pagination.
gdb/testsuite/ChangeLog:
2020-10-30 Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
* gdb.base/paginate-after-ctrl-c-running.exp: Update with no pagination
behavior.
* gdb.base/paginate-bg-execution.exp: Ditto.
* gdb.base/paginate-inferior-exit.exp: Ditto.
* gdb.base/double-prompt-target-event-error.c: Remove.
* gdb.base/double-prompt-target-event-error.exp: Remove.
|
|
If the remote target closes while we're reading registers/memory for
restoring the selected frame in scoped_restore_current_thread's dtor,
the corresponding TARGET_CLOSE_ERROR error is swallowed by the
scoped_restore_current_thread's dtor, because letting exceptions
escape from a dtor is bad. It isn't great to lose that errors like
that, though. I've been thinking about how to avoid it, and I came up
with this patch.
The idea here is to make scoped_restore_current_thread's dtor do as
little as possible, to avoid any work that might throw in the first
place. And to do that, instead of having the dtor call
restore_selected_frame, which re-finds the previously selected frame,
just record the frame_id/level of the desired selected frame, and have
get_selected_frame find the frame the next time it is called. In
effect, this implements most of Cagney's suggestion, here:
/* On demand, create the selected frame and then return it. If the
selected frame can not be created, this function prints then throws
an error. When MESSAGE is non-NULL, use it for the error message,
otherwize use a generic error message. */
/* FIXME: cagney/2002-11-28: At present, when there is no selected
frame, this function always returns the current (inner most) frame.
It should instead, when a thread has previously had its frame
selected (but not resumed) and the frame cache invalidated, find
and then return that thread's previously selected frame. */
extern struct frame_info *get_selected_frame (const char *message);
The only thing missing to fully implement that would be to make
reinit_frame_cache just clear selected_frame instead of calling
select_frame(NULL), and the call select_frame(NULL) explicitly in the
places where we really wanted reinit_frame_cache to go back to the
current frame too. That can done separately, though, I'm not
proposing to do that in this patch.
Note that this patch renames restore_selected_frame to
lookup_selected_frame, and adds a new restore_selected_frame function
that doesn't throw, to be paired with the also-new save_selected_frame
function.
There's a restore_selected_frame function in infrun.c that I think can
be replaced by the new one in frame.c.
Also done in this patch is make the get_selected_frame's parameter be
optional, so that we don't have to pass down nullptr explicitly all
over the place.
lookup_selected_frame should really move from thread.c to frame.c, but
I didn't do that here, just to avoid churn in the patch while it
collects comments. I did make it extern and declared it in frame.h
already, preparing for the move. I will do the move as a follow up
patch if people agree with this approach.
Incidentally, this patch alone would fix the crashes fixed by the
previous patches in the series, because with this,
scoped_restore_current_thread's constructor doesn't throw either.
gdb/ChangeLog:
* blockframe.c (block_innermost_frame): Use get_selected_frame.
* frame.c
(scoped_restore_selected_frame::scoped_restore_selected_frame):
Use save_selected_frame. Save language as well.
(scoped_restore_selected_frame::~scoped_restore_selected_frame):
Use restore_selected_frame, and restore language as well.
(selected_frame_id, selected_frame_level): New.
(selected_frame): Update comments.
(save_selected_frame, restore_selected_frame): New.
(get_selected_frame): Use lookup_selected_frame.
(get_selected_frame_if_set): Delete.
(select_frame): Record selected_frame_level and selected_frame_id.
* frame.h (scoped_restore_selected_frame) <m_level, m_lang>: New
fields.
(get_selected_frame): Make 'message' parameter optional.
(get_selected_frame_if_set): Delete declaration.
(select_frame): Update comments.
(save_selected_frame, restore_selected_frame)
(lookup_selected_frame): Declare.
* gdbthread.h (scoped_restore_current_thread) <m_lang>: New field.
* infrun.c (struct infcall_control_state) <selected_frame_level>:
New field.
(save_infcall_control_state): Use save_selected_frame.
(restore_selected_frame): Delete.
(restore_infcall_control_state): Use restore_selected_frame.
* stack.c (select_frame_command_core, frame_command_core): Use
get_selected_frame.
* thread.c (restore_selected_frame): Rename to ...
(lookup_selected_frame): ... this and make extern. Select the
current frame if the frame level is -1.
(scoped_restore_current_thread::restore): Also restore the
language.
(scoped_restore_current_thread::~scoped_restore_current_thread):
Don't try/catch.
(scoped_restore_current_thread::scoped_restore_current_thread):
Save the language as well. Use save_selected_frame.
Change-Id: I73fd1cfc40d8513c28e5596383b7ecd8bcfe700f
|
|
I noticed that the closure parameter of
gdbarch_displaced_step_hw_singlestep is never used by any
implementation of the method, so this patch removes it.
gdb/ChangeLog:
* gdbarch.sh (displaced_step_hw_singlestep): Remove closure
parameter.
* aarch64-tdep.c (aarch64_displaced_step_hw_singlestep):
Likewise.
* aarch64-tdep.h (aarch64_displaced_step_hw_singlestep):
Likewise.
* arch-utils.c (default_displaced_step_hw_singlestep):
Likewise.
* arch-utils.h (default_displaced_step_hw_singlestep):
Likewise.
* rs6000-tdep.c (ppc_displaced_step_hw_singlestep):
Likewise.
* s390-tdep.c (s390_displaced_step_hw_singlestep):
Likewise.
* gdbarch.c: Re-generate.
* gdbarch.h: Re-generate.
* infrun.c (resume_1): Adjust.
Change-Id: I7354f0b22afc2692ebff0cd700a462db8f389fc1
|
|
Use the inferior parameter now available in jit_inferior_created_hook.
It is passed down to jit_inferior_init, which uses it as much as
possible instead of the current inferior or current program space.
gdb/ChangeLog:
* jit.c (jit_reader_load_command): Pass current inferior.
(jit_inferior_init): Change parameter type to inferior, use it.
(jit_inferior_created): Remove.
(jit_inferior_created_hook): Pass inferior parameter down.
(_initialize_jit): Use jit_inferior_created_hook instead of
jit_inferior_created.
* jit.h (jit_inferior_created_hook): Add inferior parameter.
* infrun.c (follow_exec): Pass inferior to
jit_inferior_created_hook.
Change-Id: If3a2114a933370dd313d5abd623136d273cdb8fa
|
|
These comments are stale, they refer to non-existent parameters. Fix
that.
gdb/ChangeLog:
* infrun.c (displaced_step_in_progress_thread): Fix comment.
(displaced_step_in_progress): Fix comment.
Change-Id: I7a39f1338fbfbf73153b49cbca0345d495d12762
|
