| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
The help text for various "internal problem" settings is longer than
80 columns. This patch tightens this up a bit. Note that these
commands are all "maint" commands so, IMO, it is sufficient if they
are clear to a gdb developer.
Reviewed-By: Eli Zaretskii <eliz@gnu.org>
|
|
This patch adds separate styling for line numbers. That is, whenever
gdb prints a source line number, it uses this style.
v2 includes a change to ensure that %ps works in query.
Reviewed-By: Eli Zaretskii <eliz@gnu.org>
Reviewed-by: Keith Seitz <keiths@redhat.com>
|
|
With the same trigger patch adding "set horizontal-scroll-mode on" to INPUTRC
as used in commit 250f1bbaf33 ("[gdb/testsuite] Fix gdb.tui/wrap-line.exp with
wrapping disabled"), we can easily reproduce a failure in
gdb.tui/wrap-line.exp mentioned in PR testsuite/31201:
...
(gdb) 78901234567890123456789012345678901234567890123456789012345678901234567^M<890123456789012345678901234567890123456789012345678 ^H^H^H^H^H^H^H^H^H^H^H^H^H^H^H^H^H^H^H^H^H^H^H^H^H9WFAIL: gdb.base/wrap-line.exp: term=ansi: width-hard-coded: wrap (timeout)
...
The test-case expects wrapping, but that's disabled by horizontal-scroll-mode.
Add a new line to "maint info screen", that describes the current readline
wrapping mode, and use it in the test-case to handle the different cases.
The reported values for the wrapping mode are as follows.
Unsupported because of running in batch mode:
...
$ gdb -q -batch -ex "maint info screen"
Readline wrapping mode: unsupported (gdb batch mode).
...
Unsupported because the terminal is not capable to move the cursor up:
...
$ TERM=dumb gdb -q -ex "maint info screen" -ex q
Readline wrapping mode: unsupported (terminal is not Cursor Up capable).
...
Disabled by horizontal-scroll-mode:
...
$ grep horizontal-scroll-mode ~/.inputrc
set horizontal-scroll-mode on
$ gdb -q -ex "maint info screen" -ex q
Readline wrapping mode: disabled (horizontal-scroll-mode).
...
Wrap done by readline because terminal is not auto wrap capable:
...
$ TERM=ansi gdb -q -ex "maint info screen" -ex q
Readline wrapping mode: readline (terminal is not auto wrap capable, last column reserved).
...
Wrap done by terminal autowrap:
...
$ TERM=xterm gdb -q -ex "maint info screen" -ex q
Readline wrapping mode: terminal (terminal is auto wrap capable).
...
Tested on x86_64-linux.
Co-Authored-By: Bernd Edlinger <bernd.edlinger@hotmail.de>
Approved-By: Tom Tromey <tom@tromey.com>
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31201
|
|
This commit is in preparation for the next few commits, this commit
adds a new function extract_single_filename_arg.
This new function will be used to convert GDB commands that expect a
single filename argument to have these commands take a possibly quoted
or escaped string.
There's no use of the new function in this commit, for that see the
following commits.
|
|
C++ 11 has a built-in attribute for this, no need to use a compat macro.
Change-Id: I90e4220d26e8f3949d91761f8a13cd9c37da3875
Reviewed-by: Lancelot Six <lancelot.six@amd.com>
|
|
Most files including gdbcmd.h currently rely on it to access things
actually declared in cli/cli-cmds.h (setlist, showlist, etc). To make
things easy, replace all includes of gdbcmd.h with includes of
cli/cli-cmds.h. This might lead to some unused includes of
cli/cli-cmds.h, but it's harmless, and much faster than going through
the 170 or so files by hand.
Change-Id: I11f884d4d616c12c05f395c98bbc2892950fb00f
Approved-By: Tom Tromey <tom@tromey.com>
|
|
Move some declarations related to the "quit" machinery from defs.h to
event-top.h. Most of the definitions associated to these declarations
are in event-top.c. The exceptions are `quit()` and `maybe_quit()`,
that are defined in utils.c. For consistency, move these two
definitions to event-top.c.
Include "event-top.h" in many files that use these things.
Change-Id: I6594f6df9047a9a480e7b9934275d186afb14378
Approved-By: Tom Tromey <tom@tromey.com>
|
|
Change the functions in target-debug.h to return string representations
in an std::string, such that they don't need to know how the printing
part is done. This also helps the following patch that makes the debug
prints in debug_target one-liners.
Update target-delegates.c (through make-target-delegates.py) to do the
printing.
Add an overload of gdb_puts to avoid using `.c_str ()`.
Change-Id: I55cbff1c1b03a3b24a81740e34c6ad41ac4f8453
Approved-By: Tom Tromey <tom@tromey.com>
|
|
The mingw build currently issues a warning:
./../../src/gdb/utils.h:378:56: warning: ignoring attributes on template argument 'void(const char*, va_list)' {aka 'void(const char*, char*)'} [-Wignored-attributes]
This patch fixes the problem as suggested by Simon:
https://sourceware.org/pipermail/gdb-patches/2024-April/207908.html
...that is, by changing the warning interceptor to a class with a
single 'warn' method.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
Now that defs.h, server.h and common-defs.h are included via the
`-include` option, it is no longer necessary for source files to include
them. Remove all the inclusions of these files I could find. Update
the generation scripts where relevant.
Change-Id: Ia026cff269c1b7ae7386dd3619bc9bb6a5332837
Approved-By: Pedro Alves <pedro@palves.net>
|
|
This changes the global 'warning_hook' to use the warning_hook_handler
typedef in its definition. This makes it a little easier to change
the type, should that be needed.
|
|
The warning_hook_handler function pointer takes va_list as
an argument, which on some platforms (mingw64) includes some
attributes. Attributes get ignored in template arguments.
This led to the compiler warning:
error: ignoring attributes on template argument 'warning_hook_handler' {aka 'void (*)(const char*, char*)'} [-Werror=ignored-attributes]
387 | scoped_restore_tmpl<warning_hook_handler> m_save;
| ^
By manually implementing the save/restore behaviour, rather
than using the helper template, this warning is fixed.
Approved-By: Tom Tromey <tom@tromey.com>
|
|
A recent commit broke AIX build. The thread_local type defined functions
were being considered a weak symbol and hence while creating the binary these
symbols were not visible.
This patch is a fix for the same.
|
|
This commit is the result of the following actions:
- Running gdb/copyright.py to update all of the copyright headers to
include 2024,
- Manually updating a few files the copyright.py script told me to
update, these files had copyright headers embedded within the
file,
- Regenerating gdbsupport/Makefile.in to refresh it's copyright
date,
- Using grep to find other files that still mentioned 2023. If
these files were updated last year from 2022 to 2023 then I've
updated them this year to 2024.
I'm sure I've probably missed some dates. Feel free to fix them up as
you spot them.
|
|
Commit deb1ba4e38b ("[gdb/tui] Fix TUI resizing for TERM=ansi") introduced a
dependency on readline private variable _rl_term_autowrap.
There is precedent for this, but it's something we want to get rid of
(PR build/10723).
Remove the dependency on _rl_term_autowrap, and instead calculate
readline_hidden_cols by comparing the environment variable COLS with cols as
returned by rl_get_screen_size.
Tested on x86_64-linux.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=10723
|
|
This changes gdb to use the C++17 [[fallthrough]] attribute rather
than special comments.
This was mostly done by script, but I neglected a few spellings and so
also fixed it up by hand.
I suspect this fixes the bug mentioned below, by switching to a
standard approach that, presumably, clang supports.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=23159
Approved-By: John Baldwin <jhb@FreeBSD.org>
Approved-By: Luis Machado <luis.machado@arm.com>
Approved-By: Pedro Alves <pedro@palves.net>
|
|
Since GDB now requires C++17, we don't need the internally maintained
gdb::optional implementation. This patch does the following replacing:
- gdb::optional -> std::optional
- gdb::in_place -> std::in_place
- #include "gdbsupport/gdb_optional.h" -> #include <optional>
This change has mostly been done automatically. One exception is
gdbsupport/thread-pool.* which did not use the gdb:: prefix as it
already lives in the gdb namespace.
Change-Id: I19a92fa03e89637bab136c72e34fd351524f65e9
Approved-By: Tom Tromey <tom@tromey.com>
Approved-By: Pedro Alves <pedro@palves.net>
|
|
common-defs.h has a few defines that I suspect were used during the
transition to C++. These aren't needed any more, so remove them.
Tested by rebuilding.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
Approved-By: Andrew Burgess <aburgess@redhat.com>
|
|
This started with me running into this comment in symfile.c:
/* FIXME, should use print_sys_errmsg but it's not filtered. */
gdb_printf (_("`%ps' has disappeared; keeping its symbols.\n"),
styled_string (file_name_style.style (), filename));
In this particular case I think I disagree with the comment; I think
the output should be a warning rather than just a message printed to
gdb_stdout, I think when the executable, or some other objfile that is
currently being debugged, disappears from disk, this is likely an
unexpected situation, and worth warning the user about.
So, in theory, I could just call print_sys_errmsg and remove the
comment, but that would mean loosing the filename styling in the
output... so in the end I remove the comment and updated the code to
call warning.
But that got me looking at print_sys_errmsg and how it's used.
Currently the function takes a string and an errno, and prints, to
stderr, the string followed by the result of calling strerror on the
errno.
In some places the string passed to print_sys_errmsg is just a
filename, and this is used when something goes wrong. In these cases,
I think calling warning rather than gdb_printf to gdb_stderr, would be
better, and in fact, in a couple of places we manually print a
"warning" prefix, and then call print_sys_errmsg. And so, for these
users I have added a new function warning_filename_and_errno, which
takes a filename, which is printed with styling, and an errno, which
is passed through strerror and the resulting string printed. This new
function calls warning to print its output. I then updated some of
the print_sys_errmsg users to use this new function.
Some other users of print_sys_errmsg are also emitting what is clearly
a warning, however, the string being passed in is more than just a
filename, so the new warning_filename_and_errno function can't be
used, it would style the whole string. For these users I have
switched to calling warning directly, this allows me to style the
warning message correctly.
Finally, in inflow.c there is one last call to print_sys_errmsg, in
this case I just inlined the definition of print_sys_errmsg. This is
a really weird case, as after printing this message GDB just does a
hard exit. This is pretty old code, dating back to the initial GDB
import, I guess it should be updated to call error() maybe, but I'm
reluctant to make this change as part of this commit, just in case
there's some reason why we can't throw an error at this point.
With that done there are now no users of print_sys_errmsg, and so the
old function can be removed.
While I was doing all of the above I added some additional filename
styling in soure.c, this is in an else block where the if contained
the print_sys_errmsg call, so these felt related.
And finally, while I was updating the uses of print_sys_errmsg in
procfs.c, I noticed that we used a static errmsg buffer to format some
error strings. As the above changes got rid of one of the users of
errmsg I also removed the other two users, and the static buffer.
There were a couple of tests that depended on the existing output
message format that needed updating. In one case we gained an extra
'warning: ' prefix, and in the other 'Warning: ' becomes 'warning: ',
I think in both cases the new output is an improvement.
Approved-By: Tom Tromey <tom@tromey.com>
|
|
I noticed a comment by an include and remembered that I think these
don't really provide much value -- sometimes they are just editorial,
and sometimes they are obsolete. I think it's better to just remove
them. Tested by rebuilding.
Approved-By: Andrew Burgess <aburgess@redhat.com>
|
|
This reverts commit 02601231fdd91a7bd4837ce202906ea2ce661489.
This commit was a refactoring to remove an xrealloc and simplify
utils.[ch]. However, it has a flaw -- it mishandles a substitution
like "$datadir/subdir".
I am backing out the patch in the interests of fixing the regression
before GDB 14. It can be reinstated (with modifications) later if we
like.
Regression tested on x86-64 Fedora 36.
|
|
Add template functions assign_return_if_changed and assign_set_if_changed in
gdb/utils.h:
...
template<typename T> void assign_set_if_changed (T &lval, const T &val, bool &changed)
{ ... }
template<typename T> bool assign_return_if_changed (T &lval, const T &val)
{ ... }
...
This allows us to rewrite code like this:
...
if (tui_border_attrs != entry->value)
{
tui_border_attrs = entry->value;
need_redraw = true;
}
...
into this:
...
need_redraw |= assign_return_if_changed<int> (tui_border_attrs, entry->value);
...
or:
...
assign_set_if_changed<int> (tui_border_attrs, entry->value, need_redraw);
...
The names are a composition of the functionality. The functions:
- assign VAL to LVAL, and either
- return true if the assignment changed LVAL, or
- set CHANGED to true if the assignment changed LVAL.
Tested on x86_64-linux.
|
|
I. Auto-detected width (xterm vs. ansi)
Say we have a terminal with a width of 40 chars:
...
$ echo $COLUMNS
40
...
With TERM=xterm, we report a width of 40 chars:
...
$ TERM=xterm gdb
(gdb) show width
Number of characters gdb thinks are in a line is 40.
...
And with TERM=ansi, a width of 39 chars:
...
$ TERM=ansi gdb
(gdb) show width
Number of characters gdb thinks are in a line is 39.
...
Gdb uses readline to auto-detect screen size, and readline decides in the
TERM=ansi case that the terminal does not have reliable auto-wrap, and
consequently decides to hide the last terminal column from the readline user
(in other words GDB), hence we get 39 instead of 40.
II. Types of wrapping
Looking a bit more in detail inside gdb, it seems there are two types of
wrapping:
- readline wrapping (in other words, prompt edit wrapping), and
- gdb output wrapping (can be observed by issuing "info sources").
This type of wrapping attempts to wrap some of the gdb output earlier
than the indicated width, to not break lines in inconvenient places.
III. Readline wrapping, auto-detected screen size
Let's investigate readline wrapping with the auto-detected screen widths.
First, let's try with xterm:
...
$ TERM=xterm gdb
(gdb) 7890123456789012345678901234567890
123
...
That looks as expected, wrapping occurs after 40 chars.
Now, let's try with ansi:
...
$ TERM=ansi gdb
(gdb) 78901234567890123456789012345678
90123
...
It looks like wrapping occurred after 38, while readline should be capable of
wrapping after 39 chars.
This is caused by readline hiding the last column, twice.
In more detail:
- readline detects the screen width: 40,
- readline hides the last column, setting the readline screen width to 39,
- readline reports 39 to gdb as screen width,
- gdb sets its width setting to 39,
- gdb sets readline screen width to 39,
- readline hides the last column, again, setting the readline screen width to
38.
This is reported as PR cli/30346.
IV. gdb output wrapping, auto-detected screen size
Say we set the terminal width to 56. With TERM=xterm, we have:
...
/home/abuild/rpmbuild/BUILD/glibc-2.31/csu/elf-init.c,
/data/vries/hello.c,
...
but with TERM=ansi:
...
/home/abuild/rpmbuild/BUILD/glibc-2.31/csu/elf-init.c, /
data/vries/hello.c,
...
So what happened here? With TERM=ansi, the width setting is auto-detected to
55, and gdb assumes the terminal inserts a line break there, which it doesn't
because the terminal width is 56.
This is reported as PR cli/30411.
V. Fix PRs
Fix both mentioned PRs by taking into account the hidden column when readline
reports the screen width in init_page_info, and updating chars_per_line
accordingly.
Note that now we report the same width for both TERM=xterm and TERM=ansi,
which is much clearer.
The point where readline respectively expects or ensures wrapping is still
indicated by "maint info screen", for xterm:
...
Number of characters readline reports are in a line is 40.
...
and ansi:
...
Number of characters readline reports are in a line is 39.
...
VI. Testing
PR cli/30346 is covered by existing regression tests gdb.base/wrap-line.exp
and gdb.tui/wrap-line.exp, so remove the KFAILs there.
I didn't manage to come up with a regression test for PR cli/30411. Perhaps
that would be easier if we had a maintenance command that echoes its arguments
while applying gdb output wrapping.
Tested on x86_64-linux.
PR cli/30346
PR cli/30411
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30346
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30411
|
|
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.
|
|
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.
|
|
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
|
|
With TERM=ansi, when resizing a TUI window from LINES/COLUMNS 31/118
(maximized) to 20/78 (de-maximized), I get a garbled screen (that ^L doesn't
fix) and a message:
...
@@ resize done 0, size = 77x20
...
with the resulting width being 77 instead of the expected 78.
[ The discrepancy also manifests in CLI, filed as PR30346. ]
The discrepancy comes from tui_resize_all, where we ask readline for the
screen size:
...
rl_get_screen_size (&screenheight, &screenwidth);
...
As it happens, when TERM is set to ansi, readline decides that the terminal
cannot auto-wrap lines, and reserves one column to deal with that, and as a
result reports back one less than the actual screen width:
...
$ echo $COLUMNS
78
$ TERM=xterm gdb -ex "show width" -ex q
Number of characters gdb thinks are in a line is 78.
$ TERM=ansi gdb -ex "show width" -ex q
Number of characters gdb thinks are in a line is 77.
...
In tui_resize_all, we need the actual screen width, and using a screenwidth of
one less than the actual value garbles the screen.
This is currently not causing trouble in testing because we have a workaround
in place in proc Term::resize. If we disable the workaround:
...
- stty columns [expr {$_cols + 1}] < $::gdb_tty_name
+ stty columns $_cols < $::gdb_tty_name
...
and dump the screen we get the same type of screen garbling:
...
0 +---------------------------------------+|
1 ||
2 ||
3 ||
...
Another way to reproduce the problem is using command "maint info screen".
After starting gdb with TERM=ansi, entering TUI, and issuing the command, we
get:
...
Number of characters curses thinks are in a line is 78.
...
and after maximizing and demaximizing the window we get:
...
Number of characters curses thinks are in a line is 77.
...
If we use TERM=xterm, we do get the expected 78.
Fix this by:
- detecting when readline will report back less than the actual screen width,
- accordingly setting a new variable readline_hidden_cols,
- using readline_hidden_cols in tui_resize_all to fix the resize problem, and
- removing the workaround in Term::resize.
The test-case gdb.tui/empty.exp serves as regression test.
I've applied the same fix in tui_async_resize_screen, the new test-case
gdb.tui/resize-2.exp serves as a regression test for that change. Without
that fix, we have:
...
FAIL: gdb.tui/resize-2.exp: again: gdb width 80
...
Tested on x86_64-linux.
PR tui/30337
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30337
|
|
With a build without ncurses we run into:
...
src/gdb/utils.c: In function ‘void maintenance_info_screen(const char*, int)’:
src/gdb/utils.c:1310:7: error: ‘COLS’ was not declared in this scope
COLS);
^~~~
src/gdb/utils.c:1331:8: error: ‘LINES’ was not declared in this scope
LINES);
^~~~~
...
Fix this by using HAVE_LIBCURSES.
Tested on x86_64-linux.
PR build/30391
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30391
|
|
While working on PRs tui/30337 and cli/30346 I came across various notions of
width in gdb, as reported by gdb, readline, curses and the environment
variables.
As for gdb, readline and the environment variables, the way things work
is:
- Gdb asks readline to detect screen size,
- readline sets the actual screen size in the environment variables
COLUMNS and LINES,
- readline reports back a screen size to gdb, which may have one column
less than the actual screen size, to deal with lack of auto-wrap.
This becomes gdb's notion of screen size (in other words the point where
we can expect the gdb command line to wrap),
- Gdb then explicitly sets readline's screen size, which readline itself may
adjust to deal with lack of auto-wrap. This becomes readlines notion
of screen size (well, internally the unadjusted one, but it'll report back
the adjusted one).
Add a command "maint info screen" that prints these notions, both for width
and height.
For TERM=xterm we have:
...
$ TERM=xterm gdb -ex "maint info screen"
Number of characters gdb thinks are in a line is 118.
Number of characters readline reports are in a line is 118.
Number of characters curses thinks are in a line is 118.
Number of characters environment thinks are in a line is 118 (COLUMNS).
Number of lines gdb thinks are in a page is 27.
Number of lines readline reports are in a page is 27.
Number of lines curses thinks are in a page is 27.
Number of lines environment thinks are in a page is 27 (LINES).
...
And for TERM=ansi:
...
$ TERM=ansi gdb -ex "maint info screen"
Number of characters gdb thinks are in a line is 117.
Number of characters readline reports are in a line is 116.
Number of characters curses thinks are in a line is 118.
Number of characters environment thinks are in a line is 118 (COLUMNS).
Number of lines gdb thinks are in a page is 27.
Number of lines readline reports are in a page is 27.
Number of lines curses thinks are in a page is 27.
Number of lines environment thinks are in a page is 27 (LINES).
...
[ The fact that we have "characters readline reports are in a line is 116" is
is due to gdb making readline adjust twice for the lack of auto-wrap, this is
PR cli/30346.
Likewise we can detect tui/30337 by doing a resize in TUI mode and doing
"maint info screen":
...
Number of characters characters curses thinks are in a line is 110.
Number of characters environment thinks are in a line is 111 (COLUMNS). ]
And for TERM=ansi, with width and heigth set to 0:
...
Number of characters gdb thinks are in a line is 4294967295 (unlimited).
Number of characters readline reports are in a line is 32766 (unlimited - 1).
Number of characters curses thinks are in a line is 118.
Number of characters environment thinks are in a line is 118 (COLUMNS).
Number of lines gdb thinks are in a page is 4294967295 (unlimited).
Number of lines readline reports are in a page is 32767 (unlimited).
Number of lines curses thinks are in a page is 27.
Number of lines environment thinks are in a page is 27 (LINES).
...
[ Note that when doing a resize by say maximizing or de-maximizing a terminal,
all reported values are updated, except for curses when not in TUI mode.
Maybe that means there's a bug. If not, then maybe we should not print
the curses lines unless in TUI mode, or annotate those lines such that it's
clear that the values may be not up-to-date. ]
I'd like to use this command in the regression test for PR cli/30346.
Tested on x86_64-linux.
Reviewed-By: Eli Zaretskii <eliz@gnu.org>
Reviewed-By: Tom Tromey <tom@tromey.com>
|
|
I noticed that language_demangle shadows the global
"current_language". When I went to fix this, though, I then saw that
language_demangle is only called in two places, and has a comment
saying it should be removed. This patch removes it. Note that the
NULL check in language_demangle is not needed by either of the
existing callers.
Regression tested on x86-64 Fedora 36.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
This changes gdb to use scalar arithmetic for expression evaluation.
I suspect this patch is not truly complete, as there may be code paths
that still don't correctly handle 128-bit integers. However, many
things do work now.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30190
|
|
In some cases GDB will fail when attempting to complete a command that
involves a rust symbol, the failure can manifest as a crash.
The problem is caused by the completion_match_for_lcd object being
left containing invalid data during calls to cp_symbol_name_matches_1.
The first question to address is why we are calling a C++ support
function when handling a rust symbol. That's due to GDB's auto
language detection for msymbols, in some cases GDB can't tell if a
symbol is a rust symbol, or a C++ symbol.
The test application contains symbols for functions which are
statically linked in from various rust support libraries. There's no
DWARF for these symbols, so all GDB has is the msymbols built from the
ELF symbol table.
Here's the problematic symbol that leads to our crash:
mangled: _ZN4core3str21_$LT$impl$u20$str$GT$5parse17h5111d2d6a50d22bdE
demangled: core::str::<impl str>::parse
As an msymbol this is initially created with language auto, then GDB
eventually calls symbol_find_demangled_name, which loops over all
languages calling language_defn::sniff_from_mangled_name, the first
language that can demangle the symbol gets assigned as the language
for that symbol.
Unfortunately, there's overlap in the mangled symbol names,
some (legacy) rust symbols can be demangled as both rust and C++, see
cplus_demangle in libiberty/cplus-dem.c where this is mentioned.
And so, because we check the C++ language before we check for rust,
then the msymbol is (incorrectly) given the C++ language.
Now it's true that is some cases we might be able to figure out that a
demangled symbol is not actually a valid C++ symbol, for example, in
our case, the construct '::<impl str>::' is not, I believe, valid in a
C++ symbol, we could look for ':<' and '>:' and refuse to accept this
as a C++ symbol.
However, I'm not sure it is always possible to tell that a demangled
symbol is rust or C++, so, I think, we have to accept that some times
we will get this language detection wrong.
If we accept that we can't fix the symbol language detection 100% of
the time, then we should make sure that GDB doesn't crash when it gets
the language wrong, that is what this commit addresses.
In our test case the user tries to complete a symbol name like this:
(gdb) complete break pars
This results in GDB trying to find all symbols that match 'pars',
eventually we consider our problematic symbol, and we end up with a
call stack that looks like this:
#0 0x0000000000f3c6bd in strncmp_iw_with_mode
#1 0x0000000000706d8d in cp_symbol_name_matches_1
#2 0x0000000000706fa4 in cp_symbol_name_matches
#3 0x0000000000df3c45 in compare_symbol_name
#4 0x0000000000df3c91 in completion_list_add_name
#5 0x0000000000df3f1d in completion_list_add_msymbol
#6 0x0000000000df4c94 in default_collect_symbol_completion_matches_break_on
#7 0x0000000000658c08 in language_defn::collect_symbol_completion_matches
#8 0x0000000000df54c9 in collect_symbol_completion_matches
#9 0x00000000009d98fb in linespec_complete_function
#10 0x00000000009d99f0 in complete_linespec_component
#11 0x00000000009da200 in linespec_complete
#12 0x00000000006e4132 in complete_address_and_linespec_locations
#13 0x00000000006e4ac3 in location_completer
In cp_symbol_name_matches_1 we enter a loop, this loop repeatedly
tries to match the demangled problematic symbol name against the user
supplied text ('pars'). Each time around the loop another component
of the symbol name is stripped off, thus, we check 'pars' against
these options:
core::str::<impl str>::parse
str::<impl str>::parse
<impl str>::parse
parse
As soon as we get a match the cp_symbol_name_matches_1 exits its loop
and returns. In our case, when we're looking for 'pars', the match
occurs on the last iteration of the loop, when we are comparing to
'parse'.
Now the problem here is that cp_symbol_name_matches_1 uses the
strncmp_iw_with_mode, and inside strncmp_iw_with_mode we allow for
skipping over template parameters. This allows GDB to match the
symbol name 'foo<int>(int,int)' if the user supplies 'foo(int,'.
Inside strncmp_iw_with_mode GDB will record any template arguments
that it has skipped over inside the completion_match_for_lcd object
that is passed in as an argument.
And so, when GDB tries to match against '<impl str>::parse', the first
thing it sees is '<impl str>', GDB assumes this is a template argument
and records this as a skipped region within the
completion_match_for_lcd object. After '<impl str>' GDB sees a ':'
character, which doesn't match with the 'pars' the user supplied, so
strncmp_iw_with_mode returns a value indicating a non-match. GDB then
removes the '<impl str>' component from the symbol name and tries
again, this time comparing to 'parse', which does match.
Having found a match, then in cp_symbol_name_matches_1 we record the
match string, and the full symbol name within the
completion_match_result object, and return.
The problem here is that the skipped region, the '<impl str>' that we
recorded in the penultimate loop iteration was never discarded, its
still there in our returned result.
If we look at what the pointers held in the completion_match_result
that cp_symbol_name_matches_1 returns, this is what we see:
core::str::<impl str>::parse
| \________/ |
| | '--- completion match string
| '---skip range
'--- full symbol name
When GDB calls completion_match_for_lcd::finish, GDB tries to create a
string using the completion match string (parse), but excluding the
skip range, as the stored skip range is before the start of the
completion match string, then GDB tries to do some weird string
creation, which will cause GDB to crash.
The reason we don't often see this problem in C++ is that for C++
symbols there is always some non-template text before the template
argument. This non-template text means GDB is likely to either match
the symbol, or reject the symbol without storing a skip range.
However, notice, I did say, we don't often see this problem. Once I
understood the issue, I was able to reproduce the crash using a pure
C++ example:
template<typename S>
struct foo
{
template<typename T>
foo (int p1, T a)
{
s = 0;
}
S s;
};
int
main ()
{
foo<int> obj (2.3, 0);
return 0;
}
Then in GDB:
(gdb) complete break foo(int
The problem here is that the C++ symbol for the constructor looks like
this:
foo<int>::foo<double>(int, double)
When GDB enters cp_symbol_name_matches_1 the symbols it examines are:
foo<int>::foo<double>(int, double)
foo<double>(int, double)
The first iteration of the loop will match the 'foo', then add the
'<int>' template argument will be added as a skip range. When GDB
find the ':' after the '<int>' the first iteration of the loop fails
to match, GDB removes the 'foo<int>::' component, and starts the
second iteration of the loop.
Again, GDB matches the 'foo', and now adds '<double>' as a skip
region. After that the '(int' successfully matches, and so the second
iteration of the loop succeeds, but, once again we left the '<int>' in
place as a skip region, even though this occurs before the start of
our match string, and this will cause GDB to crash.
This problem was reported to the mailing list, and a solution
discussed in this thread:
https://sourceware.org/pipermail/gdb-patches/2023-January/195166.html
The solution proposed here is similar to one proposed by the original
bug reported, but implemented in a different location within GDB.
Instead of placing the fix in strncmp_iw_with_mode, I place the fix in
cp_symbol_name_matches_1. I believe this is a better location as it
is this function that implements the loop, and it is this loop, which
repeatedly calls strncmp_iw_with_mode, that should be resetting the
result object state (I believe).
What I have done is add an assert to strncmp_iw_with_mode that the
incoming result object is empty.
I've also added some other asserts in related code, in
completion_match_for_lcd::mark_ignored_range, I make some basic
assertions about the incoming range pointers, and in
completion_match_for_lcd::finish I also make some assertions about how
the skip ranges relate to the match pointer.
There's two new tests. The original rust example that was used in the
initial bug report, and a C++ test. The rust example depends on which
symbols are pulled in from the rust libraries, so it is possible that,
at some future date, the problematic symbol will disappear from this
test program. The C++ test should be more reliable, as this only
depends on symbols from within the C++ source code.
Since I originally posted this patch to the mailing list, the
following patch has been merged:
commit 6e7eef72164c00d6a5a7b0bce9fa01f5481f33cb
Date: Sun Mar 19 09:13:10 2023 -0600
Use rust_demangle to fix a crash
This solves the problem of a rust symbol ending up in the C++ specific
code by changing the order languages are sorted. However, this new
commit doesn't address the issue in the C++ code which was fixed with
this commit.
Given that the C++ issue is real, and has a reproducer, I'm still
going to merge this fix. I've left the discussion of rust in this
commit message as I originally wrote it, but it should be read within
the context of GDB prior to commit 6e7eef72164c00d6a5a7.
Co-Authored-By: Zheng Zhan <zzlossdev@163.com>
|
|
At the moment, handle_sigterm() in event-top.c does the following:
sync_quit_force_run = 1;
set_quit_flag ();
This was used several more times in a later patch in this series, so
I'm introducing (at Pedro's suggestion) a new function named
'set_force_quit_flag'. It simply sets sync_quit_force_run and also
calls set_quit_flag(). I've revised the later patch to call
set_force_quit_flag instead.
I noticed that sync_quit_force_run is declared as an int but is being
used as a bool, so I also changed its type to bool in this commit.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=26761
Approved-By: Pedro Alves <pedro@palves.net>
|
|
When a GDB process receives the SIGTERM signal, handle_sigterm() in
event-top.c is called. The global variable 'sync_quit_force_run' is
set by this signal handler. It does some other things too, but the
setting of this global is the important bit for the SIGTERM part of
this discussion.
GDB will periodically check to see whether a Ctrl-C or SIGTERM has
been received. This is performed via use of the QUIT macro in
GDB's code. QUIT is defined to invoke maybe_quit(), which will be
periodically called during any lengthy operation. This is supposed to
ensure that the user won't have to wait too long for a Ctrl-C or
SIGTERM to be acted upon.
When a Ctrl-C / SIGINT is received, quit_handler() will decide whether
to pass the SIGINT onto the inferior or to call quit() which causes
gdb_exception_quit to be thrown. This exception (usually) propagates
to the top level. Control is then returned to the top level event
loop.
At the moment, SIGTERM is handled very differently. Instead of
throwing an exception, quit_force() is called. This does eventually
cause GDB to exit(), but prior to that happening, the inferiors
are killed or detached and other target related cleanup occurs.
As shown in this discussion between Pedro Alves and myself...
https://sourceware.org/pipermail/gdb-patches/2021-July/180802.html
https://sourceware.org/pipermail/gdb-patches/2021-July/180902.html
https://sourceware.org/pipermail/gdb-patches/2021-July/180903.html
...we found that it is possible for inferior_ptid and current_thread_
to get out of sync. When that happens, the "current_thread_ != nullptr"
assertion in inferior_thread() can fail resulting in a GDB internal
error.
Pedro recommended that we "let the normal quit exception propagate all
the way to the top level, and then have the top level call quit_force
if sync_quit_force_run is set." However, after the v2 series for this
patch set, we tweaked that idea by introducing a new exception for
handling SIGTERM.
This commit implements the obvious part of Pedro's suggestion:
Instead of calling quit_force from quit(), throw_forced_quit() is now
called instead. This causes the new exception 'gdb_exception_forced_quit'
to be thrown.
At the top level, I changed catch_command_errors() and captured_main()
to catch gdb_exception_forced_quit and then call quit_force() from the
catch block. I also changed start_event_loop() to also catch
gdb_exception_forced_quit; while we could also call quit_force() from
that catch block, it's sufficient to simply rethrow the exception
since it'll be caught by the newly added code in captured_main().
Making these changes fixed the failure / regression that I was seeing
for gdb.base/gdb-sigterm.exp when run on a machine with glibc-2.34.
However, there are many other paths back to the top level which this
test case does not test. I did an audit of all of the try / catch
code in GDB in which calls in the try-block might (eventually) call
QUIT. I found many cases where gdb_exception_quit and the new
gdb_exception_forced_quit will be swallowed. (When using GDB, have
you ever hit Ctrl-C and not have it do anything; if so, it could be
due to a swallowed gdb_exception_quit in one of the cases I've
identified.) The rest of the patches in this series deal with this
concern.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=26761
Tested-by: Tom de Vries <tdevries@suse.de>
Approved-by: Pedro Alves <pedro@palves.net>
|
|
This commit should fix PR gdb/20091, PR gdb/17201, and PR gdb/17071.
Additionally, PR gdb/17199 relates to this area of code, but is more
of a request to refactor some parts of GDB, this commit does not
address that request, but it is probably worth reading that PR when
looking at this commit.
When the current language is C++, and the user places a breakpoint on
a function in a shared library, GDB will currently find two locations
for the breakpoint, one location will be within the function itself as
we would expect, but the other location will be within the PLT table
for the call to the named function. Consider this session:
$ gdb -q /tmp/breakpoint-shlib-func
Reading symbols from /tmp/breakpoint-shlib-func...
(gdb) start
Temporary breakpoint 1 at 0x40112e: file /tmp/breakpoint-shlib-func.cc, line 20.
Starting program: /tmp/breakpoint-shlib-func
Temporary breakpoint 1, main () at /tmp/breakpoint-shlib-func.cc:20
20 int answer = foo ();
(gdb) break foo
Breakpoint 2 at 0x401030 (2 locations)
(gdb) info breakpoints
Num Type Disp Enb Address What
2 breakpoint keep y <MULTIPLE>
2.1 y 0x0000000000401030 <foo()@plt>
2.2 y 0x00007ffff7fc50fd in foo() at /tmp/breakpoint-shlib-func-lib.cc:20
This is not the expected behaviour. If we compile the same test using
a C compiler then we see this:
(gdb) break foo
Breakpoint 2 at 0x7ffff7fc50fd: file /tmp/breakpoint-shlib-func-c-lib.c, line 20.
(gdb) info breakpoints
Num Type Disp Enb Address What
2 breakpoint keep y 0x00007ffff7fc50fd in foo at /tmp/breakpoint-shlib-func-c-lib.c:20
Here's what's happening. When GDB parses the symbols in the main
executable and the shared library we see a number of different symbols
for foo, and use these to create entries in GDB's msymbol table:
- In the main executable we see a symbol 'foo@plt' that points at
the plt entry for foo, from this we add two entries into GDB's
msymbol table, one called 'foo@plt' which points at the plt entry
and has type mst_text, then we create a second symbol, this time
called 'foo' with type mst_solib_trampoline which also points at
the plt entry,
- Then, when the shared library is loaded we see another symbol
called 'foo', this one points at the actual implementation in the
shared library. This time GDB creates a msymbol called 'foo' with
type mst_text that points at the implementation.
This means that GDB creates 3 msymbols to represent the 2 symbols
found in the executable and shared library.
When the user creates a breakpoint on 'foo' GDB eventually ends up in
search_minsyms_for_name (linespec.c), this function then calls
iterate_over_minimal_symbols passing in the name we are looking for
wrapped in a lookup_name_info object.
In iterate_over_minimal_symbols we iterate over two hash tables (using
the name we're looking for as the hash key), first we walk the hash
table of symbol linkage names, then we walk the hash table of
demangled symbol names.
When the language is C++ the symbols for 'foo' will all have been
mangled, as a result, in this case, the iteration of the linkage name
hash table will find no matching results.
However, when we walk the demangled hash table we do find some
results. In order to match symbol names, GDB obtains a symbol name
matching function by calling the get_symbol_name_matcher method on the
language_defn class. For C++, in this case, the matching function we
use is cp_fq_symbol_name_matches, which delegates the work to
strncmp_iw_with_mode with mode strncmp_iw_mode::MATCH_PARAMS and
language set to language_cplus.
The strncmp_iw_mode::MATCH_PARAMS mode means that strncmp_iw_mode will
skip any parameters in the demangled symbol name when checking for a
match, e.g. 'foo' will match the demangled name 'foo()'. The way this
is done is that the strings are matched character by character, but,
once the string we are looking for ('foo' here) is exhausted, if we
are looking at '(' then we consider the match a success.
Lets consider the 3 symbols GDB created. If the function declaration
is 'void foo ()' then from the main executable we added symbols
'_Z3foov@plt' and '_Z3foov', while from the shared library we added
another symbol call '_Z3foov'. When these are demangled they become
'foo()@plt', 'foo()', and 'foo()' respectively.
Now, the '_Z3foov' symbol from the main executable has the type
mst_solib_trampoline, and in search_minsyms_for_name, we search for
any symbols of type mst_solib_trampoline and filter these out of the
results.
However, the '_Z3foov@plt' symbol (from the main executable), and the
'_Z3foov' symbol (from the shared library) both have type mst_text.
During the demangled name matching, due to the use of MATCH_PARAMS
mode, we stop the comparison as soon as we hit a '(' in the demangled
name. And so, '_Z3foov@plt', which demangles to 'foo()@plt' matches
'foo', and '_Z3foov', which demangles to 'foo()' also matches 'foo'.
By contrast, for C, there are no demangled hash table entries to be
iterated over (in iterate_over_minimal_symbols), we only consider the
linkage name symbols which are 'foo@plt' and 'foo'. The plain 'foo'
symbol obviously matches when we are looking for 'foo', but in this
case the 'foo@plt' will not match due to the '@plt' suffix.
And so, when the user asks for a breakpoint in 'foo', and the language
is C, search_minsyms_for_name, returns a single msymbol, the mst_text
symbol for foo in the shared library, while, when the language is C++,
we get two results, '_Z3foov' for the shared library function, and
'_Z3foov@plt' for the plt entry in the main executable.
I propose to fix this in strncmp_iw_with_mode. When the mode is
MATCH_PARAMS, instead of stopping at a '(' and assuming the match is a
success, GDB will instead search forward for the matching, closing,
')', effectively skipping the parameter list, and then resume
matching. Thus, when comparing 'foo' to 'foo()@plt' GDB will
effectively compare against 'foo@plt' (skipping the parameter list),
and the match will fail, just as it does when the language is C.
There is one slight complication, which is revealed by the test
gdb.linespec/cpcompletion.exp, when searching for the symbol of a
const member function, the demangled symbol will have 'const' at the
end of its name, e.g.:
struct_with_const_overload::const_overload_fn() const
Previously, the matching would stop at the '(' character, but after my
change the whole '()' is skipped, and the match resumes. As a result,
the 'const' modifier results in a failure to match, when previously
GDB would have found a match.
To work around this issue, in strncmp_iw_with_mode, when mode is
MATCH_PARAMS, after skipping the parameter list, if the next character
is '@' then we assume we are looking at something like '@plt' and
return a value indicating the match failed, otherwise, we return a
value indicating the match succeeded, this allows things like 'const'
to be skipped.
With these changes in place I now see GDB correctly setting a
breakpoint only at the implementation of 'foo' in the shared library.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=20091
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=17201
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=17071
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=17199
Tested-By: Bruno Larsen <blarsen@redhat.com>
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
gdbsupport/errors.h declares perror_with_name and leaves the
implementation to the clients.
However gdb and gdbserver's implementations are essentially the
same, resulting in unnecessary code duplication.
Fix this by implementing perror_with_name in gdbsupport. Add an
optional parameter for specifying the errno used to generate the
error message.
Also move the implementation of perror_string to gdbsupport since
perror_with_name requires it.
Approved-By: Tom Tromey <tom@tromey.com>
|
|
When an assert triggers in tui mode the output is not great, the
internal backtrace that is generated is printed directly to the file
descriptor for gdb_stderr, and, as a result, does not currently format
itself correctly - the output uses only '\n' at the end of each line,
and so, when the terminal is in raw mode, the cursor does not return
to the start of each line after the '\n'.
This is mostly fixable, we could update bt-utils.c to use '\r\n'
instead of just '\n', and this would fix most of the problems. The
one we can't easily fix is if/when GDB is built to use execinfo
instead of libbacktrace, in this case we use backtrace_symbols_fd to
print the symbols, and this function only uses '\n' as the line
terminator. Fixing this would require switching to backtrace_symbols,
but that API uses malloc, which is something we're trying to
avoid (this code is called when GDB hits an error, so ideally we don't
want to rely on malloc).
However, the execinfo code is only used when libbacktrace is not
available (or the user specifically disables libbacktrace) so maybe we
can ignore that problem...
... but there is another problem. When the backtrace is printed in
raw mode, it is possible that the backtrace fills the screen. With
the terminal in raw mode we don't have the ability to scroll back,
which means we loose some of the backtrace, which isn't ideal.
In this commit I propose that we should disable tui mode whenever we
handle a fatal signal, or when we hit the internal error code
path (e.g. when an assert triggers). With this done then we don't
need to update the bt-utils.c code, and the execinfo version of the
code (using backtrace_symbols_fd) works just fine. We also get the
ability to scroll back to view the error message and all of the
backtrace, assuming the users terminal supports scrolling back.
The only downside I see with this change is if the tui_disable call
itself causes an error for some reason, or, if we handle a single at a
time when it is not safe to call tui_disable, in these cases the extra
tui_disable call might cause GDB to loose the original error.
However, I think (just from personal experience) that the above two
issues are pretty rare and the benefits from this change far out
weighs the possible drawbacks.
|
|
This commit is the result of running the gdb/copyright.py script,
which automated the update of the copyright year range for all
source files managed by the GDB project to be updated to include
year 2023.
|
|
PR gdb/28947
The address_significant gdbarch setting was introduced as a way to remove
non-address bits from pointers, and it is specified by a constant. This
constant represents the number of address bits in a pointer.
Right now AArch64 is the only architecture that uses it, and 56 was a
correct option so far.
But if we are using Pointer Authentication (PAuth), we might use up to 2 bytes
from the address space to store the required information. We could also have
cases where we're using both PAuth and MTE.
We could adjust the constant to 48 to cover those cases, but this doesn't
cover the case where GDB needs to sign-extend kernel addresses after removal
of the non-address bits.
This has worked so far because bit 55 is used to select between kernel-space
and user-space addresses. But trying to clear a range of bits crossing the
bit 55 boundary requires the hook to be smarter.
The following patch renames the gdbarch hook from significant_addr_bit to
remove_non_address_bits and passes a pointer as opposed to the number of
bits. The hook is now responsible for removing the required non-address bits
and sign-extending the address if needed.
While at it, make GDB and GDBServer share some more code for aarch64 and add a
new arch-specific testcase gdb.arch/aarch64-non-address-bits.exp.
Bug-url: https://sourceware.org/bugzilla/show_bug.cgi?id=28947
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
It seems to me that streq and compare_cstrings belong near the other
string utility functions in common-utils.h; and furthermore that streq
ought to be inlined. This patch makes this change.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
I stumbled across subset_compare today, and after looking at the
callers I realized it could be removed and replaced with calls to
startswith.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
Currently, every internal_error call must be passed __FILE__/__LINE__
explicitly, like:
internal_error (__FILE__, __LINE__, "foo %d", var);
The need to pass in explicit __FILE__/__LINE__ is there probably
because the function predates widespread and portable variadic macros
availability. We can use variadic macros nowadays, and in fact, we
already use them in several places, including the related
gdb_assert_not_reached.
So this patch renames the internal_error function to something else,
and then reimplements internal_error as a variadic macro that expands
__FILE__/__LINE__ itself.
The result is that we now should call internal_error like so:
internal_error ("foo %d", var);
Likewise for internal_warning.
The patch adjusts all calls sites. 99% of the adjustments were done
with a perl/sed script.
The non-mechanical changes are in gdbsupport/errors.h,
gdbsupport/gdb_assert.h, and gdb/gdbarch.py.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
Change-Id: Ia6f372c11550ca876829e8fd85048f4502bdcf06
|
|
This replaces the global input_interactive_p function with a new
method ui::input_interactive_p.
|
|
This patch removes ui_register_input_event_handler and
ui_unregister_input_event_handler, replacing them with methods on
'ui'. It also changes gdb to use these methods everywhere, rather
than sometimes reaching in to the ui to manage the file descriptor
directly.
|
|
The throw_perror_with_name function is not used outside of utils.c
right now. And as perror_with_name is just a wrapper around
throw_perror_with_name, then any future calls would be to
perror_with_name.
Lets make throw_perror_with_name static.
There should be no user visible changes after this commit.
|
|
I don't think it's very useful to return the character from gdb_putc,
so this patch changes it to return void.
|
|
PR cli/17151 points out that "set height 1" has pathological behavior
in gdb. What I see is that gdb will endlessly print the pagination
prompt. This patch takes a simple and expedient approach to a fix:
pretend that the height is really 2.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=17151
|
|
PR cli/20741 points out that when pagination is disabled, this also
disabled word wrapping. However, the manual documents that these
settings are separate -- if you intend to disable the wrapping, you
must use "set width unlimited".
This patch fixes the bug by letting the pagination-disabled case fall
through to the code that also handles word-wrapping.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=20741
|
|
While working on a different patch, I triggered an assertion from the
initialize_current_architecture code, specifically from one of
the *_gdbarch_init functions in a *-tdep.c file. This exposes a
couple of issues with GDB.
This is easy enough to reproduce by adding 'gdb_assert (false)' into a
suitable function. For example, I added a line into i386_gdbarch_init
and can see the following issue.
I start GDB and immediately hit the assert, the output is as you'd
expect, except for the very last line:
$ ./gdb/gdb --data-directory ./gdb/data-directory/
../../src.dev-1/gdb/i386-tdep.c:8455: internal-error: i386_gdbarch_init: Assertion `false' failed.
A problem internal to GDB has been detected,
further debugging may prove unreliable.
----- Backtrace -----
... snip ...
---------------------
../../src.dev-1/gdb/i386-tdep.c:8455: internal-error: i386_gdbarch_init: Assertion `false' failed.
A problem internal to GDB has been detected,
further debugging may prove unreliable.
Quit this debugging session? (y or n) ../../src.dev-1/gdb/ser-event.c:212:16: runtime error: member access within null pointer of type 'struct serial'
Something goes wrong when we try to query the user. Note, I
configured GDB with --enable-ubsan, I suspect that without this the
above "error" would actually just be a crash.
The backtrace from ser-event.c:212 looks like this:
(gdb) bt 10
#0 serial_event_clear (event=0x675c020) at ../../src/gdb/ser-event.c:212
#1 0x0000000000769456 in invoke_async_signal_handlers () at ../../src/gdb/async-event.c:211
#2 0x000000000295049b in gdb_do_one_event () at ../../src/gdbsupport/event-loop.cc:194
#3 0x0000000001f015f8 in gdb_readline_wrapper (
prompt=0x67135c0 "../../src/gdb/i386-tdep.c:8455: internal-error: i386_gdbarch_init: Assertion `false' failed.\nA problem internal to GDB has been detected,\nfurther debugging may prove unreliable.\nQuit this debugg"...)
at ../../src/gdb/top.c:1141
#4 0x0000000002118b64 in defaulted_query(const char *, char, typedef __va_list_tag __va_list_tag *) (
ctlstr=0x2e4eb68 "%s\nQuit this debugging session? ", defchar=0 '\000', args=0x7fffffffa6e0)
at ../../src/gdb/utils.c:934
#5 0x0000000002118f72 in query (ctlstr=0x2e4eb68 "%s\nQuit this debugging session? ")
at ../../src/gdb/utils.c:1026
#6 0x00000000021170f6 in internal_vproblem(internal_problem *, const char *, int, const char *, typedef __va_list_tag __va_list_tag *) (problem=0x6107bc0 <internal_error_problem>, file=0x2b976c8 "../../src/gdb/i386-tdep.c",
line=8455, fmt=0x2b96d7f "%s: Assertion `%s' failed.", ap=0x7fffffffa8e8) at ../../src/gdb/utils.c:417
#7 0x00000000021175a0 in internal_verror (file=0x2b976c8 "../../src/gdb/i386-tdep.c", line=8455,
fmt=0x2b96d7f "%s: Assertion `%s' failed.", ap=0x7fffffffa8e8) at ../../src/gdb/utils.c:485
#8 0x00000000029503b3 in internal_error (file=0x2b976c8 "../../src/gdb/i386-tdep.c", line=8455,
fmt=0x2b96d7f "%s: Assertion `%s' failed.") at ../../src/gdbsupport/errors.cc:55
#9 0x000000000122d5b6 in i386_gdbarch_init (info=..., arches=0x0) at ../../src/gdb/i386-tdep.c:8455
(More stack frames follow...)
It turns out that the problem is that the async event handler
mechanism has been invoked, but this has not yet been initialized.
If we look at gdb_init (in gdb/top.c) we can indeed see the call to
gdb_init_signals is after the call to initialize_current_architecture.
If I reorder the calls, moving gdb_init_signals earlier, then the
initial error is resolved, however, things are still broken. I now
see the same "Quit this debugging session? (y or n)" prompt, but when
I provide an answer and press return GDB immediately crashes.
So what's going on now? The next problem is that the call_readline
field within the current_ui structure is not initialized, and this
callback is invoked to process the reply I entered.
The problem is that call_readline is setup as a result of calling
set_top_level_interpreter, which is called from captured_main_1.
Unfortunately, set_top_level_interpreter is called after gdb_init is
called.
I wondered how to solve this problem for a while, however, I don't
know if there's an easy "just reorder some lines" solution here.
Looking through captured_main_1 there seems to be a bunch of
dependencies between printing various things, parsing config files,
and setting up the interpreter. I'm sure there is a solution hiding
in there somewhere.... I'm just not sure I want to spend any longer
looking for it.
So.
I propose a simpler solution, more of a hack/work-around. In utils.c
we already have a function filtered_printing_initialized, this is
checked in a few places within internal_vproblem. In some of these
cases the call gates whether or not GDB will query the user.
My proposal is to add a new readline_initialized function, which
checks if the current_ui has had readline initialized yet. If this is
not the case then we should not attempt to query the user.
After this change GDB prints the error message, the backtrace, and
then aborts (including dumping core). This actually seems pretty sane
as, if GDB has not yet made it through the initialization then it
doesn't make much sense to allow the user to say "no, I don't want to
quit the debug session" (I think).
|
|
I noticed that GDB will display URLs in a few spots. This changes
them to be styled. Originally I thought I'd introduce a new "url"
style, but there aren't many places to use this, so I just reused
filename styling instead. This patch also changes the debuginfod URL
list to be printed one URL per line. I think this is probably a bit
easier to read.
|
