When using QEMU as a RISCV simulator, hardware watchpoint events are
reported to GDB before the target memory gets written. GDB currently
expects the event to be reported after it is written. As a result of
this mismatch, upon receiving the event, GDB sees that the target
memory region has not changed, and therefore decides to ignore the
event. It therefore resumes the program's execution with a continue,
which is the start of an infinite loop between QEMU repeatedly
reporting the same watchpoint event over and over, and GDB repeatedly
ignoring it.
This patch fixes the issue by telling GDB to expect the watchpoint
event to be reported ahead of the memory region being modified.
Upon receiving the event, GDB then single-steps the program before
checking the watched memory value.
gdb/ChangeLog:
* riscv-tdep.c (riscv_gdbarch_init): Set the gdbarch's
have_nonsteppable_watchpoint attribute to 1.
Some confusion over how the register names and aliases are setup in
riscv means that we currently can't access registers through their
architectural name.
This commit fixes this issue, and moves some of the csr register
handling out of the alias handling code and deals with it separately.
This has the benefit that we can now directly access some arrays
rather than having to iterate over them.
A new test is added to ensure that register aliases now work
correctly.
gdb/ChangeLog:
* riscv-tdep.c (riscv_gdb_reg_names): Update comment, and all
register names.
(struct register_alias): Rename to...
(struct riscv_register_alias): ...this, and update comment.
(riscv_register_aliases): Update type, and alias names. Remove
CSR names from this list.
(riscv_register_name): Use riscv_gdb_reg_names for int and float
register names. Add an extra assertion.
(riscv_is_regnum_a_named_csr): New function.
(riscv_register_reggroup_p): Use riscv_is_regnum_a_named_csr.
gdb/testsuite/ChangeLog:
* gdb.arch/riscv-reg-aliases.c: New file.
* gdb.arch/riscv-reg-aliases.exp: New file.
The hardware requires that values in FP registers be NaN-boxed, so we must
extend them with 1's instead of 0's as we do for integer values.
gdb/
* riscv-tdep.c (riscv_push_dummy_call) <in_reg>: Check for value in
FP reg smaller than FP reg size, and fill with -1 instead of 0.
A 64-bit FP register can hold either a single or double float value, so
print it as both types by using a union type for FP registers. Likewise
for 128-bit regs which can also hold long double.
gdb/
* riscv-tdep.c (riscv_fpreg_d_type, riscv_fpreg_q_type): New.
(riscv_register_type): Use them.
(riscv_print_one_register_info): Handle union of floats same as float.
* riscv-tdep.h (struct gdbarch_tdep): Add riscv_fpreg_d_type and
riscv_fpreg_q_type fields.
Tests that target md_apply_fix can handle fixups that have resolved
down to a constant and thus do not need relocations.
Also a fix for eqv-dot xfails.
* testsuite/gas/all/simple-forward.d,
* testsuite/gas/all/simple-forward.s: New test.
* testsuite/gas/all/gas.exp: Run it.
* testsuite/gas/all/eqv-dot.d: xfail tic30 and tic54x.
* config/tc-alpha.c (md_apply_fix): Handle BFD_RELOC_8 for fixups
without a symbol.
* testsuite/gas/all/gas.exp: Don't xfail forward test here..
* testsuite/gas/all/forward.d: ..do so here, removing alpha.
What a trip down a rabbit hole this bug has been.
First observation: You can't use deferred_expression in s_leb128.
deferred_expression implements the semantics of .eqv or '==', saving
an expression with minimal simplification for assignment to a symbol
so that the expression is evaluated at uses of the symbol. In
particular, the value of "dot" is not evaluated at the .eqv symbol
assignment, but later. When s_leb128 uses deferred_expression,
"later" is at the end of assembly, giving entirely the wrong value of
"dot". There is no way to fix this for the s_leb128 use without
breaking .equ (which incidentally was already somewhat broken, see
commit e4c2619ad1). So, don't use deferred_expression in s_leb128.
But that leads to the gas test elf/dwarf2-17 failing, because view
symbols are calculated with a chain of expression symbols. In the
dwarf2-17 .L1 case there is a "temp_sym_1 > temp_sym_2" expression,
with temp_sym_1 and temp_sym_2 on either side of a ".balign". Since
".balign" and many other directives moving "dot" are not calculated on
the first (and only) pass over source, .L1 cannot be calculated until
final addresses are assigned to frags. However, ".uleb128 .L1" *is*
calculated immediately, resulting in the wrong value.
The reason why .L1 is calculated immediately is that code in
expr.c:operand after the comment
/* If we have an absolute symbol or a reg, then we know its
value now. */
does as it says and fixes the value of .L1, because .L1 is assigned
to absolute_section in dwarf2dbg.c:set_or_check_view. So, correct
that to expr_section.
Unfortunately that fix leads to failure of the elf/dwarf2-5 test with
../gas/elf/dwarf2-5.s: Error: attempt to get value of unresolved symbol `.L5'
../gas/elf/dwarf2-5.s: Error: attempt to get value of unresolved symbol `.L11'
../gas/elf/dwarf2-5.s: Error: attempt to get value of unresolved symbol `.L12'
So why is that? Well, it turns out that .L5 is defined in terms of
.L4, and apparently .L4 is undefined. But .L4 clearly is defined,
otherwise we would hit an error when trying to use .L4 a little
earlier. There are two copies of .L4! So, symbols are cloned when
that should not happen.
Symbol cloning is a technique used by gas to support saving the value
of symbols that change between uses, but that isn't the case with
.L4. Only one value is set and used for .L4, but indeed .L4 was being
cloned by symbol_clone_if_forward_ref. This despite no forward refs
being present. Also, .L4 is a local symbol and a cursory glance at
symbol_clone_if_forward_ref "if (symbolP && !LOCAL_SYMBOL_CHECK (symbolP))"
would seem to prevent cloning of local symbols. All is not as it
seems though, a curse of using macros. LOCAL_SYMBOL_CHECK modifies
its argument if a "struct local_symbol" is converted to the larger
"struct symbol", as happens when assigning a view symbol value.
That fact results in the recursive call to symbol_clone_if_forward_ref
returning a different address for "add_symbol". This problem could
have been fixed by using symbol_same_p rather than comparing symbol
pointers, but I thought it better to use the real symbol throughout.
Note that symbol_find_exact also returns the real symbol for a
converted local symbol.
Finally, this patch does expose lack of support for forward symbol
definitions in various targets. For example:
alpha-linux +ERROR: ../ld/testsuite/ld-elf/pr11138-2.c: compilation failed
This is caused by view symbol uses. On alpha-linux-gcc (GCC) 8.1.1
20180502 they happen to occur in .byte directives so were silently
broken in cases like elf/dwarf2-17 anyway.
/tmp/ccvtsMfU.s: Assembler messages:
/tmp/ccvtsMfU.s: Fatal error: unhandled relocation type BFD_RELOC_8
/tmp/ccvtsMfU.s: Fatal error: unhandled relocation type BFD_RELOC_8
md_apply_fix on those targets needs to handle fixups that resolve down
to a constant.
PR 23040
* symbols.c (get_real_sym): New function.
(symbol_same_p): Use get_real_sym.
(symbol_clone_if_forward_ref): Save real original add_symbol and
op_symbol for comparison against that returned from lookup or
recursive calls.
* dwarf2dbg.c (set_or_check_view): Use expr_section for
expression symbols, not absolute_section.
(dwarf2_directive_loc): Check symbol_equated_p and tidy cloning
of view symbols.
* read.c (s_leb128): Don't use deferred_expression.
Running "maintenance selftest" on an amd64 build with AddressSanitizer
enabled, I get this:
==18126==ERROR: AddressSanitizer: dynamic-stack-buffer-overflow on address 0x7ffdf72397c1 at pc 0x7fb5f437b011 bp 0x7ffdf7239740 sp 0x7ffdf7238ee8
WRITE of size 8 at 0x7ffdf72397c1 thread T0
#0 0x7fb5f437b010 in __interceptor_memcpy /build/gcc/src/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:737
#1 0x55a1f899c1b3 in readable_regcache::raw_read(int, unsigned char*) /home/simark/src/binutils-gdb/gdb/regcache.c:530
#2 0x55a1f7db241b in amd64_pseudo_register_read_value /home/simark/src/binutils-gdb/gdb/amd64-tdep.c:384
#3 0x55a1f8413a2e in gdbarch_pseudo_register_read_value(gdbarch*, readable_regcache*, int) /home/simark/src/binutils-gdb/gdb/gdbarch.c:1992
#4 0x55a1f899c9d1 in readable_regcache::cooked_read(int, unsigned char*) /home/simark/src/binutils-gdb/gdb/regcache.c:636
#5 0x55a1f89a2251 in cooked_read_test /home/simark/src/binutils-gdb/gdb/regcache.c:1649
In amd64_pseudo_register_read_value, when we try to read the al
register, for example, we need to read rax and extract al from it. We
allocate a buffer of the size of al (1 byte):
gdb_byte *raw_buf = (gdb_byte *) alloca (register_size (gdbarch, regnum));
but read in it the whole rax value (8 bytes):
status = regcache->raw_read (gpnum, raw_buf);
Fix it by allocating a buffer correctly sized for the full register from
which the smaller register is extracted. The
amd64_pseudo_register_write function had the same problem.
gdb/ChangeLog:
* amd64-tdep.c (amd64_pseudo_register_read_value): Use
correctly-sized buffer with raw_read.
(amd64_pseudo_register_write): Use correctly-sized buffer for
raw_read/raw_write.
Without this patch:
(gdb) help set print type
Generic command for setting how types print.
List of show print type subcommands:
show print type methods -- Set printing of methods defined in classes
...
With this patch:
(gdb) h set print type
Generic command for setting how types print.
List of set print type subcommands:
set print type methods -- Set printing of methods defined in classes
...
"make info" in gdb was broken by my last patch. This fixes it.
Tested by rebuilding; committing as obvious.
gdb/doc/ChangeLog
2018-10-20 Tom Tromey <tom@tromey.com>
* gdb.texinfo (TUI Commands): Add @end table.
(TUI Configuration): Remove stray @end table.
.eqv (and ==) ought not simplify expressions involving dot or other
symbols set by .eqv. If such simplification occurs, the value of dot
will be that at the assignment rather than at the place where the
symbol is used.
PR 23800
* expr.c (expr): Don't simplify expressions involving forward_ref
symbols when mode is expr_defer.
* config/tc-spu.c (spu_cons): Parse expression using normal
expression evaluation if @ppu is not detected.
* testsuite/gas/all/eqv-dot.d,
* testsuite/gas/all/eqv-dot.s: New test.
* testsuite/gas/all/gas.exp: Run it.
Symbol tables can change when a number of objcopy options are used.
I figure string tables are similarly changeable.
PR 23788
* elf.c (section_match): Don't require a size match for SHT_SYMTAB
or SHT_STRTAB.
A symbol root is not needed if --gc-keep-exported is also given.
* ldlang.c (lang_end): Don't error if no --entry or --undefined
is given with -r -gc-sections if --gc-keep-exported.
The "tabset" command sets the tab width as used by the TUI for source
and disassembly display.
This command has long seemed to be misnamed to me. It is more in
keeping with gdb design to call it "set tui tab-width". Also, making
this change allows for the corresponding "show" command to work.
gdb/ChangeLog
2018-10-19 Tom Tromey <tom@tromey.com>
PR tui/18388:
* NEWS: Mention tabset deprecation.
* tui/tui-win.c (tui_tab_width, internal_tab_width): New globals.
(update_tab_width): New function.
(tui_set_tab_width, tui_show_tab_width): New functions.
(tui_set_tab_width_command): Use update_tab_width.
(_initialize_tui_win): Move to end of file. Deprecate "tabset".
Add new "set tui tab-width" command.
* tui/tui-source.c (tui_set_source_content): Update.
* tui/tui-disasm.c (tui_set_disassem_content): Update.
* tui/tui-data.h (tui_default_tab_len, tui_set_default_tab_len):
Don't declare.
(tui_tab_width): Declare.
* tui/tui-data.c (default_tab_len, tui_default_tab_len)
(tui_set_default_tab_len): Remove.
gdb/doc/ChangeLog
2018-10-19 Tom Tromey <tom@tromey.com>
PR tui/18388:
* gdb.texinfo (TUI Commands): Remove tabset documentation.
(TUI Configuration): Document "set tui tab-width".
I noticed that a couple of functions in tui-io.c could be static, and
that a couple more were unused and could be removed.
gdb/ChangeLog
2018-10-19 Tom Tromey <tom@tromey.com>
* tui/tui-io.h (key_is_start_sequence, key_is_end_sequence)
(key_is_backspace, tui_getc): Don't declare.
* tui/tui-io.c (key_is_start_sequence): Now static.
(key_is_end_sequence, key_is_backspace): Remove.
(tui_getc): Now static.
-fsanitize=address pointed out a use-after free in
objfile_register_static_link. The bug turned out to be that
reread_symbols does not clear the static_links field; this leaves a
hash table that is filled with freed pointers.
Jan's (now quite old) idea of replacing reread_symbols with a simple
delete/new still seems good to me, and it's worth noting that it would
have avoided this bug.
Tested by the buildbot and by observing the change with
-fsanitize=address.
gdb/ChangeLog
2018-10-19 Tom Tromey <tom@tromey.com>
* symfile.c (reread_symbols): Clear "static_links".
The two new test I added require the use of the ".inst" directive which the PE
targets don't support. Because of that I excluded *-unknown-pe but the mask needs
to be wider. I am now excluding *-*-pe.
gas/ChangeLog:
* testsuite/gas/arm/undefined-insn-arm.d: Widen pe skip.
* testsuite/gas/arm/undefined-insn-thumb.d: Likewise.
When reading the reserved section in the sigcontext ensure the
address is updated on an unknown section. Also add additional
checks to prevent reading past the end of the array.
Fixes gdb.base/savedregs.exp
* aarch64-linux-tdep.c (AARCH64_SIGCONTEXT_RESERVED_SIZE): New
define.
(aarch64_linux_sigframe_init): Extra boundary checks.
I've encountered a GDB crash when trying to read registers from a remote
stub that provided a target.xml with vector registers, but without the
'vec128' data type. The crash is caused by NULL register type entries for
the "concatenated" pseudo-registers v0-v15. These NULL entries are
introduced by the logic in s390_pseudo_register_type(), where the tdesc
type 'vec128' is returned unconditionally -- even if it doesn't exist (is
NULL).
The fixed logic for determining a "concatenated" vector register's type
now returns the type of the raw register v16 instead. This also makes
sure that all vector register have the same type.
gdb/ChangeLog:
* s390-tdep.c (s390_pseudo_register_type): For v0-v15 don't yield
the possibly non-existent tdesc type 'vec128', but the type of raw
register v16 instead.
This commit fixes a resource leak found by Coverity, where cli_interp's
constructor creates a new cli_ui_out, but there was no corresponding
destructor to free it.
gdb/ChangeLog:
* cli/cli-interp.c (cli_interp::~cli_interp): New function.
The test loop in test_python_inline_or_multiline was not checking for the
gdb prompt. readline_is_used then reads in the prompt causing it to fail
and the final chunk of tests to be skipped.
Spotted this error because sometimes the gdb prompt does get processed
correctly and the final tests are run, causing it to show up in the
racy tests script output.
Fix by ensuring the prompt is always checked for. To do this the list
style needs reformatting so that variables get expanded.
2018-10-19 Alan Hayward <alan.hayward@arm.com>
* gdb.python/py-cmd.exp: Check for gdb_prompt.
On aarch64 (and possibly other ports), for O3 the sizeof "a" can still be shown.
2018-10-19 Alan Hayward <alan.hayward@arm.com>
* gdb.base/vla-optimized-out.exp: Allow either optimized out or 6.
Fix the layout used in the regexp for breakpoints.
Gets rid of two FAILS.
2018-10-18 Alan Hayward <alan.hayward@arm.com>
* gdb.cp/ovldbreak.exp: Fix regexps.
skip_compile_feature_tests is only valid if the inferior has already
been started (see proc comments). Move the runto_main earlier.
2018-10-18 Alan Hayward <alan.hayward@arm.com>
* gdb.compile/compile-cplus-print.exp: Start inferior earlier.
The disassembler for Arm has some aborts in it in places it assumes can never
be reached. Under normal circumstances they indeed cannot be reached because
the right options are selected from the ARM attributes in the ELF file.
However when disassembling with -b binary then if you do not get the options
right the disassembler just aborts. This changes it so it just prints how it
was trying to interpret the instruction and prints UNKNOWN instructions next to it.
This way the user has an idea of what's going.
gas/ChangeLog:
* testsuite/gas/arm/undefined-insn-arm.d: New test.
* testsuite/gas/arm/undefined-insn-thumb.d: New test.
* testsuite/gas/arm/undefined-insn.s: New test.
opcodes/ChangeLog:
* arm-dis.c (UNKNOWN_INSTRUCTION_32BIT): Format specifier for arm mode.
(UNKNOWN_INSTRUCTION_16BIT): Format specifier for thumb mode.
(print_insn_arm, print_insn_thumb16, print_insn_thumb32): Use them.
* testsuite/gas/mips/r5900.s: Extend the R5900 short loop fix
test with border cases.
* testsuite/gas/mips/r5900.d: Add extra expected disassembly.
* config/tc-mips.c (can_swap_branch_p): Clarify the R5900 short
loop hardware bug conditions. Correct note on the R5900
instruction count short loop fix.
This simple patch fixes the segfault reported on PR cli/23785, which
happens when using the "restore FILE binary" command with a
non-existent file. We just have to check if the file handler returned
by "gdb_fopen_cloexec" is not NULL, and error out if it is.
A test has also been added to gdb.base/restore.exp in order to
exercise this scenario.
No regressions introduced.
gdb/ChangeLog:
2018-10-18 Sergio Durigan Junior <sergiodj@redhat.com>
PR cli/23785
* cli/cli-dump.c (restore_binary_file): Check if "file" is
NULL.
gdb/testsuite/ChangeLog:
2018-10-18 Sergio Durigan Junior <sergiodj@redhat.com>
PR cli/23785
* gdb.base/restore.exp: New test to check if "restore" with an
invalid file doesn't segfault.
At https://sourceware.org/gdb/wiki/GDBTestcaseCookbook\
#Follow_the_test_name_convention we find:
..
Test names should start with a lower case and don't need to end with a period
(they are not sentences).
...
Fix some capitalized test names.
Tested on x86_64-linux.
2018-10-18 Tom de Vries <tdevries@suse.de>
* gdb.ada/bp_inlined_func.exp: Fix capitalized test name.
* gdb.ada/excep_handle.exp: Same.
* gdb.ada/mi_string_access.exp: Same.
* gdb.ada/mi_var_union.exp: Same.
* gdb.arch/arc-analyze-prologue.exp: Same.
* gdb.arch/arc-decode-insn.exp: Same.
* gdb.base/readnever.exp: Same.
* gdb.fortran/printing-types.exp: Same.
* gdb.guile/scm-lazy-string.exp: Same.
gdb/ChangeLog:
* fbsd-nat.c (fbsd_nat_target::info_proc) Use
fbsd_info_proc_mappings_header and fbsd_info_proc_mappings_entry.
* fbsd-tdep.c (fbsd_vm_map_entry_flags): Mark static.
(fbsd_info_proc_mappings_header, fbsd_info_proc_mappings_entry):
New functions.
(fbsd_core_info_proc_mappings): Use fbsd_info_proc_mappings_header
and fbsd_info_proc_mappings_header.
* fbsd-tdep.h (fbsd_vm_map_entry_flags): Remove.
(fbsd_info_proc_mappings_header, fbsd_info_proc_mappings_entry):
New.
This patch improves dynobj selection. This allows the testcase in the
PR to proceed further before segfaulting due to other bugs in the
sparc backend.
../ld/ld-new --eh-frame-hdr -m elf_x86_64 -shared -o bug.so.5 bug.o ./libc.so.6 ./crtendS.o
../ld/ld-new: sparc architecture of input file `bug.o' is incompatible with i386:x86-64 output
../ld/ld-new: bug.o: in function `a':
bug.c:(.text+0x4): undefined reference to `_GLOBAL_OFFSET_TABLE_'
../ld/ld-new: bug.c:(.text+0x8): undefined reference to `_GLOBAL_OFFSET_TABLE_'
Segmentation fault
PR 23653
* elflink.c (_bfd_elf_link_create_dynstrtab): Match elf_object_id
too when choosing dynobj.
There are two problems with the current catch-follow-exec.exp:
- INTERNAL_GDBFLAGS (containing the datadir setting) is not used
- remote host testing doesn't work
Fix the former by using gdb_spawn_with_cmdline_opts. Fix the latter by
requiring gdb-native.
Build on x86_64-linux with and without ubsan, and tested.
2018-10-16 Tom de Vries <tdevries@suse.de>
PR gdb/23730
* gdb.base/catch-follow-exec.c: Add copyright notice.
* gdb.base/catch-follow-exec.exp: Rewrite to use
gdb_spawn_with_cmdline_opts. Require gdb-native.
Committed on behalf of Matthew Malcomson:
The SIMD UDOT instruction assembly has an unusual operand that selects a single
32 bit element with the mnemonic 4B.
This unusual mnemonic is handled by a special operand qualifier and associated
qualifier data in `aarch64_opnd_qualifiers`.
The current qualifier data describes 4 1-byte elements with the structure
{1, 4, 0x0, "4b", OQK_OPD_VARIANT}
This makes sense, as the instruction does work on 4 1-byte elements, however
some logic in the `operand_general_constraint_met_p` makes assumptions about
the range of index allowed when selecting a SIMD_ELEMENT depending on element
size.
That function reasons that e.g. in order to select a byte-sized element in a 16
byte V register an index must allow selection of one of the 16 elements and
hence its range will be in [0,15].
This reasoning breaks with the above description of a 4 part selection of 1
byte elements and allows an index outside the valid [0,3] range, triggering an
assert later on in the program in `aarch64_ins_reglane`.
vshcmd: > echo 'udot v0.2s, v1.8b, v2.4b[4]' | ../src/binutils-build/gas/as-new -march=armv8.4-a
as-new: ../../binutils-gdb/opcodes/aarch64-asm.c:134: aarch64_ins_reglane: Assertion `reglane_index < 4' failed.
{standard input}: Assembler messages:
{standard input}:1: Internal error (Aborted).
Please report this bug.
This patch changes the operand qualifier data so that it describes a single
32 bit element.
{4, 1, 0x0, "4b", OQK_OPD_VARIANT}
Hence the calculation in `operand_general_constraint_met_p` provides the
correct answer and the usual error checking machinery is used.
vshcmd: > echo 'udot v0.2s, v1.8b, v2.4b[4]' | ../src/binutils-build/gas/as-new -march=armv8.4-a
{standard input}: Assembler messages:
{standard input}:1: Error: register element index out of range 0 to 3 at operand 3 -- `udot v0.2s,v1.8b,v2.4b[4]'
The negative size check can be rolled into the "exceeds space left in
section" check if that is done using an unsigned comparison. We know
that "addr - section->vma" is never larger than section->size since
the section is found by find_section_by_vma.
* peXXigen.c (_bfd_XX_bfd_copy_private_bfd_data_common): Simplify
PE_DEBUG_DATA size checks.
