| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
flake8 warns about dap/__init__.py because it has a number of unused
imports. Most of these are intentional: the import is done to ensure
that the a DAP request is registered with the server object.
This patch applies a "noqa" comment to these imports, and also removes
one import that is truly unnecessary.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
Commit 032d23a6 ("Fix stray KeyboardInterrupt after cancel")
introduced some errors into dap/server.py. A function is called but
not imported, and the wrong variable name is used. This patch
corrects both errors.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
I re-ran flake8 today and was puzzled to see W503 warnings.
Eventually I found out that the setup.cfg config overrides .flake8.
This patch merges the two and removes .flake8, to avoid future
confusion.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
On fedora rawhide, when running test-case gdb.base/ctf-ptype.exp, I get:
...
gdb compile failed, ctf-ptype.c: In function 'main':
ctf-ptype.c:242:29: error: implicit declaration of function 'malloc' \
[-Wimplicit-function-declaration]
242 | v_char_pointer = (char *) malloc (1);
| ^~~~~~
ctf-ptype.c:1:1: note: include '<stdlib.h>' or provide a declaration of 'malloc'
+++ |+#include <stdlib.h>
1 | /* This test program is part of GDB, the GNU debugger.
...
Fix this by adding the missing include.
Tested on aarch64-linux.
|
|
In an aarch32-linux chroot on an aarch64-linux system, I run into:
...
(gdb) print x^M
$1 = 9223372036854775807^M
(gdb) FAIL: gdb.ada/verylong.exp: print x
...
A passing version on aarch64-linux looks like:
...
(gdb) print x^M
$1 = 170141183460469231731687303715884105727^M
(gdb) PASS: gdb.ada/verylong.exp: print x
...
The difference is caused by the size of the type Long_Long_Long_Integer, which
is:
- a 128-bit signed on 64-bit targets, and
- a 64-bit signed on 32-bit target.
Fix this by detecting the size of the Long_Long_Long_Integer type, and
handling it.
Tested on aarch64-linux and aarch32-linux.
Approved-By: Tom Tromey <tom@tromey.com>
PR testsuite/31574
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31574
[1] https://gcc.gnu.org/onlinedocs/gnat_rm/Implementation-Defined-Characteristics.html
|
|
After starting TUI like this with a hello world a.out:
...
$ gdb -q a.out -ex start -ex "tui enable"
...
we get:
...
┌─hello.c──────────────────────────────┐
│ 5 { │
│ 6 printf ("hello\n"); │
│ 7 │
│ 8 return 0; │
│ 9 } │
│ │
└──────────────────────────────────────┘
...
This is a regression since commit ee1e9bbb513 ("[gdb/tui] Fix displaying main
after resizing"), before which we had instead:
...
┌─hello.c──────────────────────────────┐
│ 4 main (void) │
│ 5 { │
│ > 6 [7m printf ("hello\n");[0m │
│ 7 │
│ 8 return 0; │
│ 9 } │
└──────────────────────────────────────┘
...
In other words, the problems are:
- the active line (source line 6) is no longer highlighted, and
- the active line is not vertically centered (screen line 2 out 6 instead of
screen line 3 out of 6).
Fix these problems respectively by:
- in tui_enable, instead of "tui_show_frame_info (0)" using
'tui_show_frame_info (deprecated_safe_get_selected_frame ())", and
- in tui_source_window_base::rerender, adding centering functionality.
Tested on aarch64-linux.
Co-Authored-By: Tom Tromey <tom@tromey.com>
Approved-By: Tom Tromey <tom@tromey.com>
PR tui/31522
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31522
|
|
I noticed in code_breakpoint::code_breakpoint that we are calling
update_dprintf_command_list once for each breakpoint location, when we
really only need to call this once per breakpoint -- the data updated
by this function, the breakpoint command list -- is per breakpoint,
not per breakpoint location. Calling update_dprintf_command_list
multiple times is just wasted effort, there's no per location error
checking, we don't even pass the current location to the function.
This commit moves the update_dprintf_command_list call outside of the
per-location loop.
There should be no user visible changes after this commit.
|
|
Given the changes in the previous couple of commits, this commit
cleans up some of the asserts and 'if' checks related to the
extra_string within a dprintf breakpoint.
This commit:
1. Adds some asserts to update_dprintf_command_list about the
breakpoint type, and that the extra_string is not nullptr,
2. Given that we know extra_string is not nullptr (this is enforced
when the breakpoint is created), we can simplify
code_breakpoint::code_breakpoint -- it no longer needs to check for
the extra_string is nullptr case,
3. In dprintf_breakpoint::re_set we can remove the assert (this will
be checked within update_dprintf_command_list, we can also remove
the redundant 'if' check.
There should be no user visible changes after this commit.
|
|
I noticed in update_dprintf_command_list that we handle the case where
the bp_dprintf style breakpoint doesn't have a format and args string.
However, I don't believe such a situation is possible. The obvious
approach certainly already catches this case:
(gdb) dprintf main
Format string required
If it is possible to create a dprintf breakpoint without a format and
args string then I think we should be catching this case and handling
it at creation time, rather than having GDB just ignore the situation
later on.
And so, I propose that we change the 'if' that ignores the case where
the format/args string is empty, and instead assert that we do always
have a format/args string. The original code, that handled an empty
format/args string has existed since commit e7e0cddfb0d4, which is
when dprintf support was added to GDB.
If I'm correct and this situation can't ever happen then there should
be no user visible changes after this commit.
|
|
The goal of this commit is to better define the API for
create_breakpoint especially around the use of extra_string and
parse_extra. This will be useful in the next commit when I plan to
make some changes to create_breakpoint.
This commit makes one possibly breaking change: until this commit it
was possible to create thread-specific dprintf breakpoint like this:
(gdb) dprintf call_me, thread 1 "%s", "hello"
Dprintf 2 at 0x401152: file /tmp/hello.c, line 8.
(gdb) info breakpoints
Num Type Disp Enb Address What
2 dprintf keep y 0x0000000000401152 in call_me at /tmp/hello.c:8 thread 1
stop only in thread 1
printf "%s", "hello"
(gdb)
This feature of dprintf was not documented, was not tested, and is
slightly different in syntax to how we create thread specific
breakpoints and/or watchpoints -- the thread condition appears after
the first ','.
I believe that this worked at all was simply by luck. We happen to
pass the parse_extra flag as true from dprintf_command to
create_breakpoint.
So in this commit I made the choice to change this. We now pass
parse_extra as false from dprintf_command to create_breakpoint. With
this done it is assumed that the only thing in the extra_string is the
dprintf format and arguments.
Beyond this change I've updated the comment on create_breakpoint in
breakpoint.h, and I've then added some asserts into
create_breakpoint as well as moving around some of the error
handling.
- We now assert on the incoming argument values,
- I've moved an error check to sit after the call to
find_condition_and_thread_for_sals, this ensures the extra_string
was parsed correctly,
In dprintf_command:
- We now throw an error if there is no format string after the
dprintf location. This error was already being thrown, but was
being caught later in the process. With this change we catch the
missing string earlier,
- And, as mentioned earlier, we pass parse_extra as false when
calling create_breakpoint,
In create_tracepoint_from_upload:
- We now throw an error if the parsed location doesn't completely
consume the addr_str variable. This error has now effectively
moved out of create_breakpoint.
|
|
This commit extends the asserts on create_breakpoint (in the header
file), and adds some additional assertions into the definition.
The new assert confirms that when the thread and inferior information
is going to be parsed from the extra_string, then the thread and
inferior arguments should be -1. That is, the caller of
create_breakpoint should not try to create a thread/inferior specific
breakpoint by *both* specifying thread/inferior *and* asking to parse
the extra_string, it's one or the other.
There should be no user visible changes after this commit.
|
|
I noticed a redundant assignment to 'prev_col' in
tui_redisplay_readline, and then went ahead and lowered most of the
variable definitions in that function to their initialization point.
|
|
The gdb.python/py-cmd-prompt.exp script includes a test that has a
gdbserver port number within a test name. As port numbers can change
from one test run to the next (depending on what else is running on
the machine at the time), this can make it hard to compare test
results between runs.
Give the test a specific name to avoid including the port number.
There is no change in what is tested after this commit.
|
|
Provide an explicit name for a test in gdb.base/pc-not-saved.exp to
avoid printing $pc and $sp values in the test name -- these values
might change between different test runs, which makes it harder to
compare test results.
There is no change in what is actually being tested with this commit.
|
|
On fedora rawhide, with test-case gdb.trace/collection.exp, I get:
...
gdb compile failed, collection.c: In function 'strings_test_func':
collection.c:227:13: error: implicit declaration of function 'malloc' \
[-Wimplicit-function-declaration]
227 | longloc = malloc(500);
| ^~~~~~
collection.c:1:1: note: \
include '<stdlib.h>' or provide a declaration of 'malloc'
+++ |+#include <stdlib.h>
1 | /* This testcase is part of GDB, the GNU debugger.
collection.c:228:3: error: implicit declaration of function 'strcpy' \
[-Wimplicit-function-declaration]
228 | strcpy(longloc, ... );
| ^~~~~~
collection.c:1:1: note: include '<string.h>' or provide a declaration of \
'strcpy'
+++ |+#include <string.h>
1 | /* This testcase is part of GDB, the GNU debugger.
collection.c:230:8: error: implicit declaration of function 'strlen' \
[-Wimplicit-function-declaration]
230 | i += strlen (locstr);
| ^~~~~~
collection.c:230:8: note: include '<string.h>' or provide a declaration of \
'strlen'
...
Fix this by adding the missing includes.
Tested on aarch64-linux.
Approved-By: John Baldwin <jhb@FreeBSD.org>
|
|
On fedora rawhide, when running test-case gdb.linespec/break-asm-file.exp, I
get:
...
gdb compile failed, break-asm-file.c:21:8: error: \
return type defaults to 'int' [-Wimplicit-int]
21 | static func()
| ^~~~
...
Fix this by adding the missing return type.
Tested on aarch64-linux.
Approved-By: John Baldwin <jhb@FreeBSD.org>
|
|
PR gdb/31524 points out a crash when pascal_language::print_type is
called with varstring==nullptr. This crash is a regression arising
from the printf/pager rewrite -- that indirectly removed a NULL check
from gdb's "puts".
This patch instead fixes the problem by adding a check to print_type.
Passing nullptr here seems to be expected in other places (e.g., there
is a call to type_print like this in expprint.c), and other
implementations of this method (or related helpers) explicitly check
for NULL.
I didn't write a test case for this because it seemed like overkill
for a Pascal bug that only occurs with -i=mi. However, if you want
one, let me know and I will do it.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31524
Approved-By: John Baldwin <jhb@FreeBSD.org>
|
|
On aarch64-linux, using the manjaro linux distro, I run into:
...
(gdb) next^M
32 }^M
(gdb) next^M
0x0000fffff7d67b80 in ?? () from /usr/lib/libc.so.6^M
(gdb) FAIL: gdb.base/ending-run.exp: step out of main
...
What happens here is described in detail in this clause:
...
-re "0x.*\\?\\? \\(\\) from /lib/powerpc.*$gdb_prompt $" {
# This case occurs on Powerpc when gdb steps out of main and the
# needed debug info files are not loaded on the system, preventing
# GDB to determine which function it reached (__libc_start_call_main).
# Ideally, the target system would have the necessary debugging
# information, but in its absence, GDB's behavior is as expected.
...
}
...
but the clause only matches for powerpc.
Fix this by:
- making the regexp generic enough to also match /usr/lib/libc.so.6, and
- updating the comment to not mention powerpc.
Tested on aarch64-linux.
PR testsuite/31450
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31450
|
|
When running test-case gdb.threads/attach-stopped.exp on aarch64-linux, using
the manjaro linux distro, I get:
...
(gdb) thread apply all bt^M
^M
Thread 2 (Thread 0xffff8d8af120 (LWP 278116) "attach-stopped"):^M
#0 0x0000ffff8d964864 in clock_nanosleep () from /usr/lib/libc.so.6^M
#1 0x0000ffff8d969cac in nanosleep () from /usr/lib/libc.so.6^M
#2 0x0000ffff8d969b68 in sleep () from /usr/lib/libc.so.6^M
#3 0x0000aaaade370828 in func (arg=0x0) at attach-stopped.c:29^M
#4 0x0000ffff8d930aec in ?? () from /usr/lib/libc.so.6^M
#5 0x0000ffff8d99a5dc in ?? () from /usr/lib/libc.so.6^M
^M
Thread 1 (Thread 0xffff8db62020 (LWP 278111) "attach-stopped"):^M
#0 0x0000ffff8d92d2d8 in ?? () from /usr/lib/libc.so.6^M
#1 0x0000ffff8d9324b8 in ?? () from /usr/lib/libc.so.6^M
#2 0x0000aaaade37086c in main () at attach-stopped.c:45^M
(gdb) FAIL: gdb.threads/attach-stopped.exp: threaded: attach2 to stopped bt
...
The problem is that the test-case expects to see start_thread:
...
gdb_test "thread apply all bt" ".*sleep.*start_thread.*" \
"$threadtype: attach2 to stopped bt"
...
but lack of symbols makes that impossible.
Fix this by allowing " in ?? () from " as well.
Tested on aarch64-linux.
PR testsuite/31451
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31451
|
|
On fedora rawhide, with test-case gdb.base/rtld-step.exp I get:
...
static-pie-static-libc.c: In function '_start':^M
static-pie-static-libc.c:1:22: error: \
implicit declaration of function '_exit' [-Wimplicit-function-declaration]^M
1 | void _start (void) { _exit (0); }^M
| ^~~~~^M
compiler exited with status 1
...
UNTESTED: gdb.base/rtld-step.exp: failed to compile \
(-static-pie not supported or static libc missing)
...
Fix this by adding the missing include.
Tested on aarch64-linux.
Approved-by: Kevin Buettner <kevinb@redhat.com>
|
|
Simon pointed out that commit 818ef5f4 ("Capture warnings when writing
to the index cache") broke the build with clang. This patch fixes the
breakage.
|
|
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>
|
|
The motivation for this change is for analysis tools and IDEs to be
better at analyzing header files on their own.
There are some definitions and includes we want to occur at the very
beginning of all translation units. The way we currently do that is by
requiring all source files (.c and .cc files) to include one of defs.h
(for gdb), server.h (for gdbserver) of common-defs.h (for gdbsupport and
shared source files). These special header files define and include
everything that needs to be included at the very beginning. Other
header files are written in a way that assume that these special
"prologue" header files have already been included.
My problem with that is that my editor (clangd-based) provides a very
bad experience when editing header files. Since clangd doesn't know
that one of defs.h/server.h/common-defs.h was included already, a lot of
things are flagged as errors. For instance, CORE_ADDR is not known.
It's possible to edit the files in this state, but a lot of the power of
the editor is unavailable.
My proposal to help with this is to include those things we always want
to be there using the compilers' `-include` option. Tom Tromey said
that the current approach might exist because not all compilers used to
have an option like this. But I believe that it's safe to assume they
do today.
With this change, clangd picks up the -include option from the compile
command, and is able to analyze the header file correctly, as it sees
all that stuff included or defined by that -include option. That works
because when editing a header file, clangd tries to get the compilation
flags from a source file that includes said header file.
This change is a bit self-serving, because it addresses one of my
frustrations when editing header files, but it might help others too.
I'd be curious to know if others encounter the same kinds of problems
when editing header files. Also, even if the change is not necessary by
any means, I think the solution of using -include for stuff we always
want to be there is more elegant than the current solution.
Even with this -include flag, many header files currently don't include
what they use, but rather depend on files included before them. This
will still cause errors when editing them, but it should be easily
fixable by adding the appropriate include. There's no rush to do so, as
long as the code still compiles, it's just a convenience thing.
The changes are:
- Add the appropriate `-include` option to the various Makefiles.
- There is one particularity for gdbserver's Makefile: we do not want
to include server.h when building `gdbreplay.o`, as `gdbreplay.cc`
doesn't include it. So we can't simply put the `-include` in
`INTERNAL_CFLAGS`. Add the `-include server.h` option to the
`COMPILE` and `IPAGENT_COMPILE` variables, and added a special rule
to compile `gdbreplay.o` with `-include gdbsupport/common-defs.h`.
- Remove the `-include` option from the `check-headers` rule in
gdb/Makefile.in, since it is already included in `INTERNAL_CFLAGS`.
Change-Id: If3e345d00a9fc42336322f1d8286687d22134340
Approved-By: Pedro Alves <pedro@palves.net>
|
|
Remove `INTERNAL_CFLAGS_BASE` and `INTERNAL_WARN_CFLAGS`, inline their
contents in `INTERNAL_CFLAGS`. Not functional changes expected.
Change-Id: I6a09794835ca2cfd4a88a3e9f2e627c8f5bd569f
Approved-By: Pedro Alves <pedro@palves.net>
|
|
Reformat some variables definitions. I think it makes them easier to
read, and it also makes diffs clearer.
Change-Id: I82f63ba0e6d0fe268eb1f1ad5ab22c3cd016ab02
Approved-By: Pedro Alves <pedro@palves.net>
|
|
I noticed that gdbarch_types.py is executable. It's not needed, since
it's only imported from gdbarch.py.
Change-Id: I481170714af66fc3fc3a48c55a7268e0789cf83e
|
|
This reverts commit 01ed1674d4435aa4e194fd9373b7705e425ef354.
|
|
This reverts commit 7816b81e9b36ea0f57662bfd7446b573bf0c9e54.
|
|
This reverts commit cd9b374ffe372dcaf7e4c15548cf53a301d8dcdd.
|
|
This reverts commit efba976d9713a92b4507ccfef2257e4589da2798.
|
|
This reverts commit 198ff6ff819c240545f9fc68b39636fd376d4ba9.
|
|
This reverts commit 24df37a10f8773ad5db07dc000f694d6405e3a36.
|
|
This reverts commit f4c19f89ef43dbce8065532c808e1aeb05d08994.
|
|
A user on irc pointed out that parse_number.exp has a redundant check.
This patch removes the duplicate.
|
|
On debian 12, I run into:
...
(gdb) target remote | vgdb --wait=2 --max-invoke-ms=2500 --pid=618591^M
Remote debugging using | vgdb --wait=2 --max-invoke-ms=2500 --pid=618591^M
relaying data between gdb and process 618591^M
warning: remote target does not support file transfer, \
attempting to access files from local filesystem.^M
Reading symbols from /lib/ld-linux-aarch64.so.1...^M
(No debugging symbols found in /lib/ld-linux-aarch64.so.1)^M
0x000000000401a980 in ?? () from /lib/ld-linux-aarch64.so.1^M
(gdb) FAIL: gdb.base/valgrind-infcall.exp: target remote for vgdb
...
The problem is that we're expecting to match either of these regexps:
...
set start_re1 " in \\.?_start "
set start_re2 "\\.?_start \\(\\) at "
...
but there are no dwarf or elf symbols present.
Fix this by also allowing:
...
set start_re3 "$::hex in \\?\\? \\(\\) from "
...
Tested on aarch64-linux.
Approved-By: Tom Tromey <tom@tromey.com>
|
|
PR symtab/30837 points out a race that can occur when writing to the
index cache: a call to ada_encode can cause a warning, which is
forbidden on a worker thread.
This patch fixes the problem by arranging to capture any such
warnings.
This is v2 of the patch. It is rebased on top of some other changes
in the same area. v1 was here:
https://sourceware.org/pipermail/gdb-patches/2024-February/206595.html
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30837
|
|
This commit:
commit 198ff6ff819c240545f9fc68b39636fd376d4ba9
Date: Tue Jan 30 15:37:23 2024 +0000
gdb/gdbserver: share x86/linux tdesc caching
added some functions which are always defined, but their use is
guarded within various #ifdef blocks. As a result we were seeing
errors about defined, but unused, functions.
I've fixed this problem in this commit by wrapping the function
definitions within #ifdef blocks.
I'm a little worried that there might be too many #ifdef blocks within
this file, however, I'm going to commit this fix for now as this will
fix the build, then I'll think about if there's a better way to split
this file so we might avoid some of these #ifdef blocks.
|
|
After this commit:
commit 198ff6ff819c240545f9fc68b39636fd376d4ba9
Date: Tue Jan 30 15:37:23 2024 +0000
gdb/gdbserver: share x86/linux tdesc caching
a possible use of an uninitialised variable was introduced, the
'tdesc' variable in i386_linux_core_read_description might be read
without being written too if 'xcr0' was 0.
This is fixed in this commit. I've updated the function to follow the
same pattern as amd64_linux_core_read_description, if xcr0 is 0 then
we select a default xcr0 value and use that to select a tdesc.
|
|
When building GDB with clang, I see:
/usr/lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/unique_ptr.h:95:2: error: delete called on non-final 'addrmap_mutable' that has virtual functions but non-virtual destructor [-Werror,-Wdelete-non
-abstract-non-virtual-dtor]
95 | delete __ptr;
| ^
/usr/lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/unique_ptr.h:396:4: note: in instantiation of member function 'std::default_delete<addrmap_mutable>::operator()' requested here
396 | get_deleter()(std::move(__ptr));
| ^
/home/smarchi/src/binutils-gdb/gdb/addrmap.c:422:14: note: in instantiation of member function 'std::unique_ptr<addrmap_mutable>::~unique_ptr' requested here
422 | auto map = std::make_unique<struct addrmap_mutable> ();
| ^
Fix that by making `addrmap_mutable` final, and `addrmap_fixed` too
while at it.
Change-Id: I03aa0b0907c8d0e3390ddbedeb77d73b19b2b526
Approved-By: Tom Tromey <tom@tromey.com>
|
|
Cygwin/MinGW testing links in a set_unbuffered_mode.o object to all
test programs. When running the testsuite in parallel mode, on
Cygwin, I noticed errors like:
ERROR: remote_download to host of ..../build/set_unbuffered_mode.o to ..../build/set_unbuffered_mode_saved.o: cp: cannot open '..../build/set_unbuffered_mode.o' for reading: No such file or directory
...
ERROR: remote_download to host of ..../build/set_unbuffered_mode.o to ..../build/set_unbuffered_mode_saved.o: cp: cannot stat '..../build/set_unbuffered_mode.o': No such file or directory
...
ERROR: remote_download to host of ..../build/set_unbuffered_mode.o to ..../build/set_unbuffered_mode_saved.o: cp: skipping file '..../build/set_unbuffered_mode.o', as it was replaced while being copied
(Absolute paths elided above.)
The problem is that gdb_compile's unbuffered_mode_obj cache isn't
parallel safe. This is fixed in this commit.
Reviewed-by: Kevin Buettner <kevinb@redhat.com>
Change-Id: I67a289473c14ce0603d4b0beb755b124588f18d2
|
|
While working on Windows non-stop mode, I managed to introduce a bug
that led to fake_create_process being called. That then resulted in
GDB crashes later on, because fake_create_process added a thread with
an incorrect ptid for this target. It is putting dwThreadId in the
tid field of the ptid instead of on the lwp field. This is fixed by
this patch.
Change-Id: Iaee5d2deaa57c501f7e6909f8ac242af9b183215
|
|
Move more setup of the readline global state relating to tab
completion into completer.c out of top.c.
Lots of the readline setup is done in init_main (top.c). This commit
moves those bits of initialisation that relate to completion, and
which are only set the one time, into completer.c. This does mean
that readline initialisation is now done in multiple locations, some
in init_main (top.c) and some in completer.c, but I think this is OK.
The work done in init_main is the general readline setup.
I think making static what can be made static, and having it all in
one file, makes things easier to reason about. So I'm OK with having
this split initialisation.
The only completion related thing which is still setup in top.c is
rl_completion_display_matches_hook. I've left this where it is for
now as rl_completion_display_matches_hook is also updated in the tui
code, and the display hook functions are not in completer.c anyway, so
moving this initialisation to completer.c would not allow anything
else to be made static.
There should be no user visible changes after this commit.
|
|
I noticed that completion_find_completion_word is only used within
completer.c, so lets make it static.
There should be no user visible changes after this commit.
|
|
This commit removes some code which is special casing the filename
completion logic. The code in question relates to finding the
beginning of the completion word and was first introduced, or modified
into its existing form in commit 7830cf6fb9571c3357b1a0 (from 2001).
The code being removed moved the start of the completion word backward
until a character in gdb_completer_file_name_break_characters was
found, or until we reached the end of the actual command.
However, I doubt that this is needed any more. The filename completer
has a corresponding filename_completer_handle_brkchars function which
provides gdb_completer_file_name_break_characters as the word break
characters to readline, and also sets rl_completer_quote_characters.
As such, I would expect readline to be able to correctly find the
start of the completion word.
There is one change which I've needed to make as a consequence of
removing the above code, and I think this is a bug fix.
In complete_line_internal_normal_command we initialised temporary
variable P to the CMD_ARGS; this is the complete text after the
command name. Meanwhile, complete_line_internal_normal_command also
accepts an argument WORD, which is the completion word that readline
found for us.
In the code I removed P was updated, it was first set to WORD, and
then moved backwards to the "new" start of the completion word.
But notice, the default for P is the complete command argument text,
and only if we are performing filename completion do we modify P to be
the completion word.
We then passed P through to the actual commands completion function.
If we are doing anything other than filename completion then the value
of P passed is the complete argument text.
If we are doing filename completion then the value of P passed is the
completion word.
In filename_completer we get two arguments TEXT and WORD, the TEXT
argument is the value of P which is the "new" completion word, while
WORD is the completion word that readline calculated.
After simplifying complete_line_internal_normal_command, and the
temporary P is removed, we always pass the complete argument text into
TEXT, while WORD remains the completion word that readline found.
Previously in filename_completer we actually tried to generate
completions based on TEXT, which worked fine as TEXT actually
contained the completion word that we found in
complete_line_internal_normal_command. But I believe that we should
be fine to use the completion word that readline found, so I have
updated filename_completer to generate completions based on WORD.
If I'm correct, then I don't expect to see any user visible changes
after this commit.
|
|
In completer.c there is some code that is surrounded with '#if 0',
this code:
#if 0
/* There is no way to do this just long enough to affect quote
inserting without also affecting the next completion. This
should be fixed in readline. FIXME. */
/* Ensure that readline does the right thing
with respect to inserting quotes. */
rl_completer_word_break_characters = "";
#endif
This code, in some form, and always defined out, has been around since
the original import of GDB. Though the comment hints at what the
problem might be, it's not really clear what the issue is. And
completion within GDB has moved on a long way since this code was
written ... but not used.
I'm proposing that we just remove this code.
If/when a problem comes up then we can look at how to solve it. Maybe
this code would be the answer ... but also, I suspect, given all the
changes ... maybe not. I'm not sure carrying around this code for
another 20+ years adds much value.
There should be no user visible changes after this commit.
|
|
Currently GDB only supports using single quotes for quoting things,
the reason for this, as explained in completer.c (next to the variable
gdb_completer_expression_quote_characters) is that double quoted
strings need to be treated differently by the C expression parser.
But for filenames I don't believe this restriction holds. The file
names as passed to things like the 'file' command are not passing
through the C expression parser, so it seems like we should be fine to
allow double quotes for quoting in this case.
And so, this commit extends GDB to allow double quotes for quoting
filenames. Maybe in future we might be able to allow double quote
quoting in additional places, but this seems enough for now.
The testing has been extended to cover double quotes in addition to
the existing single quote testing.
This change does a number of things:
1. Set rl_completer_quote_characters in filename_completer and
filename_completer_handle_brkchars, this overrides the default which
is set in complete_line_internal_1,
2. In advance_to_completion_word we now take a set of quote
characters as a parameter, the two callers
advance_to_expression_complete_word_point and
advance_to_filename_complete_word_point now pass in the required set
of quote characters,
3. In completion_find_completion_word we now use the currently active
set of quote characters, this means we'll use
gdb_completer_expression_quote_characters or
gdb_completer_file_name_quote_characters depending on what type of
things we are completing.
|
|
In gdb_completion_word_break_characters_throw, after calling
complete_line_internal, if the completion function chose to use a
custom word point then we set rl_completer_quote_characters to NULL.
However, nowhere do we set rl_completer_quote_characters back to its
default value, which is setup in init_main (top.c).
An example of something that uses a custom word point for its
completion is 'thread apply all ...'.
An example of something that relies on rl_completer_quote_characters
would be completion of a quoted filename that contains white space.
Consider this shell and GDB session. The <TAB> markers indicate where
I've used tab to trigger completion:
$ mkdir /tmp/aaa\ bbb
$ touch /tmp/aaa\ bbb/xx\ 11
$ touch /tmp/aaa\ bbb/xx\ 22
$ gdb -q
(gdb) file '/tmp/aaa bbb/xx<TAB><TAB>
xx 11 xx 22
(gdb) thread apply all hel<TAB>
(gdb) thread apply all help
(gdb) file '/tmp/aaa bbb/xx<TAB><TAB>
First I create a directory structure which uses white space within
file and directory names. Then within GDB I use the 'file' command
and use a single quote to quote the filename. When I tab complete GDB
correctly offers the two files within the directory '/tmp/aaa bbb/'.
This works because rl_completer_quote_characters contains the single
quote, and so readline knows that it is trying to complete the string
that starts after the single quote: /tmp/aaa bbb/xx
Next I invoke the completer for the 'thread apply all' command, to do
this I type 'thread apply all hel' and hit tab, this expands to the
one completion 'thread apply all help'. We can run this command or
not, it doesn't matter (there are no threads, so we'll get no output).
Now I repeat the original 'file' completion. This time though I don't
get offered any completions.
The reason is that the 'thread apply all' completer set
rl_completer_quote_characters to nullptr. Now, when readline tries to
figure out the word to complete it doesn't see the single quote as the
start of a quoted word, so instead readline falls back to the word
break characters, and in this case spots the white space. As a result
readline tries to complete the string 'bbb/xx' which obviously doesn't
have any completions.
By setting rl_completer_quote_characters each time completion is
invoked this problem is resolved and the second 'file' command
completes as expected.
I've extended gdb.base/filename-completion.exp to also test with
quoted filenames, and added a 'thread apply all' completion at the
start to expose this bug.
As setting of rl_completer_quote_characters is now all done in the
completer.c file the function get_gdb_completer_quote_characters()
could be made static. However, as this function is only used one time
to initialise rl_completer_quote_characters, I've instead just deleted
get_gdb_completer_quote_characters() and used
gdb_completer_quote_characters directly.
|
|
The function skip_quoted_chars (completer.c) is only used by
skip_quoted (also completer.c), so could be made static. The function
skip_quoted just calls directly to skip_quoted_chars but fills in some
default arguments.
The function skip_quoted is only used by the Pascal expression parser,
and is only used in one place.
The skip_quoted_chars function skips a single string; it either looks
for a string between matching quotes, or for a string up to a word
break character.
However, given how the Pascal expression parser calls this function,
we know that the first character will always be a single quote, in
which case skip_quoted_chars will looks for a string between matching
single quotes.
The skip_quoted_chars doesn't do any escaped character handling, it
will just stop at the next single quote character.
In this commit I propose to remove skip_quoted and skip_quoted_chars,
and replace these with a smaller function pascal_skip_string which
I've placed in p-exp.y. This new function only skips a string between
matching single quotes, which is exactly the use case that we need.
The benefit of this change is to remove (some) code duplication. It
feels like skip_quoted is similar in some ways to
extract_string_maybe_quoted, however, there are some differences;
skip_quoted uses the quotes and word break characters from the
completion engine which extract_string_maybe_quoted does not.
However, I'm currently working on improving filename completion, one
part of this is that I'm looking at allowing filenames to be quoted
with single or double quotes, while the default string quoting in
GDB (for expressions) can only use single quotes. If I do end up
allowing single and double quotes in some cases, but we retain the
single quotes only for expressions then skip_quoted starts to become a
problem, should it accept both quote types, or only one?
But given how skip_quoted is used, I can avoid worrying about this by
simply removing skip_quoted.
The Pascal tests do still pass. The code that called skip_quoted is
called at least once in the Pascal tests (adding an abort() call
causes gdb.pascal/types.exp to fail), but I doubt the testing is
extensive. Not sure how widely used GDB for Pascal actually is
though.
|
|
We now have unwind-on-timeout and unwind-on-terminating-exception, and
then the odd one out unwindonsignal.
I'm not a great fan of these squashed together command names, so in
this commit I propose renaming this to unwind-on-signal.
Obviously I've added the hidden alias unwindonsignal so any existing
GDB scripts will keep working.
There's one test that I've extended to test the alias works, but in
most of the other test scripts I've changed over to use the new name.
The docs are updated to reference the new name.
Reviewed-By: Eli Zaretskii <eliz@gnu.org>
Tested-By: Luis Machado <luis.machado@arm.com>
Tested-By: Keith Seitz <keiths@redhat.com>
|
|
Now that inferior function calls can timeout (see the recent
introduction of direct-call-timeout and indirect-call-timeout), this
commit adds a new setting unwind-on-timeout.
This new setting is just like the existing unwindonsignal and
unwind-on-terminating-exception, but the new setting will cause GDB to
unwind the stack if an inferior function call times out.
The existing inferior function call timeout tests have been updated to
cover the new setting.
Reviewed-By: Eli Zaretskii <eliz@gnu.org>
Tested-By: Luis Machado <luis.machado@arm.com>
Tested-By: Keith Seitz <keiths@redhat.com>
|
