Age | Commit message (Collapse) | Author | Files | Lines |
|
The argument documentation for -data-disassemble looks like this:
-data-disassemble
[ -s @var{start-addr} -e @var{end-addr} ]
| [ -a @var{addr} ]
| [ -f @var{filename} -l @var{linenum} [ -n @var{lines} ] ]
-- @var{mode}
However, I believe, according to the 'Notation and Terminology'
section, this means that the there are 3 optional location
specification argument groups for the command, followed by a
non-optional '-- mode'.
However, this is not true, one of the location specifications must be
given, i.e. we can't choose to give NO location specification, which
is what the above implies.
I propose that we change this to instead be:
-data-disassemble
( -s @var{start-addr} -e @var{end-addr}
| -a @var{addr}
| -f @var{filename} -l @var{linenum} [ -n @var{lines} ] )
-- @var{mode}
By placing all the location specifications within '( ... )' we
indication that these are a group, from which one of the options,
separated by '|', must be selected.
However, the 'Notation and Terminology' section only describes two
uses for parenthesis: '( GROUP )*' and '( GROUP )+', in the first case
GROUP is repeated zero or more times, and in the second GROUP is
repeated 1 or more times.
Neither of those exactly describe what I want, which is GROUP must
appear exactly once. I propose to extend 'Notation and Terminology'
to include '( GROUP )' which means that GROUP should appear exactly
once.
This change is important because, in a later commit, I want to add
additional optional arguments to the -data-disassemble command, and
things start to get confusing with the original syntax.
|
|
In a later commit I want to use operator~ on a gdb_disassembly_flag
flag value. This is currently not possible as gdb_disassembly_flag
is, by default, signed.
This commit just makes this enum unsigned.
There should be no user visible changes after this commit.
|
|
This commit changes the format of 'disassemble /r' to match GNU
objdump. Specifically, GDB will now display the instruction bytes in
as 'objdump --wide --disassemble' does.
Here is an example for RISC-V before this patch:
(gdb) disassemble /r 0x0001018e,0x0001019e
Dump of assembler code from 0x1018e to 0x1019e:
0x0001018e <call_me+66>: 03 26 84 fe lw a2,-24(s0)
0x00010192 <call_me+70>: 83 25 c4 fe lw a1,-20(s0)
0x00010196 <call_me+74>: 61 65 lui a0,0x18
0x00010198 <call_me+76>: 13 05 85 6a addi a0,a0,1704
0x0001019c <call_me+80>: f1 22 jal 0x10368 <printf>
End of assembler dump.
And here's an example after this patch:
(gdb) disassemble /r 0x0001018e,0x0001019e
Dump of assembler code from 0x1018e to 0x1019e:
0x0001018e <call_me+66>: fe842603 lw a2,-24(s0)
0x00010192 <call_me+70>: fec42583 lw a1,-20(s0)
0x00010196 <call_me+74>: 6561 lui a0,0x18
0x00010198 <call_me+76>: 6a850513 addi a0,a0,1704
0x0001019c <call_me+80>: 22f1 jal 0x10368 <printf>
End of assembler dump.
There are two differences here. First, the instruction bytes after
the patch are grouped based on the size of the instruction, and are
byte-swapped to little-endian order.
Second, after the patch, GDB now uses the bytes-per-line hint from
libopcodes to add whitespace padding after the opcode bytes, this
means that in most cases the instructions are nicely aligned.
It is still possible for a very long instruction to intrude into the
disassembled text space. The next example is x86-64, before the
patch:
(gdb) disassemble /r main
Dump of assembler code for function main:
0x0000000000401106 <+0>: 55 push %rbp
0x0000000000401107 <+1>: 48 89 e5 mov %rsp,%rbp
0x000000000040110a <+4>: c7 87 d8 00 00 00 01 00 00 00 movl $0x1,0xd8(%rdi)
0x0000000000401114 <+14>: b8 00 00 00 00 mov $0x0,%eax
0x0000000000401119 <+19>: 5d pop %rbp
0x000000000040111a <+20>: c3 ret
End of assembler dump.
And after the patch:
(gdb) disassemble /r main
Dump of assembler code for function main:
0x0000000000401106 <+0>: 55 push %rbp
0x0000000000401107 <+1>: 48 89 e5 mov %rsp,%rbp
0x000000000040110a <+4>: c7 87 d8 00 00 00 01 00 00 00 movl $0x1,0xd8(%rdi)
0x0000000000401114 <+14>: b8 00 00 00 00 mov $0x0,%eax
0x0000000000401119 <+19>: 5d pop %rbp
0x000000000040111a <+20>: c3 ret
End of assembler dump.
Most instructions are aligned, except for the very long instruction.
Notice too that for x86-64 libopcodes doesn't request that GDB group
the instruction bytes. This matches the behaviour of objdump.
In case the user really wants the old behaviour, I have added a new
modifier 'disassemble /b', this displays the instruction byte at a
time. For x86-64, which never groups instruction bytes, /b and /r are
equivalent, but for RISC-V, using /b gets the old layout back (except
that the whitespace for alignment is still present). Consider our
original RISC-V example, this time using /b:
(gdb) disassemble /b 0x0001018e,0x0001019e
Dump of assembler code from 0x1018e to 0x1019e:
0x0001018e <call_me+66>: 03 26 84 fe lw a2,-24(s0)
0x00010192 <call_me+70>: 83 25 c4 fe lw a1,-20(s0)
0x00010196 <call_me+74>: 61 65 lui a0,0x18
0x00010198 <call_me+76>: 13 05 85 6a addi a0,a0,1704
0x0001019c <call_me+80>: f1 22 jal 0x10368 <printf>
End of assembler dump.
Obviously, this patch is a potentially significant change to the
behaviour or /r. I could have added /b with the new behaviour and
left /r alone. However, personally, I feel the new behaviour is
significantly better than the old, hence, I made /r be what I consider
the "better" behaviour.
The reason I prefer the new behaviour is that, when I use /r, I almost
always want to manually decode the instruction for some reason, and
having the bytes displayed in "instruction order" rather than memory
order, just makes this easier.
The 'record instruction-history' command also takes a /r modifier, and
has been modified in the same way as disassemble; /r gets the new
behaviour, and /b has been added to retain the old behaviour.
Finally, the MI command -data-disassemble, is unchanged in behaviour,
this command now requests the raw bytes of the instruction, which is
equivalent to the /b modifier. This means that the MI output will
remain backward compatible.
|
|
This commit reduces the number of times we call read_code when
printing the instruction opcode bytes during disassembly.
I've added a new gdb::byte_vector within the
gdb_pretty_print_disassembler class, in line with all the other
buffers that gdb_pretty_print_disassembler needs. This byte_vector is
then resized as needed, and filled with a single read_code call for
each instruction.
There should be no user visible changes after this commit.
|
|
Add another test for the output of MI command -data-disassemble. The
new check validates the format of the 'opcodes' field, specifically,
this test checks that the field contains a series of bytes, separated
by a single space.
We also check that the bytes are in the correct order, that is, the
first byte is from the lowest address, and subsequent bytes are from
increasing addresses.
The motivation for this test (besides more tests being generally good)
is that I plan to make changes to how opcode bytes are displayed in
the disassembler output, and I want to ensure that I don't break any
existing MI behaviour.
There should be no user visible changes to GDB after this commit.
|
|
On openSUSE Tumbleweed, I run into:
...
FAIL: gdb.mi/mi-sym-info.exp: List all functions from debug information only
...
The problem is in matching this string:
...
{name="_start",type="void (void)",description="void _start(void);"}
...
using regexp fun_re, which requires a line field:
...
set fun_re \
"\{line=\"$decimal\",name=${qstr},type=${qstr},description=${qstr}\}"
...
Fix this by making the line field optional in fun_re.
Tested on x86_64-linux.
|
|
Clang generates a warning if there is a variable that is set but not used
otherwise ("-Wunused-but-set-variable"). On the default configuration, it
causes a build failure (unless "--disable-werror" is specified).
The only extra_lines use in arrange_linetable function is removed on the
commit 558802e4d1c5dcbd0df7d2c6ef62a6deac247a2f
("gdb: change subfile::line_vector to an std::vector"). So, this variable
should be removed to prevent a build failure.
|
|
Since commit 52b920c5d20 ("[gdb/testsuite] Fix gdb.dwarf2/dw2-dir-file-name.exp
for ppc64le"), the test-case fails with target board cc-with-debug-names, due
to missing .debug_aranges info.
Add the missing .debug_aranges info.
Also add a file_id option to Dwarf::assemble, to make it possible to contribute
to an already open file.
Tested on x86_64-linux.
|
|
Consider the test-case contained in this patch.
With g++ (7.5.0) we have for "ptype A":
...
type = class A {
public:
int a;
A(void);
~A();
}
...
and with clang++ (13.0.1):
...
type = class A {
public:
int a;
A(void);
~A(void);
}
...
and we observe that the destructor is printed differently.
There's a difference in debug info between the two cases: in the clang case,
there's one artificial parameter, but in the g++ case, there are two, and
these similar cases are handled differently in cp_type_print_method_args.
This is due to this slightly convoluted bit of code:
...
i = staticp ? 0 : 1;
if (nargs > i)
{
while (i < nargs)
...
}
else if (varargs)
gdb_printf (stream, "...");
else if (language == language_cplus)
gdb_printf (stream, "void");
...
The purpose of "i = staticp ? 0 : 1" is to skip the printing of the implicit
this parameter.
In commit 5f4d1085085 ("c++/8218: Destructors w/arguments"), skipping of other
artificial parameters was added, but using a different method: rather than
adjusting the potential loop start, it skips the parameter in the loop.
The observed difference in printing is explained by whether we enter the loop:
- in the clang case, the loop is not entered and we print "void".
- in the gcc case, the loop is entered, and nothing is printed.
Fix this by rewriting the code to:
- always enter the loop
- handle whether arguments need printing in the loop
- keep track of how many arguments are printed, and
use that after the loop to print void etc.
such that we have the same for both gcc and clang:
...
A(void);
~A(void);
...
Note that I consider the discussion of whether we want to print:
- A(void) / ~A(void), or
- A() / ~A()
out-of-scope for this patch.
Tested on x86_64-linux.
|
|
It is only used in auxv.c. Also, given it is not strictly a wrapper
around target_ops::auxv (since 27a48a9223d0 "Add auxv parsing to the
architecture vector."), I think that the name prefixed with target is a
bit misleading. Rename to just parse_auxv.
Change-Id: I41cca055b92c8ede37c258ba6583746a07d8f77e
|
|
It's only used in auxv.c.
Change-Id: I4992d9aae37b6631a074ab99bbab2f619725b642
|
|
Constify the input parameters of the various auxv parse functions, they
don't need to modify the raw auxv data.
Change-Id: I13eacd5ab8e925ec2b5c1f7722cbab39c41516ec
|
|
The target_ops parameters here can be made const.
Change-Id: Ibc18b17d6b21d06145251a03e68aca90538117d6
|
|
This patch changes various global target_desc declarations to const, thereby
correcting a prominent source of ODR violations in PowerPC-related target code.
The majority of files/changes are mechanical const-ifications accomplished by
regenerating the C files in features/.
This also required manually updating mips-linux-tdep.h, s390-linux-tdep.h,
nios2-tdep.h, s390-tdep.h, arch/ppc-linux-tdesc.h, arch/ppc-linux-common.c,
and rs6000-tdep.c.
Patch tested against the sourceware trybot, and fully regression tested against
our (Red Hat's) internal test infrastructure on Rawhide aarch64, s390x, x86_64,
and powerpcle.
With this patch, I can finally enable LTO in our GDB package builds. [Tested
with a rawhide scratch build containing this patch.]
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=22395
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=24835
|
|
This patch adds some missing .xml files to features/Makefile so that when the
directory's C files are regenerated, all files are appropriately remade.
This has demonstrated that there have been several "misses" in regenerating
files in this directory. Namely, arm-secext.c and sparc{32,64}-solaris.c. For
the former case, there was what essentially amounts to a typo regarding the
create feature function's name. In the later case, this file has missed at least
one important update in July, 2020, when allocate_target_description was
changed to return a unique pointer.
Those corrections are included.
|
|
GDB fails to build for me, on Ubuntu 20.04. I get:
...
CXXLD gdb
/usr/bin/ld: linux-tdep.o: in function `linux_corefile_thread(thread_info*, linux_corefile_thread_data*)':
/home/pedro/gdb/binutils-gdb/src/gdb/linux-tdep.c:1831: undefined reference to `gcore_elf_build_thread_register_notes(gdbarch*, thread_info*, gdb_signal, bfd*, std::unique_ptr<char, gdb::xfree_deleter<char> >*, int*)'
/usr/bin/ld: linux-tdep.o: in function `linux_make_corefile_notes(gdbarch*, bfd*, int*)':
/home/pedro/gdb/binutils-gdb/src/gdb/linux-tdep.c:2117: undefined reference to `gcore_elf_make_tdesc_note(bfd*, std::unique_ptr<char, gdb::xfree_deleter<char> >*, int*)'
collect2: error: ld returned 1 exit status
make[2]: *** [Makefile:2149: gdb] Error 1
make[2]: Leaving directory '/home/pedro/gdb/binutils-gdb/build/gdb'
make[1]: *** [Makefile:11847: all-gdb] Error 2
make[1]: Leaving directory '/home/pedro/gdb/binutils-gdb/build'
make: *** [Makefile:1004: all] Error 2
Those undefined functions exist in gdb/gcore-elf.c, which is only
included in the build if GDB's configure thinks that the target you're
configuring for is an ELF target. GDB's configure thinks my system
isn't ELF, which is incorrect.
For the ELF support check, gdb/config.log shows:
configure:17387: checking for ELF support in BFD
configure:17407: gcc -o conftest -I/home/pedro/gdb/binutils-gdb/src/gdb/../include -I../bfd -I/home/pedro/gdb/binutils-gdb/src/gdb/../bfd -g3 -O0 -L../bfd -L../libiberty -lzstd conftest.c -lbfd -liberty -lz -lncursesw -lm -ldl >&5
/usr/bin/ld: ../bfd/libbfd.a(compress.o): in function `decompress_contents':
/home/pedro/gdb/binutils-gdb/src/bfd/compress.c:42: undefined reference to `ZSTD_decompress'
/usr/bin/ld: /home/pedro/gdb/binutils-gdb/src/bfd/compress.c:44: undefined reference to `ZSTD_isError'
/usr/bin/ld: ../bfd/libbfd.a(compress.o): in function `bfd_compress_section_contents':
/home/pedro/gdb/binutils-gdb/src/bfd/compress.c:195: undefined reference to `ZSTD_compress'
/usr/bin/ld: /home/pedro/gdb/binutils-gdb/src/bfd/compress.c:198: undefined reference to `ZSTD_isError'
collect2: error: ld returned 1 exit status
configure:17407: $? = 1
...
configure:17417: result: no
Note how above, in the gcc command line, "-lzstd" appears before
"-lbfd". That explain the link failure. It should appear after, like
-lz does.
This commit fixes it, by moving ZSTD_LIBS from LDFLAGS to LIBS, next
to -lz, in GDB_AC_CHECK_BFD, and regenerating gdb/configure.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29630
Change-Id: I1f4128dde634e8ea04c9002904f1005a8b3a6863
|
|
Change-Id: Ic7f8e415acd1bff6194cf08ed646bff45571f165
|
|
A user noticed that gdb would assert when printing a certain array
with array-indexes enabled. This turned out to be caused by the array
having an index type of Character, which is completely valid in Ada.
This patch changes the Ada support to recognize Character as a
discrete type, and adds some tests.
Because this is Ada-specific and was also reviewed internally, I am
checking it in.
|
|
Pick up config/lib-ld.m4 changes from:
commit 67d1991b785bdfef1d70cddfa0202b99b43ccce9
Author: Alan Modra <amodra@gmail.com>
AuthorDate: Wed Sep 28 13:37:31 2022 +0930
Commit: Alan Modra <amodra@gmail.com>
CommitDate: Wed Sep 28 13:37:31 2022 +0930
egrep in binutils
Change-Id: Ifc84d30f1fca015e80bafa80f9a35616b0077220
|
|
PR29397 PR29563: Add new configure option --with-zstd which defaults to
auto. If pkgconfig/libzstd.pc is found, define HAVE_ZSTD and support
zstd compressed debug sections for most tools.
* bfd: for addr2line, objdump --dwarf, gdb, etc
* gas: support --compress-debug-sections=zstd
* ld: support ELFCOMPRESS_ZSTD input and --compress-debug-sections=zstd
* objcopy: support ELFCOMPRESS_ZSTD input for
--decompress-debug-sections and --compress-debug-sections=zstd
* gdb: support ELFCOMPRESS_ZSTD input. The bfd change references zstd
symbols, so gdb has to link against -lzstd in this patch.
If zstd is not supported, ELFCOMPRESS_ZSTD input triggers an error. We
can avoid HAVE_ZSTD if binutils-gdb imports zstd/ like zlib/, but this
is too heavyweight, so don't do it for now.
```
% ld/ld-new a.o
ld/ld-new: a.o: section .debug_abbrev is compressed with zstd, but BFD is not built with zstd support
...
% ld/ld-new a.o --compress-debug-sections=zstd
ld/ld-new: --compress-debug-sections=zstd: ld is not built with zstd support
% binutils/objcopy --compress-debug-sections=zstd a.o b.o
binutils/objcopy: --compress-debug-sections=zstd: binutils is not built with zstd support
% binutils/objcopy b.o --decompress-debug-sections
binutils/objcopy: zstd.o: section .debug_abbrev is compressed with zstd, but BFD is not built with zstd support
...
```
|
|
Patches that renamed the type::length and type::target_type fields
didn't update gdb-gdb.py.in accordingly, do that.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29599
Change-Id: I0f3f37a94d43497789156b0ded4d2f2dd5b89496
|
|
If some command in there gives the wrong answer, we currently have to
wait for a timeout for the test to continue. For instance, I currently
see:
print *val->type
$1 = Python Exception <class 'gdb.error'>: Cannot take address of method length.
(outer-gdb) FAIL: gdb.gdb/python-helper.exp: pretty print type (timeout)
We can avoid this and modernize the test at the same time by using the
-prompt option of gdb_test.
gdb_test_no_output currently accepts a -prompt_re option (the variable
name passed to parse_args defines the option name), but I think
it's a typo. It's supposed to be -prompt, like gdb_test. I can't find
anything using -prompt_re using grep. Change it to just "prompt".
Change-Id: Icc0a9a0ef482e62460c708bccdd544c11d711eca
|
|
When running gdb.gdb/python-helper.exp, I get some timeouts:
continue
Continuing.
print 1
FAIL: gdb.gdb/python-helper.exp: hit breakpoint in outer gdb (timeout)
At this time, GDB is actually processing the stop and reading in some
CUs. selftest_setup does bump the timeout, but it's not for the whole
test.
Since debugging GDB with GDB is (unfortunately) a bit slow, bump the
timeout for the whole duration of the setup and body. On my optimized
build, the command takes just a bit more than the current timeout of 10
seconds. But it's much slower if running the test on an unoptimized
build, so I think it's necessary to bump the timeout for that in any
case.
Change-Id: I4d38285870e76c94f9d0bfdb60648a2e7f2cfa5d
|
|
When running test-case gdb.dwarf2/dw2-unspecified-type-foo.c with target board
unix/-m32, I run into:
...
(gdb) PASS: gdb.dwarf2/dw2-unspecified-type.exp: ptype foo
p ((int (*) ()) foo) ()^M
$1 = -135698472^M
...
Add the missing "return 0" in foo, which fixes this.
Tested on x86_64-linux.
|
|
When a source file's dirname is solely made up of directory separators
we end up trying to dereference the last character of an empty string
with std::string::back, which results in undefined behaviour. A typical
use case where this can happen is when the root directory "/" is used as
a compilation directory.
With libstdc++.so.6.0.28 we get no out-of-bounds checks and the byte
preceding the storage of the empty string is returned. The character
value of this byte depends on heap implementation and usage, but when
this byte happens to hold the value of the directory separator character
we go on to call std::string::pop_back on the empty string which results
in an out_of_range exception which terminates GDB.
Fix this by using path_join. prepare_path_for_appending ensures that the
filename component is relative.
The testsuite has been run before and after the change and no
regressions were found.
|
|
FreeBSD's kernel has recently added two new ELF auxiliary vector
entries to describe the location of the user stack for the initial
thread in a process.
This change displays the proper name and description of these entries
in 'info auxv'.
|
|
The test gdb.tui/tui-missing-src.exp fails on my CI machine, and I
concluded that it is caused by the long source directory name:
/home/jenkins/workspace/binutils-gdb_master_linuxbuild/platform/jammy-amd64/target_board/unix/src/binutils-gdb
The long name causes some particular redrawing that doesn't happen for
shorter directories, and causes a Term::command call to return too
early.
This can be reproduced by cloning the binutils-gdb repo in a directory
with a name similar to the one shown above.
$ pwd
/home/simark/workspace/binutils-gdb_master_linuxbuild/platform/jammy-amd64/target_board/unix/src/binutils-gdb/build/gdb
$ make check-read1 TESTS="gdb.tui/tui-missing-src.exp"
FAIL: gdb.tui/tui-missing-src.exp: checking if inside f2 ()
FAIL: gdb.tui/tui-missing-src.exp: f2.c must be displayed in source window
FAIL: gdb.tui/tui-missing-src.exp: check source box is empty after return
FAIL: gdb.tui/tui-missing-src.exp: Back in main
Note that using "make check" instead of "make check-read1" only shows
the last 2 failures for me.
When running gdb.tui/tui-missing-src.exp in a directory with a shorter
name, the terminal looks like this by the time the "checking if inside
f2" test runs:
Screen Dump (size 80 columns x 24 rows, cursor at column 6, row 23):
0 +-...ld/binutils-gdb-noasan/gdb/testsuite/outputs/gdb.tui/tui-missing-src/f2.c-+
1 | 1 |
2 | 2 int |
3 | 3 f2 (int x) |
4 | 4 { |
5 | > 5 x <<= 1; |
6 | 6 return x+5; |
7 | 7 } |
8 | 8 |
9 | 9 |
10 | 10 |
11 | 11 |
12 | 12 |
13 | 13 |
14 +------------------------------------------------------------------------------+
15 multi-thre Thread 0x7ffff7cc07 In: f2 L5 PC: 0x555555555143
16 at /home/simark/build/binutils-gdb-noasan/gdb/testsuite/outputs/gdb.tui/tui-
17 missing-src/main.c:6
18 (gdb) next
19 (gdb) step
20 f2 (x=4)
21 at /home/simark/build/binutils-gdb-noasan/gdb/testsuite/outputs/gdb.tui/tui-
22 missing-src/f2.c:5
23 (gdb)
PASS: gdb.tui/tui-missing-src.exp: checking if inside f2 ()
When running the `Term::command "step"` just before, GDB writes the
"step", which makes the `wait_for` proc go in the "looking for the
prompt" mode, to know when the command's execution is complete. As some
new output appears, lines that must disappear are deleted using the
"Delete Line" operation [1] and some new ones are drawn. The source
window gets redrawn with the contents of the f2.c file. Then, GDB
writes the prompt (at line 23 above), which satisfies `wait_for`, which
then returns. The state of the terminal is therefore correct for the
"check if inside f2" and "f2.c must be displayed in the source window"
tests.
In the non-working case, the terminal looks like this by the time the
"check if inside f2" test runs:
Screen Dump (size 80 columns x 24 rows, cursor at column 6, row 17):
0 +------------------------------------------------------------------------------+
1 | |
2 | |
3 | |
4 | |
5 | |
6 | |
7 | [ No Source Available ] |
8 | |
9 | |
10 | |
11 | |
12 | |
13 | |
14 +------------------------------------------------------------------------------+
15 multi-thre Thread 0x7ffff7cc1b In: main L7 PC: 0x555555555128
16 sing-src/main.c:6
17 (gdb) ary breakpoint 1, main ()
18 at /home/simark/workspace/binutils-gdb_master_linuxbuild/platform/jammy-amd6
19 4/target_board/unix/src/binutils-gdb/build/gdb/testsuite/outputs/gdb.tui/tui-mis
20 sing-src/main.c:6
21 (gdb) next
22 (gdb) step
23
FAIL: gdb.tui/tui-missing-src.exp: checking if inside f2 ()
What happened is: GDB wrote the "step" command, which make the
`wait_for` proc go in its "looking for the prompt" mode. However,
curses decided to redraw whatever scrolled up to line 17 using some
standard character insertion operations:
+++ Cursor Down (1), cursor: (16, 0) -> (17, 0)
+++ Inserting string '('
+++ Inserted char '(', cursor: (17, 0) -> (17, 1)
+++ Inserted string '(', cursor: (17, 0) -> (17, 1)
+++ Inserting string 'g'
+++ Inserted char 'g', cursor: (17, 1) -> (17, 2)
+++ Inserted string 'g', cursor: (17, 1) -> (17, 2)
+++ Inserting string 'd'
+++ Inserted char 'd', cursor: (17, 2) -> (17, 3)
+++ Inserted string 'd', cursor: (17, 2) -> (17, 3)
+++ Inserting string 'b'
+++ Inserted char 'b', cursor: (17, 3) -> (17, 4)
+++ Inserted string 'b', cursor: (17, 3) -> (17, 4)
+++ Inserting string ')'
+++ Inserted char ')', cursor: (17, 4) -> (17, 5)
+++ Inserted string ')', cursor: (17, 4) -> (17, 5)
+++ Inserting string ' '
+++ Inserted char ' ', cursor: (17, 5) -> (17, 6)
+++ Inserted string ' ', cursor: (17, 5) -> (17, 6)
And that causes `wait_for` to think the "step" command is complete.
This is wrong, as the prompt at line 17 isn't the prompt drawn after the
completion of the "step" command. The subsequent tests now run with a
partially updated screen (what is shown above) and obviously fail.
The ideal way to fix this would be for `wait_for` to be smarter, to
avoid it confusing the different prompts drawn.
However, I would also like to reduce the variations in TUI test results
due to the directories (source and build) in which tests are ran. TUI
tests are more prone to differences in test results due to variations in
directory names than other tests, as it makes curses take different
redrawing decisions. So in this patch, I propose to make TUI tests use
"set filename-display basename", which makes GDB omit directory names
when it prints file names. This way, regardless of where you run the
tests, you should get the same results (all other things being equal).
Doing this happens to fix my failures and makes my CI happy (which in
turns makes me happy). To be clear, I understand that this does not fix
the root issue of `proc wait_for` being confused. However, it makes TUI
test runs be more similar for everyone, such that there's less chance of
TUI tests randomly failing for somebody. If some other change triggers
the `wait_for` problem again in the future, hopefully everybody will see
the problem and we can work on getting it fixed more easily than if just
one unlucky person sees the problem.
Note that there are other reasons why TUI tests could vary, like
different curses library versions taking different re-drawing decisions.
However, I think my change is a good step towards more stable test
results.
[1] https://vt100.net/docs/vt510-rm/DL.html
Change-Id: Ib18da83317e7b78a46f77892af0d2e39bd261bf5
|
|
Add cskyv2-linux.xml for re-generating cskyv2-linux.c if needed.
Also update cskyv2-linux.c.
|
|
GDB's testsuite can override dejagnu's default_target_compile if the
system provided dejagnu installation does not provide support to compile
languages GDB needs.
Recent version of dejagnu (1.6.3, installed on RHEL-9) includes ba60272
"Establish a default C compiler by evaluating [find_gcc] if no other
compiler is given."[1]. This commit removed calls such as
`set_board_info compiler "[find_gcc]"` from the various baseboards
and has default_target_compile call `find_gcc` itself to find a compiler
if none was specified by the board description.
On systems with dejagnu-1.6.3, if GDB's overrides is needed to support
languages still unknown to dejagnu, we end up in the following
situation:
- The system board files do not set the C compiler anymore,
- GDB's replacement for default_target_compile assumes that the
compiler should have been set up by the board file.
In this situation, no one sets the C compiler for the board and as a
result many test are not compiled and not executed:
[...]
Running .../gdb/testsuite/gdb.base/bt-on-error-and-warning.exp ...
gdb compile failed, default_target_compile: No compiler to compile with
Running .../gdb/testsuite/gdb.base/dprintf-non-stop.exp ...
gdb compile failed, default_target_compile: No compiler to compile with
Running .../gdb/testsuite/gdb.base/structs3.exp ...
gdb compile failed, default_target_compile: No compiler to compile with
[...]
We are observing this error with ROCgdb[2], a downstream port of GDB
supporting AMD GPUs. This port needs to use GDB's override of
default_target_compile to compile HIP programs since dejagnu does not
provide support for this language yet.
This patch changes gdb_default_target_compile_1 in a similar way
default_target_compile has been updated so both implementations remain
compatible. Even if this is not strictly required by GDB just yet,
I believe keeping both implementations in sync desirable.
Using board files provided with dejagnu <=1.6.2 is still supported: if
the compiler is set by the board file, gdb_default_target_compile_1 uses
it and does not need `find_gcc`.
Patch tested on x86_64 RHEL-9 and ubuntu-20.04 on top of GDB and ROCgdb.
[1] http://git.savannah.gnu.org/gitweb/?p=dejagnu.git;a=commit;h=ba60272a5ac6f6a7012acca03f596a6ed003f044
[2] https://github.com/ROCm-Developer-Tools/ROCgdb
Change-Id: Ibff52684d9cab8243a7c6748ecbd29f50c37e669
|
|
Add all_comp_units/all_type_units views on all_units.
Having the views allows us to:
- easily get the number of CUs or TUs in all_units, and
- easily access the nth CU or TU.
This minimizes the use of tu_stats.nr_tus.
Tested on x86_64-linux.
|
|
Mechanically rename all_comp_units to all_units:
...
$ sed -i 's/all_comp_units/all_units/' gdb/dwarf2/*
...
Tested on x86_64-linux.
|
|
I noticed that, from Python, I could register a new TUI window that
had whitespace in its name, like this:
gdb.register_window_type('my window', MyWindowType)
however, it is not possible to then use this window in a new TUI
layout, e.g.:
(gdb) tui new-layout foo my window 1 cmd 1
Unknown window "my"
(gdb) tui new-layout foo "my window" 1 cmd 1
Unknown window ""my"
(gdb) tui new-layout foo my\ window 1 cmd 1
Unknown window "my\"
GDB clearly uses the whitespace to split the incoming command line.
I could fix this by trying to add a mechanism by which we can use
whitespace within a window name, but it seems like an easier solution
if we just forbid whitespace within a window name. Not only is this
easier, but I think this is probably the better solution, identifier
names with spaces in would mean we'd need to audit all the places a
window name could be printed and ensure that the use of a space didn't
make the output ambiguous.
So, having decided to disallow whitespace, I then thought about other
special characters. We currently accept anything as a window name,
and I wondered if this was a good idea.
My concerns were about how special characters used in a window name
might cause confusion, for example, we allow '$' in window names,
which is maybe fine now, but what if one day we wanted to allow
variable expansion when creating new layouts? Or what about starting
a window name with '-'? We already support a '-horizontal' option,
what if we want to add more in the future? Or use of the special
character '{' which has special meaning within a new layout?
In the end I figured it might make sense to place some restrictive
rules in place, and then relax the rules later if/when users complain,
we can consider each relaxation as its requested.
So, I propose that window names should match this regular expression:
[a-zA-Z][-_.a-zA-Z0-9]*
There is a chance that there is user code in the wild which will break
with the addition of this change, but hopefully adapting to the new
restrictions shouldn't be too difficult.
|
|
The testsuite implicitly tests GDB's ability to step through epilogues
in multiple tests, without doing it explicitly anywhere. This is
unfortunate, as clang does not emit epilogue information, so using clang
on our testsuite makes many tests fail. This patch adds a central,
explicit test for walking through the epilogue so we can safely remove
this from other tests and have them working with clang.
The test created attempts to step through a simple epilogue, an
epilogue that ends on another epilogue, and epilogues leading to other
function calls.
|
|
gdb.base/skip.exp was making use of a fixed number of step commands to
exit some functions. This caused some problems when using clang to test
GDB, as GDB would need fewer steps to reach the desired spots. For
instance, when testing in the section "step after disabling 3", the log
looks like this:
Breakpoint 4, main () at binutils-gdb/gdb/testsuite/gdb.base/skip.c:32
32 x = baz ((bar (), foo ()));
(gdb) step
bar () at binutils-gdb/gdb/testsuite/gdb.base/skip1.c:21
21 return 1;
(gdb) PASS: gdb.base/skip.exp: step after disabling 3: step 1
step
foo () at binutils-gdb/gdb/testsuite/gdb.base/skip.c:42
42 return 0;
(gdb) PASS: gdb.base/skip.exp: step after disabling 3: step 2
step
main () at binutils-gdb/gdb/testsuite/gdb.base/skip.c:34
34 test_skip_file_and_function ();
(gdb) step
test_skip_file_and_function () at binutils-gdb/gdb/testsuite/gdb.base/skip.c:59
59 test_skip ();
(gdb) FAIL: gdb.base/skip.exp: step after disabling 3: step 3
step
test_skip () at binutils-gdb/gdb/testsuite/gdb.base/skip.c:48
48 }
(gdb) PASS: gdb.base/skip.exp: step after disabling 3: step 4
step
test_skip_file_and_function () at binutils-gdb/gdb/testsuite/gdb.base/skip.c:60
60 skip1_test_skip_file_and_function ();
(gdb) FAIL: gdb.base/skip.exp: step after disabling 3: step 5
This shows that the feature is working but because the inferior lands in
a different location, it registers as a failure. Seeing as along with
this difference, there are also some differences that depend on gcc
versions (where gdb might stop back at line 32 before entering foo), it
would not be easy to test for this behavior using steps and analzing
where the inferior stops at each point. On the other hand, using
gdb_step_until is not feasible because we'd possibly gloss over stepping
into baz and rendering the whole test useless. Instead, skip.exp now
uses finish to leave functions, synchronizing through compilers and
compiler versions. Some test names were also changed to be a bit more
descriptive. The new log looks like this, independently of compiler used:
Breakpoint 4, main () at binutils-gdb/gdb/testsuite/gdb.base/skip.c:32
32 x = baz ((bar (), foo ()));
(gdb) step
bar () at binutils-gdb/gdb/testsuite/gdb.base/skip1.c:21
21 return 1;
(gdb) PASS: gdb.base/skip.exp: step after disabling 3: step into bar
finish
Run till exit from #0 bar () at binutils-gdb/gdb/testsuite/gdb.base/skip1.c:21
main () at binutils-gdb/gdb/testsuite/gdb.base/skip.c:32
32 x = baz ((bar (), foo ()));
Value returned is $2 = 1
(gdb) PASS: gdb.base/skip.exp: step after disabling 3: return from bar
step
foo () at binutils-gdb/gdb/testsuite/gdb.base/skip.c:42
42 return 0;
(gdb) PASS: gdb.base/skip.exp: step after disabling 3: step into foo
finish
Run till exit from #0 foo () at binutils-gdb/gdb/testsuite/gdb.base/skip.c:42
main () at binutils-gdb/gdb/testsuite/gdb.base/skip.c:32
32 x = baz ((bar (), foo ()));
Value returned is $3 = 0
(gdb) PASS: gdb.base/skip.exp: step after disabling 3: Return from foo
step
34 test_skip_file_and_function ();
(gdb) PASS: gdb.base/skip.exp: step after disabling 3: step and skip baz
|
|
When using clang as the compiler for the target, gdb.base/jit-elf.exp
was failing because the filename displayed when GDB attached to the
inferior was only showing up as with a relative path, like so:
(gdb) attach 3674146
Attaching to program: /home/blarsen/Documents/gdb-build/gdb/testsuite/outputs/gdb.base/jit-elf/jit-elf-main, process 3674146
Reading symbols from /lib64/libm.so.6...
Reading symbols from .gnu_debugdata for /lib64/libm.so.6...
(No debugging symbols found in .gnu_debugdata for /lib64/libm.so.6)
Reading symbols from /lib64/libc.so.6...
(No debugging symbols found in /lib64/libc.so.6)
Reading symbols from /lib64/ld-linux-x86-64.so.2...
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib64/libthread_db.so.1".
0x00000000004013ff in main (argc=3, argv=0x7fffffffd820) at ../../../common/git-repos/binutils-gdb/gdb/testsuite/gdb.base/jit-elf-main.c:118
118| WAIT_FOR_GDB; i = 0; /* gdb break here 1 */
(gdb) FAIL: gdb.base/jit-elf.exp: attach: one_jit_test-2: break here 1: attach
While gcc's output is as follows:
(gdb) attach 3592961
Attaching to program: /home/blarsen/Documents/gdb-build/gdb/testsuite/outputs/gdb.base/jit-elf/jit-elf-main, process 3592961
Reading symbols from /lib64/libm.so.6...
Reading symbols from .gnu_debugdata for /lib64/libm.so.6...
(No debugging symbols found in .gnu_debugdata for /lib64/libm.so.6)
Reading symbols from /lib64/libc.so.6...
(No debugging symbols found in /lib64/libc.so.6)
Reading symbols from /lib64/ld-linux-x86-64.so.2...
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib64/libthread_db.so.1".
main (argc=3, argv=0x7fffffffd860) at /home/blarsen/Documents/gdb-build/gdb/testsuite/../../../common/git-repos/binutils-gdb/gdb/testsuite/gdb.base/jit-elf-main.c:118
118| WAIT_FOR_GDB; i = 0; /* gdb break here 1 */
(gdb) PASS: gdb.base/jit-elf.exp: attach: one_jit_test-2: break here 1: attach
This difference only happens when GDB's configure is ran using a
relative path, but seeing as testing the full path is not important for
this specific test, it feels worth fixing anyway. To fix the false
positive, the regexp for checking where gdb has stopped was relaxed a
little to allow the relative path.
|
|
When trying to test gdb.base/msym-bp-shl.exp using clang, it would have
many failures because one of the version of the foo function was being
optimized away. Adding __attribute__ ((used)) to it fixed this.
|
|
When testing gdb.base/skip-inline.exp using clang, we get failures
when trying to step out of functions, since clang requires one fewer
step when compared to gcc. The inferior gets increasingly out of sync
as the test continues because of this difference, which generates those
failures.
This commit fixes this by switching those hardcoded steps to
gdb_step_until, to guarantee that the inferior is always synced to what
the test expects. This approach does not work for the parts that use
step 2 or step 3, so when we identify that clang is being used, those
tests are skipped.
|
|
When running gdb.base/skip-solib.exp, the backtrace tests could fail with
compilers that associated epilogue instructions with the last statement
line of the function, instead of associating it with the closing brace,
despite the feature being fully functional. As an example, when testing
skipping the function square, the testsuite would show
Breakpoint 1, main () at (...)/binutils-gdb/gdb/testsuite/gdb.base/skip-solib-main.c:5
5 return square(0);
(gdb) step
0x00007ffff7cef560 in __libc_start_call_main () from /lib64/libc.so.6
(gdb) PASS: gdb.base/skip-solib.exp: ignoring solib file: step
bt
#0 0x00007ffff7cef560 in __libc_start_call_main () from /lib64/libc.so.6
#1 0x00007ffff7cef60c in __libc_start_main_impl () from /lib64/libc.so.6
#2 0x0000000000401065 in _start ()
(gdb) FAIL: gdb.base/skip-solib.exp: ignoring solib file: bt
Which means that the feature is working, the testsuite is just
mis-identifying it. To avoid this problem, the skipped function calls
have been sent to a line before `return`, so epilogues won't factor in.
|
|
Different compilers link the epilogue of functions to different lines.
As an example, gcc links it to the closing brace of the function,
whereas clang links it to the last statement of the function. This
difference is important for the testsuite, since the where GDB will land
after a step can be wildly different. Where possible, this dependency
should be side-stepped in the testsuite, but it isn't always possible,
so this commit adds a gdb_caching_proc that is able to detect where the
epilogue is linked, so tests can react accordingly.
|
|
gdb_bfd.c and remote.c contain identical implementations of a
fileio_error -> errno function. Factor that out to
gdbsupport/fileio.{h,cc}.
Rename it fileio_error_to_host, for symmetry with host_to_fileio_error.
Change-Id: Ib9b8807683de2f809c94a5303e708acc2251a0df
|
|
Converting from free-form macros to an enum gives a bit of type-safety.
This caught places where we would assign host error numbers to what
should contain a target fileio error number, for instance in
target_fileio_pread.
I added the FILEIO_SUCCESS enumerator, because
remote.c:remote_hostio_parse_result initializes the remote_errno output
variable to 0. It seems better to have an explicit enumerator than to
assign a value for which there is no enumerator. I considered
initializing this variable to FILEIO_EUNKNOWN instead, such that if the
remote side replies with an error and omits the errno value, we'll get
an errno that represents an error instead of 0 (which reprensents no
error). But it's not clear what the consequences of that change would
be, so I prefer to err on the side of caution and just keep the existing
behavior (there is no intended change in behavior with this patch).
Note that remote_hostio_parse_resul still reads blindly what the remote
side sends as a target errno into this variable, so we can still end up
with a nonsensical value here. It's not good, but out of the scope of
this patch.
Convert host_to_fileio_error and fileio_errno_to_host to return / accept
a fileio_error instead of an int, and cascade the change in the whole
chain that uses that.
Change-Id: I454b0e3fcf0732447bc872252fa8e57d138b0e03
|
|
I don't see why include/gdb/fileio.h is placed there. It's not
installed by "make install", and it's not included by anything outside
of gdb/gdbserver/gdbsupport.
Move its content back to gdbsupport/fileio.h. I have omitted the bits
inside an `#if 0`, since it's obviously not used, as well as the
"limits" constants, which are also unused.
Change-Id: I6fbc2ea10fbe4cfcf15f9f76006b31b99c20e5a9
|
|
When a GDB built with -D_GLIBCXX_DEBUG=1 reads a binary with a single
character name, we hit this assertion failure:
$ ./gdb -q --data-directory=data-directory -nx ./x
/usr/include/c++/12.1.0/string_view:239: constexpr const std::basic_string_view<_CharT, _Traits>::value_type& std::basic_string_view<_CharT, _Traits>::operator[](size_type) const [with _CharT = char; _Traits = std::char_traits<char>; const_reference = const char&; size_type = long unsigned int]: Assertion '__pos < this->_M_len' failed.
The backtrace:
#3 0x00007ffff6c0f002 in std::__glibcxx_assert_fail (file=<optimized out>, line=<optimized out>, function=<optimized out>, condition=<optimized out>) at /usr/src/debug/gcc/libstdc++-v3/src/c++11/debug.cc:60
#4 0x000055555da8a864 in std::basic_string_view<char, std::char_traits<char> >::operator[] (this=0x7fffffffcc30, __pos=1) at /usr/include/c++/12.1.0/string_view:239
#5 0x00005555609dcb88 in path_join[abi:cxx11](gdb::array_view<std::basic_string_view<char, std::char_traits<char> > const>) (paths=...) at /home/simark/src/binutils-gdb/gdbsupport/pathstuff.cc:203
#6 0x000055555e0443f4 in path_join<char const*, char const*> () at /home/simark/src/binutils-gdb/gdb/../gdbsupport/pathstuff.h:84
#7 0x00005555609dc336 in gdb_realpath_keepfile[abi:cxx11](char const*) (filename=0x6060000a8d40 "/home/simark/build/binutils-gdb-one-target/gdb/./x") at /home/simark/src/binutils-gdb/gdbsupport/pathstuff.cc:122
#8 0x000055555ebd2794 in exec_file_attach (filename=0x7fffffffe0f9 "./x", from_tty=1) at /home/simark/src/binutils-gdb/gdb/exec.c:471
#9 0x000055555f2b3fb0 in catch_command_errors (command=0x55555ebd1ab6 <exec_file_attach(char const*, int)>, arg=0x7fffffffe0f9 "./x", from_tty=1, do_bp_actions=false) at /home/simark/src/binutils-gdb/gdb/main.c:513
#10 0x000055555f2b7e11 in captured_main_1 (context=0x7fffffffdb60) at /home/simark/src/binutils-gdb/gdb/main.c:1209
#11 0x000055555f2b9144 in captured_main (data=0x7fffffffdb60) at /home/simark/src/binutils-gdb/gdb/main.c:1319
#12 0x000055555f2b9226 in gdb_main (args=0x7fffffffdb60) at /home/simark/src/binutils-gdb/gdb/main.c:1344
#13 0x000055555d938c5e in main (argc=5, argv=0x7fffffffdcf8) at /home/simark/src/binutils-gdb/gdb/gdb.c:32
The problem is this line in path_join:
gdb_assert (strlen (path) == 0 || !IS_ABSOLUTE_PATH (path));
... where `path` is "x". IS_ABSOLUTE_PATH eventually calls
HAS_DRIVE_SPEC_1:
#define HAS_DRIVE_SPEC_1(dos_based, f) \
((f)[0] && ((f)[1] == ':') && (dos_based))
This macro accesses indices 0 and 1 of the input string. However, `f`
is a string_view of length 1, so it's incorrect to try to access index
1. We know that the string_view's underlying object is a null-terminated
string, so in practice there's no harm. But as far as the string_view
is concerned, index 1 is considered out of bounds.
This patch makes the easy fix, that is to change the path_join parameter
from a vector of to a vector of `const char *`. Another solution would
be to introduce a non-standard gdb::cstring_view class, which would be a
view over a null-terminated string. With that class, it would be
correct to access index 1, it would yield the NUL character. If there
is interest in having this class (it has been mentioned a few times in
the past) I can do it and use it here.
This was found by running tests such as gdb.ada/arrayidx.exp, which
produce 1-char long filenames, so adding a new test is not necessary.
Change-Id: Ia41a16c7243614636b18754fd98a41860756f7af
|
|
Remove the macro, replace all uses with calls to type::length.
Change-Id: Ib9bdc954576860b21190886534c99103d6a47afb
|
|
Add the `length` and `set_length` methods on `struct type`, in order to remove
the `TYPE_LENGTH` macro. In this patch, the macro is changed to use the
getter, so all the call sites of the macro that are used as a setter are
changed to use the setter method directly. The next patch will remove the
macro completely.
Change-Id: Id1090244f15c9856969b9be5006aefe8d8897ca4
|
|
Remove the macro, replace all uses by calls to type::target_type.
Change-Id: Ie51d3e1e22f94130176d6abd723255282bb6d1ed
|
|
Add the `target_type` and `set_target_type` methods on `struct type`, in order
to remove the `TYPE_TARGET_TYPE` macro. In this patch, the macro is changed to
use the getter, so all the call sites of the macro that are used as a setter
are changed to use the setter method directly. The next patch will remove the
macro completely.
Change-Id: I85ce24d847763badd34fdee3e14b8c8c14cb3161
|
|
I noticed that stdin_event_handler is only used in event-top.c, so
this patch changes it to be 'static'.
|
|
This changes some target_so_ops instances to be const. This makes
their use a little more obvious (they can't be mutated) and also
allows for the removal of some initialization code.
|
|
This changs solib_ops to be an ordinary gdbarch value and updates all
the uses. This removes a longstanding FIXME and makes the code
somewhat cleaner as well.
|