Re: [PATCH] gdb.dwarf2: Testsuite 64-bit pointer truncation fixes

[Doug Evans <dje at google dot com>]

On Sun, Oct 12, 2014 at 3:26 PM, Maciej W. Rozycki
<macro@codesourcery.com> wrote:
> Hi,
>
>  Here are fixes for some issues I discovered in DWARF-2 tests with 64-bit
> MIPS targets:
>
> (gdb) PASS: gdb.dwarf2/dw2-case-insensitive.exp: set case-sensitive off
> info functions fUnC_lang
> All functions matching regular expression "fUnC_lang":
>
> File file1.txt:
> foo FUNC_lang(void);
>
> Non-debugging symbols:
> 0x0000000120000b20  FUNC_lang_start
> 0x0000000120000b40  FUNC_lang_end
> (gdb) FAIL: gdb.dwarf2/dw2-case-insensitive.exp: regexp case-sensitive off
> p fuNC_lang
> Cannot access memory at address 0x20000b20
> (gdb) FAIL: gdb.dwarf2/dw2-case-insensitive.exp: p fuNC_lang
> p fuNC_symtab
> $1 = {<text variable, no debug info>} 0x120000b40 <FUNC_symtab>
> (gdb) PASS: gdb.dwarf2/dw2-case-insensitive.exp: p fuNC_symtab
> break fuNC_lang
> Cannot access memory at address 0x20000b20
> (gdb) FAIL: gdb.dwarf2/dw2-case-insensitive.exp: setting breakpoint at fuNC_lang
>
> and:
>
> (gdb) continue
> Continuing.
>
> Breakpoint 1, 0x0000000120000c0c in main ()
> (gdb) break func
> Breakpoint 2 at 0x20000ad0: func. (3 locations)
> (gdb) continue
> Continuing.
> Warning:
> Cannot insert breakpoint 2.
> Cannot access memory at address 0x20000ad0
> Cannot insert breakpoint 2.
> Cannot access memory at address 0x20000b60
>
> (gdb) FAIL: gdb.dwarf2/dw2-skip-prologue.exp: continue to breakpoint: func
> info break $bpnum
> Num     Type           Disp Enb Address            What
> 2       breakpoint     keep y   <MULTIPLE>
> 2.1                         y     0x0000000020000ad0 in func at main.c:5
> 2.2                         y     0x0000000020000b60
> 2.3                         y     0x0000000120000adc <func+12>
> (gdb) FAIL: gdb.dwarf2/dw2-skip-prologue.exp: 2 locations found
>
> Address truncation is obvious from these logs.  The fixes follow previous
> art across our test suite and should therefore be rather obvious.
>
>  Regression tested with the mips-linux-gnu target and the following
> multilibs:
>
> -EB
> -EB -msoft-float
> -EB -mips16
> -EB -mips16 -msoft-float
> -EB -mmicromips
> -EB -mmicromips -msoft-float
> -EB -mabi=n32
> -EB -mabi=n32 -msoft-float
> -EB -mabi=64
> -EB -mabi=64 -msoft-float
>
> and the -EL variants of same, fixing the 5 failures noted above for each
> of the `-mabi=64' multilibs and with no changes otherwise.
>
>  OK to apply?
>
> 2014-10-12  Maciej W. Rozycki  <macro@codesourcery.com>
>
>         gdb/testsuite/
>         * gdb.dwarf2/dw2-case-insensitive-debug.S: Handle 64-bit pointers.
>         * gdb.dwarf2/dw2-case-insensitive.exp: Update accordingly.
>         * gdb.dwarf2/dw2-skip-prologue.S: Handle 64-bit pointers.
>         * gdb.dwarf2/dw2-skip-prologue.exp: Update accordingly.

Hi.

I'm of two minds on this.
On the one hand, what's the cost/benefit ratio of going down the road
of portable assembler testcases?
On the other hand, once someone has put in the effort for one
particular testcase, as long as it doesn't place a long term burden on
maintaining that testcase it's not that big a deal.  [If we're going
to impose something on new tests then I would want it written down in
the testsuite coding standards wiki, but that can be a separate
discussion.  We should write something down anyway.]

Although I think there's a limit to how far down the road we should
go, I'm ok with this patch.

>
>   Maciej
>
> gdb-dwarf2-test-64bit.diff
> Index: gdb-fsf-trunk-quilt/gdb/testsuite/gdb.dwarf2/dw2-case-insensitive-debug.S
> ===================================================================
> --- gdb-fsf-trunk-quilt.orig/gdb/testsuite/gdb.dwarf2/dw2-case-insensitive-debug.S      2014-10-03 04:33:45.000000000 +0100
> +++ gdb-fsf-trunk-quilt/gdb/testsuite/gdb.dwarf2/dw2-case-insensitive-debug.S   2014-10-03 04:51:44.547540632 +0100
> @@ -15,6 +15,14 @@
>     You should have received a copy of the GNU General Public License
>     along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
>
> +#if PTRBITS == 64
> +# define PTRBYTE .8byte
> +#elif PTRBITS == 32
> +# define PTRBYTE .4byte
> +#else
> +# error Unsupported pointer size
> +#endif
> +

Nit: The argument to #error is a series of preprocessor tokens, so

#error it's not ok

is not ok.  We don't have a convention (that I can remember) of always
making the error text a string, but I do like this convention.

Can you change the #error's to use a string?
[And I'll look into making and documenting this convention.
After some grepping, it is more prevalent than not quoting.]

So, LGTM with that change.