[RFC] When adding an offset to a lo_sum containing a symbol, check it is in range of the symbol's alignment

[Andrew Bennett <Andrew dot Bennett at imgtec dot com>]

Hi,

In MIPS (and similarly for other RISC architectures) to load an absolute address of an object
requires a two instruction sequence: one instruction to load the high part of the object's address, 
and one instruction to load the low part of the object's address.  Typically the result from the 
calculation of the high part of the address will only be used by one instruction to load 
the low part of the address.  However, in certain situations (for example when loading or
storing double word values) the result of computing the high part of the address can be 
used by multiple instructions to load the low parts of an address at different offsets.  Lets 
show this with an example C program.

struct
{
  short s;
  unsigned long long l;
} h;

void foo (void)
{
  h.l = 0;
}

When this is compiled for MIPS it produces the following assembly:

        lui     $2,%hi(h+8)
        sw      $0,%lo(h+8)($2)
        jr      $31
        sw      $0,%lo(h+12)($2)

	  ...

        .globl  h
        .section        .bss,"aw",@nobits
        .align  3
        .type   h, @object
        .size   h, 16
h:
        .space  16


Notice here that the high part of the address of object h is loaded into register $2,
and this is then used as part of the low part calculation by two the sw instructions which each
have different offsets.  In MIPS the value of a low part calculation is treated as a signed value.
It is therefore valid to use the result of a high part calculation with multiple low part calculations
containing different offsets so long as when adding the result of the high part to the each of the
sign extended low parts we get valid addresses.

However, if we add the packed attribute to the h structure, the fields of the structure will 
not be naturally aligned and we can break the previous condition.  Lets explain this in more 
detail with the following C program.

struct __attribute__((packed))
{
 short s;
 unsigned long long l;
} h;

void foo (void)
{
 h.l = 0;
}

When this is compiled for MIPS it produces the following assembly:

	  lui     $2,%hi(h)
        addiu   $4,$2,%lo(h+2)
        addiu   $3,$2,%lo(h+6)
        swl     $0,3($4)
        swr     $0,%lo(h+2)($2)
        swl     $0,3($3)
        jr      $31
        swr     $0,%lo(h+6)($2)

        ...

	  .globl  h
        .section        .bss,"aw",@nobits
        .align  2
        .type   h, @object
        .size   h, 10
h:
        .space  10


There are two things to highlight here.  Firstly the alignment of the h structure has decreased
from 8 bytes to 4 bytes.  Secondly we have a low part calculation which adds an offset of 6
to the address of the h structure which is greater than its alignment.

When the MIPS linker resolves a HI relocation (i.e. %hi(h)) it finds the next LO 
relocation (i.e. %lo(h+2)) in the relocation table and using the information from both 
of these relocations it computes the object's address and extracts its high part.  Then, when the 
MIPS linker resolves a LO relocation it adds the offset to the object's address and then extracts 
the low part.

Lets assume that object h has an address of 0x80007ffc.  When the MIPS linker resolves the value 
of the HI relocation for object h, it will also use the value of the LO relocation for object h 
with an offset of 2.  The high part value is therefore:

HIGH (0x80007ffc + 2) = HIGH (0x80007ffe) = 0x8000


Then the MIPS linker resolves the value of LO relocation for object h with an offset of 2:

LO (0x80007ffc + 2) = LO (0x80007ffe) = 0x7ffe


Finally the MIPS linker resolves the value of the LO relocation for object h with an offset of 6:

LO (0x80007ffc + 6) = LO (0x80008002) = 0x8002

In MIPS the value of a LO relocation is treated as a signed value, so when the program is run the address 
of h+6 will be 0x7fff8002 when it should be 0x80008002.


The place in the GCC middle end where the invalid RTX is generated is in the adjust_address_1 function 
in emit-rtl.c, when it adjusts the offset of an address which is a MEM rtx containing a LO_SUM, 
by producing a new LO_SUM containing the offset.  In the case where the original LO_SUM contains a SYMBOL_REF 
this is only valid if the alignment of the object is less than the offset.

The potential fix (shown below) is to perform this check.  Before I produce a formal patch and testcase 
for this issue I wanted to discuss it with the GCC community to ensure this is the best place to fix such 
a problem.

So, if required please feel free to suggest improvements or other more appropriate places to apply this fix.


Many thanks,



Andrew



diff --git a/gcc/emit-rtl.c b/gcc/emit-rtl.c
index 3041b9e..b20de6f 100644
--- a/gcc/emit-rtl.c
+++ b/gcc/emit-rtl.c
@@ -2036,12 +2036,30 @@ adjust_address_1 (rtx memref, enum machine_mode mode, HOST_WIDE_INT offset,
 
   if (adjust_address)
     {
+      unsigned HOST_WIDE_INT symbol_alignment;
+
+      symbol_alignment = GET_MODE_ALIGNMENT (GET_MODE (memref)) / BITS_PER_UNIT;
+
+      if (GET_CODE (addr) == LO_SUM)
+       {
+         rtx sym = XEXP (addr, 1);
+
+         if (GET_CODE (sym) == CONST
+             && GET_CODE (XEXP (sym, 0)) == PLUS)
+           sym = XEXP (XEXP (sym, 0), 0);
+
+         if (GET_CODE (sym) == SYMBOL_REF
+             && SYMBOL_REF_DECL (sym))
+           symbol_alignment = DECL_ALIGN_UNIT (SYMBOL_REF_DECL (sym));
+       }
+
       /* If MEMREF is a LO_SUM and the offset is within the alignment of the
         object, we can merge it into the LO_SUM.  */
       if (GET_MODE (memref) != BLKmode && GET_CODE (addr) == LO_SUM
          && offset >= 0
-         && (unsigned HOST_WIDE_INT) offset
+         && ((unsigned HOST_WIDE_INT) offset
              < GET_MODE_ALIGNMENT (GET_MODE (memref)) / BITS_PER_UNIT)
+         && ((unsigned HOST_WIDE_INT) offset < symbol_alignment))
        addr = gen_rtx_LO_SUM (address_mode, XEXP (addr, 0),
                               plus_constant (address_mode,
                                              XEXP (addr, 1), offset));