They are unreachable because there is a return statement in all possible code
paths before.
Change-Id: I9db2f41f8017380c0c79f97af9bbf8734038ba9a
Approved-By: Tom Tromey <tom@tromey.com>
This changes gdbpy_registry::lookup to return a gdbpy_ref<>, using the
type system to convey that a new reference is always returned.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
This changes gdbarch_to_arch_object to return a gdbpy_ref<>,
using the type system to convey that a new reference is always
returned.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
This changes symtab_to_linetable_object to return a gdbpy_ref<>,
using the type system to convey that a new reference is always
returned.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
This changes frame_info_to_frame_object to return a gdbpy_ref<>,
using the type system to convey that a new reference is always
returned.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
This changes type_to_type_object to return a gdbpy_ref<>,
using the type system to convey that a new reference is always
returned.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
This changes value_to_value_object to return a gdbpy_ref<>,
using the type system to convey that a new reference is always
returned.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
This changes block_to_block_object to return a gdbpy_ref<>,
using the type system to convey that a new reference is always
returned.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
This changes symtab_to_symtab_object to return a gdbpy_ref<>,
using the type system to convey that a new reference is always
returned.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
This changes symbol_to_symbol_object to return a gdbpy_ref<>,
using the type system to convey that a new reference is always
returned.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
This changes symtab_and_line_to_sal_object to return a gdbpy_ref<>,
using the type system to convey that a new reference is always
returned.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
This commit removes some uses of gdb::argv_vec from GDB.
The gdb::argv_vec class exists in part due to our need to convert from
one container type to another in a few places, and I think by using
templates we can push this conversion down into
gdbsupport/common-inferior.{cc,h} and remove the conversion logic from
higher level code in GDB. This should make core GDB simpler.
For examples of this simplification, see python/py-inferior.c,
remote.c, and unittests/remote-arg-selftests.c, where this commit has
allowed for the removal of some code that only exists in order to
convert the container type.
Ideally I'd like to see all uses of gdb::argv_vec removed, but I'm
still not sure about the use in nat/fork-inferior.c, I think its use
there might be the best solution, so for now at least, I have no plans
to touch that code.
There should be no user visible changes after this commit.
Approved-By: Tom Tromey <tom@tromey.com>
We have many times the pattern:
current_source_location *loc
= current_source_key.get (pspace);
if (loc == nullptr)
loc = current_source_key.emplace (pspace);
return loc;
I thought it would be nice to have it directly part of registry::key.
Add a try_emplace method, which is like emplace, except that it returns
the existing data, if any. The try_emplace name comes from similar
methods in std::map or gdb::unordered_map
(ankerl::unordered_dense::map).
Replace as many callers as I could find to use it.
Change-Id: I21f922ca065a354f041caaf70484d6cfbcbb76ed
Approved-By: Tom Tromey <tom@tromey.com>
Since we use C++ and not C, I think that we should gradually move to
using references for things that can never be nullptr. One example of
this is the return value of the emplace method.
Change it to return a reference, and (to keep the patch straightforward)
update all callers to take the address. More patches could follow to
propagate the use of references further.
Change-Id: I725539694cf496f8288918cc29d7aaae9aca2292
Approved-By: Tom Tromey <tom@tromey.com>
This patch changes how gdb output redirection is done.
Currently, output is done via the UI. gdb_stdout, for example, is a
define the expands to an lvalue referencing a field in the current UI.
When redirecting, this field may temporarily be reset; and when
logging is enabled or disabled, this is also done.
This has lead to bugs where the combination of redirection and logging
results in use-after-free. Crashes are readily observable; see the
new test cases.
This patch upends this. Now, gdb_stdout is simply an rvalue, and
refers to the current interpreter. The interpreter provides ui_files
that do whatever rewriting is needed (mostly for MI); then output is
forward to the current UI via an indirection (see the new
ui::passthrough_file).
The ui provides paging, logging, timestamps, and the final stream that
writes to an actual file descriptor.
Redirection is handled at the ui layer. Rather than changing the
output pipeline, new ui_files are simply swapped in by rewriting
pointers, hopefully with a scoped_restore.
Redirecting at the ui layer means that interpreter rewriting is still
applied when capturing output. This fixes one of the reported bugs.
Not changing the pipeline means that the problems with the combination
of redirect and logging simply vanish. Logging just changes a flag
and doesn't involve object destruction.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=17697
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28620
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28798
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28948
Approved-By: Andrew Burgess <aburgess@redhat.com>
While working on this series, I found a number of odd styling issues
recurred. For instance, the issue where the pager would lose track
and style subsequent output incorrectly reappeared.
It turned out that different ui_file objects in the output pipeline
would get confused about their current style. And, looking deeper at
this, I realized that mainly it is the pager that really needs to
track the current style at all. All the other file implementations
can be purely reactive (except the buffered stream code, as Andrew
pointed out).
This patch moves m_applied_style from ui_file and into pager_file.
This necessitated making ui_file::vprintf virtual, so that the base
class could pass in the "plain" style as the starting point, whereas
the pager could use the applied style. (I did not investigate whether
this was truly necessary, and I somewhat suspect it might not be.)
This straightforward approach caused some regressions, mostly
involving extra ANSI escapes being emitted. I fixed most of these by
arranging for ui_out::call_do_message to track styles a little more
thoroughly.
Co-Authored-By: Andrew Burgess <aburgess@redhat.com>
Approved-By: Andrew Burgess <aburgess@redhat.com>
Fix a whitespace problem pre-commit complains about:
...
check-whitespace........................................................Failed
- hook id: check-whitespace
- exit code: 2
gdb/python/py-ref.h:58: indent with spaces.
+ "The __dict__ for this object.", nullptr },
...
Previous patches made some Python extension objects ipublicly inherit
directly or indirectly from PyObject.
In the interest of consistency, this patch makes all remaining Python
extension objects still not inheriting from PyObject do so.
Approved-By: Tom Tromey <tom@tromey.com>
When enabling the Python limited API, pointers to Python C extension
objects can no longer be implicitly converted to 'PyObject *' by the
compiler.
gdbpy_ref_policy is a templated class that provides a generic interface
for incrementing and decrementing the reference counter on the given
object. It is used as a specialisation of the policy parameter in
gdb::ref_ptr, together with PyObject as the parameter type. As a result,
gdbpy_ref_policy always expects an argument derived from PyObject.
This patch fixes the resulting compilation issue by adding an explicit
static_cast to 'PyObject *' before passing the value to Py_INCREF and
Py_DECREF. As a side effect, these casts enforce, at compile time, that
the template type passed to gdbpy_ref_policy is a subclass of PyObject.
To provide a clearer diagnostic when an incorrect type is used, a
static_assert is added to gdbpy_ref_policy, avoiding obscure errors
originating from the static_cast. Finally, all C Python extension types
passed to gdbpy_ref_policy are updated to inherit from PyObject.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=23830
Approved-By: Tom Tromey <tom@tromey.com>
GDB is currently using the Python unlimited API. Migrating the codebase
to the Python limited API would have for benefit to make a GDB build
artifacts compatible with older and newer versions of Python that they
were built with.
This patch prepares the ground for migrating the existing C extension
types from static types to heap-allocated ones, by removing the
dependency on tp_dictoffset, which is unavailable when using the limited
API.
One of the most common incompatibilities in the current static type
declarations is the tp_dictoffset slot, which specifies the dictionary
offset within the instance structure. Historically, the unlimited
API has provided two approaches to supply a dictionary for __dict__:
* A managed dictionary.
Setting Py_TPFLAGS_MANAGED_DICT in tp_flags indicates that the
instances of the type have a __dict__ attribute, and that the
dictionary is managed by Python.
According to the Python documentation, this is the recommended approach.
However, this flag was introduced in 3.12, together with
PyObject_VisitManagedDict() and PyObject_ClearManagedDict(), neither
of which is part of the limited API (at least for now). As a result,
this recommended approach is not viable in the context of the limited
API.
* An instance dictionary, for which the offset in the instance is
provided via tp_dictoffset.
According to the Python documentation, this "tp slot" is on the
deprecation path, and Py_TPFLAGS_MANAGED_DICT should be used instead.
Given the age of the GDB codebase and the requirement to support older
Python versions (>= 3.4), no need to argue that today, the implementation
of __dict__ relies on tp_dictoffset. However, in the context of the
limited API, PyType_Slot does not provide a Py_tp_dictoffset member, so
another approach is needed to provide __dict__ to instances of C extension
types.
Given the constraints of the limited API, the proposed solution consists
in providing a dictionary through a common base class, gdbpy__dict__wrapper.
This helper class owns a dictionary member corresponding to __dict__, and
any C extension type requiring a __dict__ must inherit from it. Since
extension object must also be convertible to PyObject, this wrapper class
publicly inherits from PyObject as well.
Access to the dictionary is provided via a custom getter defined in a
PyGetSetDef, similarily to what was previously done with gdb_py_generic_dict().
Because __dict__ participates in attribute look-up, and since this dictionary
is neither managed by Python nor exposed via tp_dictoffset, custom
implementations of tp_getattro and tp_setattro are required to correctly
redirect attribute look-ups to the dictionary. These custom implementations
— equivalent to PyObject_GenericGetAttr() and PyObject_GenericSetAttr() —
must be installed via tp_getattro / tp_setattro for static types, or
Py_tp_getattro / Py_tp_setattro for heap-allocated types.
- gdbpy__dict__wrapper: a base class for C extension objects that own a
__dict__.
- gdb_py_generic_dict_getter: a __dict__ getter for extension types
derived from gdbpy__dict__wrapper.
- gdb_py_generic_getattro: equivalent of PyObject_GenericGetAttr, but
fixes the look-up of __dict__.
- gdb_py_generic_setattro: equivalent of PyObject_GenericSetAttr, but
fixes the look-up of __dict__.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=23830
Approved-By: Tom Tromey <tom@tromey.com>
When enabling the Python limited API, pointers to Python C extension
objects can no longer be implicitly converted to 'PyObject *' by the
compiler.
The lookup() method of gbdpy_registry returns a new reference to the
type object of the looked-up entry. It does so by calling Py_XINCREF()
to increment the reference counter of the returned type object. The
template parameter obj_type corresponds to the type of C extension
object type. With the Python limited API enabled, obj_type can no longer
be implicitly converted to 'PyObject *' when passed to Py_XINCREF().
This patch fixes the resulting compilation issue by adding an explicit
static_cast to 'PyObject *' before passing the value to Py_XINCREF().
As a side effect, this cast enforces, at compile time, that the template
type 'Storage::obj_type' passed to gdbpy_registry is a subclass of
PyObject. To provide a clearer diagnostic when an incorrect type is used,
a static_assert is added to gdbpy_registry, avoiding obscure errors
originating from the static_cast. Finally, the relevant C extension types
passed to gdbpy_registry are updated to inherit publicly from PyObject.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=23830
Approved-By: Tom Tromey <tom@tromey.com>
This patch replaces PyImport_ExtendInittab () with its limited C
API equivalent, PyImport_AppendInittab (), a convenience wrapper
around PyImport_ExtendInittab ().
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=23830
Approved-By: Tom Tromey <tom@tromey.com>
This patch replaces Py_CompileStringExFlags () with its limited C API
equivalent, Py_CompileString (). The eval_python_command () helper is
now exposed through the private GDB Python API as a utility function.
PyRun_SimpleString () is replaced with eval_python_command () to avoid
code duplication.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=23830
Approved-By: Tom Tromey <tom@tromey.com>
"pre-commit autoupdate" suggests a new version of black. This version
seems to want to change how destructuring assignments are formatted.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
This replaces a number of uses of 'ptr.reset (xstrdup ())'
with 'ptr = make_unique_xstrdup ()'.
The main motivation for this is that, IMO, it's better to avoid the
reset method when possible.
Approved-By: Andrew Burgess <aburgess@redhat.com>
A previous patch [1] enabled readline in Python in a GDB-specific way
and blocked the standard Python readline module to prevent conflicts
with GDB by adding a custom importer raising an exception for the readline
module.
This custom importer was written back in 2012 for old Python versions,
and does not seem to work anymore with Python 3.x. find_module() and
load_module() have been deprecated since Python 3.4, and the first one
has been removed since 3.12, and the second will be removed in 3.15.
The GDB testsuite does not cover this use case, and the removal of
find_module() was not detected by the testsuite. This issue is tracked
in bug 32473.
importlib.abc.MetaPathFinder:
find_module(fullname, path)
Deprecated since version 3.4: Use find_spec() instead.
Changed in version 3.10: Use of find_module() by the import
system now raises ImportWarning.
Changed in version 3.12: find_module() has been removed. Use
find_spec() instead.
find_spec(fullname, path, target=None)
New in version 3.4, as a replacement for find_module.
importlib.abc.Loader:
load_module(fullname):
Deprecated since version 3.4, will be removed in version 3.15
The recommended API for loading a module is exec_module()
(and create_module()).
This patch uses Patryk Sondej's approach detailed in bug 32473, but with
a slight variation regarding the finder insertion in sys.meta_path.
It also adds a new test to prevent future regression.
[1]: 037bbc8eea
Approved-By: Tom Tromey <tom@tromey.com>
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32473
This changes the DAP code so that constants will now be returned by a
DAP scopes request. This is perhaps slightly strange with Ada
enumerators, but on the other hand this is consistent with what the
CLI does.
Reviewed-By: Eli Zaretskii <eliz@gnu.org>
A co-worker reported that certain symbols weren't appearing in the DAP
'scopes' response. In particular, symbols with non-ASCII names didn't
appear; though further research showed that this was in fact a result
of the variable in question actually being a constant.
Unfortunately Ada still requires the user to set the Ada source
character set in order to properly display symbol names. For DAP, it
seemed to make sense to allow this as a launch parameter. This patch
implements this.
Reviewed-By: Eli Zaretskii <eliz@gnu.org>
This changes some DAP code to explicitly use a symbol's print name.
Some places were using '.name'; and while 'str' does use the print
name, it seems better to be both consistent and explicit.
interp::inited is currently public, because interp_set does the task
of making sure the interpreter is only initialized a single time.
However, the interpreter can do this job itself, and this member can
be private.
This updates the copyright headers to include 2026. I did this by
running gdb/copyright.py and then manually modifying a few files as
noted by the script.
This adds a way for DAP clients to catch unhandled Ada exceptions,
similar to the "catch exception unhandled" CLI command.
Reviewed-By: Eli Zaretskii <eliz@gnu.org>
In PR dap/33228, we changed gdb to gracefully report an error if a DAP
'variables' request asked for more variables than had been reported.
This behavior was clarified in the spec, see
https://github.com/microsoft/debug-adapter-protocol/issues/571
This patch changes gdb to conform to the specified approach, namely
truncating the list rather than erroring.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=33228
Approved-By: Andrew Burgess <aburgess@redhat.com>
gdb's implementation of the DAP 'disconnect' request was incorrect in
a few ways.
First, the 'terminateDebuggee' field is optional, and has a special
meaning when not supplied: it should do whatever the default is.
Second, if the inferior was attached, it should detach rather than
terminate by default.
Finally, if the inferior was not started at all, it seems reasonable
for this request to simply succeed silently -- currently it returns
"success: false" with the reason being that the inferior isn't
running.
Approved-By: Andrew Burgess <aburgess@redhat.com>
This changes user_reg_map_name_to_regnum to use std::string_view.
This pointed out some dead code in that function: the "len < 0" test
in the loop can never be true, because earlier code changes 'len' in
this case.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
In C++20, "module" is an "identifier with special meaning". While it
can be used as a variable name, it highlights as a keyword in Emacs,
and I think it's nicer to just avoid it.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32660
Some Debug Adapter Protocol clients like Helix set the optional
"arguments" field of the ConfigurationDone request to null, which is a
bit odd but seems to be allowed by the protocol specification. Before
this patch, Python exceptions would be raised on such requests. This
patch makes it so these requests are treated as if the "arguments"
field was absent.
This changes DAP to ignore the case where a pretty-printer returns a
negative number from the num_children method. It didn't seem worth
writing a test case for this.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=33594
Reviewed-By: Ciaran Woodward <ciaranwoodward@xmos.com>
A user pointed out that if multiple breakpoints are set at the same
spot, in DAP mode, then changing the breakpoints won't reset all of
them.
The problem here is that the breakpoint map only stores a single
breakpoint, so if two breakpoints have the same key, only one will be
stored. Then, when breakpoints are changed, the "missing" breakpoint
will not be deleted.
The fix is to change the map to store a list of breakpoints.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=33467
Reviewed-By: Ciaran Woodward <ciaranwoodward@xmos.com>
This renames the variable 'breakpoint_map' in DAP's breakpoint.py,
adding an underscore to make it clear that it is private to the
module.
Reviewed-By: Ciaran Woodward <ciaranwoodward@xmos.com>
A user pointed out that the Python API can't create a type that is
both const and volatile.
The bug is that the calls to make_cv_type did not preserve the "other"
flag.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=33585
Reviewed-By: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
I noticed that when printing a gdb.Block object in Python, I would
occasionally get corrupted, nonsensical output, like this:
<gdb.Block <anonymous> {intintyinty_1inty_3inty_5... (-5 more symbols)}>
The symbol list is missing commas, it should be:
int, inty, inty_1, inty_3, inty_5, ...
And the '-5 more symbols' is clearly not right.
The problem is in python/py-block.c, we use this line to calculate the
number of symbols in a block:
const int len = mdict_size (block->multidict ());
Then we loop over the symbols in the block like this:
for (struct symbol *symbol : block_iterator_range (block))
...
The problem here is that 'block_iterator_range (block)' can loop over
more symbols than just those within 'block'. For global and static
blocks, block_iterator_range() takes into account included CUs; and so
can step through multiple global or static blocks. See
block_iterator_step and find_iterator_compunit_symtab in block.c for
more details.
In contrast, 'mdict_size (block->multidict ())' only counts the
symbols contained within 'block' itself.
I could fix this by either fixing LEN, or by only iterating over the
symbols within 'block'.
I assume that printing a gdb.Block object is used mostly for debug
purposes; the output isn't really user friendly, so I cannot imagine a
user script that is relying on printing a gdb.Block as a way to inform
the user about blocks in their program. As such, I think it makes
more sense if the symbols listed are restricted to those strictly held
within the block.
And so, instead of block_iterator_range, I've switched to iterating
over the multidict symbols. Now the calculated LEN will match the
number of symbols being printed, which fixes the output seen above.
However, as we're now only printing symbols that are within the block
being examined, the output above becomes:
<gdb.Block <anonymous> {}>
All the symbols that GDB previously tried to print, are coming from an
included CU.
For testing, I've made use of an existing DWARF test that tests
DW_AT_import. In the wild I saw this in an inferior that used
multiple shared libraries that has their debug information stored in a
separate debug file, and then parts of that debug information was
combined into a third separate file using the DWZ tool. I made a few
attempts to craft a simpler reproducer, but failed. In the end it was
easier to just use a DWARF assembler test to reproduce the issue.
I have added some more typedef symbols into the DWARF test, I don't
believe that this will impact the existing test, but makes the
corrupted output more obvious.
Approved-By: Tom Tromey <tom@tromey.com>
Currently, there is no way for a new user to have an idea of common
useful commands and behaviors from the GDB interface itself, without
checking the example session in the documentation. This command class
aims to close that gap by providing a set of quickstart commands that
allows for any simple debug session to happen without anything too
egregious missing.
The set of commands was chosen somewhat arbitrarily, based on what I
used or missed the most. The one overarching important thing, however,
is that the list is kept short, so as to not overwhelm new users. This
is confirmed by the newly introduced selftest, essential_command_count,
which ensures there are 20 or fewer essential commands.
Here's the reasoning for some of the choices:
* The command "start" was picked over "run" because combining it with
"continue" achieves the same effect, and I prefer it over needing to set
a breakpoint on main to stop at the start of the inferior.
* The command "ptype" is chosen because I believe it is important to
provide a way for the user to check a variable's type from inside GDB,
and ptype is a more complete command than the alternative, "whatis".
Reviewed-By: Eli Zaretskii <eliz@gnu.org>
Approved-By: Tom Tromey <tom@tromey.com>
Convert symtab to use obstack_new, and have a real constructor. The
filename, filename_for_id and m_compunit, members should really not
change once the symtab has been created, so make these members private
(m_compunit was already private) and set them just once from the
constructor. The set_compunit function has been deleted, and new
getter functions for filename and filename_for_id have been added.
The language is also set at construction time, but can be updated
later, so set the language in the constructor, but retain
symtab::set_language for when the language needs to be updated.
Prior to this patch the symtab was allocated with OBSTACK_ZALLOC which
would zero out the symtab object. With the call to objstack_new
fields in the symtab would no longer be initialised, so I've added
default member initialisation for everything not set in the
constructor.
The interesting changes are in symtab.h, and symfile.c. Everything
else is just updating to handle symfile::filename and
symfile::filename_for_id becoming methods.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
Add gdb_rl_tilde_expand, a wrapper around readline's tilde_expand that
returns a gdb::unique_xmalloc_ptr<char>. Change all callers of
tilde_expand to use gdb_rl_tilde_expand (even the couple of spots that
release it immediatly, for consistency). This simplifies a few callers.
The name gdb_tilde_expand is already taken by a home-made implementation
in gdbsupport/gdb_tilde_expand.{h.cc}. I wonder if we could just use
that one instead of readline's tilde_expand, but that's an orthogonal
question. I don't know how they differ, and I don't want to introduce
behavior changes in this patch.
Change-Id: I6d34eef19f86473226df4ae56d07dc01912e3131
Approved-By: Tom Tromey <tom@tromey.com>
This commit changes find_pcs_for_symtab_line() to return complete
linetable entries instead of just PCs. This is a preparation for adding
more attributes to gdb.LinetableEntry objects.
I also renamed the function to find_linetable_entries_for_symtab_line()
to better reflect what it does.
Approved-By: Tom Tromey <tom@tromey.com>
The GCC plugin that implements the Python API checker does not appear
to really be maintained. And, as far as I know, it never really
worked for C++ code anyway. Considering those factors, and that no
one has tried to run it in years, I think it's time to remove the
macros from the gdb source.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
