A co-worker pointed out that gdb's DAP implementation might return an
integer for the name of a stack frame, like:
{"id": 1, "name": 93824992310799, ...}
This can be seen currently in the logs of the bt-nodebug.exp test
case.
What is happening is that FrameDecorator falls back on returning the
PC when the frame's function symbol cannot be found, relying on the
gdb core to look up the minsym and print its name.
This can actually yield the wrong answer sometimes, because it falls
into the get_frame_pc / get_frame_address_in_block problem -- if the
frame is at a call to a noreturn function, the PC in this case might
appear to be in the next function in memory. For more on this, see:
https://sourceware.org/bugzilla/show_bug.cgi?id=8416
and related bugs.
However, there's a different approach we can take: the code here can
simply use Frame.name. This handles the PC problem correctly, and
gets us the information we need.
DAP requires python module typing, which is supported starting python 3.5.
Make this formal by:
- disabling the dap interpreter for python version < 3.5
- returning 0 in allow_dap_tests for python version < 3.5
Approved-By: Tom Tromey <tom@tromey.com>
PR dap/30708
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30708
This implements the DAP "source" request. I renamed the
"loadedSources" function from "sources" to "loaded_sources" to avoid
any confusion. I also moved the loadedSources test to the new
sources.exp.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30691
This changes the gdb DAP implementation to emit a real
sourceReference, rather than emitting 0. Sources are tracked in some
maps in sources.py, and a new helper function is introduced to compute
the "Source" object that can be sent to the client.
The DAP 'module' event may include a 'path' component. I noticed that
this is supplied even when the module in question does not come from a
file.
This patch only emits this field when the objfile corresponds to a
real file.
No test case, because I wasn't sure how to write a portable one.
However, it's clear from gdb.log on Linux:
{"type": "event", "event": "module", "body": {"reason": "new", "module": {"id": "system-supplied DSO at 0x7ffff7fc4000", "name": "system-supplied DSO at 0x7ffff7fc4000"}}, "seq": 21}
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30676
This patch implements ValueFormat for DAP. Currently this only means
supporting "hex".
Note that StackFrameFormat is defined to have many more options, but
none are currently recognized. It isn't entirely clear how these
should be handled. I'll file a new gdb bug for this, and perhaps an
upstream DAP bug as well.
New in v2:
- I realized that the "hover" context was broken, and furthermore
that we only had tests for "hover" failing, not for it succeeding.
This version fixes the oversight and adds a test.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30469
I noticed that the support for memoryReference in the "variables"
output is gated on the client "supportsMemoryReferences" capability.
This patch implements this and makes some other changes to the DAP
memory reference code:
* Remove the memoryReference special case from _SetResult.
Upstream DAP fixed this oversight in response to
https://github.com/microsoft/debug-adapter-protocol/issues/414
* Don't use the address of a variable as its memoryReference -- only
emit this for pointer types. There's no spec support for the
previous approach.
* Use strip_typedefs to handle typedefs of pointers.
This implements the DAP 'terminated' event. Vladimir Makaev noticed
that VSCode will not report the debug session as over unless this is
sent.
It's not completely clear when exactly this event ought to be sent.
Here I've done it when the inferior exits.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30681
When the DAP client sets a breakpoint, gdb currently sends a "new
breakpoint" event. However, Vladimir Makaev discovered that this
causes VSCode to think there are two breakpoints.
This patch changes gdb to suppress the event in this case.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30678
A subsequent patch will add the ability to suppress breakpoint events
to DAP. My first attempt at this ended up with recurse imports,
causing Python failures. So, this patch moves all the DAP breakpoint
event code to breakpoint.py in preparation for the change.
I've renamed breakpoint_descriptor here as well, because it can now be
private to breakpoint.py.
Vladimir Makaev noticed that, in some cases, a DAP stackTrace response
would include a relative path name for the "path" component.
This patch changes the frame decorator code to add a new DAP-specific
decorator, and changes the DAP entry point to frame filters to use it.
This decorator prefers the symtab's full name, and does not fall back
to the solib's name.
I'm not entirely happy with this patch, because if a user frame filter
uses FrameDecorator, it may still do the wrong thing. It would be
better to have frame filters return symtab-like objects instead, or to
have a separate method to return the full path to the source file.
I also tend to think that the solib fallback behavior of
FrameDecorator is a mistake. If this is ever needed, it seems to me
that it should be a separate method.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30665
In Python, a member name starting with "__" is specially handled to
make it "more private" to the class -- it isn't truly private, but it
is renamed to make it less likely to be reused by mistake. This patch
ensures that this is done for the private method of FrameDecorator.
Tom de Vries found a bug where, sometimes, a SIGCHLD would be
delivered to a non-main thread, wreaking havoc.
The problem is that gdb.block_signals after first blocking a set of
signals, then unblocked the same set rather than restoring the initial
situation. This function being called from the DAP thread lead to
SIGCHLD being unblocked there.
This patch fixes the problem by restoring the previous set of signals
instead.
Tested-by: Tom de Vries <tdevries@suse.de>
Reviewed-By: Tom de Vries <tdevries@suse.de>
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30680
Tom de Vries filed a bug about an intermittent gdb DAP failure -- and
coincidentally, at the same time, Alexandra Hájková sent email about a
somewhat similar failure.
After looking into this for a while (with no results) using ASan and
valgrind, I found that setting PYTHONMALLOC=malloc_debug found the bug
instantly.
The problem is that gdbpy_parse_and_eval releases the GIL while
calling parse_and_eval, but fails to re-acquire it before calling
value_to_value_object. This is easily fixed by introducing a new
scope.
I wonder whether the test suite should unconditionally set
PYTHONMALLOC=malloc_debug.
Tested-by: Tom de Vries <tdevries@suse.de>
Reviewed-By: Tom de Vries <tdevries@suse.de>
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30686
While looking at the DAP spec, I noticed that the breakpointLocations
request is gated behind a capability. This patch changes gdb to
report this capability.
I've also added a comment to explain the fact that arguments to
breakpointLocations are not optional, even though the spec says they
are.
In gdb/tui/tui-layout.c, we have:
...
static window_types_map known_window_types;
...
and in gdb/python/py-tui.c:
...
/* A global list of all gdbpy_tui_window_maker objects. */
static intrusive_list<gdbpy_tui_window_maker> m_window_maker_list;
};
/* See comment in class declaration above. */
intrusive_list<gdbpy_tui_window_maker>
gdbpy_tui_window_maker::m_window_maker_list;
...
With a gdb build with -O0 or -O2, the static destructor calling order seems to be:
- first gdb/tui/tui-layout.c,
- then gdb/python/py-tui.c.
So when running test-case gdb.python/tui-window-factory.exp, we see the
following order of events:
- the destructor for known_window_types is called, which triggers calling the
destructor for the only element E of m_window_maker_list. The destructor
destroys E, and also removes E from m_window_maker_list, leaving it empty.
- the destructor for m_window_maker_list is called. It's empty, so it's a nop.
However, when building gdb with -O2 -flto=auto, the static destructor calling
order seems to be reversed.
Instead, we have these events:
- the destructor for m_window_maker_list is called. This doesn't destroy it's
only element E, but it does make m_window_maker_list empty.
- the destructor for known_window_types is called, which triggers calling the
destructor for E. An attempt is done to remove E from m_window_maker_list,
but we run into an assertion failure, because the list is empty.
Fix this by checking is_linked () before attempting to remove from
m_window_maker_list, similar to how things were addressed in commit 995a34b177
("Guard against frame.c destructors running before frame-info.c's").
Tested on x86_64-linux.
PR tui/30646
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30646
The DAP start_thread helper function has a 'name' parameter that is
unused. Apparently I forgot to hook it up to the thread constructor.
This patch fixes the oversight.
While working on an experiment, I realized that I needed the DAP
block_signals function. I figured other developers may need it as
well, so this patch moves it from DAP to the gdb module and exports
it.
I also added a new subclass of threading.Thread that ensures that
signals are blocked in the new thread.
Finally, this patch slightly rearranges the documentation so that
gdb-side threading issues and functions are all discussed in a single
node.
This adds a new objfile_for_address method to gdb.Progspace. This
makes it easy to find the objfile for a given address.
There's a related PR; and while this change would have been sufficient
for my original need, it's not clear to me whether I should close the
bug. Nevertheless I think it makes sense to at least mention it here.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=19288
Reviewed-By: Eli Zaretskii <eliz@gnu.org>
I noticed an unused import in dap/evaluate.py; and also I found out
that my recent changes to use frame filters from DAP left some unused
imports in dap/bt.py.
The DAP disassemble command lets the client return the underlying
bytes of the instruction in an implementation-defined format. This
patch updates gdb to return this, and simply uses a hex string of the
bytes as the format.
Reviewed-By: Eli Zaretskii <eliz@gnu.org>
Andrew reported that the previous change to gdb.Inferior.read_memory &
friends introducing scoped_restore_current_inferior_for_memory broke
gdb.dap/stop-at-main.exp. This is also reported as PR dap/30644.
The root of the problem is that all the methods that now use
scoped_restore_current_inferior_for_memory cause GDB to crash with a
failed assert if they are run on an inferior that is not yet started.
E.g.:
(gdb) python i = gdb.selected_inferior ()
(gdb) python i.read_memory (4,4)
gdb/thread.c:626: internal-error: any_thread_of_inferior: Assertion `inf->pid != 0' failed.
This patch fixes the problem by removing
scoped_restore_current_inferior_for_memory's ctor ptid parameter and
the any_thread_of_inferior calls completely, and making
scoped_restore_current_inferior_for_memory switch inferior_ptid to a
pid ptid.
I was a little worried that some port might be assuming inferior_ptid
points at a thread in the xfer_partial memory access routines. We
know that anything that supports forks must not assume that, due to
how detach_breakpoints works. I looked at a number of xfer_partial
implementations, and didn't see anything that is looking at
inferior_ptid in a way that would misbehave. I'm thinking that we
could go forward with this and just fix ports if they break.
While on some ports like on AMD GPU we have thread-specific address
spaces, and so when accessing memory for those address spaces, we must
have the right thread context (via inferior_ptid) selected, in
Inferior.read_memory, we only have the inferior to work with, so this
API as is can't be used to access thread-specific address spaces.
IOW, it can only be used to access the global address space that is
visible to both the CPU and the GPUs.
In proc-service.c:ps_xfer_memory, the other spot using
scoped_restore_current_inferior_for_memory, we're always accessing
per-inferior memory.
If we end up using scoped_restore_current_inferior_for_memory later to
set up the context to read memory from a specific thread, then we can
add an alternative ctor that takes a thread_info pointer, and make
inferior_ptid point to the thread, for example.
New test added to gdb.python/py-inferior.exp, exercising
Inferior.read_memory without execution.
No regressions on native and extended-gdbserver x86_64 GNU/Linux.
Reviewed-By: Tom Tromey <tom@tromey.com>
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30644
Change-Id: I11309c5ddbbb51a4594cf63c21b3858bfd9aed19
This commit builds on this earlier work:
commit 9fe01a376b
Date: Thu Jun 29 11:26:55 2023 -0600
Update TUI window title when changed
and makes tui_win_info::title private, renaming to m_title at the same
time. There's a new tui_win_info::title() member function to provide
read-only access to the title.
There should be no user visible changes after this commit.
Approved-By: Tom Tromey <tom@tromey.com>
A user noticed that Inferior.read_memory and a few other Python APIs
will always use the currently selected inferior, not the one passed to
the call.
This patch fixes the bug by arranging to switch to the inferior. I
found this same issue in several APIs, so this fixes them all.
I also added a few missing calls to INFPY_REQUIRE_VALID to these
methods.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30615
Approved-By: Pedro Alves <pedro@palves.net>
I wrote a TUI window in Python, and I noticed that setting its title
did not result in a refresh, so the new title did not appear. This
patch corrects this problem.
The no-ops pretty-printers that were introduced for DAP have a classic
gdb bug: they neglect to call check_typedef. This will cause some
strange behavior; for example not showing the children of a variable
whose type is a typedef of a structure type. This patch fixes the
oversight.
This reimplements DAP stack traces using frame filters. This slightly
simplifies the code, because frame filters and DAP were already doing
some similar work. This also renames RegisterReference and
ScopeReference to make it clear that these are private (and so changes
don't have to worry about other files).
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30468
The frame decorator "FrameVars" code misses a couple of cases,
discovered when working on related DAP changes.
First, fetch_frame_locals does not stop when reaching a function
boundary. This means it would return locals from any enclosing
functions.
Second, fetch_frame_args assumes that all arguments are at the
outermost scope, but this doesn't seem to be required by gdb.
This patch adds a new function, frame_iterator, that wraps the
existing code to find and execute the frame filters. However, unlike
execute_frame_filters, it will always return an iterator -- whereas
execute_frame_filters will return None if no frame filters apply.
Nothing uses this new function yet, but it will used by a subsequent
DAP patch.
When reading the doc string for execute_frame_filters, I wasn't sure
if the ranges were inclusive or exclusive. This patch updates the doc
string to reflect my findings, and also fixes an existing typo.
A co-worker, Andry, noticed that the DAP setExpression implementation
returned the wrong fields -- it used "result" rather than "value", and
included "memoryReference", which isn't in the spec (an odd oversight,
IMO).
This patch fixes the problems.
Only a few types in the Python API currently have __repr__()
implementations. This patch adds a few more of them. specifically: it
adds __repr__() implementations to gdb.Symbol, gdb.Architecture,
gdb.Block, gdb.Breakpoint, gdb.BreakpointLocation, and gdb.Type.
This makes it easier to play around the GDB Python API in the Python
interpreter session invoked with the 'pi' command in GDB, giving more
easily accessible tipe information to users.
An example of how this would look like:
(gdb) pi
>> gdb.lookup_type("char")
<gdb.Type code=TYPE_CODE_INT name=char>
>> gdb.lookup_global_symbol("main")
<gdb.Symbol print_name=main>
The gdb.Block.__repr__() method shows the first 5 symbols from the
block, and then a message to show how many more were elided (if any).
PyModule_AddObject steals a reference on success, but not on error,
which is why we have gdb_pymodule_addobject. I found one spot still
calling the former, which could in theory leak memory on failure.
This patch fixes this.
In the same function I found an unchecked call to
PyDict_SetItemString. This patch fixes this as well.
Approved-By: Andrew Burgess <aburgess@redhat.com>
I found some Python 2 compatibility code in gdb's Python library.
There's no need for this any more, so this removes it. There is still
a bit more of this remaining in __init__.py, but I haven't tried
removing that yet.
Reviewed-By: Bruno Larsen <blarsen@redhat.com>
A DAP client can request that an expression be evaluated in "hover"
context, meaning that it should not cause side effects. In gdb, this
can be implemented by temporarily setting a few "may-" parameters to
"off".
In order to make this work, I had to also change "may-write-registers"
so that it can be changed while the program is running. I don't think
there was any reason for this prohibition in the first place.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30476
DAP allows a source breakpoint to specify a log message. When this is
done, the breakpoint acts more like gdb's dprintf: it logs a message
but does not cause a stop.
I looked into implement this using dprintf with the new %V printf
format. However, my initial attempt at this did not work, because
when the inferior is continued, the dprintf output is captured by the
gdb.execute call. Maybe this could be fixed by having all
inferior-continuation commands use the "&" form; the main benefit of
this would be that expressions are only parsed a single time.
A bug report about the supportsVariablePaging capability in DAP
resulted in a clarification: when this capability is not present, DAP
implementations should ignore the paging parameters to the "variables"
request. This patch implements this clarification.
I realized that with a small refactoring, it is possible to type-check
the parameters to the various DAP breakpoint requests. This would
have caught the earlier bug involving hitCondition.
When creating a DAP breakpoint, a failure should be returned by
setting "verified" to False. gdb didn't properly implement this, and
there was a FIXME comment to this effect. This patch fixes the
problem.
The DAP breakpoint code tries to reuse a breakpoint when possible.
Currently it uses the condition and the hit condition (aka ignore
count) when making this determination. However, these attributes are
just going to be reset anyway, so this patch changes the code to
exclude these from the reuse decision.
DAP specifies a breakpoint's hitCondition as a string, meaning it is
an expression to be evaluated. However, gdb implemented this as if it
were an integer instead. This patch fixes this oversight.
v2.
EvaluateResult does not need a name, just as what Tom pointed out in
previous review. It's only the *children* that need to be made sure that
their names are valid. An identifier for a variable, can't ever have an
integer as a name, anyhow (not as far as I am aware, no programming
languages allow for that).
Removed the f-strings and use str() instead as pointed out that
f-strings might not be supported fully.
v1.
This patch fixes a few bugs.
First of all, name of VariableReferences must always be of string type.
This patch makes sure that this is the case by formatting the name. If
(when) the name is an integer, this will cause clients to fail or throw
errors.
Fixes a bug in NoOpArrayPrinter that calculated children to be N, but
only ever retrieves N-1 children, which makes Python at some time later
(during fetch_children -> fetch_one_child(N) ) raise an exception (out
of list index) which makes the entire request go bad.
The result[self.result_name] also f-strings the printer.to_string()
value, because this can potentially be a LazyString (which is a Python
object, not a string) and is not serializable by json.dumps.
Approved-By: Tom Tromey <tom@tromey.com>
