Sandra Loosemore <san...@codesourcery.com> writes:
> This patch is for PR target/92499, a bug reported by GLIBC maintainers 
> against nios2 target.  The initial failure symptom was a linker error 
> resulting from use of GP-relative addressing on an object that wasn't 
> allocated in the small data section.  It turns out that the real problem 
> is that the TARGET_IN_SMALL_DATA_P hook uses the function 
> int_size_in_bytes to check whether an object is "small", and this 
> function treats flexible array members as having size 0.  All backends 
> except for tic6x that implement TARGET_IN_SMALL_DATA_P use a similar 
> implementation of the hook that has the same problem.
>
> The solution here is to add a new function that returns -1 as the size 
> of any type with a flexible array member, and adjust all the affected 
> backends to use it.  I swiped the predicate for identifying these types 
> from the C front end.
>
> I regression-tested this on nios2-elf and nios2-linux-gnu.  I'm not set 
> up to test any of the other affected back ends, but the code change is 
> trivial and identical to what I did for nios2.  OK to commit?  I'd like 
> to backport this patch to all active branches as well, once it is in on 
> trunk.

For local decls like the ones in the testcase, we should be able to
query the actual size of the decl using DECL_SIZE(_UNIT).  But for
externs we have to make conservatively correct assumptions based only
on the type, so I agree this looks like the right way of fixing it.

The problem is that it's going to be an ABI break for existing code that
does something like:

    struct flexible
    {
      int length;
      int data[];
    };

    extern struct flexible foo;
    int get_foo (void) { return foo.data[0]; }

since the code might have been compiled on the assumption that foo
fits within the small-data limit.

So I think we need to restrict the change to targets that have
explicitly bought into the ABI change, i.e. just nios2 for now.
It might also be worth mentioning in the release notes.

The target-independent bits are OK for trunk unless someone objects
in the next 24 hrs.  I'm not sure it's appropriate to backport given
the ABI change though.

Thanks,
Richard

Reply via email to