Tom Tromey [Fri, 27 Jan 2023 16:20:43 +0000 (09:20 -0700)]
More const-correctness in cooked indexer
I noticed that iterating over the index yields non-const
cooked_index_entry objects. However, after finalization, they should
not be modified. This patch enforces this by adding const where
needed.
v2 makes the find, all_entries, and wait methods const as well.
Recent commit 1d98e564c97 ("[gdb/testsuite] Add
gdb.base/unwind-on-each-insn-{amd64,i386}.exp") broke commit eb015bf86b6
("[gdb/testsuite] Avoid using .eh_frame in gdb.base/unwind-on-each-insn.exp"),
in the sense that gdb.base/unwind-on-each-insn.exp no longer uses
-fno-asynchronous-unwind-tables, due to trying to concatenate two lists using:
...
lappend srcfile2_flags $nodebug_flags
...
which should instead be:
...
lappend srcfile2_flags {*}$nodebug_flags
...
Fix this by simplifying gdb.base/unwind-on-each-insn.exp.tcl, completely
leaving the responsibility to set srcfile_flags and srcfile2_flags to each
includer.
Andrew Burgess [Thu, 5 Jan 2023 12:26:09 +0000 (12:26 +0000)]
gdb/tui: avoid extra refresh_window on vertical scroll
While working on the previous couple of patches I noticed that when I
scroll the src and asm windows vertically, I get two refresh_window
calls.
The two calls can be traced back to
tui_source_window_base::update_source_window_as_is, in here we call
show_source_content, which calls refresh_window, and then
update_exec_info, which also calls refresh_window.
In this commit I propose making the refresh_window call in
update_exec_info optional. In update_source_window_as_is I'll then
call update_exec_info before calling show_source_content, and pass a
flag to update_exec_info to defer the refresh.
There are places where update_exec_info is used without any subsequent
refresh_window call (e.g. when a breakpoint is updated), so
update_exec_info does not to call refresh_window in some cases, which
is why I'm using a flag to control the refresh.
With this changes I'm now only seeing a single refresh_window call for
each vertical scroll.
There should be no user visible changes after this commit.
Andrew Burgess [Thu, 5 Jan 2023 12:18:05 +0000 (12:18 +0000)]
gdb/tui: avoid extra refresh_window on horizontal scroll
While working on the previous patches I noticed that in some cases I
was seeing two calls to tui_source_window_base::refresh_window when
scrolling the window horizontally.
The two calls would trigger in for the tui-disasm-long-lines.exp test
when the pad needed to be refilled. The two called both come from
tui_source_window_base::show_source_content. The first call is nested
within check_and_display_highlight_if_needed, while the second call is
done directly at the end of show_source_content.
The check_and_display_highlight_if_needed is being used to draw the
window box to the window, this is needed here because
show_source_content is what gets called when the window needs
updating, e.g. after a resize. We could potentially do the boxing in
refresh_window, but then we'd be doing it each time we scroll, even
though the box doesn't need changing in this case.
However, we can move the check_and_display_highlight_if_needed to be
the last thing done in show_source_content, this means that we can
rely on the refresh_window call within it to be our single refresh
call.
There should be no user visible changes after this commit.
Andrew Burgess [Sat, 24 Dec 2022 20:45:51 +0000 (20:45 +0000)]
gdb/tui: rewrite of tui_source_window_base to handle very long lines
This commit addresses an issue that is exposed by the test script
gdb.tui/tui-disasm-long-lines.exp, that is, tui_source_window_base
does not handle very long lines.
The problem can be traced back to the newpad call in
tui_source_window_base::show_source_content, this is where we allocate
a backing pad to hold the window content.
Unfortunately, there appears to be a limit to the size of pad that can
be allocated, and the gdb.tui/tui-disasm-long-lines.exp test goes
beyond this limit. As a consequence the newpad call fails and returns
nullptr.
It just so happens that the reset of the tui_source_window_base code
can handle the pad being nullptr (this happens anyway when the window
is first created, so we already depend on nullptr handling), so all
that happens is the source window displays no content.
... well, sort of ... something weird does happen in the command
window, we seem to see a whole bunch of blank lines. I've not
bothered to track down exactly what's happening there, but it's some
consequence of GDB attempting to write content to a WINDOW* that is
nullptr.
Before explaining my solution, I'll outline how things currently work:
Consider we have the following window content to display:
the longest line here is 20 characters. If our display window is 10
characters wide, then we will create a pad that is 20 characters wide,
and then copy the lines of content into the pad:
As the user scrolls left and right we adjust m_horizontal_offset and
use this to select which part of the pad is copied onto the display.
The benefit of this is that we only need to copy the content to the
pad once, which includes processing the ansi escape sequences, and
then the user can scroll left and right as much as they want
relatively cheaply.
The problem then, is that if the longest content line is very long,
then we try to allocate a very large pad, which can fail.
What I propose is that we allow both the pad and the display view to
scroll. Once we allow this, then it becomes possible to allocate a
pad that is smaller than the longest display line. We then copy part
of the content into the pad. As the user scrolls the view left and
right GDB will continue to copy content from the pad just as it does
right now. But, when the user scrolls to the edge of the pad, GDB
will copy a new block of content into the pad, and then update the
view as normal. This all works fine so long as the maximum pad size
is larger than the current window size - which seems a reasonable
restriction, if ncurses can't support a pad of a given size it seems
likely it will not support a display window of that size either.
If we return to our example above, but this time we assume that the
maximum pad size is 15 characters, then initially the pad would be
loaded like this:
Notice that the last 5 characters from the 'b' line are no longer
included in the pad. There is still enough content though to fill the
10 character wide display, just as we did before.
The pad contents remain unchanged until the user scrolls the display
right to this point:
The display can now be updated from the pad again just like normal.
With this change in place the gdb.tui/tui-disasm-long-lines.exp test
now correctly loads the assembler code, and we can scroll around as
expected.
Most of the changes are pretty mundane, just updating to match the
above. One interesting change though is the new member function
tui_source_window_base::puts_to_pad_with_skip. This replaces direct
calls to tui_puts when copying content to the pad.
The content strings contain ansi escape sequences. When these strings
are written to the pad these escape sequences are translated into
ncurses attribute setting calls.
Now however, we sometimes only write a partial string to the pad,
skipping some of the leading content. Imagine then that we have a
content line like this:
"\033[31mABCDEFGHIJKLM\033[0m"
Now the escape sequences in this content mean that the actual
content (the 'ABCDEFGHIJKLM') will have a red foreground color.
If we want to copy this to the pad, but skip the first 3 characters,
then what we expect is to have the pad contain 'DEFGHIJKLM', but this
text should still have a red foreground color.
It is this problem that puts_to_pad_with_skip solves. This function
skips some number of printable characters, but processes all the
escape sequences. This means that when we do start printing the
actual content the content will have the expected attributes.
/
Andrew Burgess [Wed, 4 Jan 2023 17:07:47 +0000 (17:07 +0000)]
gdb/tui: make m_horizontal_offset private
I noticed that tui_source_window_base::m_horizontal_offset was
protected, but could be made private, so lets do that.
This makes more sense in the context of a later commit where I plan to
add another member variable that is similar to m_horizontal_offset.
The new member variable could also be private.
So I had to choose, place the new member variable next to
m_horizontal_offset making it protected, but grouping similar
variables together, or make m_horizontal_offset private, and then add
the new variable as private too.
I chose to make m_horizontal_offset private, which is this commit.
There should be no user visible changes after this commit.
Andrew Burgess [Wed, 4 Jan 2023 09:48:31 +0000 (09:48 +0000)]
gdb/tui: disable tui mode when an assert triggers
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.
Andrew Burgess [Thu, 22 Dec 2022 16:26:37 +0000 (16:26 +0000)]
gdb/tui: improve errors from tui focus command
This commit improves (I think) the errors from the tui focus command.
There are a number of errors that can be triggered by the focus
command, they include:
(1) Window name "NAME" is ambiguous
(2) Unrecognized window name "NAME"
(3) Window "NAME" cannot be focused
Error (1) is triggered when the user gives a partial window name, and
the name matches multiple windows in the current layout.
It is worth noting that the ambiguity must be within the current
layout; if the partial name matches one window in the current layout,
and one or more windows not in the current layout, then this is not
ambiguous, and focus will shift to the matching window in the current
layout.
This error was not previous being tested, but in this commit I make
use of the Python API to trigger and test this error.
Error (3) is simple enough, and was already being tested. This is
triggered by something like 'focus status'. The named window needs to
be present in the current layout, and non-focusable in order to
trigger the error.
Error (2) is what I'd like to improve in this commit. This error
triggers if the name the user gives doesn't match any window in the
current layout. Even if GDB does know about the window, but the
window isn't in the current layout, then GDB will say it doesn't
recognize the window name.
In this commit I propose to to split this error into three different
errors. These will be:
(a) Unrecognized window name "NAME"
(b) No windows matching "NAME" in the current layout
(c) Window "NAME" is not in the current layout
Error (a) is the same as before, but will now only trigger if GDB
doesn't know about window NAME at all. If the window is known, but
not in the current layout then one of the other errors will trigger.
Error (b) will trigger if NAME is ambiguous for multiple windows that
are not in the current layout. If NAME identifies a single window in
the current layout then that window will continue to be selected, just
as it currently is. Only in the case where NAME doesn't identify a
window in the current layout do we then check all the other known
windows, if NAME matches multiple of these, then (b) is triggered.
Finally, error (c) is used when NAME uniquely identifies a single
window that is not in the current layout.
The hope with these new errors is that the user will have a better
understanding of what went wrong. Instead of GDB claiming to not know
about a window, the mention of the current layout will hint to the
user that they should first switch layouts.
Andrew Burgess [Thu, 22 Dec 2022 16:26:50 +0000 (16:26 +0000)]
gdb/testsuite: fix line feed scrolling in tuiterm.exp
In a following commit I managed to trigger the line feed scrolling
case in tuiterm.exp. This case is currently unhandled, and this
commit fills in this missing functionality.
The implementation is pretty simple, just scroll all the content up
one line at a time until the cursor is back on the screen (a single
line of scroll is all that should be needed).
This change is untested in this commit, but is required by the next
commit.
Jan Beulich [Fri, 27 Jan 2023 08:23:38 +0000 (09:23 +0100)]
x86: move reg_operands adjustment
Ideally we'd do away with this somewhat questionable adjustment (leaving
i.types[] untouched). That's non-trivial though as it looks, so only
- move the logic into process_operands(), putting it closer to related
logic and eliminating a conditional for operand-less insns,
- make it consistent (i.e. also affect %xmm0), eliminating an ugly
special case later in the function.
Jan Beulich [Fri, 27 Jan 2023 08:23:12 +0000 (09:23 +0100)]
x86: drop dead SSE2AVX-related code
All (there are just two) SSE2AVX templates with %xmm0 as first operand
also specify VEX3SOURCES. Hence there's no need for an "else" to the
respective if(), and the if() itself can become an assertion.
Jan Beulich [Fri, 27 Jan 2023 08:21:24 +0000 (09:21 +0100)]
x86/Intel: improve special casing of certain insns
Now that we have identifiers for the mnemonic strings we can avoid
opcode based comparisons, for (in many cases) being more expensive and
(in a few cases) being a little fragile and not self-documenting.
Jan Beulich [Fri, 27 Jan 2023 08:20:58 +0000 (09:20 +0100)]
opcodes: suppress internationalization on build helper tools
While one of the two actually having been instrumented (i386-gen.c) now
has that instrumentation dropped, there's still no point in honoring
such instrumentation in general (i.e. now for ia64-gen.c only), as that
only leads to a waste of resources.
With CFILES then being merely an alias of LIBOPCODES_CFILES, drop the
former variable altogether.
Alan Modra [Fri, 27 Jan 2023 05:02:06 +0000 (15:32 +1030)]
Call bfd_close_all_done in ld_cleanup
This is similar to "Call bfd_close_all_done in output_file_close",
but with some code tidying in the pe/pep write_build_id functions.
write_build_id is passed the output bfd as its parameter, so there is
no need to go looking for the output bfd via link_info (and doing so
will no longer work since I clear link_info.output_bfd before calling
bfd_close).
* emultempl/pe.em (write_build_id): Rename t to td. Formatting.
Don't access pe_data(link_info.output_bfd), use td instead.
(setup_build_id): Rename t to td. Formatting.
* emultempl/pep.em: As for pe.em.
* ldmain.c (ld_cleanup): Call bfd_close_all_done on linker bfds.
(main): Clear link_info.output_bfd when closing.
Alan Modra [Fri, 27 Jan 2023 04:46:03 +0000 (15:16 +1030)]
Perform cleanup in bfd_close after errors
It seems reasonable to continue after errors in bfd_close_all_done,
particularly since bfd_close_all_done is typically called on an output
file after we've hit some sort of error elsewhere. The iovec test is
necessary if bfd_close_all_done is to work on odd bfd's opened by
bfd_create.
* opncls.c (bfd_close): Call bfd_close_all_done after errors
from _bfd_write_contents.
(bfd_close_all_done): Call _bfd_delete_bfd after errors.
Don't call iovec->bclose when iovec is NULL.
Alan Modra [Fri, 27 Jan 2023 00:01:56 +0000 (10:31 +1030)]
gas macro memory leaks
This tidies memory allocated for entries in macro_hash. Freeing the
macro name requires a little restructuring of the define_macro
interface due to the name being used in the error message, and exposed
the fact that the name and other fields were not initialised by the
iq2000 backend.
There is also a fix for
.macro .macro
.endm
.macro .macro
.endm
which prior to this patch reported
mac.s:1: Warning: attempt to redefine pseudo-op `.macro' ignored
mac.s:3: Error: Macro `.macro' was already defined
rather than reporting the attempt to redefine twice.
* macro.c (macro_del_f): New function.
(macro_init): Use it when creating macro_hash.
(free_macro): Free macro name too.
(define_macro): Return the macro_entry, remove idx, file, line and
namep params. Call as_where. Report errors here. Delete macro
from macro_hash on attempt to redefined pseudo-op.
(delete_macro): Don't call free_macro.
* macro.h (define_macro): Update prototype.
* read.c (s_macro): Adjust to suit.
* config/tc-iq2000.c (iq2000_add_macro): Init all fields of
macro_entry.
Tom Tromey [Wed, 25 Jan 2023 17:01:57 +0000 (10:01 -0700)]
Use mi_clean_restart more
This changes a number of MI tests to use mi_clean_restart rather than
separate calls. This reduces the number of lines, which is nice, and
also provides a nicer model to copy for future tests.
Tom Tromey [Wed, 25 Jan 2023 17:00:58 +0000 (10:00 -0700)]
Start gdb after building executable in mi-basics.exp
A lot of the MI tests start gdb and only then build the executable.
This just seemed weird to me, so I've fixed this up. In this patch,
no other cleanups are done, the startup is just moved to a more
logical (to me) spot.
Tom Tromey [Wed, 25 Jan 2023 16:25:36 +0000 (09:25 -0700)]
Use ordinary calling convention for clean_restart
clean_restart accepts a single optional argument. Rather than using
{args} and handling the argument by hand, change it to use Tcl's own
argument-checking.
Alan Modra [Thu, 26 Jan 2023 08:29:07 +0000 (18:59 +1030)]
Sanity check dwarf5 form of .file
There's a comment a few lines earlier saying that demand_copy_C_string
has already reported an error if it returns NULL. Given the proximity
I decided not to duplicate the comment.
* dwarf2dbg.c (dwarf2_directive_filename): Check return of
demand_copy_C_string for file.
Simon Marchi [Thu, 26 Jan 2023 20:46:51 +0000 (15:46 -0500)]
gdb/testsuite: initialize "correct" variable in gdb.cp/cpexprs.exp.tcl
Due to a GDB bug (visible when building with -D_GLIBCXX_DEBUG), GDB
crashes somewhere in the middle of gdb.cp/cpexprs.exp, and thus fails to
read the string, at gdb.cp/cpexprs.exp.tcl:725. The "correct" variable
doesn't get set, and I then see this TCL error:
ERROR: can't read "correct": no such variable
Avoid the TCL error by initializing the "correct" variable to a dummy
value.
{"request_seq": 17, "type": "response", "command": "disassemble", "success": false, "message": "Cannot access memory at address 0x115d", "seq": 41}FAIL: gdb.dap/basic-dap.exp: disassemble one instruction success
FAIL: gdb.dap/basic-dap.exp: instructions in disassemble output
The problem is that the PC to disassemble is taken from the breakpoint
insertion response, which happens before running. With a PIE
executable, that PC is unrelocated, but the disassembly request happens
after relocation.
I chose to fix this by watching for a breakpoint changed event giving
the new breakpoint address, and recording the address from there. I
think this is an interesting way to fix it, because it adds a bit of
test coverage, I don't think these events are checked right now.
Other ways to fix it would be:
- Get the address by doing a breakpoint insertion after the program is
started, or some other way.
- Do the disassembly by symbol instead of by address.
- Do the disassembly before running the program.
Simon Marchi [Fri, 6 Jan 2023 18:27:14 +0000 (13:27 -0500)]
gdb/testsuite/dap: make dap_wait_for_event_and_check return preceding messages
In the following patch, I change gdb.dap/basic-dap.exp such that after
waiting for some event, it checks if it received another event
meanwhile. To help with this, make dap_wait_for_event_and_check and
_dap_dap_wait_for_event return a list with everything received before
the event of interest. This is similar to what
dap_check_request_and_response returns.
Simon Marchi [Fri, 6 Jan 2023 16:15:32 +0000 (11:15 -0500)]
gdb/testsuite/dap: pass around dicts instead of TON objects
The DAP helper functions generally return TON objects. However, callers
almost all immediately use ton::2dict to convert them to dicts, to
access their contents. This commits makes things a bit simpler for them
by having function return dicts directly instead.
The downside is that the TON objects contain type information. For
instance, a "2" in a TCL dict could have been the integer 2 or the
string "2" in JSON. By converting to TCL dicts, we lose that
information. If some tests specifically want to check the types of some
fields, I think we can add intermediary functions that return TON
objects, without having to complicate other callers who don't care.
Simon Marchi [Fri, 6 Jan 2023 16:39:29 +0000 (11:39 -0500)]
gdb/testsuite/dap: remove catch from dap_read_event
This catch didn't cause me any trouble, but for the same reason as the
preceding patch, I think it's a bit better to just let any exception
propagate, to make for easier debugging.
Simon Marchi [Fri, 6 Jan 2023 16:09:00 +0000 (11:09 -0500)]
gdb/testsuite/dap: make dap_request_and_response not catch / issue test result
Following some of my changes, dap_request_and_response was failing and I
didn't know why. I think it's better to make it not catch any
exception, and just make it do a simple "send request, read response".
If an exception is thrown while sending a request or reading a response,
things are going really badly, it's not like we'll want to recover from
that and continue the test.
Simon Marchi [Fri, 6 Jan 2023 15:37:10 +0000 (10:37 -0500)]
gdb/testsuite/dap: write requests to gdb.log
This helps following what happens when reading gdb.log. The downside is
that it becomes harder to tell what text is from GDB and what text is
going to GDB, but I think that seeing responses without seeing requests
is even more confusing. At least, the lines are prefix with >>>, so
when you see this, you know that until the end of the line, it's
something that was sent to GDB, and not GDB output.
Simon Marchi [Fri, 6 Jan 2023 15:35:53 +0000 (10:35 -0500)]
gdb/testsuite/dap: prefix some procs with _
Prefix some procs that are only used internally with an underscore, to
make it clear they are internal. If they need to be used by some test
later, we can always un-prefix them.
The gcc 4.4.x (and earlier) compilers had the problem that the unwind info in
the epilogue was inaccurate.
In order to work around this in gdb, epilogue unwinders were added with a
higher priority than the dwarf unwinders in the amd64 and i386 targets:
- amd64_epilogue_frame_unwind, and
- i386_epilogue_frame_unwind.
Subsequently, the epilogue unwind info problem got fixed in gcc 4.5.0.
However, the epilogue unwinders prevented gdb from taking advantage of the
fixed epilogue unwind info, so the scope of the epilogue unwinders was
limited, bailing out for gcc >= 4.5.0.
There was no regression test added for this preference scheme, so if we now
declare epilogue unwind info from all gcc versions as trusted, no test will
start failing.
Fix this by adding an amd64 and i386 regression test for this.
I have no gcc 4.4.x lying around, so I fabricated the assembly files by:
- commenting out some .cfi directives to break the epilogue unwind info, and
- hand-editing the producer info to 4.4.7 to activate the fix.
Tested on x86_64-linux, target boards unix/{-m64,-m32}.
Mark Harmstone [Mon, 23 Jan 2023 23:01:54 +0000 (23:01 +0000)]
ld: Add pdb support to aarch64-w64-mingw32
This extends PDB support to the aarch64 PE targets.
The changes to the test files are just to make it so they can be assembled as
either x86, x86_64, or aarch64, mainly by changing the comment style.
The only actual code change here is in adding the architecture constants
to pdb.c.
This patch resolves the performance issue reported in pr/29738 by
caching the values for the stack pointers for the inner frame. By
doing so, the impact can be reduced to checking the state and
returning the appropriate value.
gdb: dwarf2 generic implementation for caching function data
When there is no dwarf2 data for a register, a function can be called
to provide the value of this register. In some situations, it might
not be trivial to determine the value to return and it would cause a
performance bottleneck to do the computation each time.
This patch allows the called function to have a "cache" object that it
can use to store some metadata between calls to reduce the performance
impact of the complex logic.
The cache object is unique for each function and frame, so if there are
more than one function pointer stored in the dwarf2_frame_cache->reg
array, then the appropriate pointer will be supplied (the type is not
known by the dwarf2 implementation).
dwarf2_frame_get_fn_data can be used to retrieve the function unique
cache object.
dwarf2_frame_allocate_fn_data can be used to allocate and retrieve the
function unique cache object.
Tom Tromey [Wed, 25 Jan 2023 15:04:39 +0000 (08:04 -0700)]
Clean up unusual code in mi-cmd-stack.c
I noticed some unusual code in mi-cmd-stack.c. This code is a switch,
where one of the cases appears in the middle of another block. It
seemed cleaner to me to have the earlier case just conditionally fall
through.
Tom Tromey [Sun, 22 Jan 2023 20:38:26 +0000 (13:38 -0700)]
Use require with is_remote
This changes some tests to use require with 'is_remote', rather than
an explicit test. This adds uniformity helps clean up more spots
where a test might exit early without any notification.
Tom Tromey [Sun, 22 Jan 2023 17:01:05 +0000 (10:01 -0700)]
Add unsupported calls where needed
A number of tests will exit early without saying why. This patch adds
"unsupported" at spots like this that I could readily find.
There are definitely more of these; for example dw2-ranges.exp fails
silently because a compilation fails. I didn't attempt to address
these as that is a much larger task.
Tom Tromey [Sun, 22 Jan 2023 17:05:00 +0000 (10:05 -0700)]
Introduce and use is_any_target
A few tests work on two different targets that can't be detected with
a single call to istarget -- that proc only accepts globs, not regular
expressions.
This patch introduces a new is_any_target proc and then converts these
tests to use it in a 'require'.
Tom Tromey [Sun, 22 Jan 2023 05:41:25 +0000 (22:41 -0700)]
Use require with istarget
This changes many tests to use require when checking 'istarget'. A
few of these conversions were already done in earlier patches.
No change was needed to 'require' to make this work, due to the way it
is written. I think the result looks pretty clear, and it has the
bonus of helping to ensure that the reason that a test is skipped is
always logged.
Tom de Vries [Wed, 25 Jan 2023 15:35:53 +0000 (16:35 +0100)]
[gdb/testsuite] Fix gdb.base/unwind-on-each-insn.exp for -m32
With test-case gdb.base/unwind-on-each-insn.exp and target board unix/-m32, I
now get:
...
# of expected passes 25
...
instead of:
...
# of expected passes 133
...
as I used to get before commit d25a8dbc7c3 ("[gdb/testsuite] Allow debug
srcfile2 in gdb.base/unwind-on-each-insn.exp"), due to the test-case trying to match
"rip = " and info frame printing "eip = " instead.
Fix this by dropping "rip" from the regexp.
Tested on x86_64-linux, target boards unix/{-m64,-m32}.
Tom de Vries [Wed, 25 Jan 2023 12:44:17 +0000 (13:44 +0100)]
[gdb/testsuite] Allow nodebug srcfile in gdb.base/unwind-on-each-insn.exp
Test-case gdb.base/unwind-on-each-insn.exp compiles $srcfile with debug info, and
$srcfile2 without.
Occasionally, I try both files with debug info:
...
- $srcfile $debug_flags $srcfile2 $nodebug_flags]]} {
+ $srcfile $debug_flags $srcfile2 $debug_flags]]} {
...
and both files without:
...
- $srcfile $debug_flags $srcfile2 $nodebug_flags]]} {
+ $srcfile $nodebug_flags $srcfile2 $nodebug_flags]]} {
...
In the latter case, I run into:
...
FAIL: gdb.base/unwind-on-each-insn.exp: foo: instruction 1: bt 2
FAIL: gdb.base/unwind-on-each-insn.exp: foo: instruction 1: up
...
due to a mismatch between the regexp and the different output due to using
nodebug.
Tom de Vries [Wed, 25 Jan 2023 12:27:03 +0000 (13:27 +0100)]
[gdb/testsuite] Analyze non-leaf fn in gdb.base/unwind-on-each-insn.exp
In test-case gdb.base/unwind-on-each-insn.exp, we stepi through function foo
to check various invariants at each insn, in particular hoping to excercise
insns that modify stack and frame pointers.
Function foo is a leaf function, so add a non-leaf function bar, and step
through it as well (using nexti instead of stepi).
With the updated test-case, on powerpc64le-linux, I run into PR tdep/30049:
...
FAIL: bar: instruction 5: bt 2
FAIL: bar: instruction 5: up
FAIL: bar: instruction 5: [string equal $fid $::main_fid]
FAIL: bar: instruction 6: bt 2
FAIL: bar: instruction 6: up
FAIL: bar: instruction 6: [string equal $fid $::main_fid]
...
Tested on:
- x86_64-linux (-m64 and -m32)
- s390x-linux (-m64 and -m31)
- aarch64-linux
- powerpc64le-linux
Tom de Vries [Wed, 25 Jan 2023 12:27:03 +0000 (13:27 +0100)]
[gdb/testsuite] Improve leaf fn in gdb.base/unwind-on-each-insn.exp
In test-case gdb.base/unwind-on-each-insn.exp, we stepi through function foo
to check various invariants at each insn, in particular hoping to excercise
insns that modify stack and frame pointers.
For x86_64-linux, we have a reasonably complex foo (and similar for -m32):
...
4004bc: 55 push %rbp
4004bd: 48 89 e5 mov %rsp,%rbp
4004c0: 90 nop
4004c1: 5d pop %rbp
4004c2: c3 ret
...
Both stack pointer (%rsp) and frame pointer (%rbp) are modified.
Also for s390x-linux (and similar for -m31):
... 0000000001000668 <foo>: 1000668: e3 b0 f0 58 00 24 stg %r11,88(%r15) 100066e: b9 04 00 bf lgr %r11,%r15 1000672: e3 b0 b0 58 00 04 lg %r11,88(%r11) 1000678: 07 fe br %r14
...
Frame pointer (%r11) is modified.
But for aarch64-linux, we have:
... 00000000004005fc <foo>:
4005fc: d503201f nop
400600: d65f03c0 ret
...
There's no insn that modifies stack or frame pointer.
Fix this by making foo more complex, adding an (unused) argument:
... 0000000000400604 <foo>:
400604: d10043ff sub sp, sp, #0x10
400608: f90007e0 str x0, [sp, #8]
40060c: d503201f nop
400610: 910043ff add sp, sp, #0x10
400614: d65f03c0 ret
...
such that the stack pointer (sp) is modified.
[ Note btw that we're seeing the effects of -momit-leaf-frame-pointer, with
-mno-omit-leaf-frame-pointer we have instead:
... 0000000000400604 <foo>:
400604: a9be7bfd stp x29, x30, [sp, #-32]!
400608: 910003fd mov x29, sp
40060c: f9000fa0 str x0, [x29, #24]
400610: d503201f nop
400614: a8c27bfd ldp x29, x30, [sp], #32
400618: d65f03c0 ret
...
such that also the frame pointer (x29) is modified. ]
Having made foo more complex, we now run into the following fail on
x86_64/-m32:
...
FAIL: gdb.base/unwind-on-each-insn.exp: instruction 1: $sp_value == $main_sp
....
The problem is that the stack pointer has changed inbetween the sampling of
main_sp and *foo, in particular by the push insn:
... 804841a: 68 c0 84 04 08 push $0x80484c0 804841f: e8 10 00 00 00 call 8048434 <foo>
...
Fix this by updating main_sp.
On powerpc64le-linux, with gcc 7.5.0 I now run into PR tdep/30021:
...
FAIL: gdb.base/unwind-on-each-insn.exp: insn 7: $fba_value == $main_fba
FAIL: gdb.base/unwind-on-each-insn.exp: insn 7: [string equal $fid $main_fid]
...
but I saw the same failure before this commit with gcc 4.8.5.
Tested on:
- x86_64-linux (-m64 and -m32)
- s390x-linux (-m64 and -m31)
- powerpc64le-linux
- aarch64-linux
Also I've checked that the test-case still functions as regression test for PR
record/16678 on x86_64.
Andrew Burgess [Tue, 20 Dec 2022 15:45:32 +0000 (15:45 +0000)]
gdb/tui: better filtering of tab completion results for focus command
While working on the previous couple of commits, I noticed that the
'focus' command would happily suggest 'status' as a possible focus
completion, even though the 'status' window is non-focusable, and,
after the previous couple of commits, trying to focus the status
window will result in an error.
This commit improves the tab-completion results for the focus command
so that the status window is not included.
Andrew Burgess [Thu, 22 Dec 2022 12:43:38 +0000 (12:43 +0000)]
gdb/tui: convert if/error to an assert
While working on the previous commit, I realised that there was an
error in tui_set_focus_command that could never be triggered.
Since the big tui rewrite (adding dynamic layouts) it is no longer
true that there is a tui_win_info object for every window at all
times. We now only create a tui_win_info object for a particular
window, when the window is part of the current layout. As a result,
if we have a tui_win_info pointer, then the window must be visible
inside tui_set_focus_command (this function calls tui_enable as its
first action, which makes the current layout visible).
The gdb.tui/tui-focus.exp test script exercises this area of code, and
doesn't trigger the assert, nor do any of our other existing tui
tests.
Andrew Burgess [Tue, 20 Dec 2022 15:01:29 +0000 (15:01 +0000)]
gdb/testsuite/tui: more testing of the 'focus' command
I noticed that we didn't have many tests of the tui 'focus' command,
so I started adding some. This exposed a bug in GDB; we are able to
focus windows that should not be focusable, e.g. the 'status' window.
This is harmless until we then do 'focus next' or 'focus prev', along
this code path we assert that the currently focused window is
focusable, which obviously, is not always true, so GDB fails with an
assertion error.
The fix is simple; add a check to the tui_set_focus_command function
to ensure that the selected window is focusable. If it is not then an
error is thrown. The new tests I've added cover this case.
Following on from the previous commit, in this commit I am updating
the test script gdb.tui/tui-nl-filtered-output.exp to take account of
the changes in commit:
In the above commit the TERM environment variable was changed to be
'dumb' by default, which means that tests, that previously activated
tui mode, no longer do unless TERM is set to 'ansi'.
As the gdb.tui/tui-nl-filtered-output.exp script didn't do this, the
test stopped working. As the expect patterns in this script were
pretty generic no tests actually started failing, and we never
noticed.
In this commit I update the test script to correctly activate our
terminal emulator, the test continues to pass after this update, but
now we are testing in tui mode.
Following on from the previous commit, in this commit I am updating
the test script gdb.tui/tui-disasm-long-lines.exp to take account of
the changes in commit:
In the above commit the TERM environment variable was changed to be
'dumb' by default, which means that tests, that previously activated
tui mode, no longer do unless TERM is set to 'ansi'.
As the gdb.tui/tui-disasm-long-lines.exp script didn't do this, the
test stopped working. As the expect patterns in this script were
pretty generic no tests actually started failing, and we never
noticed.
In this commit I update the script to use Term::clean_restart, which
correctly sets TERM to 'ansi'. I've also added a check that the asm
box does appear on the screen, which should indicate that tui mode has
correctly activated.
However, I also notice that GDB doesn't appear to fully work
correctly. The test should display the disassembly for the test
program, but it doesn't.
The test is trying to disassemble some code that (deliberately) uses a
very long symbol name, this eventually results in GDB entering
tui_source_window_base::show_source_content and trying to allocate an
ncurses pad in order to hold the current page of disassembler output.
Unfortunately, due to the very long line, the call to newpad fails,
meaning that tui_source_window_base::m_pad is nullptr. Luckily non of
the following calls appear to crash when passed a nullptr, however,
all the output that is written to the pad is lost, which is why we
don't see any assembly code written to the screen.
As the test history indicates that the script was originally checking
for a crash in GDB when the long identifier was encountered, I think
there is value in just leaving the test as it is for now, I have a fix
for the issue of the newpad call failing, which I'll post in a follow
up commit later.
Andrew Burgess [Fri, 25 Nov 2022 18:19:16 +0000 (18:19 +0000)]
gdb/testsuite: extend gdb.tui/tui-layout.exp test script
In passing I noticed that the gdb.tui/tui-layout.exp test script was a
little strange, it tests the layout command multiple times, but never
sets up our ANSI terminal emulator, so every layout command fails with
a message about the terminal lacking the required abilities.
This was when we changed the testsuite to set the TERM environment
variable to "dumb" by default.
After this, any tui test that didn't set the terminal mode back to
'ansi' would fail to activate tui mode.
For the tui-layout.exp test it just so happens that the test patterns
are generic enough that the test continued to pass, even after this
change.
In this commit I have updated the test so we now check the layout
command both with a 'dumb' terminal and with the 'ansi' terminal.
When testing with the 'ansi' terminal, I have some limited validation
that GDB correctly entered tui mode.
I figured that it is probably worth having at least one test in the
test suite that deliberately tries to enter tui mode in a dumb
terminal, it would be sad if we one day managed to break GDB such that
this caused a crash, and never noticed.