Fix gdb.dwarf2/dw2-dir-file-name.exp test for MIPS architectures
Commit Message
This patch fixes the failures that occur with the
gdb.dwarf2/dw2-dir-file-name.exp test on 64-bit MIPS and compressed MIPS
ISAs (i.e. MIPS16 and microMIPS).
The failures on 64-bit occur because the generated DWARF address
information is always 32-bit, which causes the upper 32-bits of
addresses to be truncated and causes breakpoints to be set on the wrong
address if any of the upper 32-bits are non-zero. I suspect that other
64-bit architectures get away with it because they place all their
instructions at a VMA lower than 2^32 by default. This patch causes
64-bit addresses to be generated if a 64-bit target is detected.
The failures on MIPS16 and microMIPS occur because the breakpoint
address needs to have the LSB set to 1 (used to indicate that the code
is compressed). However, the function name is interpreted as a data
label, causing GDB to set breakpoints at even addresses. This is fixed
by explicitly adding a '.insn' directive (see
https://sourceware.org/binutils/docs/as/MIPS-insn.html) after the label
on MIPS only.
Kwok
2014-08-14 Kwok Cheung Yeung <kcy@codesourcery.com>
gdb/testsuite/
* gdb.dwarf2/dw2-dir-file-name.exp (addr_len): New.
(out_cu): Use addr_len for the size of addresses.
(out_line): Likewise. Size DW_LNE_set_address instruction
according to addr_len.
* gdb.dwarf2/dw2-dir-file-name.c (START_INSNS): New.
(FUNC): Add START_INSNS to definition.
Comments
On 10/11/2014 10:01 PM, Kwok Cheung Yeung wrote:
> This patch fixes the failures that occur with the
> gdb.dwarf2/dw2-dir-file-name.exp test on 64-bit MIPS and compressed MIPS
> ISAs (i.e. MIPS16 and microMIPS).
>
> The failures on 64-bit occur because the generated DWARF address
> information is always 32-bit, which causes the upper 32-bits of
> addresses to be truncated and causes breakpoints to be set on the wrong
> address if any of the upper 32-bits are non-zero. I suspect that other
> 64-bit architectures get away with it because they place all their
> instructions at a VMA lower than 2^32 by default. This patch causes
> 64-bit addresses to be generated if a 64-bit target is detected.
>
> The failures on MIPS16 and microMIPS occur because the breakpoint
> address needs to have the LSB set to 1 (used to indicate that the code
> is compressed). However, the function name is interpreted as a data
> label, causing GDB to set breakpoints at even addresses. This is fixed
> by explicitly adding a '.insn' directive (see
> https://sourceware.org/binutils/docs/as/MIPS-insn.html) after the label
> on MIPS only.
Looks fine to me, but I'd like to hear Maciej's OK on the MIPS specifics too.
Please make sure x86_64 still works.
I wonder if this test could be simplified using the testsuite's dwarf
assembler (Dwarf::assemble).
>
> Kwok
>
> 2014-08-14 Kwok Cheung Yeung <kcy@codesourcery.com>
>
> gdb/testsuite/
> * gdb.dwarf2/dw2-dir-file-name.exp (addr_len): New.
> (out_cu): Use addr_len for the size of addresses.
> (out_line): Likewise. Size DW_LNE_set_address instruction
> according to addr_len.
> * gdb.dwarf2/dw2-dir-file-name.c (START_INSNS): New.
> (FUNC): Add START_INSNS to definition.
>
Thanks,
Pedro Alves
Maciej has already reviewed this patch internally, and I can confirm
that the test still passes cleanly on x86_64 Linux.
If there are no objections I will go ahead and commit it tomorrow.
Thanks,
Kwok
On 17/10/2014 1:07 PM, Pedro Alves wrote:
> Looks fine to me, but I'd like to hear Maciej's OK on the MIPS specifics too.
>
> Please make sure x86_64 still works.
>
> I wonder if this test could be simplified using the testsuite's dwarf
> assembler (Dwarf::assemble).
>
>>
>> Kwok
>>
>> 2014-08-14 Kwok Cheung Yeung <kcy@codesourcery.com>
>>
>> gdb/testsuite/
>> * gdb.dwarf2/dw2-dir-file-name.exp (addr_len): New.
>> (out_cu): Use addr_len for the size of addresses.
>> (out_line): Likewise. Size DW_LNE_set_address instruction
>> according to addr_len.
>> * gdb.dwarf2/dw2-dir-file-name.c (START_INSNS): New.
>> (FUNC): Add START_INSNS to definition.
>>
>
>
> Thanks,
> Pedro Alves
>
Kwok Cheung Yeung writes:
> Maciej has already reviewed this patch internally, and I can confirm
> that the test still passes cleanly on x86_64 Linux.
>
> If there are no objections I will go ahead and commit it tomorrow.
>
> Thanks,
>
> Kwok
>
> On 17/10/2014 1:07 PM, Pedro Alves wrote:
> > Looks fine to me, but I'd like to hear Maciej's OK on the MIPS specifics too.
> >
> > Please make sure x86_64 still works.
> >
> > I wonder if this test could be simplified using the testsuite's dwarf
> > assembler (Dwarf::assemble).
> >
> >>
> >> Kwok
> >>
> >> 2014-08-14 Kwok Cheung Yeung <kcy@codesourcery.com>
> >>
> >> gdb/testsuite/
> >> * gdb.dwarf2/dw2-dir-file-name.exp (addr_len): New.
> >> (out_cu): Use addr_len for the size of addresses.
> >> (out_line): Likewise. Size DW_LNE_set_address instruction
> >> according to addr_len.
> >> * gdb.dwarf2/dw2-dir-file-name.c (START_INSNS): New.
> >> (FUNC): Add START_INSNS to definition.
Hi.
One nit.
I'm guessing there's an attempt here to have the comments align
properly in the generated file.
@@ -54,8 +62,8 @@ proc out_cu { name cu_dir cu_name line_dir line_name } {
.ascii \"GNU C\\0\" /* DW_AT_producer */
.byte 2 /* DW_AT_language (DW_LANG_C) */
.4byte .Lline_${name}_begin /* DW_AT_stmt_list */
- .4byte ${name}_start /* DW_AT_low_pc */
- .4byte ${name}_end /* DW_AT_high_pc */
+ .${addr_len}byte ${name}_start /* DW_AT_low_pc */
+ .${addr_len}byte ${name}_end /* DW_AT_high_pc */
"
if { $cu_dir != "" } {
puts $f " .ascii $cu_dir /* DW_AT_comp_dir */"
I'd rather not go down this path, and just keep the source
reasonably clean in this regard.
E.g.,
@@ -54,8 +62,8 @@ proc out_cu { name cu_dir cu_name line_dir line_name } {
.ascii \"GNU C\\0\" /* DW_AT_producer */
.byte 2 /* DW_AT_language (DW_LANG_C) */
.4byte .Lline_${name}_begin /* DW_AT_stmt_list */
- .4byte ${name}_start /* DW_AT_low_pc */
- .4byte ${name}_end /* DW_AT_high_pc */
+ .${addr_len}byte ${name}_start /* DW_AT_low_pc */
+ .${addr_len}byte ${name}_end /* DW_AT_high_pc */
"
if { $cu_dir != "" } {
puts $f " .ascii $cu_dir /* DW_AT_comp_dir */"
Thanks.
On Fri, 17 Oct 2014, Kwok Cheung Yeung wrote:
> Maciej has already reviewed this patch internally, and I can confirm
> that the test still passes cleanly on x86_64 Linux.
Indeed; it may be worth noting that depending on specific circumstances
for correct results the use of `.insn' may require commit 7bb01e2 I just
pushed as we had a long-standing bug in GAS there.
Maciej
@@ -63,6 +63,12 @@ FUNC (compdir_absolute_ldir_absolute_file_relative_different) \
FUNC (compdir_absolute_ldir_absolute_file_absolute_same) \
FUNC (compdir_absolute_ldir_absolute_file_absolute_different)
+#ifdef __mips__
+#define START_INSNS asm (".insn\n");
+#else
+#define START_INSNS
+#endif
+
/* Notes: (1) The '*_start' label below is needed because 'name' may
point to a function descriptor instead of to the actual code. (2)
The '.balign' should specify the highest possible function
@@ -72,6 +78,7 @@ FUNC (compdir_absolute_ldir_absolute_file_absolute_different)
#define FUNC(name) \
asm (".balign 8"); \
asm (#name "_start: .globl " #name "_start\n"); \
+ START_INSNS \
static void \
name (void) \
{ \
@@ -19,6 +19,13 @@ if {![dwarf2_support]} {
return 0
}
+# Find length of addresses in bytes.
+if {[is_64_target]} {
+ set addr_len 8
+} else {
+ set addr_len 4
+}
+
standard_testfile
set asmsrcfile [standard_output_file ${testfile}asm.S]
set asmobjfile [standard_output_file ${testfile}asm.o]
@@ -36,6 +43,7 @@ puts $f "/* DO NOT EDIT! GENERATED AUTOMATICALLY! */"
proc out_cu { name cu_dir cu_name line_dir line_name } {
global f
+ global addr_len
puts -nonewline $f "\
.L${name}_begin:
@@ -43,7 +51,7 @@ proc out_cu { name cu_dir cu_name line_dir line_name } {
.L${name}_start:
.2byte 2 /* DWARF Version */
.4byte .Labbrev1_begin /* Offset into abbrev section */
- .byte 4 /* Pointer size */
+ .byte ${addr_len} /* Pointer size */
"
if { $cu_dir != "" } {
puts $f " .uleb128 ABBREV_COMP_DIR_NAME /* Abbrev: DW_TAG_compile_unit */"
@@ -54,8 +62,8 @@ proc out_cu { name cu_dir cu_name line_dir line_name } {
.ascii \"GNU C\\0\" /* DW_AT_producer */
.byte 2 /* DW_AT_language (DW_LANG_C) */
.4byte .Lline_${name}_begin /* DW_AT_stmt_list */
- .4byte ${name}_start /* DW_AT_low_pc */
- .4byte ${name}_end /* DW_AT_high_pc */
+ .${addr_len}byte ${name}_start /* DW_AT_low_pc */
+ .${addr_len}byte ${name}_end /* DW_AT_high_pc */
"
if { $cu_dir != "" } {
puts $f " .ascii $cu_dir /* DW_AT_comp_dir */"
@@ -65,8 +73,8 @@ proc out_cu { name cu_dir cu_name line_dir line_name } {
.uleb128 3 /* Abbrev: DW_TAG_subprogram */
.asciz \"${name}\" /* DW_AT_name */
- .4byte ${name}_start /* DW_AT_low_pc */
- .4byte ${name}_end /* DW_AT_high_pc */
+ .${addr_len}byte ${name}_start /* DW_AT_low_pc */
+ .${addr_len}byte ${name}_end /* DW_AT_high_pc */
.byte 0 /* End of children of CU */
.L${name}_end:
@@ -75,6 +83,7 @@ proc out_cu { name cu_dir cu_name line_dir line_name } {
proc out_line { name cu_dir cu_name line_dir line_name } {
global f
+ global addr_len
puts -nonewline $f "\
.Lline_${name}_begin:
@@ -120,16 +129,16 @@ proc out_line { name cu_dir cu_name line_dir line_name } {
.byte 3 /* DW_LNS_advance_line */
.sleb128 998 /* ... to 999 */
.byte 0 /* DW_LNE_set_address */
- .uleb128 5
+ .uleb128 ${addr_len}+1
.byte 2
- .4byte ${name}_start
+ .${addr_len}byte ${name}_start
.byte 1 /* DW_LNS_copy */
.byte 3 /* DW_LNS_advance_line */
.sleb128 1 /* ... to 1000 */
.byte 0 /* DW_LNE_set_address */
- .uleb128 5
+ .uleb128 ${addr_len}+1
.byte 2
- .4byte ${name}_end
+ .${addr_len}byte ${name}_end
.byte 1 /* DW_LNS_copy */
.byte 0 /* DW_LNE_end_of_sequence */
.uleb128 1