Commit Graph

788 Commits

Author SHA1 Message Date
Joel Brobecker
213516ef31 Update copyright year range in header of all files managed by GDB
This commit is the result of running the gdb/copyright.py script,
which automated the update of the copyright year range for all
source files managed by the GDB project to be updated to include
year 2023.
2023-01-01 17:01:16 +04:00
Andrew Burgess
c8181f706f gdb: remove the pop_all_targets (and friends) global functions
This commit removes the global functions pop_all_targets,
pop_all_targets_above, and pop_all_targets_at_and_above, and makes
them methods on the inferior class.

As the pop_all_targets functions will unpush each target, which
decrements the targets reference count, it is possible that the target
might be closed.

Right now, closing a target, in some cases, depends on the current
inferior being set correctly, that is, to the inferior from which the
target was popped.

To facilitate this I have used switch_to_inferior_no_thread within the
new methods.  Previously it was the responsibility of the caller to
ensure that the correct inferior was selected.

In a couple of places (event-top.c and top.c) I have been able to
remove a previous switch_to_inferior_no_thread call.

In remote_unpush_target (remote.c) I have left the
switch_to_inferior_no_thread call as it is required for the
generic_mourn_inferior call.
2022-12-14 13:57:22 +00:00
Andrew Burgess
9678f8fe97 gdb: remove decref_target
The decref_target function is not really needed.  Calling
target_ops::decref will just redirect to decref_target anyway, so why
not just rename decref_target to target_ops::decref?

That's what this commit does.

It's not exactly renaming to target_ops::decref, because the decref
functionality is handled by a policy class, so the new name is now
target_ops_ref_policy::decref.

There should be no user visible change after this commit.
2022-12-14 13:57:22 +00:00
Andrew Burgess
91e3d1d1a5 gdb: have target_stack automate reference count handling
This commit changes the target_stack class from using a C style array
of 'target_ops *' to using a C++ std::array<target_ops_ref, ...>.  The
benefit of this change is that some of the reference counting of
target_ops objects is now done automatically.

This commit fixes a crash in gdb.python/py-inferior.exp where GDB
crashes at exit, leaving a core file behind.

The crash occurs in connpy_connection_dealloc, and is actually
triggered by this assert:

gdb_assert (conn_obj->target == nullptr);

Now a little aside...

    ... the assert is never actually printed, instead GDB crashes due
    to calling a pure virtual function.  The backtrace at the point of
    crash looks like this:

      #7  0x00007fef7e2cf747 in std::terminate() () from /lib64/libstdc++.so.6
      #8  0x00007fef7e2d0515 in __cxa_pure_virtual () from /lib64/libstdc++.so.6
      #9  0x0000000000de334d in target_stack::find_beneath (this=0x4934d78, t=0x2bda270 <the_dummy_target>) at ../../s>
      #10 0x0000000000df4380 in inferior::find_target_beneath (this=0x4934b50, t=0x2bda270 <the_dummy_target>) at ../.>
      #11 0x0000000000de2381 in target_ops::beneath (this=0x2bda270 <the_dummy_target>) at ../../src/gdb/target.c:3047
      #12 0x0000000000de68aa in target_ops::supports_terminal_ours (this=0x2bda270 <the_dummy_target>) at ../../src/gd>
      #13 0x0000000000dde6b9 in target_supports_terminal_ours () at ../../src/gdb/target.c:1112
      #14 0x0000000000ee55f1 in internal_vproblem(internal_problem *, const char *, int, const char *, typedef __va_li>

    Notice in frame #12 we called target_ops::supports_terminal_ours,
    however, this is the_dummy_target, which is of type dummy_target,
    and so we should have called dummy_target::supports_terminal_ours.
    I believe the reason we ended up in the wrong implementation of
    supports_terminal_ours (which is a virtual function) is because we
    made the call during GDB's shut-down, and, I suspect, the vtables
    were in a weird state.

    Anyway, the point of this patch is not to fix GDB's ability to
    print an assert during exit, but to address the root cause of the
    assert.  With that aside out of the way, we can return to the main
    story...

Connections are represented in Python with gdb.TargetConnection
objects (or its sub-classes).  The assert in question confirms that
when a gdb.TargetConnection is deallocated, the underlying GDB
connection has itself been removed from GDB.  If this is not true then
we risk creating multiple different gdb.TargetConnection objects for
the same connection, which would be bad.

To ensure that we have one gdb.TargetConnection object for each
connection, the all_connection_objects map exists, this maps the
process_stratum_target object (the connection) to the
gdb.TargetConnection object that represents the connection.

When a connection is removed in GDB the connection_removed observer
fires, which we catch with connpy_connection_removed, this function
then sets conn_obj->target to nullptr, and removes the corresponding
entry from the all_connection_objects map.

The first issue here is that connpy_connection_dealloc is being called
as part of GDB's exit code, which is run after the Python interpreter
has been shut down.  The connpy_connection_dealloc function is used to
deallocate the gdb.TargetConnection Python object.  Surely it is
wrong for us to be deallocating Python objects after the interpreter
has been shut down.

The reason why connpy_connection_dealloc is called during GDB's exit
is that the global all_connection_objects map is still holding a
reference to the gdb.TargetConnection object.  When the map is
destroyed during GDB's exit, the gdb.TargetConnection objects within
the map can finally be deallocated.

The reason why all_connection_objects has contents when GDB exits, and
the reason the assert fires, is that, when GDB exits, there are still
some connections that have not yet been removed from GDB, that is,
they have a non-zero reference count.

If we take a look at quit_force (top.c) you can see that, for each
inferior, we call pop_all_targets before we (later in the function)
call do_final_cleanups.  It is the do_final_cleanups call that is
responsible for shutting down the Python interpreter.  The
pop_all_targets calls should, in theory, cause all the connections to
be removed from GDB.

That this isn't working indicates that some targets have a non-zero
reference count even after this final pop_all_targets call, and
indeed, when I debug GDB, that is what I see.

I tracked the problem down to delete_inferior where we do some house
keeping, and then delete the inferior object, which calls
inferior::~inferior.

In neither delete_inferior or inferior::~inferior do we call
pop_all_targets, and it is this missing call that means we leak some
references to the target_ops objects on the inferior's target_stack.

In this commit I will provide a partial fix for the problem.  I say
partial fix, but this will actually be enough to resolve the crash.
In a later commit I will provide the final part of the fix.

As mentioned at the start of the commit message, this commit changes
the m_stack in target_stack to hold target_ops_ref objects.  This
means that when inferior::~inferior is called, and m_stack is
released, we automatically decrement the target_ops reference count.
With this change in place we no longer leak any references, and now,
in quit_force the final pop_all_targets calls will release the final
references.  This means that the targets will be correctly closed at
this point, which means the connections will be removed from GDB and
the Python objects deallocated before the Python interpreter shuts
down.

There's a slight oddity in target_stack::unpush, where we std::move
the reference out of m_stack like this:

  auto ref = std::move (m_stack[stratum]);

the `ref' isn't used explicitly, but it serves to hold the
target_ops_ref until the end of the scope while allowing the m_stack
entry to be reset back to nullptr.  The alternative would be to
directly set the m_stack entry to nullptr, like this:

  m_stack[stratum] = nullptr;

The problem here is that when we set the m_stack entry to nullptr we
first decrement the target_ops reference count, and then set the array
entry to nullptr.

If the decrement means that the target_ops object reaches a zero
reference count then the target_ops object will be closed by calling
target_close.  In target_close we ensure that the target being closed
is not in any inferiors target_stack.

As we decrement before clearing, then this check in target_close will
fail, and an assert will trigger.

By using std::move to move the reference out of m_stack, this clears
the m_stack entry, meaning the inferior no longer contains the
target_ops in its target_stack.  Now when the REF object goes out of
scope and the reference count is decremented, target_close can run
successfully.

I've made use of the Python connection_removed listener API to add a
test for this issue.  The test installs a listener and then causes
delete_inferior to be called, we can then see that the connection is
then correctly removed (because the listener triggers).
2022-12-14 13:57:21 +00:00
Simon Marchi
3fe639b81b gdb: constify auxv parse functions
Constify the input parameters of the various auxv parse functions, they
don't need to modify the raw auxv data.

Change-Id: I13eacd5ab8e925ec2b5c1f7722cbab39c41516ec
2022-09-29 16:42:34 -04:00
Simon Marchi
31282a8491 gdb: constify target_stack::is_pushed
The target_ops parameters here can be made const.

Change-Id: Ibc18b17d6b21d06145251a03e68aca90538117d6
2022-09-29 16:39:13 -04:00
Simon Marchi
b872057a63 gdbsupport: convert FILEIO_* macros to an enum
Converting from free-form macros to an enum gives a bit of type-safety.
This caught places where we would assign host error numbers to what
should contain a target fileio error number, for instance in
target_fileio_pread.

I added the FILEIO_SUCCESS enumerator, because
remote.c:remote_hostio_parse_result initializes the remote_errno output
variable to 0.  It seems better to have an explicit enumerator than to
assign a value for which there is no enumerator.  I considered
initializing this variable to FILEIO_EUNKNOWN instead, such that if the
remote side replies with an error and omits the errno value, we'll get
an errno that represents an error instead of 0 (which reprensents no
error).  But it's not clear what the consequences of that change would
be, so I prefer to err on the side of caution and just keep the existing
behavior (there is no intended change in behavior with this patch).

Note that remote_hostio_parse_resul still reads blindly what the remote
side sends as a target errno into this variable, so we can still end up
with a nonsensical value here.  It's not good, but out of the scope of
this patch.

Convert host_to_fileio_error and fileio_errno_to_host to return / accept
a fileio_error instead of an int, and cascade the change in the whole
chain that uses that.

Change-Id: I454b0e3fcf0732447bc872252fa8e57d138b0e03
2022-09-21 14:11:03 -04:00
Tom Tromey
4a570176b4 Change target_ops::async to accept bool
This changes the parameter of target_ops::async from int to bool.
Regression tested on x86-64 Fedora 34.
2022-07-22 11:06:51 -06:00
Tom Tromey
0e90c44162 Constify target_pid_to_exec_file
This changes target_pid_to_exec_file and target_ops::pid_to_exec_file
to return a "const char *".  I couldn't build many of these targets,
but did examine the code by hand -- also, as this only affects the
return type, it's normally pretty safe.  This brings gdb and gdbserver
a bit closer, and allows for the removal of a const_cast as well.
2022-05-13 08:21:16 -06:00
Pedro Alves
d51926f06a Slightly tweak and clarify target_resume's interface
The current target_resume interface is a bit odd & non-intuitive.
I've found myself explaining it a couple times the recent past, while
reviewing patches that assumed STEP/SIGNAL always applied to the
passed in PTID.  It goes like this today:

  - if the passed in PTID is a thread, then the step/signal request is
    for that thread.

  - otherwise, if PTID is a wildcard (all threads or all threads of
    process), the step/signal request is for inferior_ptid, and PTID
    indicates which set of threads run free.

Because GDB always switches the current thread to "leader" thread
being resumed/stepped/signalled, we can simplify this a bit to:

  - step/signal are always for inferior_ptid.

  - PTID indicates the set of threads that run free.

Still not ideal, but it's a minimal change and at least there are no
special cases this way.

That's what this patch does.  It renames the PTID parameter to
SCOPE_PTID, adds some assertions to target_resume, and tweaks
target_resume's description.  In addition, it also renames PTID to
SCOPE_PTID in the remote and linux-nat targets, and simplifies their
implementation a little bit.  Other targets could do the same, but
they don't have to.

Change-Id: I02a2ec2ab3a3e9b191de1e9a84f55c17cab7daaf
2022-04-29 12:33:27 +01:00
Tom Tromey
b17c7ab380 Move target_read_string to target/target.c
This moves the two overloads of target_read_string to a new file,
target/target.c, and updates both gdb and gdbserver to build this.
2022-04-14 12:12:34 -06:00
Andrew Burgess
c8154ce0d6 gdb: move gdb_disassembly_flag into a new disasm-flags.h file
While working on the disassembler I was getting frustrated.  Every
time I touched disasm.h it seemed like every file in GDB would need to
be rebuilt.  Surely the disassembler can't be required by that many
parts of GDB, right?

Turns out that disasm.h is included in target.h, so pretty much every
file was being rebuilt!

The only thing from disasm.h that target.h needed is the
gdb_disassembly_flag enum, as this is part of the target_ops api.

In this commit I move gdb_disassembly_flag into its own file.  This is
then included in target.h and disasm.h, after which, the number of
files that depend on disasm.h is much reduced.

I also audited all the other includes of disasm.h and found that the
includes in mep-tdep.c and python/py-registers.c are no longer needed,
so I've removed these.

Now, after changing disasm.h, GDB rebuilds much quicker.

There should be no user visible changes after this commit.
2022-04-06 13:09:44 +01:00
Markus Metzger
696c0d5ef2 gdb, gdbserver: update thread identifier in enable_btrace target method
The enable_btrace target method takes a ptid_t to identify the thread on
which tracing shall be enabled.

Change this to thread_info * to avoid translating back and forth between
the two.  This will be used in a subsequent patch.
2022-01-27 13:31:20 +01:00
Tom Tromey
bc521517b7 Introduce target_announce_attach
This introduces target_announce_attach, by analog with
target_announce_detach.  Then it converts existing targets to use
this, rather than emitting their own output by hand.
2022-01-06 08:56:20 -07:00
Joel Brobecker
4a94e36819 Automatic Copyright Year update after running gdb/copyright.py
This commit brings all the changes made by running gdb/copyright.py
as per GDB's Start of New Year Procedure.

For the avoidance of doubt, all changes in this commits were
performed by the script.
2022-01-01 19:13:23 +04:00
Andrew Burgess
200fd2874d gdb: make post_startup_inferior a virtual method on inf_ptrace_target
While working on a later patch that required me to understand how GDB
starts up inferiors, I was confused by the
target_ops::post_startup_inferior method.

The post_startup_inferior target function is only called from
inf_ptrace_target::create_inferior.

Part of the target class hierarchy looks like this:

  inf_child_target
     |
     '-- inf_ptrace_target
            |
            |-- linux_nat_target
            |
            |-- fbsd_nat_target
            |
            |-- nbsd_nat_target
            |
            |-- obsd_nat_target
            |
            '-- rs6000_nat_target

Every sub-class of inf_ptrace_target, except rs6000_nat_target,
implements ::post_startup_inferior.  The rs6000_nat_target picks up
the implementation of ::post_startup_inferior not from
inf_ptrace_target, but from inf_child_target.

No descendent of inf_child_target, outside the inf_ptrace_target
sub-tree, implements ::post_startup_inferior, which isn't really
surprising, as they would never see the method called (remember, the
method is only called from inf_ptrace_target::create_inferior).

What I find confusing is the role inf_child_target plays in
implementing, what is really a helper function for just one of its
descendents.

In this commit I propose that we formally make ::post_startup_inferior
a helper function of inf_ptrace_target.  To do this I will remove the
::post_startup_inferior from the target_ops API, and instead make this
a protected, pure virtual function on inf_ptrace_target.

I'll remove the empty implementation of ::post_startup_inferior from
the inf_child_target class, and add a new empty implementation to the
rs6000_nat_target class.

All the other descendents of inf_ptrace_target already provide an
implementation of this method and so don't need to change beyond
making the method protected within their class declarations.

To me, this makes much more sense now.  The helper function, which is
only called from within the inf_ptrace_target class, is now a part of
the inf_ptrace_target class.

The only way in which this change is visible to a user is if the user
turns on 'set debug target 1'.  With this debug flag on, prior to this
patch the user would see something like:

  -> native->post_startup_inferior (...)
  <- native->post_startup_inferior (2588939)

After this patch these lines are no longer present, as the
post_startup_inferior is no longer a top level target method.  For me,
this is an acceptable change.
2021-12-13 11:10:29 +00:00
Andrew Burgess
0c1e6e265b gdb: introduce a new overload of target_can_async_p
There are a few places where we call the target_ops::can_async_p
member function directly, instead of using the target_can_async_p
wrapper.

In some of these places this is because we need to ask before the
target has been pushed, and in another location (in target.c) it seems
unnecessary to go through the wrapper when we are already in target.c
code.

However, in the next commit I'd like to hoist some common checks out
of target specific code into target.c.  To achieve this, in this
commit, I introduce a new overload of target_can_async_p which takes a
target_ops pointer, and calls the ::can_async_p method directly.  I
then make use of the new overload where appropriate.

There should be no user visible changes after this commit.
2021-11-25 09:54:58 +00:00
Simon Marchi
25558938d0 gdb: change thread_info::name to unique_xmalloc_ptr, add helper function
This started out as changing thread_info::name to a unique_xmalloc_ptr.
That showed that almost all users of that field had the same logic to
get a thread's name: use thread_info::name if non-nullptr, else ask the
target.  Factor out this logic in a new thread_name free function.  Make
the field private (rename to m_name) and add some accessors.

Change-Id: Iebdd95f4cd21fbefc505249bd1d05befc466a2fc
2021-09-24 17:25:55 -04:00
Tom Tromey
c80e29dba9 Change get_ada_task_ptid parameter type
get_ada_task_ptid currently takes a 'long' as its 'thread' parameter
type.  However, on some platforms this is actually a pointer, and
using 'long' can sometimes end up with the value being sign-extended.
This sign extension can cause problems later, if the tid is then later
used as an address again.

This patch changes the parameter type to ULONGEST and updates all the
uses.  This approach preserves sign extension on the targets where it
is apparently intended, while avoiding it on others.

Co-Authored-By: John Baldwin <jhb@FreeBSD.org>
2021-09-23 09:30:54 -06:00
Simon Marchi
82d1f134cc gdb: follow-fork: push target and add thread in target_follow_fork
In the context of ROCm-gdb [1], the ROCm target sits on top of the
linux-nat target.  when a process forks, it needs to carry over some
data from the forking inferior to the fork child inferior.  Ideally, the
ROCm target would implement the follow_fork target_ops method, but there
are some small problems.  This patch fixes these, which helps the ROCm
target, but also makes things more consistent and a bit nicer in
general, I believe.

The main problem is: when follow-fork-mode is "parent",
target_follow_fork is called with the parent as the current inferior.
When it's "child", target_follow_fork is called with the child as the
current inferior.  This means that target_follow_fork is sometimes
called on the parent's target stack and sometimes on the child's target
stack.

The parent's target stack may contain targets above the process target,
such as the ROCm target.  So if follow-fork-child is "parent", the ROCm
target would get notified of the fork and do whatever is needed.  But
the child's target stack, at that moment, only contains the exec and
process target copied over from the parent.  The child's target stack is
set up by follow_fork_inferior, before calling target_follow_fork.  In
that case, the ROCm target wouldn't get notified of the fork.

For consistency, I think it would be good to always call
target_follow_fork on the parent inferior's target stack.  I think it
makes sense as a way to indicate "this inferior has called fork, do
whatever is needed".  The desired outcome of the fork (whether an
inferior is created for the child, do we need to detach from the child)
can be indicated by passed parameter.

I therefore propose these changes:

 - make follow_fork_inferior always call target_follow_fork with the
   parent as the current inferior.  That lets all targets present on the
   parent's target stack do some fork-related handling and push
   themselves on the fork child's target stack if needed.

   For this purpose, pass the child inferior down to target_follow_fork
   and follow_fork implementations.  This is nullptr if no inferior is
   created for the child, because we want to detach from it.

 - as a result, in follow_fork_inferior, detach from the parent inferior
   (if needed) only after the target_follow_fork call.  This is needed
   because we want to call target_follow_fork before the parent's
   target stack is torn down.

 - hand over to the targets in the parent's target stack (including the
   process target) the responsibility to push themselves, if needed, to
   the child's target stack.  Also hand over the responsibility to the
   process target, at the same time, to create the child's initial
   thread (just like we do for follow_exec).

 - pass the child inferior to exec_on_vfork, so we don't need to swap
   the current inferior between parent and child.  Nothing in
   exec_on_vfork depends on the current inferior, after this change.

   Although this could perhaps be replaced with just having the exec
   target implement follow_fork and push itself in the child's target
   stack, like the process target does... We would just need to make
   sure the process target calls beneath()->follow_fork(...).  I'm not
   sure about this one.

gdb/ChangeLog:

	* target.h (struct target_ops) <follow_fork>: Add inferior*
	parameter.
	(target_follow_fork): Likewise.
	* target.c (default_follow_fork): Likewise.
	(target_follow_fork): Likewise.
	* fbsd-nat.h (class fbsd_nat_target) <follow_fork>: Likewise.
	(fbsd_nat_target::follow_fork): Likewise, and call
	inf_ptrace_target::follow_fork.
	* linux-nat.h (class linux_nat_target) <follow_fork>: Likewise.
	* linux-nat.c (linux_nat_target::follow_fork): Likewise, and
	call inf_ptrace_target::follow_fork.
	* obsd-nat.h (obsd_nat_target) <follow_fork>: Likewise.
	* obsd-nat.c (obsd_nat_target::follow_fork): Likewise, and call
	inf_ptrace_target::follow_fork.
	* remote.c (class remote_target) <follow_fork>: Likewise.
	(remote_target::follow_fork): Likewise, and call
	process_stratum_target::follow_fork.
	* process-stratum-target.h (class process_stratum_target)
	<follow_fork>: New.
	* process-stratum-target.c
	(process_stratum_target::follow_fork): New.
	* target-delegates.c: Re-generate.

[1] https://github.com/ROCm-Developer-Tools/ROCgdb

Change-Id: I460bd0af850f0485e8aed4b24c6d8262a4c69929
2021-08-03 20:26:49 -04:00
Simon Marchi
3a849a3454 gdb: pass child_ptid and fork kind to target_ops::follow_fork
This is a small cleanup I think would be nice, that I spotted while
doing the following patch.

gdb/ChangeLog:

	* target.h (struct target_ops) <follow_fork>: Add ptid and
	target_waitkind parameters.
	(target_follow_fork): Likewise.
	* target.c (default_follow_fork): Likewise.
	(target_follow_fork): Likewise.
	* fbsd-nat.h (class fbsd_nat_target) <follow_fork>: Likewise.
	* 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): Likewise.
	* obsd-nat.h (class obsd_nat_target) <follow_fork>: Likewise.
	* obsd-nat.c (obsd_nat_target::follow_fork): Likewise.
	* remote.c (class remote_target) <follow_fork>: Likewise.
	* target-debug.h (target_debug_print_target_waitkind): New.
	* target-delegates.c: Re-generate.

Change-Id: I5421a542f2e19100a22b74cc333d2b235d0de3c8
2021-07-14 23:21:30 -04:00
Pedro Alves
d7cb0ef35b Fix detach with target remote (PR gdb/28080)
Commit 408f66864a ("detach in all-stop
with threads running") regressed "detach" with "target remote":

 (gdb) detach
 Detaching from program: target:/any/program, process 3671843
 Detaching from process 3671843
 Ending remote debugging.
 [Inferior 1 (process 3671843) detached]
 In main
 terminate called after throwing an instance of 'gdb_exception_error'
 Aborted (core dumped)

Here's the exception above being thrown:

 (top-gdb) bt
 #0  throw_error (error=TARGET_CLOSE_ERROR, fmt=0x555556035588 "Remote connection closed") at src/gdbsupport/common-exceptions.cc:222
 #1  0x0000555555bbaa46 in remote_target::readchar (this=0x555556a11040, timeout=10000) at src/gdb/remote.c:9440
 #2  0x0000555555bbb9e5 in remote_target::getpkt_or_notif_sane_1 (this=0x555556a11040, buf=0x555556a11058, forever=0, expecting_notif=0, is_notif=0x0) at src/gdb/remote.c:9928
 #3  0x0000555555bbbda9 in remote_target::getpkt_sane (this=0x555556a11040, buf=0x555556a11058, forever=0) at src/gdb/remote.c:10030
 #4  0x0000555555bc0e75 in remote_target::remote_hostio_send_command (this=0x555556a11040, command_bytes=13, which_packet=14, remote_errno=0x7fffffffcfd0, attachment=0x0, attachment_len=0x0) at src/gdb/remote.c:12137
 #5  0x0000555555bc1b6c in remote_target::remote_hostio_close (this=0x555556a11040, fd=8, remote_errno=0x7fffffffcfd0) at src/gdb/remote.c:12455
 #6  0x0000555555bc1bb4 in remote_target::fileio_close (During symbol reading: .debug_line address at offset 0x64f417 is 0 [in module build/gdb/gdb]
 this=0x555556a11040, fd=8, remote_errno=0x7fffffffcfd0) at src/gdb/remote.c:12462
 #7  0x0000555555c9274c in target_fileio_close (fd=3, target_errno=0x7fffffffcfd0) at src/gdb/target.c:3365
 #8  0x000055555595a19d in gdb_bfd_iovec_fileio_close (abfd=0x555556b9f8a0, stream=0x555556b11530) at src/gdb/gdb_bfd.c:439
 #9  0x0000555555e09e3f in opncls_bclose (abfd=0x555556b9f8a0) at src/bfd/opncls.c:599
 #10 0x0000555555e0a2c7 in bfd_close_all_done (abfd=0x555556b9f8a0) at src/bfd/opncls.c:847
 #11 0x0000555555e0a27a in bfd_close (abfd=0x555556b9f8a0) at src/bfd/opncls.c:814
 #12 0x000055555595a9d3 in gdb_bfd_close_or_warn (abfd=0x555556b9f8a0) at src/gdb/gdb_bfd.c:626
 #13 0x000055555595ad29 in gdb_bfd_unref (abfd=0x555556b9f8a0) at src/gdb/gdb_bfd.c:715
 #14 0x0000555555ae4730 in objfile::~objfile (this=0x555556515540, __in_chrg=<optimized out>) at src/gdb/objfiles.c:573
 #15 0x0000555555ae955a in std::_Sp_counted_ptr<objfile*, (__gnu_cxx::_Lock_policy)2>::_M_dispose (this=0x555556c20db0) at /usr/include/c++/9/bits/shared_ptr_base.h:377
 #16 0x000055555572b7c8 in std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release (this=0x555556c20db0) at /usr/include/c++/9/bits/shared_ptr_base.h:155
 #17 0x00005555557263c3 in std::__shared_count<(__gnu_cxx::_Lock_policy)2>::~__shared_count (this=0x555556bf0588, __in_chrg=<optimized out>) at /usr/include/c++/9/bits/shared_ptr_base.h:730
 #18 0x0000555555ae745e in std::__shared_ptr<objfile, (__gnu_cxx::_Lock_policy)2>::~__shared_ptr (this=0x555556bf0580, __in_chrg=<optimized out>) at /usr/include/c++/9/bits/shared_ptr_base.h:1169
 #19 0x0000555555ae747e in std::shared_ptr<objfile>::~shared_ptr (this=0x555556bf0580, __in_chrg=<optimized out>) at /usr/include/c++/9/bits/shared_ptr.h:103
 #20 0x0000555555b1c1dc in __gnu_cxx::new_allocator<std::_List_node<std::shared_ptr<objfile> > >::destroy<std::shared_ptr<objfile> > (this=0x5555564cdd60, __p=0x555556bf0580) at /usr/include/c++/9/ext/new_allocator.h:153
 #21 0x0000555555b1bb1d in std::allocator_traits<std::allocator<std::_List_node<std::shared_ptr<objfile> > > >::destroy<std::shared_ptr<objfile> > (__a=..., __p=0x555556bf0580) at /usr/include/c++/9/bits/alloc_traits.h:497
 #22 0x0000555555b1b73e in std::__cxx11::list<std::shared_ptr<objfile>, std::allocator<std::shared_ptr<objfile> > >::_M_erase (this=0x5555564cdd60, __position=std::shared_ptr<objfile> (expired, weak count 1) = {get() = 0x555556515540}) at /usr/include/c++/9/bits/stl_list.h:1921
 #23 0x0000555555b1afeb in std::__cxx11::list<std::shared_ptr<objfile>, std::allocator<std::shared_ptr<objfile> > >::erase (this=0x5555564cdd60, __position=std::shared_ptr<objfile> (expired, weak count 1) = {get() = 0x555556515540}) at /usr/include/c++/9/bits/list.tcc:158
 #24 0x0000555555b19576 in program_space::remove_objfile (this=0x5555564cdd20, objfile=0x555556515540) at src/gdb/progspace.c:210
 #25 0x0000555555ae4502 in objfile::unlink (this=0x555556515540) at src/gdb/objfiles.c:487
 #26 0x0000555555ae5a12 in objfile_purge_solibs () at src/gdb/objfiles.c:875
 #27 0x0000555555c09686 in no_shared_libraries (ignored=0x0, from_tty=1) at src/gdb/solib.c:1236
 #28 0x00005555559e3f5f in detach_command (args=0x0, from_tty=1) at src/gdb/infcmd.c:2769

So frame #28 already detached the remote process, and then we're
purging the shared libraries.  GDB had opened remote shared libraries
via the target: sysroot, so it tries closing them.  GDBserver is
tearing down already, so remote communication breaks down and we close
the remote target and throw TARGET_CLOSE_ERROR.

Note frame #14:

 #14 0x0000555555ae4730 in objfile::~objfile (this=0x555556515540, __in_chrg=<optimized out>) at src/gdb/objfiles.c:573

That's a dtor, thus noexcept.  That's the reason for the
std::terminate.

Stepping back a bit, why do we still have open remote files if we've
managed to detach already, and, we're debugging with "target remote"?
The reason is that commit 408f66864a
makes detach_command hold a reference to the target, so the remote
target won't be finally closed until frame #28 returns.  It's closing
the target that invalidates target file I/O handles.

This commit fixes the issue by not relying on target_close to
invalidate the target file I/O handles, instead invalidate them
immediately in remote_unpush_target.  So when GDB purges the solibs,
and we end up in target_fileio_close (frame #7 above), there's nothing
to do, and we don't try to talk with the remote target anymore.

The regression isn't seen when testing with
--target_board=native-gdbserver, because that does "set sysroot" to
disable the "target:" sysroot, for test run speed reasons.  So this
commit adds a testcase that explicitly tests detach with "set sysroot
target:".

gdb/ChangeLog:
yyyy-mm-dd  Pedro Alves  <pedro@palves.net>

	PR gdb/28080
	* remote.c (remote_unpush_target): Invalidate file I/O target
	handles.
	* target.c (fileio_handles_invalidate_target): Make extern.
	* target.h (fileio_handles_invalidate_target): Declare.

gdb/testsuite/ChangeLog:
yyyy-mm-dd  Pedro Alves  <pedro@palves.net>

	PR gdb/28080
	* gdb.base/detach-sysroot-target.exp: New.
	* gdb.base/detach-sysroot-target.c: New.

Reported-By: Jonah Graham <jonah@kichwacoders.com>

Change-Id: I851234910172f42a1b30e731161376c344d2727d
2021-07-13 15:30:04 +01:00
Tom de Vries
fbf3c4b979 [gdb/tdep] Use pid to choose process 64/32-bitness
In a linux kernel mailing list discussion, it was mentioned that "gdb has
this odd thing where it takes the 64-bit vs 32-bit data for the whole process
from one thread, and picks the worst possible thread to do it (ie explicitly
not even the main thread, ...)" [1].

The picking of the thread is done here in
x86_linux_nat_target::read_description:
...
  /* GNU/Linux LWP ID's are process ID's.  */
  tid = inferior_ptid.lwp ();
  if (tid == 0)
    tid = inferior_ptid.pid (); /* Not a threaded program.  */
...

To understand what this code does, let's investigate a scenario in which
inferior_ptid.lwp () != inferior_ptid.pid ().

Say we start exec jit-attach-pie, identified with pid x.  The main thread
starts another thread that sleeps, and then the main thread waits for the
sleeping thread.  So we have two threads, identified with LWP IDs x and x+1:
...
PID  LWP  CMD
x    x    ./jit-attach-pie
x    x+1  ./jit-attach-pie
...
[ The thread with LWP x is known as the thread group leader. ]

When attaching to this exec using the pid, gdb does a stop_all_threads which
iterates over all the threads, first LWP x, and then LWP x+1.

So the state we arrive with at x86_linux_nat_target::read_description is:
...
(gdb) p inferior_ptid
$1 = {m_pid = x, m_lwp = x+1, m_tid = 0}
...
and consequently we probe 64/32-bitness from thread LWP x+1.

[ Note that this is different from when gdb doesn't attach but instead
launches the exec itself, in which case there's just one thread to begin with,
and consequently the probed thread is LWP x. ]

According to aforementioned remark, a better choice would have been the main
thread, that is, LWP x.

This patch implement that choice, by simply doing:
...
  tid = inferior_ptid.pid ();
...

The fact that gdb makes a per-process permanent choice for 64/32-bitness is a
problem in itself: each thread can be in either 64 or 32 bit mode, and change
forth and back.  That is a problem that this patch doesn't fix.

Now finally: why does this matter in the context of the linux kernel
discussion?  The discussion was related to a patch that exposed io_uring
threads to user-space.  This made it possible that one of those threads would
be picked out to select 64/32-bitness.  Given that such threads are atypical
user-space threads in the sense that they don't return to user-space and don't
have a userspace register state, reading their registers returns garbage, and
so it could f.i. occur that in a 64-bit process with all normal user-space
threads in 64-bit mode, the probing would return 32-bit.

It may be that this is worked-around on the kernel side by providing userspace
register state in those threads such that current gdb is happy.  Nevertheless,
it seems prudent to fix this on the gdb size as well.

Tested on x86_64-linux.

[1] https://lore.kernel.org/io-uring/CAHk-=wh0KoEZXPYMGkfkeVEerSCEF1AiCZSvz9TRrx=Kj74D+Q@mail.gmail.com/

gdb/ChangeLog:

2021-05-23  Tom de Vries  <tdevries@suse.de>

	PR tdep/27822
	* target.h (struct target_ops): Mention target_thread_architecture in
	read_description comment.
	* x86-linux-nat.c (x86_linux_nat_target::read_description): Use
	pid to determine if process is 64-bit or 32-bit.
	* aarch64-linux-nat.c (aarch64_linux_nat_target::read_description):
	Same.
	* ppc-linux-nat.c (ppc_linux_nat_target::read_description): Same.
        * riscv-linux-nat.c (riscv_linux_nat_target::read_description): Same.
	* s390-linux-nat.c (s390_linux_nat_target::read_description): Same.
	* arm-linux-nat.c (arm_linux_nat_target::read_description): Same.
	Likewise, use pid to determine if kernel supports reading VFP
	registers.
2021-05-23 10:08:45 +02:00
Simon Marchi
294c36eb6a gdb: on exec, delegate pushing / unpushing target and adding thread to 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
2021-05-13 15:29:00 -04:00
Simon Marchi
2af87c859f gdb: call target_follow_exec when "set follow-exec-mode" is "same"
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
2021-05-13 15:29:00 -04:00
Simon Marchi
e97007b64a gdb: make target_ops::follow_fork return void
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
2021-04-07 16:57:29 -04:00
Simon Marchi
b4b1a226df gdb: defer commit resume until all available events are consumed
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
2021-03-26 16:02:11 +00:00
Simon Marchi
1192f124a3 gdb: generalize commit_resume, avoid commit-resuming when threads have 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
2021-03-26 15:58:47 +00:00
Simon Marchi
328d42d87e gdb: remove current_top_target function
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
2021-03-24 18:08:24 -04:00
Simon Marchi
d777bf0df2 gdb: move all "current target" wrapper implementations to target.c
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
2021-03-24 18:07:30 -04:00
Luis Machado
dbe692af2d New target methods for memory tagging support
This patch starts adding some of the generic pieces to accomodate memory
tagging.

We have three new target methods:

- supports_memory_tagging: Checks if the target supports memory tagging. This
  defaults to false for targets that don't support memory tagging.

- fetch_memtags: Fetches the allocation tags associated with a particular
  memory range [address, address + length).

  The default is to return 0 without returning any tags. This should only
  be called if memory tagging is supported.

- store_memtags: Stores a set of allocation tags for a particular memory
  range [address, address + length).

  The default is to return 0. This should only
  be called if memory tagging is supported.

gdb/ChangeLog:

2021-03-24  Luis Machado  <luis.machado@linaro.org>

	* remote.c (remote_target) <supports_memory_tagging>: New method
	override.
	<fetch_memtags>: New method override.
	<store_memtags>: New method override.
	(remote_target::supports_memory_tagging): New method.
	(remote_target::fetch_memtags): New method.
	(remote_target::store_memtags): New method.
	* target-delegates.c: Regenerate.
	* target.h (struct target_ops) <supports_memory_tagging>: New virtual
	method.
	<fetch_memtags>: New virtual method.
	<store_memtags>: New virtual method.
	(target_supports_memory_tagging): Define.
	(target_fetch_memtags): Define.
	(target_store_memtags): Define.
	* target-debug.h (target_debug_print_size_t)
	(target_debug_print_const_gdb_byte_vector_r)
	(target_debug_print_gdb_byte_vector_r): New functions.
2021-03-24 14:47:05 -03:00
Simon Marchi
66848ebca8 gdb: remote target_longname
I noticed it was unused.

gdb/ChangeLog:

	* target.h (target_longname): Remove.

Change-Id: Id4f514ea038a6d8d40e11179db587b11793cbbd8
2021-03-23 12:03:37 -04:00
Simon Marchi
c8fbd44a01 gdb: remove target_is_pushed free function
Same principle as the previous patches.

gdb/ChangeLog:

	* target.h (target_is_pushed): Remove, update callers to use
	inferior::target_is_pushed instead.
	* target.c (target_is_pushed): Remove.

Change-Id: I9862e6205acc65672da807cbe4b46cde009e7b9d
2021-03-23 09:50:36 -04:00
Simon Marchi
02980c5645 gdb: remove push_target free functions
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
2021-03-23 09:50:35 -04:00
Simon Marchi
fadf6add30 gdb: remove unpush_target free function
unpush_target unpushes the passed-in target from the current inferior's
target stack.  Calling it is therefore an implicit dependency on the
current global inferior.  Remove that function and make the callers use
the inferior::unpush_target method directly.  This sometimes allows
using the inferior from the context rather than the global current
inferior.

target_unpusher::operator() now needs to be implemented in target.c,
otherwise target.h and inferior.h both need to include each other, and
that wouldn't work.

gdb/ChangeLog:

	* target.h (unpush_target): Remove, update all callers
	to use `inferior::unpush_target` instead.
	(struct target_unpusher) <operator()>: Just declare.
	* target.c (unpush_target): Remove.
	(target_unpusher::operator()): New.

Change-Id: Ia5172dfb3f373e0a75b991885b50322ca2142a8c
2021-03-23 09:50:32 -04:00
Andrew Burgess
336aa7b740 gdb: move get_section_table from exec_target to dummy_target
The only target that implements target_ops::get_section_table in a
meaningful way is exec_target.  This target calls back into the
program space to return the current global section_table.

The global section table is populated whenever the user provides GDB
with an executable, or when a symbol file is loaded, e.g. when a
dynamic library is loaded, or when the user does add-symbol-file.

I recently ran into a situation where a user, debugging a remote
target, was not supplying GDB with a main executable at all.  Instead
the user attached to the target then did add-symbol-file, and then
proceeded to debug the target.

This works fine, but it was noticed that even when
trust-readonly-sections was on GDB was still accessing the target to
get the contents of readonly sections.

The problem is that by not providing an executable there was no
exec_target in the target stack, and so when GDB calls the
target_ops::get_section_table function GDB ends up in
dummy_target::get_section_table, which just returns NULL.

What I want is that even when GDB doesn't have an exec_target in the
target stack, a call to target_ops::get_section_table will still
return the section_table from the current program space.

When considering how to achieve this my first though was, why is the
request for the section table going via the target stack at all?  The
set of sections loaded is a property of the program space, not the
target.  This is, after all, why the data is being stored in the
program space.

So I initially tried changing target_get_section_table so that,
instead of calling into the target it just returns
current_program_space->target_sections ().

This would be fine except for one issue, target_bfd (from
bfd-target.c).  This code is used from solib-svr4.c to create a
temporary target_ops structure that implements two functions
target_bfd::xfer_partial and target_bfd::get_section_table.

The purpose behind the code is to enable two targets, ppc64 and frv to
decode function descriptors from the dynamic linker, based on the
non-relocated addresses from within the dynamic linker bfd object.

Both of the implemented functions in target_bfd rely on the target_bfd
object holding a section table, and the ppc64 target requires that the
target_bfd implement ::get_section_table.

The frv target doesn't require ::get_section_table, instead it
requires the ::xfer_partial.  We could in theory change the ppc64
target to use the same approach as frv, however, this would be a bad
idea.  I believe that the frv target approach is broken.  I'll
explain:

The frv target calls get_target_memory_unsigned to read the function
descriptor.  The address being read is the non-relocated address read
from the dynamic linker in solib-srv4.c:enable_break.  Calling
get_target_memory_unsigned eventually ends up in target_xfer_partial
with an object type of TARGET_OBJECT_RAW_MEMORY.  This will then call
memory_xfer_check_region.  I believe that it is quite possible that a
the non-relocated addresses pulled from the dynamic linker could be in
a memory region that is not readable, while the relocated addresses
are in a readable memory region.  If this was ever the case for the
frv target then GDB would reject the attempt to read the non-relocated
function pointer.

In contrast the ppc64 target calls target_section_by_addr, which calls
target_get_section_table, which then calls the ::get_section_table
function on the target.

Thus, when reflecting on target_bfd we see two functions,
::xfer_partial and ::get_section_table.  The former is required by the
frv target, but that target is (I think) potentially broken.  While
the latter is required by the ppc64 target, but this forces
::get_section_table to exist as a target_ops member function.

So my original plan, have target_get_section_table NOT call a
target_ops member function appears to be flawed.

My next idea was to remove exec_target::get_section_table, and instead
move the implementation into dummy_target::get_section_table.
Currently the dummy_target implementation always returns NULL
indicating no section table, but plenty of other dummy_target member
functions do more than just return null values.

So now, dummy_target::get_section_table returns the section table from
the current program space.  This allows target_bfd to remain
unchanged, so ppc64 and frv should not be affected.

Making this change removes the requirement for the user to provide an
executable, GDB can now always access the section_table, as the
dummy_target always exists in the target stack.

Finally, there's a test that the target_section table is not empty in
the case where the user does add-symbol-file without providing an
executable.

gdb/ChangeLog:

	* exec.c (exec_target::get_section_table): Delete member function.
	(section_table_read_available_memory): Use current_top_target, not
	just the exec_ops target.
	* target-delegates.c: Regenerate.
	* target.c (default_get_section_table): New function.
	* target.h (target_ops::get_section_table): Change default
	behaviour to call default_get_section_table.
	(default_get_section_table): Declare.
2021-02-24 16:58:04 +00:00
Andrew Burgess
19cf757a87 gdb: spread a little 'const' through the target_section_table code
The code to access the target section table can be made more const, so
lets do that.  There should be no user visible changes after this
commit.

gdb/ChangeLog:

	* gdb/bfd-target.c (class target_bfd) <get_section_table>: Make
	return type const.
	* gdb/exec.c (struct exec_target) <get_section_table>: Likewise.
	(section_table_read_available_memory): Make local const.
	(exec_target::xfer_partial): Make local const.
	(print_section_info): Make parameter const.
	* gdb/exec.h (print_section_info): Likewise.
	* gdb/ppc64-tdep.c (ppc64_convert_from_func_ptr_addr): Make local
	const.
	* gdb/record-btrace.c (record_btrace_target::xfer_partial):
	Likewise.
	* gdb/remote.c (remote_target::remote_xfer_live_readonly_partial):
	Likewise.
	* gdb/s390-tdep.c (s390_load): Likewise.
	* gdb/solib-dsbt.c (scan_dyntag): Likewise.
	* gdb/solib-svr4.c (scan_dyntag): Likewise.
	* gdb/target-debug.h (target_debug_print_target_section_table_p):
	Rename to...
	(target_debug_print_const_target_section_table_p): ...this.
	* gdb/target-delegates.c: Regenerate.
	* gdb/target.c (target_get_section_table): Make return type const.
	(target_section_by_addr): Likewise.  Also make some locals const.
	(memory_xfer_partial_1): Make some locals const.
	* gdb/target.h (struct target_ops) <get_section_table>: Make
	return type const.
	(target_section_by_addr): Likewise.
	(target_get_section_table): Likewise.
2021-02-24 16:58:02 +00:00
Simon Marchi
de146e1946 gdb: push target earlier in procfs_target::attach (PR 27435)
Since this is a GDB 9 -> 10 regression, I would like to push it to
gdb-10-branch.

This is a follow-up to:

  https://sourceware.org/pipermail/gdb-patches/2021-February/176202.html

This patch fixes a segfault seen when attaching to a process on Solaris.
The steps leading to the segfault are:

 - procfs_target::attach calls do_attach, at this point the inferior's
   process slot in the target stack is empty.
 - do_attach adds a thread with `add_thread (&the_procfs_target, ptid)`
 - in add_thread_silent, the passed target (&the_procfs_target) is
   passed to find_inferior_ptid
 - find_inferior_ptid returns nullptr, as there is no inferior with this
   ptid that has &the_procfs_target as its process target
 - the nullptr `inf` is passed to find_thread_ptid, which dereferences
   it, causing a segfault
 - back in procfs_target::attach, after do_attach, we push the
   the_procfs_target on the inferior's target stack, although we never
   reach this because the segfault happens before.

To fix this, I think we need to do the same as is done in
inf_ptrace_target::attach: push the target early and unpush it in case
the attach fails (and keep it if the attach succeeds).

Implement it by moving target_unpush_up to target.h, so it can be
re-used here.  Make procfs_target::attach use it.  Note that just like
is mentioned in inf_ptrace_target::attach, we should push the target
before calling target_pid_to_str, so that calling target_pid_to_str ends
up in procfs_target::pid_to_str.

Tested by trying to attach on a process on gcc211 on the gcc compile
farm.

gdb/ChangeLog:

	PR gdb/27435
	* inf-ptrace.c (struct target_unpusher): Move to target.h.
	(target_unpush_up): Likewise.
	* procfs.c (procfs_target::attach): Push target early.  Use
	target_unpush_up to unpush target in case of error.
	* target.h (struct target_unpusher): Move here.
	(target_unpush_up): Likewise.

Change-Id: I88aff8b20204e1ca1d792e27ac6bc34fc1aa0d52
2021-02-22 11:41:32 -05:00
Simon Marchi
6ff267e186 gdb: make target_is_non_stop_p return bool
gdb/ChangeLog:

	* target.c (target_is_non_stop_p): Return bool.
	* target.h (target_is_non_stop_p): Return bool.

Change-Id: Icdb37ffe917798e59b822976794d4b1b7aafd709
2021-02-04 15:47:28 -05:00
Pedro Alves
e87f0fe823 detach and breakpoint removal
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 sometimes fails with the inferior crashing
with SIGTRAP after the detach because of the bug fixed by this patch,
when tested with the native target.

The problem is that target_detach removes breakpoints from the target
immediately, and that does not work with the native GNU/Linux target
(and probably no other native target) currently.  The test wouldn't
fail with this issue when testing against gdbserver, because gdbserver
does allow accessing memory while the current thread is running, by
transparently pausing all threads temporarily, without GDB noticing.
Implementing that in gdbserver was a lot of work, so I'm not looking
forward right now to do the same in the native target.  Instead, I
came up with a simpler solution -- push the breakpoints removal down
to the targets.  The Linux target conveniently already pauses all
threads before detaching them, since PTRACE_DETACH only works with
stopped threads, so we move removing breakpoints to after that.  Only
the remote and GNU/Linux targets support support async execution, so
no other target should really need this.

gdb/ChangeLog:

	* linux-nat.c (linux_nat_target::detach): Remove breakpoints
	here...
	* remote.c (remote_target::remote_detach_1): ... and here ...
	* target.c (target_detach): ... instead of here.
	* target.h (target_ops::detach): Add comment.
2021-02-03 01:15:12 +00:00
Simon Marchi
cda09ec9f9 gdb: move remote_debug to remote.{h,c}
remote_debug is currently declared in target.h and defined in top.c.
Move them to remote.h and remote.c.

Include remote.h in remote-sim.c, as it uses remote_debug.

gdb/ChangeLog:

	* target.h (remote_debug): Move to...
	* remote.h (remote_debug): ... here.
	* top.c (remote_debug): Move to...
	* remote.c (remote_debug): ... here.
	* remote-sim.c: Include remote.h.

Change-Id: Iae632d12ff8900b23eee6b2529d6a3cd339a8caa
2021-01-22 12:39:08 -05:00
Simon Marchi
16e9019ef7 gdb: move baud_rate and serial_parity declarations to serial.h
They are currently in target.h, it would make more sense to have them in
serial.h, since they are defined in serial.c.

gdb/ChangeLog:

	* target.h (baud_rate, serial_parity): Move declarations...
	* serial.h: ... here.
	* main.c: Include serial.h.
	* serial.c (baud_rate, serial_parity): Update doc.

Change-Id: Idc983c154c80ccc29b07ce68df3483cefe03fb71
2021-01-12 14:19:49 -05:00
Joel Brobecker
3666a04883 Update copyright year range in all GDB files
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.
2021-01-01 12:12:21 +04:00
Simon Marchi
dda83cd783 gdb, gdbserver, gdbsupport: fix leading space vs tabs issues
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
2020-11-02 10:28:45 -05:00
Simon Marchi
24f5300a53 gdb: make target_ops::make_corefile_notes return a unique ptr
Since we converted gdbarch_make_corefile_notes to returning a
gdb::unique_xmalloc_ptr, I figured it would make sense to converted
target_ops::make_corefile_notes as well.

The only implementation of that is in procfs.c, and it should ideally be
re-written as a gdbarch method (see comment in write_gcore_file_1), but
in the mean time I guess it doesn't hurt to throw some unique pointer at
it.

I tested that it builds on Solaris 11 (gcc compile farm machine gcc211),
but I am not able to test it, because I can't get GDB to start a
process (I'll look at that separately).

gdb/ChangeLog:

	* target.h (struct target_ops) <make_corefile_notes>:
	Change return type to unique pointer.
	* target.c (dummy_make_corefile_notes): Likewise.
	* exec.c (struct exec_target) <make_corefile_notes>:
	Likewise.
	(exec_target::make_corefile_notes): Likewise.
	* procfs.c (class procfs_target) <make_corefile_notes>:
	Likewise.
	(procfs_do_thread_registers): Adjust to unique pointer.
	(struct procfs_corefile_thread_data): Add constructor.
	<note_data>: Change type to unique pointer.
	(procfs_corefile_thread_callback): Adjust to unique pointer.
	(procfs_target::make_corefile_notes): Change return type to
	unique pointer.
	* target-delegates.c: Re-generate.
	* gcore.c (write_gcore_file_1): Adjust.
	* target-debug.h (target_debug_print_gdb_unique_xmalloc_ptr_char):
	New.

Change-Id: I768fb17ac0f7adc67d2fe95e952c784fe0ac37ab
2020-10-22 12:58:21 -04:00
Tom Tromey
d7a78e5c41 Change target_section_table to std::vector alias
Because target_section_table only holds a vector, and because it is
used in an "open" way, this patch makes it just be an alias for the
std::vector specialization.  This makes the code less wordy.  If we do
ever want to add more specialized behavior to this type, it's simple
enough to convert it back to a struct with the few needed methods
implied by this change.

gdb/ChangeLog
2020-10-12  Tom Tromey  <tom@tromey.com>

	* target.h (struct target_ops) <get_section_table>: Update.
	(target_get_section_table): Update.
	* target.c (target_get_section_table, target_section_by_addr)
	(memory_xfer_partial_1): Update.
	* target-section.h (target_section_table): Now an alias.
	* target-delegates.c: Rebuild.
	* target-debug.h (target_debug_print_target_section_table_p):
	Rename from target_debug_print_struct_target_section_table_p.
	* symfile.c (build_section_addr_info_from_section_table): Update.
	* solib.c (solib_map_sections, solib_contains_address_p): Update.
	* solib-svr4.c (scan_dyntag): Update.
	* solib-dsbt.c (scan_dyntag): Update.
	* remote.c (remote_target::remote_xfer_live_readonly_partial):
	Update.
	* record-full.c (record_full_core_target::xfer_partial): Update.
	* progspace.h (struct program_space) <target_sections>: Update.
	* exec.h (print_section_info): Update.
	* exec.c (exec_target::close, build_section_table)
	(add_target_sections, add_target_sections_of_objfile)
	(remove_target_sections, exec_on_vfork)
	(section_table_available_memory)
	(section_table_xfer_memory_partial)
	(exec_target::get_section_table, exec_target::xfer_partial)
	(print_section_info, set_section_command)
	(exec_set_section_address, exec_target::has_memory): Update.
	* corelow.c (core_target::build_file_mappings)
	(core_target::xfer_partial, core_target::info_proc_mappings)
	(core_target::info_proc_mappings): Update.
	* bfd-target.c (class target_bfd): Update
2020-10-12 20:18:48 -06:00
Tom Tromey
7b466b1024 Introduce target-section.h
This introduces a new target-section.h file.  This makes some of the
later patches in this series a bit cleaner, because new includes of
target.h won't be required.  Also I think it's better to have small
header files for each separate data structure.

gdb/ChangeLog
2020-10-12  Tom Tromey  <tom@tromey.com>

	* target.h (struct target_section, struct target_section_table):
	Move to target-section.h.
	* target-section.h: New file.
2020-10-12 20:18:47 -06:00
Tom Tromey
4a72de7366 Move simple_search_memory to gdbsupport/search.cc
This moves the simple_search_memory function to a new file,
gdbsupport/search.cc.  The API is slightly changed to make it more
general.  This generality is useful for wiring it to gdbserver, and
also for unit testing.

gdb/ChangeLog
2020-10-07  Tom Tromey  <tromey@adacore.com>

	* target.h (simple_search_memory): Don't declare.
	* target.c (simple_search_memory): Move to gdbsupport.
	(default_search_memory): Update.
	* remote.c (remote_target::search_memory): Update.

gdbsupport/ChangeLog
2020-10-07  Tom Tromey  <tromey@adacore.com>

	* Makefile.in: Rebuild.
	* Makefile.am (libgdbsupport_a_SOURCES): Add search.cc.
	* search.h: New file.
	* search.cc: New file.
2020-10-07 12:07:55 -06:00
Tom Tromey
9aed480c3a Turn target_have_steppable_watchpoint into function
This changes the object-like macro target_have_steppable_watchpoint
into an inline function.

gdb/ChangeLog
2020-09-28  Tom Tromey  <tom@tromey.com>

	* infrun.c (displaced_step_fixup, thread_still_needs_step_over)
	(handle_signal_stop): Update.
	* procfs.c (procfs_target::insert_watchpoint): Update.
	* target.h (target_have_steppable_watchpoint): Now a function.
2020-09-28 19:52:21 -06:00
Tom Tromey
8a3ecb79b0 Turn target_can_lock_scheduler into a function
This changes the object-like macro target_can_lock_scheduler into an
inline function.

gdb/ChangeLog
2020-09-28  Tom Tromey  <tom@tromey.com>

	* infrun.c (set_schedlock_func): Update.
	* target.h (target_can_lock_scheduler): Now a function.
2020-09-28 19:52:21 -06:00