Re: [review v3] slotinfo in struct dtv_slotinfo_list should be flexible array [BZ #25...

[Carlos O'Donell <codonell at redhat dot com>]

On 11/12/19 1:42 PM, Florian Weimer wrote:
> * Sandra Loosemore:
> 
>> I don't have any state on this particular change or what it is trying to 
>> accomplish, but the linker error is the sort of thing that happens when 
>> the compiler sees a reference to an object it thinks will be put in the 
>> small data section, but the actual definition of the object puts it 
>> somewhere else (e.g., because it is too big for small data) -- in this 
>> case the .bss section.  Are there conflicting declarations about the 
>> size of the object?  If that's unavoidable for some reason, another way 
>> to suppress GP-relative addressing on this object would be to give it an 
>> explicit .bss section attribute everywhere it's declared.
> 
> Thanks for providing this information.  Here is a small reproducer:
> 
> enum { size = 100 };
> 
> struct flexible
> {
>   int length;
>   int data[];
> };
> 
> struct inflexible
> {
>   int length;
>   int data[size];
> };
> 
> static struct flexible flexible =
>   {
>    .data = { [size - 1] = 0, }
>   };
> 
> static struct inflexible inflexible =
>   {
>    .data = { [size - 1] = 0, }
>   };
> 
> struct flexible *
> get_flexible (void)
> {
>   return &flexible;
> }
> 
> struct inflexible *
> get_inflexible (void)
> {
>   return &inflexible;
> }
> 
> It results in:
> 
> 	.file	"t.c"
> 	.section	.text
> 	.align	2
> 	.global	get_flexible
> 	.type	get_flexible, @function
> get_flexible:
> 	addi	r2, gp, %gprel(flexible)
> 	ret
> 	.size	get_flexible, .-get_flexible
> 	.align	2
> 	.global	get_inflexible
> 	.type	get_inflexible, @function
> get_inflexible:
> 	movhi	r2, %hiadj(inflexible)
> 	addi	r2, r2, %lo(inflexible)
> 	ret
> 	.size	get_inflexible, .-get_inflexible
> 	.section	.bss
> 	.type	inflexible, @object
> 	.size	inflexible, 404
> 	.align	2
> inflexible:
> 	.zero	404
> 	.type	flexible, @object
> 	.size	flexible, 404
> 	.align	2
> flexible:
> 	.zero	404
> 	.ident	"GCC: (GNU) 9.2.1 20191101 [gcc-9-branch revision 277712]"
> 
> I think this shows that the backend uses the static type size (as in
> sizeof) to determine whether an object goes into the small data
> section, not the allocated object size.
> 
> The linker only sees the allocated object size, so it places the
> object wrongly (although it could perhaps do something smarter because
> it can see the relocations).
> 
> If the object is not placed into .bss, the choice of .sdata vs .data
> is also based on the static type size, so that would need fixing too.
> 
> I don't know how to work around that in the source code.  My
> preference would be to fix the backend.  I guess we could back out the
> commit and disable the warning for GCC 10 instead.
 
If the new glibc cannot be compiled with gcc8 or gcc9 for nios2, then that's
a problem though. It immediately makes it difficult to test a new glibc on
existing systems without upgrading the system compiler.

If there is a backend bug then it absolutely needs to be fixed, and it looks
like Sandra will try that, but that won't reach the affected compilers until
much much later.

I dislike backing out the changes you made because they are quite nice
cleanups, and the downstream vendors tend to upgrade glibc and gcc and binutils
all in one big group to avoid these issues so I don't expect they will ever
see this problem. It's only us as developers that see the larger version
skew problem.

My inclination is to fix the gcc problem, backport the solution to the active
FSF branches, and then that fixes our testers.

In order of importance:
- Fix all gcc10 issues.
- Fix the nios2 backend issue.
- Backport nios2 backend to active FSF branches.
- Rebuild and test to make sure we are all clean again.

How long will that take and is it OK to leave things in a broken state for that long
for a given target like nios2?

-- 
Cheers,
Carlos.