diff mbox series

env: another attempt at fixing SPL build failures

Message ID f56db3a4-29a8-9b38-4772-670142b146a9@prevas.dk
State New
Headers show
Series env: another attempt at fixing SPL build failures | expand

Commit Message

Rasmus Villemoes Jan. 11, 2020, 10:44 p.m. UTC
On 10/01/2020 15.34, Tom Rini wrote:
> On Fri, Jan 10, 2020 at 02:28:54PM +0000, Rasmus Villemoes wrote:
>> On 15/12/2019 23.29, Rasmus Villemoes wrote:
>>> I'm also seeing the build failure that commit
>>>
>>> 7d4776545b env: solve compilation error in SPL
>>>
>>
>> Yeah, I think this is a difference in how the linker works on ppc vs
>> arm. Doing
>>
>>
>> For reference, I have
>>
>> $ ${CROSS_COMPILE}ld --version
>> GNU ld (GNU Binutils for Ubuntu) 2.30
> 
> Which SPL are you using on PowerPC?  There's the one based
> CONFIG_SPL_FRAMEWORK and the one that's not.  I suspect it's a framework
> vs not problem here rather than linker exactly.
> 

No, it's nothing to do with SPL per se. Try something completely silly
such as

gets discarded just fine.

On powerpc, this fails with "undefined reference to `func9999'".

Now, changing to #if 1 instead, arm still completely eliminates both
data items as well as func9999. On powerpc, the link succeeds, and
bar9999 has been discarded, but both foo9999 and func9999 are present in
the final image.

So it seems that the ppc linker somehow fails to eliminate unreferenced
.data sections with relocation entries (it fails the same way if one
changes the pointer to an "int *").

Rasmus

Comments

Rasmus Villemoes Jan. 12, 2020, 8:23 p.m. UTC | #1
On 11/01/2020 23.44, Rasmus Villemoes wrote:
> On 10/01/2020 15.34, Tom Rini wrote:
>> On Fri, Jan 10, 2020 at 02:28:54PM +0000, Rasmus Villemoes wrote:
>>> On 15/12/2019 23.29, Rasmus Villemoes wrote:
>>>> I'm also seeing the build failure that commit
>>>>
>>>> 7d4776545b env: solve compilation error in SPL
>>>>
>>>
>>> Yeah, I think this is a difference in how the linker works on ppc vs
>>> arm. Doing
>>>
>>>
>>> For reference, I have
>>>
>>> $ ${CROSS_COMPILE}ld --version
>>> GNU ld (GNU Binutils for Ubuntu) 2.30
>>
>> Which SPL are you using on PowerPC?  There's the one based
>> CONFIG_SPL_FRAMEWORK and the one that's not.  I suspect it's a framework
>> vs not problem here rather than linker exactly.
>>
> 
> No, it's nothing to do with SPL per se. 

OK, I think I found it. On powerpc, the .fixup section contains a
reference to .data.rel.env_htab, and all of .fixup is kept (because of
KEEP(*(.fixup))), so env_htab cannot get garbage collected. Hence
anything that section refers to must also exist. So it's not a defect in
ppc-ld vs arm-ld, it's just a consequence of ppc having that .fixup section.

So, the only way to fix that is by either making sure env_flags_validate
exists in the link, or avoiding having env_htab being part of the link
in the first place. My patch does the latter.

Rasmus
Tom Rini Jan. 22, 2020, 7:03 p.m. UTC | #2
On Sun, Jan 12, 2020 at 08:23:02PM +0000, Rasmus Villemoes wrote:
> On 11/01/2020 23.44, Rasmus Villemoes wrote:
> > On 10/01/2020 15.34, Tom Rini wrote:
> >> On Fri, Jan 10, 2020 at 02:28:54PM +0000, Rasmus Villemoes wrote:
> >>> On 15/12/2019 23.29, Rasmus Villemoes wrote:
> >>>> I'm also seeing the build failure that commit
> >>>>
> >>>> 7d4776545b env: solve compilation error in SPL
> >>>>
> >>>
> >>> Yeah, I think this is a difference in how the linker works on ppc vs
> >>> arm. Doing
> >>>
> >>>
> >>> For reference, I have
> >>>
> >>> $ ${CROSS_COMPILE}ld --version
> >>> GNU ld (GNU Binutils for Ubuntu) 2.30
> >>
> >> Which SPL are you using on PowerPC?  There's the one based
> >> CONFIG_SPL_FRAMEWORK and the one that's not.  I suspect it's a framework
> >> vs not problem here rather than linker exactly.
> >>
> > 
> > No, it's nothing to do with SPL per se. 
> 
> OK, I think I found it. On powerpc, the .fixup section contains a
> reference to .data.rel.env_htab, and all of .fixup is kept (because of
> KEEP(*(.fixup))), so env_htab cannot get garbage collected. Hence
> anything that section refers to must also exist. So it's not a defect in
> ppc-ld vs arm-ld, it's just a consequence of ppc having that .fixup section.
> 
> So, the only way to fix that is by either making sure env_flags_validate
> exists in the link, or avoiding having env_htab being part of the link
> in the first place. My patch does the latter.

Thanks for digging in to the root cause on this.  I'll take a harder
look at your patch, thanks!
diff mbox series

Patch

diff --git a/common/console.c b/common/console.c
index 168ba60d0d..bbff516154 100644
--- a/common/console.c
+++ b/common/console.c
@@ -22,6 +22,21 @@ 

 DECLARE_GLOBAL_DATA_PTR;

+struct foo {
+       int (*f)(int, int);
+       long a[100];
+};
+struct bar {
+       int a; int b; int c;
+};
+extern int func9999(int a, int b);
+struct foo foo9999 = { .f = func9999, };
+struct bar bar9999 = { .a = 0xa, .b = 0xb, .c = 0xc };
+
+#if 0
+int func9999(int a, int b) { return a + b; }
+#endif
+

For an arm target, this builds just fine, and u-boot.map (and
spl/u-boot-spl.map if an SPL is built) shows that foo9999 and bar9999