aboutsummaryrefslogtreecommitdiff
path: root/gdb
AgeCommit message (Collapse)AuthorFilesLines
2021-12-07[gdb/ada] Fix assert in ada_is_unconstrained_packed_array_typeTom de Vries1-0/+2
On openSUSE Leap 42.3, with system compiler gcc 4.8.5 I run into: ... (gdb) print u_one_two_three^M src/gdb/gdbtypes.h:1050: internal-error: field: \ Assertion `idx >= 0 && idx < num_fields ()' failed.^M ... We run into trouble while doing this in ada_is_unconstrained_packed_array_type: ... 1953 return TYPE_FIELD_BITSIZE (type, 0) > 0; ... which tries to get field 0 from a type without fields: ... (gdb) p type->num_fields () $6 = 0 ... which is the case because the type is a typedef: ... (gdb) p type->code () $7 = TYPE_CODE_TYPEDEF ... Fix this by using the type referenced by the typedef instead. Tested on x86_64-linux. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28323
2021-12-06gdb: Add PowerPC support to gdb.dwarf2/frame-inlined-in-outer-frameCarl Love1-0/+24
This patch adds an #elif defined for PowerPC to setup the exit_0 macro. This patch addes the needed macro definitionald logic to handle both elfV1 and elfV2. The patch has been successfully tested on both PowerPC BE, Powerpc LE and X86_64 with no regressions.
2021-12-06[gdb/testsuite] Use precise align in gdb.arch/i386-{avx,sse}.expTom de Vries3-2/+106
Test-cases gdb.arch/i386-{avx,sse}.exp use assembly instructions that require the memory operands to be aligned to a certain boundary, and the test-cases use C11's _Alignas to make that happen. The draw-back of using _Alignas is that while it does enforce a minimum alignment, the actual alignment may be bigger, which makes the following scenario possible: - copy say, gdb.arch/i386-avx.c as basis for a new test-case - run the test-case and observe a PASS - commit the new test-case in the supposition that the test-case is correct and well-tested - run later into a failure on a different test setup (which may be a setup where reproduction and investigation is more difficult and time-consuming), and find out that the specified alignment was incorrect and should have been updated to say, 64 bytes. The initial PASS occurred only because the actual alignment happened to be greater than required. The idea of having precise alignment as a means of having more predictable execution which allows flushing out bugs earlier, has been filed as PR gcc/103095. Add a new file lib/precise-aligned-alloc.c with functions precise_aligned_alloc and precise_aligned_dup, to support precise alignment. Use precise_aligned_dup in aforementioned test-cases to: - verify that the specified alignment is indeed sufficient, rather than too little but accidentally over-aligned. - prevent the same type of problems in any new test-cases based on these Tested on x86_64-linux, with both gcc and clang.
2021-12-06[gdb/testsuite] Fix data alignment in gdb.arch/i386-{avx,sse}.expTom de Vries4-8/+24
When running test-case gdb.arch/i386-avx.exp with clang I ran into: ... (gdb) PASS: gdb.arch/i386-avx.exp: set first breakpoint in main continue^M Continuing.^M ^M Program received signal SIGSEGV, Segmentation fault.^M 0x000000000040052b in main (argc=1, argv=0x7fffffffd3c8) at i386-avx.c:54^M 54 asm ("vmovaps 0(%0), %%ymm0\n\t"^M (gdb) FAIL: gdb.arch/i386-avx.exp: continue to breakpoint: \ continue to first breakpoint in main ... The problem is that the vmovaps insn requires an 256-bit (or 32-byte) aligned address, and it's only 16-byte aligned: ... (gdb) p /x $rax $1 = 0x601030 ... Fix this by using a sufficiently aligned address, using _Alignas. Compile using -std=gnu11 to support _Alignas. Likewise in gdb.arch/i386-sse.exp. Tested on x86_64-linux, with both gcc and clang.
2021-12-05Preserve artificial CU name in process_psymtab_comp_unit_readerTom Tromey2-7/+11
This fixes a use-after-free that Simon pointed out. process_psymtab_comp_unit_reader was allocating an artificial name for a CU, and then discarding it. However, this name was preserved in the cached file_and_directory. This patch arranges for the allocated name to be preserved there.
2021-12-04Cache the result of find_file_and_directoryTom Tromey2-8/+18
This changes the DWARF reader to cache the result of find_file_and_directory. This is not especially important now, but it will help the new DWARF indexer.
2021-12-04Move file_and_directory to new file and C++-izeTom Tromey2-45/+129
This moves file_and_directory to a new file, and then C++-izes it -- replacing direct assignments with methods, and arranging for it to own any string that must be computed. Finally, the CU's objfile will only be used on demand; this is an important property for the new DWARF indexer's parallel mode.
2021-12-04Remove Irix case from find_file_and_directoryTom Tromey1-9/+0
find_file_and_directory has a special case for the Irix 6.2 compiler. Since this is long obsolete, this patch removes it.
2021-12-04gdb: don't show deprecated aliasesSimon Marchi2-3/+48
I don't think it's very useful to show deprecated aliases to the user. It encourages the user to use them, when the goal is the opposite. For example, before: (gdb) help set index-cache enabled set index-cache enabled, set index-cache off, set index-cache on alias set index-cache off = set index-cache enabled off alias set index-cache on = set index-cache enabled on Enable the index cache. When on, enable the use of the index cache. (gdb) help set index-cache on Warning: 'set index-cache on', an alias for the command 'set index-cache enabled', is deprecated. Use 'set index-cache enabled on'. set index-cache enabled, set index-cache off, set index-cache on alias set index-cache off = set index-cache enabled off alias set index-cache on = set index-cache enabled on Enable the index cache. When on, enable the use of the index cache. After: (gdb) help set index-cache enabled Enable the index cache. When on, enable the use of the index cache. (gdb) help set index-cache on Warning: 'set index-cache on', an alias for the command 'set index-cache enabled', is deprecated. Use 'set index-cache enabled on'. Enable the index cache. When on, enable the use of the index cache. Change-Id: I989b618a5ad96ba975367e9d16db95523cd57a4c
2021-12-03gdb/testsuite: fix two "maint info line-table"-related testsSimon Marchi2-0/+8
Commit 92228a334ba2 ("gdb: small "maintenance info line-table" readability improvements") change the output format of "maint info line-table" slightly, adding some empty lines between each line-table. This causes two tests to start failing, update them to account for those empty lines. Change-Id: I9d33a58fce3e860ba0554b25f5582e8066a5c519
2021-12-03gdb: revert one array_view copy change in ada-lang.cSimon Marchi1-1/+3
Commit 4bce7cdaf481 ("gdbsupport: add array_view copy function") caused an internal error when running gdb.ada/packed_array_assign.exp: print pra(1) := pr^M /home/smarchi/src/binutils-gdb/gdb/../gdbsupport/array-view.h:217: internal-error: copy: Assertion `dest.size () == src.size ()' failed.^M I am not sure what's the root cause of this, whether it is a GDB bug exposed by using the array_view copy function or not. Back out the change that triggers the internal error for now, while we investigate it. Change-Id: I055ab14143e4cfd3ca7ce8f4855c6c3c05db52a7
2021-12-03gdb: small "maintenance info line-table" readability improvementsSimon Marchi1-6/+13
- separate each entry with a newline, to visually separate them - style filenames with the filename style - print the name of the compunit_symtab A header now looks like this, with the compunit_symtab name added (and the coloring, but you can't really see it here): objfile: /home/simark/build/babeltrace/src/cli/.libs/babeltrace2 ((struct objfile *) 0x613000005980) compunit_symtab: babeltrace2-cfg-cli-args.c ((struct compunit_symtab *) 0x62100da1ed10) symtab: /usr/include/glib-2.0/glib/gdatetime.h ((struct symtab *) 0x62100d9ee530) linetable: ((struct linetable *) 0x0): Change-Id: Idc23e10aaa66e2e692adb0a6a74144f72c4fa1c7
2021-12-03gdb: change some alias functions parameters to const-referenceSimon Marchi1-32/+32
Now that we use intrusive list to link aliases, it becomes easier to pass cmd_list_element arguments by const-reference rather than by pointer to some functions, change a few. Change-Id: Id0df648ed26e9447da0671fc2c858981cda31df8
2021-12-03gdb: use intrusive_list for cmd_list_element aliases listSimon Marchi2-50/+50
Change the manually-implemented linked list to use intrusive_list. This is not strictly necessary, but it makes the code much simpler. Change-Id: Idd08090ebf2db8bdcf68e85ef72a9635f1584ccc
2021-12-03gdb: trivial changes to use array_viewSimon Marchi5-32/+19
Change a few relatively obvious spots using value contents to propagate the use array_view a bit more. Change-Id: I5338a60986f06d5969fec803d04f8423c9288a15
2021-12-03gdb: make extract_integer take an array_viewSimon Marchi14-55/+60
I think it would make sense for extract_integer, extract_signed_integer and extract_unsigned_integer to take an array_view. This way, when we extract an integer, we can validate that we don't overflow the buffer passed by the caller (e.g. ask to extract a 4-byte integer but pass a 2-byte buffer). - Change extract_integer to take an array_view - Add overloads of extract_signed_integer and extract_unsigned_integer that take array_views. Keep the existing versions so we don't need to change all callers, but make them call the array_view versions. This shortens some places like: result = extract_unsigned_integer (value_contents (result_val).data (), TYPE_LENGTH (value_type (result_val)), byte_order); into result = extract_unsigned_integer (value_contents (result_val), byte_order); value_contents returns an array view that is of length `TYPE_LENGTH (value_type (result_val))` already, so the length is implicitly communicated through the array view. Change-Id: Ic1c1f98c88d5c17a8486393af316f982604d6c95
2021-12-03gdbsupport: add array_view copy functionSimon Marchi6-62/+169
An assertion was recently added to array_view::operator[] to ensure we don't do out of bounds accesses. However, when the array_view is copied to or from using memcpy, it bypasses that safety. To address this, add a `copy` free function that copies data from an array view to another, ensuring that the destination and source array views have the same size. When copying to or from parts of an array_view, we are expected to use gdb::array_view::slice, which does its own bounds check. With all that, any copy operation that goes out of bounds should be caught by an assertion at runtime. copy is implemented using std::copy and std::copy_backward, which, at least on libstdc++, appears to pick memmove when copying trivial data. So in the end there shouldn't be much difference vs using a bare memcpy, as we do right now. When copying non-trivial data, std::copy and std::copy_backward assigns each element in a loop. To properly support overlapping ranges, we must use std::copy or std::copy_backward, depending on whether the destination is before the source or vice-versa. std::copy and std::copy_backward don't support copying exactly overlapping ranges (where the source range is equal to the destination range). But in this case, no copy is needed anyway, so we do nothing. The order of parameters of the new copy function is based on std::copy and std::copy_backward, where the source comes before the destination. Change a few randomly selected spots to use the new function, to show how it can be used. Add a test for the new function, testing both with arrays of a trivial type (int) and of a non-trivial type (foo). Test non-overlapping ranges as well as three kinds of overlapping ranges: source before dest, dest before source, and dest == source. Change-Id: Ibeaca04e0028410fd44ce82f72e60058d6230a03
2021-12-03gdb: change store_waitstatus to return a target_waitstatus by valueSimon Marchi8-19/+20
store_waitstatus is basically a translation function between a status integer and an equivalent target_waitstatus object. It would make sense for it to take the integer as a parameter and return the target_waitstatus by value. Do that, and rename to host_status_to_waitstatus. Users can then do: ws = host_status_to_waitstatus (status) which does the right thing, given the move constructor of target_waitstatus. Change-Id: I7a07d59d3dc19d3ed66929642f82f44f3e85d61b
2021-12-03gdb: return *this in target_waitstatus settersSimon Marchi1-16/+32
While playing with some code creating target_waitstatus objects, I was mildly annoyed by the fact that we can't just return a new target_waitstatus object. We have to do: target_waitstatus ws; ws.set_exited (123); return ws; Make the setters return the "this" object as a reference, such that it's possible to do: return target_waitstatus ().set_exited (123); I initially thought of adding static creation functions, which you would use like: return target_waitstatus::make_exited (123); However, making the setters return a reference to the object achieves pretty much the same thing, with less new code. Change-Id: I45159b7f9fcd9db5b20603480e323020b14ed147
2021-12-03gdb: make saved_filename an std::stringSimon Marchi1-10/+10
Make this variable an std::string, avoiding manual memory management. Change-Id: Ie7a8d7381449ab9c4dfc4cb8b99e63b9ffa8f947
2021-12-03gdb: make value_subscripted_rvalue staticAndrew Burgess2-6/+8
The function value_subscripted_rvalue is only used in valarith.c, so lets make it a static function. There should be no user visible change after this commit.
2021-12-03gdb/testsuite: give a test a real nameAndrew Burgess1-1/+2
A test in gdb.python/py-send-packet.exp added in this commit: commit 24b2de7b776f8f23788d855b1eec290c6e208821 Date: Tue Aug 31 14:04:36 2021 +0100 gdb/python: add gdb.RemoteTargetConnection.send_packet included a large amount of binary data in the command sent to GDB. As this test didn't have a real test name the binary data was included in the gdb.sum file. The contents of the binary data could change between different runs of GDB, and this makes comparing results harder. This commit gives the test a real test name.
2021-12-03gdb/remote: fix use after free bugAndrew Burgess1-2/+33
This commit: commit 288712bbaca36bff6578bc839ebcdc3707662f81 Date: Mon Nov 22 15:16:27 2021 +0000 gdb/remote: use scoped_restore to control starting_up flag introduced a use after free bug. The scoped restore added in the above commit resets a flag within a remote_target's remote_state object. However, in some situations, the remote_target can be unpushed before the error is thrown. If the only reference to the target is the one in the target stack, then unpushing the target will cause the remote_target to be deleted, which, in turn, will delete the remote_state object. The scoped restore will then try to reset the flag within a deleted object. This problem was caught in the gdb.server/server-connect.exp test, which, when run with the address sanitizer enabled, highlights the write after free bug described above. This commit resolves this issue by adding a new class specifically for the purpose of managing the starting_up flag. As well as setting, and then clearing the starting_up flag, this new class increments, and then decrements the reference count on the remote_target object. This prevents the remote_target from being deleted until after the flag has been reset. The gdb.server/server-connect.exp now runs cleanly with the address sanitizer enabled.
2021-12-02gdb: remove unexpected xstrdup in _initialize_maint_test_settingsSimon Marchi1-1/+1
That xstrdup is not correct, since we are assigning an std::string. The result of xstrdup is used to initialize the string, and then lost forever. Remove it. Change-Id: Ief7771055e4bfd643ef3b285ec9fb7b1bfd14335
2021-12-02gdb/testsuite: update tests looking for "DWARF 2" debug formatSimon Marchi9-16/+16
Commit ab557072b8ec ("gdb: use actual DWARF version in compunit's debugformat field") changes the debug format string in "info source" to show the actual DWARF version, rather than always show "DWARF 2". However, it failed to consider that some tests checked for the "DWARF 2" string to see if the test program is compiled with DWARF debug information. Since everything is compiled with DWARF 4 or 5 nowadays, that changed the behavior of those tests. Notably, it prevent the tests using skip_inline_var_tests to run. Grep through the testsuite for "DWARF 2" and change all occurrences I could find to use "DWARF [0-9]" instead (that string is passed to TCL's string match). Change-Id: Ic7fb0217fb9623880c6f155da6becba0f567a885
2021-12-02(PPC64) fix handling of fixed-point values when using "return" commandJoel Brobecker1-3/+19
In the gdb.ada/fixed_points_function.exp testcase, we have the following Ada code... type FP1_Type is delta 0.1 range -1.0 .. +1.0; -- Ordinary function Call_FP1 (F : FP1_Type) return FP1_Type is begin FP1_Arg := F; return FP1_Arg; end Call_FP1; ... used as follow: F1 : FP1_Type := 1.0; F1 := Call_FP1 (F1); The testcase, among other things, verifies that "return" works properly as follow: | (gdb) return 1.0 | Make pck.call_fp1 return now? (y or n) y | [...] | 9 F1 := Call_FP1 (F1); | (gdb) next | (gdb) print f1 | $1 = 0.0625 The output of the last command shows that we returned the wrong value. The value printed gives a clue about the problem, since it is 1/16th of the value we expected, where 1/16 is FP1_Type's scaling factor. The problem, here, comes from the fact that the function handling return values for base types (ppc64_sysv_abi_return_value_base) writes the return value using unpack_long which, upon seeing that the value being unpacked is a fixed point type, applies the scaling factor, to get the integer-representation of our fixed-point value (similar to what it does with floats, for instance). So, the fix consists in teaching ppc64_sysv_abi_return_value_base about fixed-point types, and to avoid the unwanted application of the scaling factor. Note that the "finish" function, on the other hand, does not suffer from this issue, simply becaue the value returned by the function is read from register without the use of a type, thus avoiding an unwanted application of a scaling factor. No test added, as this change is already tested by gdb.ada/fixed_points_function.exp. Co-Authored-By: Tristan Gingold <gingold@adacore.com>
2021-12-02(RISCV) fix handling of fixed-point type return valuesJoel Brobecker1-3/+44
This commit adds support for TYPE_CODE_FIXED_POINT types for "finish" and "return" commands. Consider the following Ada code... type FP1_Type is delta 0.1 range -1.0 .. +1.0; -- Ordinary function Call_FP1 (F : FP1_Type) return FP1_Type is begin FP1_Arg := F; return FP1_Arg; end Call_FP1; ... used as follow: F1 : FP1_Type := 1.0; F1 := Call_FP1 (F1); "finish" currently behaves as follow: | (gdb) finish | [...] | Value returned is $1 = 0 We expect the returned value to be "1". Similarly, "return" makes the function return the wrong value: | (gdb) return 1.0 | Make pck.call_fp1 return now? (y or n) y | [...] | 9 F1 := Call_FP1 (F1); | (gdb) next | (gdb) print f1 | $1 = 0.0625 (we expect it to print "1" instead). This problem comes from the handling of integral return values when the return value is actually fixed point type. Our type here is actually a range of a fixed point type, but the same principles should also apply to pure fixed-point types. For the record, here is what the debugging info looks like: <1><238>: Abbrev Number: 2 (DW_TAG_subrange_type) <239> DW_AT_lower_bound : -16 <23a> DW_AT_upper_bound : 16 <23b> DW_AT_name : pck__fp1_type <23f> DW_AT_type : <0x248> <1><248>: Abbrev Number: 4 (DW_TAG_base_type) <249> DW_AT_byte_size : 1 <24a> DW_AT_encoding : 13 (signed_fixed) <24b> DW_AT_binary_scale: -4 <24c> DW_AT_name : pck__Tfp1_typeB <250> DW_AT_artificial : 1 ... where the scaling factor is 1/16. Looking at the "finish" command, what happens is that riscv_arg_location determines that our return value should be returned by parameter using an integral convention (via builtin type long). And then, riscv_return_value uses a cast to that builtin type long to store the value of into a buffer with the right register size. This doesn't work in our case, because the underlying value returned by the function is unscaled, which means it is 16, and thus the cast is like doing: arg_val = (FP1_Type) 16 ... In other words, it is trying to create an FP1_Type enty whose value is 16. Applying the scaling factor, that's 256, and because the size of FP1_Type is 1 byte, we overflow and thus it ends up being zero. The same happen with the "return" function, but the other way around. The fix consists in handling fixed-point types separately from integral types.
2021-12-02(ARM/fixed-point) wrong value shown by "finish" command:Joel Brobecker1-1/+2
Consider the following Ada code: type FP1_Type is delta 0.1 range -1.0 .. +1.0; -- Ordinary FP1_Arg : FP1_Type := 0.0; function Call_FP1 (F : FP1_Type) return FP1_Type is begin FP1_Arg := F; return FP1_Arg; end Call_FP1; After having stopped inside function Call_FP1 as follow: Breakpoint 1, pck.call_fp1 (f=1) at /[...]/pck.adb:5 5 FP1_Arg := F; Returning from that function call using "finish" should show that the function return "1.0" (the same value as was passed as an argument). However, this is not the case: (gdb) finish Run till exit from #0 pck.call_fp1 (f=1) [...] 9 F1 := Call_FP1 (F1); Value returned is $1 = 0 This patch enhances the extraction of the return value to know about fixed point types.
2021-12-02(Ada/AArch64) fix fixed point argument passing in inferior funcallXavier Roirand5-1/+112
Consider the following code: type FP1_Type is delta 0.1 range -1.0 .. +1.0; -- Ordinary function Call_FP1 (F : FP1_Type) return FP1_Type is begin return F; end Call_FP1; When the default in GCC is to generate proper DWARF info for fixed point types, then in gdb, printing the result of a call to call_fp1 with a decimal parameter leads to: (gdb) p call_fp1(0.5) $1 = 0 The displayed value is wrong, and we actually expected: (gdb) p call_fp1(0.5) $1 = 0.5 What happened is that our fixed point type parameter got promoted to a 32bit integer because we detected that the length of that object was less than 4 bytes. The compiler does not perform this promotion and therefore GDB should not either. This patch fixes the behavior described above.
2021-12-02Implement 'task apply'Tom Tromey6-18/+277
This adds a 'task apply' command, which is the Ada tasking analogue of 'thread apply'. Unlike 'thread apply', it doesn't offer the 'ascending' flag; but otherwise it's essentially the same.
2021-12-02Add "task" keyword to the "watch" commandTom Tromey5-1/+175
Breakpoints in gdb can be made specific to an Ada task using the "task" qualifier. This patch applies this same idea to watchpoints.
2021-12-02gdb, include: replace pragmas with DIAGNOSTIC macros, fix build with g++ 4.8Simon Marchi2-6/+7
When introducing this code, I forgot that we had some macros for this. Replace some "manual" pragma diagnostic with some DIAGNOSTIC_* macros, provided by include/diagnostics.h. In diagnostics.h: - Add DIAGNOSTIC_ERROR, to enable a diagnostic at error level. - Add DIAGNOSTIC_ERROR_SWITCH, to enable -Wswitch at error level, for both gcc and clang. Additionally, using DIAGNOSTIC_PUSH, DIAGNOSTIC_ERROR_SWITCH and DIAGNOSTIC_POP seems to misbehave with g++ 4.8, where we see these errors: CXX ada-tasks.o /home/smarchi/src/binutils-gdb/gdb/ada-tasks.c: In function void read_known_tasks(): /home/smarchi/src/binutils-gdb/gdb/ada-tasks.c:998:10: error: enumeration value ADA_TASKS_UNKNOWN not handled in switch [-Werror=switch] switch (data->known_tasks_kind) ^ Because of the POP, the diagnostic should go back to being disabled, since it was disabled in the beginning, but that's not what we see here. Versions of GCC >= 5 compile correctly. Work around this by making DIAGNOSTIC_ERROR_SWITCH a no-op for GCC < 5. Note that this code (already as it exists in master today) enables -Wswitch at the error level even if --disable-werror is passed. It shouldn't be a problem, as it's not like a new enumerator will appear out of nowhere and cause a build error if building with future compilers. Still, for correctness, we would ideally want to ask the compiler to enable -Wswitch at its default level (as if the user had passed -Wswitch on the command-line). There doesn't seem to be a way to do this. Change-Id: Id33ebec3de39bd449409ea0bab59831289ffe82d
2021-12-01gdb: use actual DWARF version in compunit's debugformat fieldSimon Marchi1-1/+12
The "info source" command, with a DWARF-compile program, always show that the debug info is "DWARF 2": (gdb) info source Current source file is test.c Compilation directory is /home/smarchi/build/binutils-gdb/gdb Located in /home/smarchi/build/binutils-gdb/gdb/test.c Contains 2 lines. Source language is c. Producer is GNU C17 9.3.0 -mtune=generic -march=x86-64 -g3 -gdwarf-5 -O0 -fasynchronous-unwind-tables -fstack-protector-strong -fstack-clash-protection -fcf-protection. Compiled with DWARF 2 debugging format. Includes preprocessor macro info. Change it to display the actual DWARF version: (gdb) info source Current source file is test.c Compilation directory is /home/smarchi/build/binutils-gdb/gdb Located in /home/smarchi/build/binutils-gdb/gdb/test.c Contains 2 lines. Source language is c. Producer is GNU C17 9.3.0 -mtune=generic -march=x86-64 -g3 -gdwarf-5 -O0 -fasynchronous-unwind-tables -fstack-protector-strong -fstack-clash-protection -fcf-protection. Compiled with DWARF 5 debugging format. Includes preprocessor macro info. The comp_unit_head::version field is guaranteed to be between 2 and 5, thanks to the check in read_comp_unit_head. So we can still use static strings to pass to record_debugformat, and keep it efficient. In the future, when somebody will update GDB to support DWARF 6, they'll hit this assert and have to update this code. Change-Id: I3270b7ebf5e9a17b4215405bd2e365662a4d6172
2021-12-01[gdb/testsuite] Fix typo in gdb.multi/multi-arch-exec.expTom de Vries1-1/+1
With gdb.multi/multi-arch-exec.exp I run into: ... Running src/gdb/testsuite/gdb.multi/multi-arch-exec.exp ... ERROR: tcl error sourcing src/gdb/testsuite/gdb.multi/multi-arch-exec.exp. ERROR: wrong # args: extra words after "else" clause in "if" command while executing "if [istarget "powerpc64*-*-*"] { set march "-m64" } else if [istarget "s390*-*-*"] { set march "-m31" } else { set march "-m32" }" ... Fix the else if -> elseif typo. Tested on x86_64-linux.
2021-12-01[gdb/testsuite] Fix gdb.arch/i386-pkru.exp on linuxTom de Vries1-3/+10
When running test-case gdb.arch/i386-pkru.exp on a machine with "Memory Protection Keys for Userspace" support, we run into: ... (gdb) PASS: gdb.arch/i386-pkru.exp: probe PKRU support print $pkru^M $2 = 1431655764^M (gdb) FAIL: gdb.arch/i386-pkru.exp: pkru register ... The test-case expects the $pkru register to have the default value 0, matching the "init state" of 0 defined by the XSAVE hardware. Since linux kernel version v4.9 containing commit acd547b29880 ("x86/pkeys: Default to a restrictive init PKRU"), the register is set to 0x55555554 by default (which matches the printed decimal value above). Fix the FAIL by accepting this value for linux. Tested on x86_64-linux.
2021-12-01gdb/remote: use scoped_restore to control starting_up flagAndrew Burgess1-22/+22
This commit makes use of a scoped_restore object to control the remote_state::starting_up flag within the remote_target::start_remote method. Ideally I would have liked to create the scoped_restore inside start_remote and just leave the restore in place until the end of the scope, however, I'm worried that doing this would change the behaviour of GDB. Specifically, in start_remote, the following code is executed once the starting_up flag has been restored to its previous value: if (breakpoints_should_be_inserted_now ()) insert_breakpoints (); I think (but am not 100% sure) that calling install_breakpoints could end up back inside remote_target::can_download_tracepoint, which does check the value of remote_state::starting_up. And so, I'm concerned that leaving the scoped_restore in place until the end of start_remote will cause a possible change in behaviour. To avoid this, and to leave things as close to the current behaviour as possible, I've split remote_target::start_remote into two, there's the main function body which moves into remote_target::start_remote_1, this function uses the scoped_restore to change the ::starting_up flag, then there's the old remote_target::start_remote, which now just calls ::start_remote_1, and then does the insert_breakpoints call. There should be no user visible changes after this commit, unless there's a situation where the ::starting_up flag could previously have been left set, if this was the case, then this situation should no longer be possible.
2021-11-30gdb.base/corefile-buildid.exp: fix DUPLICATEs when failing to generate a ↵Simon Marchi1-19/+19
core file When my system isn't properly configured to generate core files in the local directory, I see these DUPLICATEs: DUPLICATE: gdb.base/corefile-buildid.exp: could not generate core file Fix that by having a single with_test_prefix around that message and what follows. Change-Id: I4ac245fcce1c666db56e3bad3582aa17f183dcba
2021-11-30gdb: Powerpc fix gdb.multi/multi-arch-exec.exp testCarl Love1-1/+3
The expect file has a procedure append_arch_options which sets march based the istarget. The current if / else statement does not check for powerpc64. The else statement is hit which sets march to -m32. This results in compilation errors on 64-bit PowerPC. This patch adds an if statement to check for powerpc64 and if true sets mach to -m64. The patch was tested on a Power 10 system. No compile errors were generated. The test completes with 1 expected pass and no failures.
2021-11-30gdb/python: don't use the 'p' format for parsing argsAndrew Burgess2-4/+9
When running the gdb.python/py-arch.exp tests on a GDB built against Python 2 I ran into some errors. The problem is that this test script exercises the gdb.Architecture.integer_type method, and this method uses 'p' as an argument format specifier in a call to gdb_PyArg_ParseTupleAndKeywords. Unfortunately this specified was only added in Python 3.3, so will cause an error for earlier versions of Python. This commit switches to use the 'O' specifier to collect a PyObject, and then uses PyObject_IsTrue to convert the object to a boolean. An earlier version of this patch incorrectly switched from using 'p' to use 'i', however, it was pointed out during review that this would cause some changes in behaviour, for example both of these will work with 'p', but not with 'i': gdb.selected_inferior().architecture().integer_type(32, None) gdb.selected_inferior().architecture().integer_type(32, "foo") The new approach of using 'O' works fine with these cases. I've added some new tests to cover both of the above. There should be no user visible changes after this commit.
2021-11-30[gdb/testsuite] Fix gdb.base/style.exp with stub-termcapTom de Vries1-3/+8
When running test-case gdb.base/style.exp with a gdb build using stub-termcap.c, we run into: ... (gdb) PASS: gdb.base/style.exp: all styles enabled: frame when width=20 ^M<et width 30^M (gdb) FAIL: gdb.base/style.exp: all styles enabled: set width 30 ... The problem is that we're trying to issue the command "set width 30" while width is set to 20, which causes horizontal scrolling. Fix this by resetting the width to 0 before issuing the "set width 30" command. Tested on x86_64-linux. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=24582
2021-11-30gdb/testsuite: check the python module is available before using itAndrew Burgess2-0/+31
The gdb.python/py-inferior-leak.exp test makes use of the tracemalloc module. When running the Python tests with a GDB built against Python 2 I ran into a test failure due to the tracemalloc module not being available. This commit adds a new helper function to lib/gdb-python.exp that checks if a named module is available. Using this we can then skip the py-inferior-leak.exp test when the tracemalloc module is not available.
2021-11-30gdb: fix disassembler regressions for 32-bit armAndrew Burgess1-8/+22
After this commit: commit 76b43c9b5c2b275cbf4f927bfc25984410cb5dd5 Date: Tue Oct 5 15:10:12 2021 +0100 gdb: improve error reporting from the disassembler We started seeing FAILs in the gdb.base/all-architectures*.exp tests, when running on a 32-bit ARM target, though I suspect running on any target that compiles such that bfd_vma is 32-bits would also trigger the failures. The problem is that the test is expected GDB's disassembler to print an error like this: Cannot access memory at address 0x0 However, after the above commit we see an error like: unknown disassembler error (error = -1) The reason for this is this code in opcodes/i386-dis.c (in the print_insn function): if (address_mode == mode_64bit && sizeof (bfd_vma) < 8) { (*info->fprintf_func) (info->stream, _("64-bit address is disabled")); return -1; } This code effectively disallows us from ever disassembling 64-bit x86 code if we compiled GDB with a 32-bit bfd_vma. Notice we return -1 (indicating a failure to disassemble), but never call the memory_error_func callback. Prior to the above commit GDB, when it received the -1 return value would assume that a memory error had occurred and just print whatever value happened to be in the memory error address variable, the default value of 0 just happened to be fine because the test had asked GDB to do this 'disassemble 0x0,+4'. If we instead change the test to do 'disassemble 0x100,+4' then GDB would (previously) have still reported: Cannot access memory at address 0x0 which makes far less sense. In this commit I propose to fix this issue by changing the test to accept either the "Cannot access memory ..." string, or the newer "unknown disassembler error ..." string. With this change done the test now passes. However, there is one weakness with this strategy; if GDB broke such that we _always_ reported "unknown disassembler error ..." we would never notice. This clearly would be bad. To avoid this issue I have adjusted the all-architectures*.exp tests so that, when we disassemble for the default architecture (the one selected by "auto") we _only_ expect to get the "Cannot access memory ..." error string. [ Note: In an ideal world we should be able to disassemble any architecture at all times. There's no reason why the 64-bit x86 disassembler requires a 64-bit bfd_vma, other than the code happens to be written that way. We could rewrite the disassemble to not have this requirement, but, I don't plan to do that any time soon. ] Further, I have changed the all-architectures*.exp test so that we now disassemble at address 0x100, this should avoid us being able to pass by printing a default address of 0x0. I did originally change the address we disassembled at to 0x4, however, some architectures, e.g. ia64, have a default instruction alignment that is greater than 4, so would still round down to 0x0. I could have just picked 0x8 as an address, but I figured that 0x100 was likely to satisfy most architectures alignment requirements.
2021-11-30gdb/python: add gdb.RemoteTargetConnection.send_packetAndrew Burgess10-11/+615
This commits adds a new sub-class of gdb.TargetConnection, gdb.RemoteTargetConnection. This sub-class is created for all 'remote' and 'extended-remote' targets. This new sub-class has one additional method over its base class, 'send_packet'. This new method is equivalent to the 'maint packet' CLI command, it allows a custom packet to be sent to a remote target. The outgoing packet can either be a bytes object, or a Unicode string, so long as the Unicode string contains only ASCII characters. The result of calling RemoteTargetConnection.send_packet is a bytes object containing the reply that came from the remote.
2021-11-30gdb: make packet_command function available outside remote.cAndrew Burgess4-35/+119
In a later commit I will add a Python API to access the 'maint packet' functionality, that is, sending a user specified packet to the target. To make implementing this easier, this commit refactors how this command is currently implemented so that the packet_command function is now global. The new global send_remote_packet function takes an object that is an implementation of an abstract interface. Two functions within this interface are then called, one just before a packet is sent to the remote target, and one when the reply has been received from the remote target. Using an interface object in this way allows (1) for the error checking to be done before the first callback is made, this means we only print out what packet it being sent once we know we are going to actually send it, and (2) we don't need to make a copy of the reply if all we want to do is print it. One user visible changes after this commit are the error messages, which I've changed to be less 'maint packet' command focused, this will make them (I hope) better for when send_remote_packet can be called from Python code. So: "command can only be used with remote target" Becomes: "packets can only be sent to a remote target" And: "remote-packet command requires packet text as argument" Becomes: "a remote packet must not be empty" Additionally, in this commit, I've added support for packet replies that contain binary data. Before this commit, the code that printed the reply treated the reply as a C string, it assumed that the string only contained printable characters, and had a null character only at the end. One way to show the problem with this is if we try to read the auxv data from a remote target, the auxv data is binary, so, before this commit: (gdb) target remote :54321 ... (gdb) maint packet qXfer:auxv:read::0,1000 sending: "qXfer:auxv:read::0,1000" received: "l!" (gdb) And after this commit: (gdb) target remote :54321 ... (gdb) maint packet qXfer:auxv:read::0,1000 sending: "qXfer:auxv:read::0,1000" received: "l!\x00\x00\x00\x00\x00\x00\x00\x00\xf0\xfc\xf7\xff\x7f\x00\x00\x10\x00\x00\x00\x00\x00\x00\x00\xff\xf> (gdb) The binary contents of the reply are now printed as escaped hex.
2021-11-30gdb/python: introduce gdb.TargetConnection object typeAndrew Burgess17-4/+725
This commit adds a new object type gdb.TargetConnection. This new type represents a connection within GDB (a connection as displayed by 'info connections'). There's three ways to find a gdb.TargetConnection, there's a new 'gdb.connections()' function, which returns a list of all currently active connections. Or you can read the new 'connection' property on the gdb.Inferior object type, this contains the connection for that inferior (or None if the inferior has no connection, for example, it is exited). Finally, there's a new gdb.events.connection_removed event registry, this emits a new gdb.ConnectionEvent whenever a connection is removed from GDB (this can happen when all inferiors using a connection exit, though this is not always the case, depending on the connection type). The gdb.ConnectionEvent has a 'connection' property, which is the gdb.TargetConnection being removed from GDB. The gdb.TargetConnection has an 'is_valid()' method. A connection object becomes invalid when the underlying connection is removed from GDB (as discussed above, this might be when all inferiors using a connection exit, or it might be when the user explicitly replaces a connection in GDB by issuing another 'target' command). The gdb.TargetConnection has the following read-only properties: 'num': The number for this connection, 'type': e.g. 'native', 'remote', 'sim', etc 'description': The longer description as seen in the 'info connections' command output. 'details': A string or None. Extra details for the connection, for example, a remote connection's details might be 'hostname:port'.
2021-11-29Allow DW_ATE_UTF for Rust charactersTom Tromey4-13/+75
The Rust compiler plans to change the encoding of a Rust 'char' type to use DW_ATE_UTF. You can see the discussion here: https://github.com/rust-lang/rust/pull/89887 However, this fails in gdb. I looked into this, and it turns out that the handling of DW_ATE_UTF is currently fairly specific to C++. In particular, the code here assumes the C++ type names, and it creates an integer type. This comes from commit 53e710acd ("GDB thinks char16_t and char32_t are signed in C++"). The message says: Both places need fixing. But since I couldn't tell why dwarf2read.c needs to create a new type, I've made it use the per-arch built-in types instead, so that the types are only created once per arch instead of once per objfile. That seems to work fine. ... which is fine, but it seems to me that it's also correct to make a new character type; and this approach is better because it preserves the type name as well. This does use more memory, but first we shouldn't be too concerned about the memory use of types coming from debuginfo; and second, if we are, we should implement type interning anyway. Changing this code to use a character type revealed a couple of oddities in the C/C++ handling of TYPE_CODE_CHAR. This patch fixes these as well. I filed PR rust/28637 for this issue, so that this patch can be backported to the gdb 11 branch.
2021-11-29[PR gdb/27026] CTRL-C is ignored when debug info is downloadedAaron Merey1-0/+15
During debuginfod downloads, ctrl-c should result in the download being cancelled and skipped. However in some cases, ctrl-c fails to get delivered to gdb during downloading. This can result in downloads being unskippable. Fix this by ensuring that target_terminal::ours is in effect for the duration of each download. Co-authored-by: Tom de Vries <tdevries@suse.de> https://sourceware.org/bugzilla/show_bug.cgi?id=27026#c3
2021-11-29[gdb/symtab] Fix segfault in search_one_symtabTom de Vries3-1/+105
PR28539 describes a segfault in lambda function search_one_symtab due to psymbol_functions::expand_symtabs_matching calling expansion_notify with a nullptr symtab: ... struct compunit_symtab *symtab = psymtab_to_symtab (objfile, ps); if (expansion_notify != NULL) if (!expansion_notify (symtab)) return false; ... This happens as follows. The partial symtab ps is a dwarf2_include_psymtab for some header file: ... (gdb) p ps.filename $5 = 0x64fcf80 "/usr/include/c++/11/bits/stl_construct.h" ... The includer of ps is a shared symtab for a partial unit, with as user: ... (gdb) p ps.includer().user.filename $11 = 0x64fc9f0 \ "/usr/src/debug/llvm13-13.0.0-1.2.x86_64/tools/clang/lib/AST/Decl.cpp" ... The call to psymtab_to_symtab expands the Decl.cpp symtab (and consequently the shared symtab), but returns nullptr because: ... struct dwarf2_include_psymtab : public partial_symtab { ... compunit_symtab *get_compunit_symtab (struct objfile *objfile) const override { return nullptr; } ... Fix this by returning the Decl.cpp symtab instead, which fixes the segfault in the PR. Tested on x86_64-linux. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28539
2021-11-29[gdb/testsuite] Fix typo in proc linesTom de Vries1-1/+1
Proc lines contains a typo: ... string_form { set $_line_string_form $value } ... Remove the incorrect '$' in '$_line_string_form'. Tested on x86_64-linux.
2021-11-29[gdb/testsuite] Use unique files in gdb.dwarf2/dw2-lines.expTom de Vries2-2/+30
While debugging a problem in gdb.dwarf2/dw2-lines.exp, I realized that the test-case generates all executables and associated temporary files using the same filenames. Fix this by adding a new proc prefix_id in lib/gdb.exp, and using it in the test-case. Tested on x86_64-linux.