On 11/21/19 3:59 AM, Richard Sandiford wrote:
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.
Hmmm. It sounds like I need to go back to the drawing board on this
one; I don't think the test case attached to the issue accurately
captured the actual problem affecting GLIBC. The current code is
consistent about using the same assumptions for both data placement and
addressing, but maybe the type of the object isn't being declared or
represented consistently in all compilation units? Or there are such
large flexibly-sized objects being allocated in small data that they're
causing a range overflow for %gprel? :-S
Anyway, I'll withdraw this patch for the time being, at least until I'm
convinced that it's actually solving the right problem.
-Sandra