| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
The Guile code generally checks to see if an htab is non-null before
destroying it. However, the registry code already ensures this, so we
can change these checks to asserts and simplify the code a little.
|
|
This rewrites registry.h, removing all the macros and replacing it
with relatively ordinary template classes. The result is less code
than the previous setup. It replaces large macros with a relatively
straightforward C++ class, and now manages its own cleanup.
The existing type-safe "key" class is replaced with the equivalent
template class. This approach ended up requiring relatively few
changes to the users of the registry code in gdb -- code using the key
system just required a small change to the key's declaration.
All existing users of the old C-like API are now converted to use the
type-safe API. This mostly involved changing explicit deletion
functions to be an operator() in a deleter class.
The old "save/free" two-phase process is removed, and replaced with a
single "free" phase. No existing code used both phases.
The old "free" callbacks took a parameter for the enclosing container
object. However, this wasn't truly needed and is removed here as
well.
|
|
The guile code has a couple of unused functions that touch on the
registry API. This patch removes them.
|
|
When an objfile is destroyed, types that are still in use and
allocated on that objfile are copied. A temporary hash map is created
during this process, and it is allocated on the destroyed objfile's
obstack -- which normally is fine, as that is going to be destroyed
shortly anyway.
However, this approach requires that the objfile be passed to registry
destruction, and this won't be possible in the rewritten registry.
This patch changes the copied type hash table to simply use the heap
instead. It also removes the 'objfile' parameter from
copy_type_recursive, to make this all more clear.
This patch also fixes an apparent bug in copy_type_recursive.
Previously it was copying the dynamic property list to the dying
objfile's obstack:
- = copy_dynamic_prop_list (&objfile->objfile_obstack,
However I think this is incorrect -- that obstack is about to be
destroyed.
|
|
When building gdb with guile 3.0.8, we run into:
...
gdb/guile/guile.c: In function \
'void gdbscm_initialize(const extension_language_defn*)':
gdb/guile/guile.c:680:5: error: 'scm_install_gmp_memory_functions' is \
deprecated [-Werror=deprecated-declarations]
680 | scm_install_gmp_memory_functions = 0;
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from /usr/include/guile/3.0/libguile.h:128,
from gdb/guile/guile-internal.h:30,
from gdb/guile/guile.c:36:
/usr/include/guile/3.0/libguile/deprecated.h:164:20: note: declared here
164 | SCM_DEPRECATED int scm_install_gmp_memory_functions;
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1plus: all warnings being treated as errors
make[1]: *** [Makefile:1896: guile/guile.o] Error 1
...
The variable has been deprecated because it no longer has any effect.
Fix this by disabling the specific deprecation warning.
Also handle upcoming guile versions > 3.0, in which the variable will be
removed, by limiting the usage of the variable to guile versions <= 3.0.
This does not break anything. The variable was merely used to address a
problem present in guile versions <= v3.0.5.
Note that we don't limit the usage of the variable to guile versions <= 3.0.5,
because we want to support f.i. building against 3.0.6 and then using a shared
lib with 3.0.5.
Tested on x86_64-linux.
Co-Authored-By: Tom de Vries <tdevries@suse.de>
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28994
|
|
This converts location_spec_to_string to a method of location_spec,
simplifying the code using it, as it no longer has to use
std::unique_ptr::get().
Change-Id: I621bdad8ea084470a2724163f614578caf8f2dd5
|
|
Currently, GDB internally uses the term "location" for both the
location specification the user input (linespec, explicit location, or
an address location), and for actual resolved locations, like the
breakpoint locations, or the result of decoding a location spec to
SaLs. This is expecially confusing in the breakpoints module, as
struct breakpoint has these two fields:
breakpoint::location;
breakpoint::loc;
"location" is the location spec, and "loc" is the resolved locations.
And then, we have a method called "locations()", which returns the
resolved locations as range...
The location spec type is presently called event_location:
/* Location we used to set the breakpoint. */
event_location_up location;
and it is described like this:
/* The base class for all an event locations used to set a stop event
in the inferior. */
struct event_location
{
and even that is incorrect... Location specs are used for finding
actual locations in the program in scenarios that have nothing to do
with stop events. E.g., "list" works with location specs.
To clean all this confusion up, this patch renames "event_location" to
"location_spec" throughout, and then all the variables that hold a
location spec, they are renamed to include "spec" in their name, like
e.g., "location" -> "locspec". Similarly, functions that work with
location specs, and currently have just "location" in their name are
renamed to include "spec" in their name too.
Change-Id: I5814124798aa2b2003e79496e78f95c74e5eddca
|
|
This commit is setup for the next commit.
In the next commit I will add a Python API to intercept the print_insn
calls within GDB, each print_insn call is responsible for
disassembling, and printing one instruction. After the next commit it
will be possible for a user to write Python code that either wraps
around the existing disassembler, or even, in extreme situations,
entirely replaces the existing disassembler.
This commit does not add any new Python API.
What this commit does is put the extension language framework in place
for a print_insn hook. There's a new callback added to 'struct
extension_language_ops', which is then filled in with nullptr for Python
and Guile.
Finally, in the disassembler, the code is restructured so that the new
extension language function ext_lang_print_insn is called before we
delegate to gdbarch_print_insn.
After this, the next commit can focus entirely on providing a Python
implementation of the new print_insn callback.
There should be no user visible change after this commit.
|
|
"enum string_repr_result" is defined in multiple .c files, causing ODR
warnings. This patch renames the types.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=22395
|
|
Replace with calls to blockvector::blocks, and the appropriate method
call on the returned array_view.
Change-Id: I04d1f39603e4d4c21c96822421431d9a029d8ddd
|
|
Replace with equivalent methods.
Change-Id: I334a319909a50b5cc5570a45c38c70e10dc00630
|
|
Replace with equivalent methods.
Change-Id: I31ec00f5bf85335c8b23d306ca0fe0b84d489101
|
|
Replace with equivalent methods.
Change-Id: I10a6c8a2a86462d9d4a6a6409a3f07a6bea66310
|
|
This turns symbol_symtab into a method on symbol. It also replaces
symbol_set_symtab with a method.
|
|
This turns symbol_arch into a method on symbol.
|
|
This turns symbol_objfile into a method on symbol.
|
|
Same idea as previous patch, but for symtab::objfile. I find
it clearer without this wrapper, as it shows that the objfile is
common to all symtabs of a given compunit. Otherwise, you could think
that each symtab (of a given compunit) can have a specific objfile.
Change-Id: Ifc0dbc7ec31a06eefa2787c921196949d5a6fcc6
|
|
symtab::blockvector is a wrapper around compunit_symtab::blockvector.
It is a bit misleadnig, as it gives the impression that a symtab has a
blockvector. Remove it, change all users to fetch the blockvector
through the compunit instead.
Change-Id: Ibd062cd7926112a60d52899dff9224591cbdeebf
|
|
Running gdb.guile/scm-breakpoint.exp against an --enable-ubsan build,
we see:
UNRESOLVED: gdb.guile/scm-breakpoint.exp: test_watchpoints: create a breakpoint with an invalid type number
...
guile (define wp2 (make-breakpoint "result" #:wp-class WP_WRITE #:type 999))
../../src/gdb/guile/scm-breakpoint.c:377:11: runtime error: load of value 999, which is not a valid value for type 'bptype'
ERROR: GDB process no longer exists
Fix this by parsing the user/guile input as plain int, and cast to
internal type only after we know we have a number that would be valid.
Change-Id: I03578d07db00be01b610a8f5ce72e5521aea6a4b
|
|
print_spaces_filtered is now misnamed, because whether filtering
happens is up to the stream. So, rename it.
|
|
Now that filtered and unfiltered output can be treated identically, we
can unify the printf family of functions. This is done under the name
"gdb_printf". Most of this patch was written by script.
|
|
Now that filtered and unfiltered output can be treated identically, we
can unify the putc family of functions. This is done under the name
"gdb_putc". Most of this patch was written by script.
|
|
Now that filtered and unfiltered output can be treated identically, we
can unify the puts family of functions. This is done under the name
"gdb_puts". Most of this patch was written by script.
|
|
This removes the LA_PRINT_TYPE macro, in favor of using ordinary
method calls.
|
|
Add a getter and a setter for a symbol's line. Remove the corresponding macro
and adjust all callers.
Change-Id: I229f2b8fcf938c07975f641361313a8761fad9a5
|
|
Add a getter and a setter for a symbol's type. Remove the corresponding
macro and adjust all callers.
Change-Id: Ie1a137744c5bfe1df4d4f9ae5541c5299577c8de
|
|
Add a getter and a setter for whether a symbol is an argument. Remove
the corresponding macro and adjust all callers.
Change-Id: I71b4f0465f3dfd2ed8b9e140bd3f7d5eb8d9ee81
|
|
Add a getter and a setter for whether a symbol is objfile owned. Remove
the corresponding macro and adjust all callers.
Change-Id: Ib7ef3718d65553ae924ca04c3fd478b0f4f3147c
|
|
Change-Id: I83211d5a47efc0564386e5b5ea4a29c00b1fd46a
|
|
Remove the macro, replace with an equivalent method.
Change-Id: I8f9ecd290ad28502e53c1ceca5006ba78bf042eb
|
|
Remove the macro, replace with an equivalent method.
Change-Id: Id6fe2a79c04bcd6c69ccaefb7a69bc06a476288c
|
|
Make compunit_primary_filetab a method of compunit_symtab.
Change-Id: Iee3c4f7e36d579bf763c5bba146e5e10d6766768
|
|
Remove the macro, update all users to use the getter directly.
Change-Id: I3f0fd6f4455d1c4ebd5da73b561eb18a979ef1f6
|
|
This changes all existing calls to wrap_here to call the method on the
appropriate ui_file instead. The choice of ui_file is determined by
context.
|
|
I think it only really makes sense to call wrap_here with an argument
consisting solely of spaces. Given this, it seemed better to me that
the argument be an int, rather than a string. This patch is the
result. Much of it was written by a script.
|
|
A common pattern for string_file is to want to move out the internal
string buffer, because it is the result of the computation that we want
to return. It is the reason why string_file::string returns a non-const
reference, as explained in the comment. I think it would make sense to
have a dedicated method for that instead and make string_file::string
return a const reference.
This allows removing the explicit std::move in the typical case. Note
that compile_program::compute was missing a move, meaning that the
resulting string was copied. With the new version, it's not possible to
forget to move.
Change-Id: Ieaefa35b73daa7930b2f3a26988b6e3b4121bb79
|
|
While looking into the language-capturing issue, I found another way
to crash gdb using parameters from Python:
(gdb) python print(gdb.parameter('endian'))
(This is related to PR python/12188, though this patch isn't going to
fix what that bug is really about.)
The problem here is that the global variable that underlies the
"endian" parameter is initialized to NULL. However, that's not a
valid value for an "enum" set/show parameter.
My understanding is that, in gdb, an "enum" parameter's underlying
variable must have a value that is "==" (not just strcmp-equal) to one
of the values coming from the enum array. This invariant is relied on
in various places.
I started this patch by fixing the problem with "endian". Then I
added some assertions to add_setshow_enum_cmd to try to catch other
problems of the same type.
This patch fixes all the problems that I found. I also looked at all
the calls to add_setshow_enum_cmd to ensure that they were all
included in the gdb I tested. I think they are: there are no calls in
nat-* files, or in remote-sim.c; and I was trying a build with all
targets, Python, and Guile enabled.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=12188
|
|
In an earlier version of the pager rewrite series, it was important to
audit unfiltered output calls to see which were truly necessary.
This is no longer necessary, but it still seems like a decent cleanup
to change calls to avoid explicitly passing gdb_stdout. That is,
rather than using something like fprintf_unfiltered with gdb_stdout,
the code ought to use plain printf_unfiltered instead.
This patch makes this change. I went ahead and converted all the
_filtered calls I could find, as well, for the same clarity.
|
|
This moves the gdb_argv class to a new header in gdbsupport.
|
|
In my tour of the ui_file subsystem, I found that fputstr and fputstrn
can be simplified. The _filtered forms are never used (and IMO
unlikely to ever be used) and so can be removed. And, the interface
can be simplified by removing a callback function and moving the
implementation directly to ui_file.
A new self-test is included. Previously, I think nothing was testing
this code.
Regression tested on x86-64 Fedora 34.
|
|
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.
|
|
The motivation is to reduce the number of places where unmanaged
pointers are returned from allocation type routines. All of the
callers are updated.
There should be no user visible changes after this commit.
|
|
Remove FIELD_ENUMVAL, replace its uses with field::loc_enumval.
Change-Id: Id4861cee91a8bb583a9836f1aa5da0a320fbf4d9
|
|
Remove FIELD_BITPOD, replace its uses with field::loc_bitpos.
Change-Id: Idb99297e0170661254276c206383a7e9bf1a935a
|
|
There's a common pattern to call add_basic_prefix_cmd and
add_show_prefix_cmd to add matching set and show commands. Add the
add_setshow_prefix_cmd function to factor that out and use it at a few
places.
Change-Id: I6e9e90a30e9efb7b255bf839cac27b85d7069cfd
|
|
The bug fixed by this [1] patch was caused by an out-of-bounds access to
a value's content. The code gets the value's content (just a pointer)
and then indexes it with a non-sensical index.
This made me think of changing functions that return value contents to
return array_views instead of a plain pointer. This has the advantage
that when GDB is built with _GLIBCXX_DEBUG, accesses to the array_view
are checked, making bugs more apparent / easier to find.
This patch changes the return types of these functions, and updates
callers to call .data() on the result, meaning it's not changing
anything in practice. Additional work will be needed (which can be done
little by little) to make callers propagate the use of array_view and
reap the benefits.
[1] https://sourceware.org/pipermail/gdb-patches/2021-September/182306.html
Change-Id: I5151f888f169e1c36abe2cbc57620110673816f3
|
|
This changes struct breakpoint to use unique_xmalloc_ptr in a couple
of spots, removing a bit of manual memory management.
|
|
This changes struct watchpoint to use unique_xmalloc_ptr in a couple
of places, removing a bit of manual memory management.
|
|
The pattern for using execute_command_to_string is:
...
std::string output;
output = execute_fn_to_string (fn, term_out);
...
This results in a problem when using it in a try/catch:
...
try
{
output = execute_fn_to_string (fn, term_out)
}
catch (const gdb_exception &e)
{
/* Use output. */
}
...
If an expection was thrown during execute_fn_to_string, then the output
remains unassigned, while it could be worthwhile to known what output was
generated by gdb before the expection was thrown.
Fix this by returning the string using a parameter instead:
...
execute_fn_to_string (output, fn, term_out)
...
Also add a variant without string parameter, to support places where the
function is used while ignoring the result:
...
execute_fn_to_string (fn, term_out)
...
Tested on x86_64-linux.
|
|
String-like settings (var_string, var_filename, var_optional_filename,
var_string_noescape) currently take a pointer to a `char *` storage
variable (typically global) that holds the setting's value. I'd like to
"mordernize" this by changing them to use an std::string for storage.
An obvious reason is that string operations on std::string are often
easier to write than with C strings. And they avoid having to do any
manual memory management.
Another interesting reason is that, with `char *`, nullptr and an empty
string often both have the same meaning of "no value". String settings
are initially nullptr (unless initialized otherwise). But when doing
"set foo" (where `foo` is a string setting), the setting now points to
an empty string. For example, solib_search_path is nullptr at startup,
but points to an empty string after doing "set solib-search-path". This
leads to some code that needs to check for both to check for "no value".
Or some code that converts back and forth between NULL and "" when
getting or setting the value. I find this very error-prone, because it
is very easy to forget one or the other. With std::string, we at least
know that the variable is not "NULL". There is only one way of
representing an empty string setting, that is with an empty string.
I was wondering whether the distinction between NULL and "" would be
important for some setting, but it doesn't seem so. If that ever
happens, it would be more C++-y and self-descriptive to use
optional<string> anyway.
Actually, there's one spot where this distinction mattered, it's in
init_history, for the test gdb.base/gdbinit-history.exp. init_history
sets the history filename to the default ".gdb_history" if it sees that
the setting was never set - if history_filename is nullptr. If
history_filename is an empty string, it means the setting was explicitly
cleared, so it leaves it as-is. With the change to std::string, this
distinction doesn't exist anymore. This can be fixed by moving the code
that chooses a good default value for history_filename to
_initialize_top. This is ran before -ex commands are processed, so an
-ex command can then clear that value if needed (what
gdb.base/gdbinit-history.exp tests).
Another small improvement, in my opinion is that we can now easily
give string parameters initial values, by simply initializing the global
variables, instead of xstrdup-ing it in the _initialize function.
In Python and Guile, when registering a string-like parameter, we
allocate (with new) an std::string that is owned by the param_smob (in
Guile) and the parmpy_object (in Python) objects.
This patch started by changing all relevant add_setshow_* commands to
take an `std::string *` instead of a `char **` and fixing everything
that failed to build. That includes of course all string setting
variable and their uses.
string_option_def now uses an std::string also, because there's a
connection between options and settings (see
add_setshow_cmds_for_options).
The add_path function in source.c is really complex and twisted, I'd
rather not try to change it to work on an std::string right now.
Instead, I added an overload that copies the std:string to a `char *`
and back. This means more copying, but this is not used in a hot path
at all, so I think it is acceptable.
Change-Id: I92c50a1bdd8307141cdbacb388248e4e4fc08c93
Co-authored-by: Lancelot SIX <lsix@lancelotsix.com>
|
