[PATCH 10/20] MIPS/GAS: Fail on 64-bit addresses used with LI

[PATCH 10/20] MIPS/GAS: Fail on 64-bit addresses used with LI

[Maciej W. Rozycki]

Hi,

 This change adds a suitable assertion to the LI macro where 64-bit 
addresses are not expected and not handled correctly.

2010-12-02  Maciej W. Rozycki  <macro@codesourcery.com>

	gas/
	* config/tc-mips.c (macro)[M_LI_DD]: Fail if a 64-bit address is 
	truncated to 32 bits.

 OK to apply?

  Maciej

binutils-gas-mips-lidd-assert.diff
Index: binutils-fsf-trunk-quilt/gas/config/tc-mips.c
===================================================================
--- binutils-fsf-trunk-quilt.orig/gas/config/tc-mips.c	2010-12-01 21:05:50.000000000 +0000
+++ binutils-fsf-trunk-quilt/gas/config/tc-mips.c	2010-12-01 21:05:51.000000000 +0000
@@ -7039,6 +7039,7 @@ macro (struct mips_cl_insn *ip)
 	  else
 	    {
 	      /* FIXME: This won't work for a 64 bit address.  */
+	      gas_assert (HAVE_32BIT_ADDRESSES);
 	      macro_build_lui (&offset_expr, AT);
 	    }
 

Re: [PATCH 10/20] MIPS/GAS: Fail on 64-bit addresses used with LI

[Richard Sandiford]

Patch 9 is OK.

"Maciej W. Rozycki" <macro@codesourcery.com> writes:
> binutils-gas-mips-lidd-assert.diff
> Index: binutils-fsf-trunk-quilt/gas/config/tc-mips.c
> ===================================================================
> --- binutils-fsf-trunk-quilt.orig/gas/config/tc-mips.c	2010-12-01 21:05:50.000000000 +0000
> +++ binutils-fsf-trunk-quilt/gas/config/tc-mips.c	2010-12-01 21:05:51.000000000 +0000
> @@ -7039,6 +7039,7 @@ macro (struct mips_cl_insn *ip)
>  	  else
>  	    {
>  	      /* FIXME: This won't work for a 64 bit address.  */
> +	      gas_assert (HAVE_32BIT_ADDRESSES);
>  	      macro_build_lui (&offset_expr, AT);
>  	    }
>  

Not OK.  This trivially triggers for:

	li.d	$f2,1.1

assembled with -mabi=64 -G0.

Richard

Re: [PATCH 10/20] MIPS/GAS: Fail on 64-bit addresses used with LI

[Maciej W. Rozycki]

On Tue, 7 Dec 2010, Richard Sandiford wrote:

> > binutils-gas-mips-lidd-assert.diff
> > Index: binutils-fsf-trunk-quilt/gas/config/tc-mips.c
> > ===================================================================
> > --- binutils-fsf-trunk-quilt.orig/gas/config/tc-mips.c	2010-12-01 21:05:50.000000000 +0000
> > +++ binutils-fsf-trunk-quilt/gas/config/tc-mips.c	2010-12-01 21:05:51.000000000 +0000
> > @@ -7039,6 +7039,7 @@ macro (struct mips_cl_insn *ip)
> >  	  else
> >  	    {
> >  	      /* FIXME: This won't work for a 64 bit address.  */
> > +	      gas_assert (HAVE_32BIT_ADDRESSES);
> >  	      macro_build_lui (&offset_expr, AT);
> >  	    }
> >  
> 
> Not OK.  This trivially triggers for:
> 
> 	li.d	$f2,1.1
> 
> assembled with -mabi=64 -G0.

 And rightfully so, broken code is produced:

$ cat li_d.s
	li.d	$f2, 1.1
$ mips-sde-elf-as -mabi=64 -G0 -o li_d.o li_d.s
$ mips-sde-elf-ld -m elf64btsmip -e0 -o li_d li_d.o
$ mips-sde-elf-objdump -dt li_d

li_d:     file format elf64-tradbigmips

SYMBOL TABLE:
0000000120000078 l    d  .MIPS.options	0000000000000000 .MIPS.options
0000000120000078 l    d  .text	0000000000000000 .text
0000000120000080 l    d  .rodata	0000000000000000 .rodata
0000000120010088 g       .rodata	0000000000000000 _fdata
0000000120018080 g       *ABS*	0000000000000000 _gp
0000000120000078 g       .text	0000000000000000 _ftext
0000000120010088 g       *ABS*	0000000000000000 __bss_start
0000000120010088 g       *ABS*	0000000000000000 _edata
0000000120010088 g       *ABS*	0000000000000000 _end
0000000120010088 g       *ABS*	0000000000000000 _fbss



Disassembly of section .text:

0000000120000078 <_ftext>:
   120000078:	3c012000 	lui	at,0x2000
   12000007c:	d4220080 	ldc1	$f2,128(at)
$

I don't think the code above gets at .rodata, do you?

 With some tweaking we should be able to reuse code from M_L_DAB to handle 
this case (i.e. jump to there with some prearrangements), but I may not 
have the time to implement this stuff properly now.  Why do you think 
silently making broken code is better than signalling an internal error 
(which it is) here?

  Maciej

Re: [PATCH 10/20] MIPS/GAS: Fail on 64-bit addresses used with LI

[Richard Sandiford]

"Maciej W. Rozycki" <macro@codesourcery.com> writes:
> On Tue, 7 Dec 2010, Richard Sandiford wrote:
>> Not OK.  This trivially triggers for:
>> 
>> 	li.d	$f2,1.1
>> 
>> assembled with -mabi=64 -G0.
>
>  And rightfully so, broken code is produced:
>
> $ cat li_d.s
> 	li.d	$f2, 1.1
> $ mips-sde-elf-as -mabi=64 -G0 -o li_d.o li_d.s
> $ mips-sde-elf-ld -m elf64btsmip -e0 -o li_d li_d.o
> $ mips-sde-elf-objdump -dt li_d
>
> li_d:     file format elf64-tradbigmips
>
> SYMBOL TABLE:
> 0000000120000078 l    d  .MIPS.options	0000000000000000 .MIPS.options
> 0000000120000078 l    d  .text	0000000000000000 .text
> 0000000120000080 l    d  .rodata	0000000000000000 .rodata
> 0000000120010088 g       .rodata	0000000000000000 _fdata
> 0000000120018080 g       *ABS*	0000000000000000 _gp
> 0000000120000078 g       .text	0000000000000000 _ftext
> 0000000120010088 g       *ABS*	0000000000000000 __bss_start
> 0000000120010088 g       *ABS*	0000000000000000 _edata
> 0000000120010088 g       *ABS*	0000000000000000 _end
> 0000000120010088 g       *ABS*	0000000000000000 _fbss
>
>
>
> Disassembly of section .text:
>
> 0000000120000078 <_ftext>:
>    120000078:	3c012000 	lui	at,0x2000
>    12000007c:	d4220080 	ldc1	$f2,128(at)
> $
>
> I don't think the code above gets at .rodata, do you?

You're just repeating what the FIXME already says: that the code doesn't
work correctly with addresses whose values exceed the 32-bit range.
I wasn't disputing that.

>  With some tweaking we should be able to reuse code from M_L_DAB to handle 
> this case (i.e. jump to there with some prearrangements), but I may not 
> have the time to implement this stuff properly now.  Why do you think 
> silently making broken code is better than signalling an internal error 
> (which it is) here?

An assertion failure is also a bug.  Assertion failures are for
conditions that we think should never occur, so we shouldn't add
an assertion failure for a condition that is known to trigger.
We should either fix the FIXME or issue a proper diagnostic.

Richard