I think objfile::sections makes sense as the name of the method to
iterate over an objfile's sections, so this patch renames the existing
field to objfile::sections_start in preparation for that.
Python pretty-printers haven't applied to static members for quite
some time. I tracked this down to the call to cp_print_value_fields
in cp_print_static_field -- it doesn't let pretty-printers have a
chance to print the value. This patch fixes the problem.
The way that static members are handled is very weird to me. I tend
to think this should be done more globally, like in value_print.
However, I haven't made any big change.
Reviewed-by: Keith Seitz <keiths@redhat.com>
Tested-by: Keith Seitz <keiths@redhat.com>
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30057
I found a couple of spots in ada-valprint.c that use an explicit loop,
but where discrete_position could be used instead.
Reviewed-by: Keith Seitz <keiths@redhat.com>
Currently, when we add a new python sub-system to GDB,
e.g. py-inferior.c, we end up having to create a new function like
gdbpy_initialize_inferior, which then has to be called from the
function do_start_initialization in python.c.
In some cases (py-micmd.c and py-tui.c), we have two functions
gdbpy_initialize_*, and gdbpy_finalize_*, with the second being called
from finalize_python which is also in python.c.
This commit proposes a mechanism to manage these initialization and
finalization calls, this means that adding a new Python subsystem will
no longer require changes to python.c or python-internal.h, instead,
the initialization and finalization functions will be registered
directly from the sub-system file, e.g. py-inferior.c, or py-micmd.c.
The initialization and finalization functions are managed through a
new class gdbpy_initialize_file in python-internal.h. This class
contains a single global vector of all the initialization and
finalization functions.
In each Python sub-system we create a new gdbpy_initialize_file
object, the object constructor takes care of registering the two
callback functions.
Now from python.c we can call static functions on the
gdbpy_initialize_file class which take care of walking the callback
list and invoking each callback in turn.
To slightly simplify the Python sub-system files I added a new macro
GDBPY_INITIALIZE_FILE, which hides the need to create an object. We
can now just do this:
GDBPY_INITIALIZE_FILE (gdbpy_initialize_registers);
One possible problem with this change is that there is now no
guaranteed ordering of how the various sub-systems are initialized (or
finalized). To try and avoid dependencies creeping in I have added a
use of the environment variable GDB_REVERSE_INIT_FUNCTIONS, this is
the same environment variable used in the generated init.c file.
Just like with init.c, when this environment variable is set we
reverse the list of Python initialization (and finalization)
functions. As there is already a test that starts GDB with the
environment variable set then this should offer some level of
protection against dependencies creeping in - though for full
protection I guess we'd need to run all gdb.python/*.exp tests with
the variable set.
I have tested this patch with the environment variable set, and saw no
regressions, so I think we are fine right now.
One other change of note was for gdbpy_initialize_gdb_readline, this
function previously returned void. In order to make this function
have the correct signature I've updated its return type to int, and we
now return 0 to indicate success.
All of the other initialize (and finalize) functions have been made
static within their respective sub-system files.
There should be no user visible changes after this commit.
After this commit:
commit e2f620135d
Date: Thu Mar 30 13:26:25 2023 +0100
gdb/testsuite: change newline patterns used in gdb_test
It was pointed out in PR gdb/30403 that the same patterns can be found
in other lib/gdb.exp procs and that it would probably be a good idea
if these procs remained in sync with gdb_test. Actually, the bug
specifically calls out gdb_test_multiple when using with '-wrap', but
I found a couple of other locations in gdb_continue_to_breakpoint,
gdb_test_multiline, get_valueof, and get_local_valueof.
In all these locations one or both of the following issues are
addressed:
1. A leading pattern of '[\r\n]*' is pointless. If there is a
newline it will be matched, but if there is not then the testsuite
doesn't care. Also, as expect is happy to skip non-matched output
at the start of a pattern, if there is a newline expect is happy to
skip over it before matching the rest. As such, this leading
pattern is removed.
2. Using '\[\r\n\]*$gdb_prompt' means that we will swallow
unexpected blank lines at the end of a command's output, but also,
if the pattern from the test script ends with a '\r', '\n', or '.'
then these will partially match the trailing newline, with the
remainder of the newline matched by the pattern from gdb.exp. This
split matching doesn't add any value, it's just something that has
appeared as a consequence of how gdb.exp was originally written. In
this case the '\[\r\n\]*' is replaced with '\r\n'.
I've rerun the testsuite and fixed the regressions that I saw, these
were places where GDB emits a blank line at the end of the command
output, which we now need to explicitly match in the test script, this
was for:
gdb.dwarf2/dw2-out-of-range-end-of-seq.exp
gdb.guile/guile.exp
gdb.python/python.exp
Or a location where the test script was matching part of the newline
sequence, while gdb.exp was previously matching the remainder of the
newline sequence. Now we rely on gdb.exp to match the complete
newline sequence, this was for:
gdb.base/commands.exp
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30403
I noticed in gdb.base/page.exp:
...
set fours [string repeat 4 40]
...
but then shortly afterwards:
...
[list 1\r\n 2\r\n 3\r\n 444444444444444444444444444444]
...
Summarize the long string in the same way using string repeat:
...
[list 1\r\n 2\r\n 3\r\n [string repeat 4 30]]
...
Tested on x86_64-linux.
Tighten the expected output pattern in the test script:
gdb.debuginfod/build-id-no-debug-warning.exp
While working on some other patch I broke GDB such that this warning:
warning: "FILENAME": separate debug info file has no debug info
(which is generated in build-id.c) didn't actually include the
FILENAME any more -- yet this test script continued to pass. It turns
out that this script doesn't actually check for FILENAME.
This commit extends the test pattern to check for the full warning
string, including FILENAME, and also removes some uses of '.*' to make
the test stricter.
While looking into another bug, I noticed that the DWARF cooked
indexer picks up an address for this symbol:
<1><82>: Abbrev Number: 2 (DW_TAG_variable)
<83> DW_AT_specification: <0x9f>
<87> DW_AT_location : 10 byte block: e 0 0 0 0 0 0 0 0 e0 (DW_OP_const8u: 0 0; DW_OP_GNU_push_tls_address or DW_OP_HP_unknown)
<92> DW_AT_linkage_name: (indirect string, offset: 0x156): _ZN9container8tlsvar_0E
This happens because decode_locdesc allows the use of
DW_OP_GNU_push_tls_address.
This didn't make sense to me. I looked into it a bit more, and I
think decode_locdesc is used in three ways:
1. Find a constant address of a symbol that happens to be encoded as a
location expression.
2. Find the offset of a function in a virtual table. (This one should
probably be replaced by code to just evaluate the expression in
gnu-v3-abi.c -- but there's no point yet because no compiler
actually seems to emit correct DWARF here, see the bug linked in
the patch.)
3. Find the offset of a field, if the offset is a constant.
None of these require TLS.
This patch simplifies decode_locdesc by removing any opcodes that
don't fit into the above. It also changes the API a little, to make
it less difficult to use.
Regression tested on x86-64 Fedora 36.
This simplifies auto_load_expand_dir_vars to first split the string,
then do any needed substitutions. This was suggested by Simon, and is
much simpler than the current approach.
Then this patch also removes substitute_path_component, as it is no
longer called. This is nice because it helps with the long term goal
of removing utils.h.
Regression tested on x86-64 Fedora 36.
Add a test-case that tests prompt edit wrapping in CLI, both
for TERM=xterm and TERM=ansi, both with auto-detected and hard-coded width.
In the TERM=ansi case with auto-detected width we run into PR cli/30346, so
add a KFAIL for that failure mode.
Tested on x86_64-linux.
Add a test-case that tests prompt edit wrapping behaviour in the tuiterm, both
for CLI and TUI, both with auto-detected and hard-coded width.
In the CLI case with auto-detected width we run into PR cli/30411, so add a
KFAIL for that failure mode.
Tested on x86_64-linux.
PR 30354
* elf32-arm.c (elf32_arm_gc_mark_extra_sections): If any debug sections are marked then rerun the extra marking in order to pick up any dependencies.
This reverts commit 476410b3bc.
One of Simon's recent commits (2a740b3ba4)
changed the way recording a remote target works and fixed the underlying
issue of the bug, so the KFails can be removed from the test.
Approved-By: Tom Tromey <tom@tromey.com>
SUMMARY
The '--simple-values' argument to '-stack-list-arguments' and similar
GDB/MI commands does not take reference types into account, so that
references to arbitrarily large structures are considered "simple" and
printed. This means that the '--simple-values' argument cannot be used
by IDEs when tracing the stack due to the time taken to print large
structures passed by reference.
DETAILS
Various GDB/MI commands ('-stack-list-arguments', '-stack-list-locals',
'-stack-list-variables' and so on) take a PRINT-VALUES argument which
may be '--no-values' (0), '--all-values' (1) or '--simple-values' (2).
In the '--simple-values' case, the command is supposed to print the
name, type, and value of variables with simple types, and print only the
name and type of variables with compound types.
The '--simple-values' argument ought to be suitable for IDEs that need
to update their user interface with the program's call stack every time
the program stops. However, it does not take C++ reference types into
account, and this makes the argument unsuitable for this purpose.
For example, consider the following C++ program:
struct s {
int v[10];
};
int
sum(const struct s &s)
{
int total = 0;
for (int i = 0; i < 10; ++i) total += s.v[i];
return total;
}
int
main(void)
{
struct s s = { { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 } };
return sum(s);
}
If we start GDB in MI mode and continue to 'sum', the behaviour of
'-stack-list-arguments' is as follows:
(gdb)
-stack-list-arguments --simple-values
^done,stack-args=[frame={level="0",args=[{name="s",type="const s &",value="@0x7fffffffe310: {v = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10}}"}]},frame={level="1",args=[]}]
Note that the value of the argument 's' was printed, even though 's' is
a reference to a structure, which is not a simple value.
See https://github.com/microsoft/MIEngine/pull/673 for a case where this
behaviour caused Microsoft to avoid the use of '--simple-values' in
their MIEngine debug adapter, because it caused Visual Studio Code to
take too long to refresh the call stack in the user interface.
SOLUTIONS
There are two ways we could fix this problem, depending on whether we
consider the current behaviour to be a bug.
1. If the current behaviour is a bug, then we can update the behaviour
of '--simple-values' so that it takes reference types into account:
that is, a value is simple if it is neither an array, struct, or
union, nor a reference to an array, struct or union.
In this case we must add a feature to the '-list-features' command so
that IDEs can detect that it is safe to use the '--simple-values'
argument when refreshing the call stack.
2. If the current behaviour is not a bug, then we can add a new option
for the PRINT-VALUES argument, for example, '--scalar-values' (3),
that would be suitable for use by IDEs.
In this case we must add a feature to the '-list-features' command
so that IDEs can detect that the '--scalar-values' argument is
available for use when refreshing the call stack.
PATCH
This patch implements solution (1) as I think the current behaviour of
not printing structures, but printing references to structures, is
contrary to reasonable expectation.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29554
PR 30300
* emultempl/pep.em (set_entry_point): Add an undefined reference to the entry point if it has been constructed heuristically.
* emultempl/pe.em (set_entry_point): Likewise.
* scripttempl/pru.sc (OUTPUT_SECTION_ALIGN): New helper variable to place at end of DMEM output sections.
(.data): Use the helper variable.
(.eh_frame): New output section.
(.gnu_extab): Ditto.
(.gcc_except_table): Ditto.
(.resource_table): Use the helper variable.
As per the spec merely a blank isn't okay as a separator, the operand
to the relocation function ought to be parenthesized. Enforcing this
then also eliminates an inconsistency in that
lui t0, %hi sym
lui t0, %hi 0x1000
were accepted, but
lui t0, %hi +sym
lui t0, %hi -0x1000
were not.
char is unsigned on s390x, so there are a lot of warnings like:
gas/config/tc-bpf.c: In function 'get_token':
gas/config/tc-bpf.c:900:14: error: comparison is always false due to limited range of data type [-Werror=type-limits]
900 | if (ch == EOF || len > MAX_TOKEN_SZ)
| ^~
Change its type to int, like in the other similar code.
There is also:
gas/config/tc-bpf.c:735:30: error: 'bpf_endianness' may be used uninitialized in this function [-Werror=maybe-uninitialized]
735 | dst, be ? size[endianness - BPF_BE16] : size[endianness - BPF_LE16]);
| ~~~~~~~~~~~^~~~~~~~~~
-Wmaybe-uninitialized doesn't seem to understand the FSM; just
initialize bpf_endianness to silence it. Add an assertion to
build_bpf_endianness() in order to catch potential bugs.
In commit 995a34b177 ("Guard against frame.c destructors running before
frame-info.c's") the following problem was addressed.
The frame_info_ptr destructor:
...
~frame_info_ptr ()
{
frame_list.erase (frame_list.iterator_to (*this));
}
...
uses frame_list, which is a static member of class frame_info_ptr,
instantiated in frame-info.c:
...
intrusive_list<frame_info_ptr> frame_info_ptr::frame_list;
...
Then there's a static frame_info_pointer variable named selected_frame in
frame.c:
...
static frame_info_ptr selected_frame;
...
Because the destructor of selected_frame uses frame_list, its destructor needs
to be called before the destructor of frame_list.
But because they're in different compilation units, the initialization order and
consequently destruction order is not guarantueed.
The commit fixed this by handling the case that the destructor of frame_list
is called first, adding a check on is_linked ():
...
~frame_info_ptr ()
{
- frame_list.erase (frame_list.iterator_to (*this));
+ /* If this node has static storage, it may be deleted after
+ frame_list. Attempting to erase ourselves would then trigger
+ internal errors, so make sure we are still linked first. */
+ if (is_linked ())
+ frame_list.erase (frame_list.iterator_to (*this));
}
...
However, since then frame_list has been moved into frame.c, and
initialization/destruction order is guarantueed inside a compilation unit.
Revert aforementioned commit, and fix the destruction order problem by moving
frame_list before selected_frame.
Reverting the commit is another way of fixing the already fixed
Wdangling-pointer warning reported in PR build/30413, in a different way than
commit 9b0ccb1eba ("Pass const frame_info_ptr reference for
skip_[language_]trampoline").
Approved-By: Simon Marchi <simon.marchi@efficios.com>
Tested on x86_64-linux.
PR build/30413
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30413
The show_args_command uses gdb_printf without specifying the ui_file.
This means that it prints to gdb_stdout instead of the stream given as
an argument to the function.
This commit fixes this.
Reviewed-By: Tom Tromey <tom@tromey.com>
g++ 13.1.1 produces a -Werror=dangling-pointer=
In file included from ../../binutils-gdb/gdb/frame.h:75,
from ../../binutils-gdb/gdb/symtab.h:40,
from ../../binutils-gdb/gdb/language.c:33:
In member function ‘void intrusive_list<T, AsNode>::push_empty(T&) [with T = frame_info_ptr; AsNode = intrusive_base_node<frame_info_ptr>]’,
inlined from ‘void intrusive_list<T, AsNode>::push_back(reference) [with T = frame_info_ptr; AsNode = intrusive_base_node<frame_info_ptr>]’ at gdbsupport/intrusive_list.h:332:24,
inlined from ‘frame_info_ptr::frame_info_ptr(const frame_info_ptr&)’ at gdb/frame.h:241:26,
inlined from ‘CORE_ADDR skip_language_trampoline(frame_info_ptr, CORE_ADDR)’ at gdb/language.c:530:49:
gdbsupport/intrusive_list.h:415:12: error: storing the address of local variable ‘<anonymous>’ in ‘frame_info_ptr::frame_list.intrusive_list<frame_info_ptr>::m_back’ [-Werror=dangling-pointer=]
415 | m_back = &elem;
| ~~~~~~~^~~~~~~
gdb/language.c: In function ‘CORE_ADDR skip_language_trampoline(frame_info_ptr, CORE_ADDR)’:
gdb/language.c:530:49: note: ‘<anonymous>’ declared here
530 | CORE_ADDR real_pc = lang->skip_trampoline (frame, pc);
| ~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~
gdb/frame.h:359:41: note: ‘frame_info_ptr::frame_list’ declared here
359 | static intrusive_list<frame_info_ptr> frame_list;
| ^~~~~~~~~~
Each new frame_info_ptr is being pushed on a static frame list and g++
cannot see why that is safe in case the frame_info_ptr is created and
destroyed immediately when passed as value.
It isn't clear why only in this one place g++ sees the issue (probably
because it can inline enough code in this specific case).
Since passing the frame_info_ptr as const reference is cheaper, use
that as workaround for this warning.
PR build/30413
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30413
Tested-by: Kevin Buettner <kevinb@redhat.com>
Reviewed-by: Kevin Buettner <kevinb@redhat.com>
Reviewed-by: Tom Tromey <tom@tromey.com>
* coffcode.h (coff_read_word_from_buffer): New function.
* coffcode.h (COFF_CHECKSUM_BUFFER_SIZE): New constant.
* coffcode.h (coff_compute_checksum): Improve speed by reducing the number of seeks and reads used.
bfd/doc/chew.c extracts documentation from source code comments
annotated with keywords, and generates much of bfd.h and libbfd.h from
those same comments. The docs have suffered from people (me too)
adding things like CODE_FRAGMENT to the source to put code into bfd.h
without realising that CODE_FRAGMENT also puts @example around said
code into the docs. So we have random senseless things in the docs.
This patch fixes that problem (well, the senseless things from
CODE_FRAGMENT), moves most of the code out of bfd-in.h, and improves a
few chew.c features. libbfd.h now automatically gets ATTRIBUTE_HIDDEN
prototypes, and indentation in bfd.h and libbfd.h is better.
These functions don't belong in opncls.c.
* libbfd-in.h (bfd_release): Delete prototype.
* opncls.c (bfd_alloc, bfd_zalloc, bfd_release): Move to..
* libbfd.c: ..here. Include objalloc.c and provide bfd_release
with a FUNCTION comment.
* bfd-in2.h: Regenerate.
* libbfd.h: Regenerate.
bfd_elf_bfd_from_remote_memory is just a wrapper, and the function
could be implemented for other formats. Move it to opncls.c because
it acts a little like some of the other bfd_open* routines. Also give
it the usual FUNCTION etc. comment so prototypes and docs are handled
automatically.
* elf.c (bfd_elf_bfd_from_remote_memory): Move to..
* opncls.c: ..here, add FUNCTION comment.
* bfd-in.h (bfd_elf_bfd_from_remote_memory): Delete prototype.
* bfd-in2.h: Regenerate.
* hash.c (higher_prime_number): Use uint32_t param, return value,
tables and variables.
(bfd_default_hash_table_size): Make it an unsigned int.
(bfd_hash_set_default_size): Use unsigned int param and return.
* bfd-in.h (bfd_hash_set_default_size): Update prototype.
* bfd-in2.h: Regenerate.
The crc calculated is 32 bits. Replace uses of unsigned long with
uint32_t. Also use bfd_byte* for buffers.
bfd/
* opncls.c (bfd_calc_gnu_debuglink_crc32): Use stdint types.
(bfd_get_debug_link_info_1, bfd_get_debug_link_info): Likewise.
(separate_debug_file_exists, bfd_follow_gnu_debuglink): Likewise.
(bfd_fill_in_gnu_debuglink_section): Likewise.
* bfd-in2.h: Regenerate.
gdb/
* auto-load.c (auto_load_objfile_script): Update type of
bfd_get_debug_link_info argument.
* symfile.c (find_separate_debug_file_by_debuglink): Likewise.
* gdb_bfd.c (get_file_crc): Update type of
bfd_calc_gnu_debuglink_crc32 argument.
This explains exactly why the high reloc adjustment is as it is,
replacing the rather nebulous existing comment. I've also changed the
expression from (lo+0x8000)&0xffff to (lo&0xffff)^0x8000 which better
matches part of the standard 16-bit sign extension (resulting in
exactly the same value), and hoisted the calculation out of the loop.
* elfxx-mips.c (_bfd_mips_elf_lo16_reloc): Expand vallo
comment. Hoist calculation out of loop.
Initialize wpoffset_to_wpnumto avoid TCL error which happens in some aarch64 types.
ERROR: in testcase /root/build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.base/watchpoint-unaligned.exp
ERROR: can't read "wpoffset_to_wpnum(1)": no such element in array
ERROR: tcl error code TCL READ VARNAME
ERROR: tcl error info:
can't read "wpoffset_to_wpnum(1)": no such element in array
while executing
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30340
Reviewed-by: Luis Machado <luis.machado@arm.com>
Reviewed-By: Andrew Burgess <aburgess@redhat.com>
GCC 13 points out that main_subfile has local function scope, but a
pointer to it is assigned to the global inclTable array subfile
element field:
In function ‘void process_linenos(CORE_ADDR, CORE_ADDR)’,
inlined from ‘void aix_process_linenos(objfile*)’ at xcoffread.c:727:19,
inlined from ‘void aix_process_linenos(objfile*)’ at xcoffread.c:720:1:
xcoffread.c:629:37: error: storing the address of local variable ‘main_subfile’ in ‘*inclTable.19_45 + _28._inclTable::subfile’ [-Werror=dangling-pointer=]
629 | inclTable[ii].subfile = &main_subfile;
| ~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~
xcoffread.c: In function ‘void aix_process_linenos(objfile*)’:
xcoffread.c:579:18: note: ‘main_subfile’ declared here
579 | struct subfile main_subfile;
| ^~~~~~~~~~~~
xcoffread.c:496:19: note: ‘inclTable’ declared here
496 | static InclTable *inclTable; /* global include table */
| ^~~~~~~~~
Fix this by making main_subfile file static. And allocate and
deallocated together with inclTable in allocate_include_entry and
xcoff_symfile_finish. Adjust the use of main_subfile in
process_linenos to take a pointer to the subfile.
In gdb.dwarf2/dw2-abs-hi-pc.exp we do:
...
set sources [lmap i $sources { expr { "$srcdir/$subdir/$i" } }]
...
The use of expr is not idiomatic. Fix this by using set instead:
...
set sources [lmap i $sources { set tmp $srcdir/$subdir/$i }]
...
Reported-By: Tom Tromey <tom@tromey.com>
Reviewed-By: Andreas Schwab <schwab@suse.de>
In AIX if there is a main and a thread created from it , then once the
program completed execution and goes to pd_disable () inferior_ptid
had pid 0 leading to an assertion failure while finding the thread's data
in aix-thread.c file.
This patch is a fix for the same.
error_stream is trivial and only used in a couple of spots in
breakpoint.c. This patch removes it in favor of just writing it out
at the spots where it was used.
Noticed in passing that the prepare_for_testing call in
gdb.linespec/cp-completion-aliases.exp does not pass the 'c++' flag,
despite this being a C++ test.
I guess, as the source file has the '.cc' extension, all the compilers
are doing the right thing anyway -- the source file uses templates, so
is definitely being compiled as C++.
I noticed this when I tried to set CXX_FOR_TARGET (but not
CC_FOR_TARGET) and spotted that this script was still using the C
compiler.
Fixed in this commit by adding the 'c++' flag for prepare_for_testing.
The Debugger Adapter Protocol defines a "launch" request but leaves
the parameters up to the implementation:
Since launching is debugger/runtime specific, the arguments for
this request are not part of this specification.
This patch adds some documentation for the parameter GDB currently
defines. Note that I plan to add more parameters here, and perhaps
there will be other extensions in time as well.
Reviewed-By: Eli Zaretskii <eliz@gnu.org>
I don't think that having struct ui_interp_info separated from struct ui
is very useful. As of today, it looks like an unnecessary indirection
layer. Move the contents of ui_interp_info directly into struct ui, and
update all users.
Change-Id: I817ba6e047dbcc4ba15b666af184b40bfed7e521
I'd like to move some things so they become methods on struct ui. But
first, I think that struct ui and the related things are big enough to
deserve their own file, instead of being scattered through top.{c,h} and
event-top.c.
Change-Id: I15594269ace61fd76ef80a7b58f51ff3ab6979bc
