Re: [PATCH] msp430: Set DWARF2_ADDR_SIZE to 4

[Kevin Buettner <kevinb at redhat dot com>]

On Mon, 1 Feb 2016 10:24:18 +0000
Nick Clifton <nickc@redhat.com> wrote:

> > msp430: Set DWARF2_ADDR_SIZE to 4.
> >
> > This change makes gas's notion of the msp430 dwarf2 address size match
> > that of gcc and gdb.
> 
> Hmm - will this change also affect the information in the .eh_frame
> section ?  If so then maybe this function: 
> bfd/elf32-msp430.c:elf32_msp430_eh_frame_address_size will need to
> be updated as well.

I tested GDB with the multilibs mentioned below, but I saw no change
in results when I made elf32_msp430_eh_frame_address_size() return 4
unconditionally.

So... all I can say about this is that I don't know the answer.

> > Without this change, addresses in .debug_line are encoded in two bytes
> > for some multilibs.  When GDB reads the address for
> > DW_LNE_set_address, it uses the pointer size provided by the CU.  When
> > these values don't match, GDB reads the wrong number of bytes.
> 
> Would it be possible to add a GAS test for this ?  Or maybe a BINUTILS
> test instead ?

Maybe, but the problem is readily apparent when running the GDB tests.
(See below.)

> Did you regression test your patch ?  With both the default and the
> -mlarge multilibs ?  In fact since we are talking about debug
> information generation, please could you tell us what affect this
> patch has on the MSp430 GDB testsuite results ?

I tested binutils, gas, and gdb using the following multilibs:

msp430-sim/-msim/-mcpu=msp430
msp430-sim/-msim/-mcpu=msp430x
msp430-sim/-msim/-mcpu=msp430x/-mlarge/-mdata-region=either/-mcode-region=either

I see no difference in the test results for either binutils or gas.

For gdb, there's no difference for the 2nd or 3rd multilibs.  This is
to be expected though since the address size should be 4 for these
multilibs.  (The results for the -mlarge multilib aren't especially
good, but nonetheless, they're identical before/after the patches that
I tested.)

The -mcpu=msp430 multilib shows a dramatic difference:

Without patch:

# of expected passes		10890
# of unexpected failures	5364
# of unexpected successes	1
# of expected failures		31
# of known failures		74
# of unresolved testcases	109
# of untested testcases		196
# of unsupported tests		260

With patch:

# of expected passes		16599
# of unexpected failures	265
# of expected failures		29
# of unknown successes		1
# of known failures		54
# of unresolved testcases	8
# of untested testcases		197
# of unsupported tests		262

Basically, when run without the patch under consideration, GDB is
unable to read the .debug_line section(s).  It records no line
numbers, which in turn causes GDB to display what it can sans line
numbers.  There are (obviously) more than a few GDB tests which expect
a line number to be present in GDB's output.

I should note that in addition to the tests which I recently performed
using a toolchain built with recent upstream sources, we have been
using this patch in our Red Hat internal sources for the msp430-elf
toolchain since July, 2015.

Kevin