diff mbox series

sandbox: Fix LTO to work with STACKPROTECTOR

Message ID 20240624092900.2108175-1-andrew.goodbody@linaro.org
State Superseded
Headers show
Series sandbox: Fix LTO to work with STACKPROTECTOR | expand

Commit Message

Andrew Goodbody June 24, 2024, 9:29 a.m. UTC
Add the STACKPROTECTOR symbols to the script that generates the
symbols that should not be removed by the use of LTO when linking
a shared object. This prevents a fail to build due to link errors.

https://source.denx.de/u-boot/u-boot/-/issues/35

Signed-off-by: Andrew Goodbody <andrew.goodbody@linaro.org>
---

 scripts/gen_ll_addressable_symbols.sh | 2 ++
 1 file changed, 2 insertions(+)

Comments

Simon Glass July 1, 2024, 1:57 p.m. UTC | #1
Hi Andrew,

On Mon, 24 Jun 2024 at 10:29, Andrew Goodbody
<andrew.goodbody@linaro.org> wrote:
>
> Add the STACKPROTECTOR symbols to the script that generates the
> symbols that should not be removed by the use of LTO when linking
> a shared object. This prevents a fail to build due to link errors.
>
> https://source.denx.de/u-boot/u-boot/-/issues/35
>
> Signed-off-by: Andrew Goodbody <andrew.goodbody@linaro.org>
> ---
>
>  scripts/gen_ll_addressable_symbols.sh | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/scripts/gen_ll_addressable_symbols.sh b/scripts/gen_ll_addressable_symbols.sh
> index d0864804aa..ebf89b04bf 100755
> --- a/scripts/gen_ll_addressable_symbols.sh
> +++ b/scripts/gen_ll_addressable_symbols.sh
> @@ -13,3 +13,5 @@ set -e
>  echo '#include <common.h>'
>  $@ 2>/dev/null | grep -oe '_u_boot_list_2_[a-zA-Z0-9_]*_2_[a-zA-Z0-9_]*' | \
>         sort -u | sed -e 's/^\(.*\)/extern char \1[];\n__ADDRESSABLE(\1);/'
> +$@ 2>/dev/null | grep -oe '__stack_chk_.*' | \
> +       sort -u | sed -e 's/^\(.*\)/extern char \1[];\n__ADDRESSABLE(\1);/'
> --

It is OK to add these new ones, but here you seem to be adding lots of
other things also. So far we are only allowing linker lists. So can
you update this to allow just linker lists and stack_chk? Perhaps
egrep (_u_boot_list_2_|__stack_chk_) or similar?

Regards,
Simon
Andrew Goodbody July 1, 2024, 3:01 p.m. UTC | #2
On 01/07/2024 14:57, Simon Glass wrote:
> Hi Andrew,
> 
> On Mon, 24 Jun 2024 at 10:29, Andrew Goodbody
> <andrew.goodbody@linaro.org> wrote:
>>
>> Add the STACKPROTECTOR symbols to the script that generates the
>> symbols that should not be removed by the use of LTO when linking
>> a shared object. This prevents a fail to build due to link errors.
>>
>> https://source.denx.de/u-boot/u-boot/-/issues/35
>>
>> Signed-off-by: Andrew Goodbody <andrew.goodbody@linaro.org>
>> ---
>>
>>   scripts/gen_ll_addressable_symbols.sh | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/scripts/gen_ll_addressable_symbols.sh b/scripts/gen_ll_addressable_symbols.sh
>> index d0864804aa..ebf89b04bf 100755
>> --- a/scripts/gen_ll_addressable_symbols.sh
>> +++ b/scripts/gen_ll_addressable_symbols.sh
>> @@ -13,3 +13,5 @@ set -e
>>   echo '#include <common.h>'
>>   $@ 2>/dev/null | grep -oe '_u_boot_list_2_[a-zA-Z0-9_]*_2_[a-zA-Z0-9_]*' | \
>>          sort -u | sed -e 's/^\(.*\)/extern char \1[];\n__ADDRESSABLE(\1);/'
>> +$@ 2>/dev/null | grep -oe '__stack_chk_.*' | \
>> +       sort -u | sed -e 's/^\(.*\)/extern char \1[];\n__ADDRESSABLE(\1);/'
>> --
> 
> It is OK to add these new ones, but here you seem to be adding lots of
> other things also. So far we are only allowing linker lists. So can
> you update this to allow just linker lists and stack_chk? Perhaps
> egrep (_u_boot_list_2_|__stack_chk_) or similar?
> 
> Regards,
> Simon

I am sorry but I do not follow you here. I am not sure what you mean by 
'linker lists'. The script is matching on symbols from object files as 
output by nm. I have not changed that original match expression.

My addition only adds the following 3 symbols to the output of 
gen_ll_addressable_symbols.sh

__stack_chk_guard
__stack_chk_fail
__stack_chk_fail_local

These all come from the same file, common/stackprot.c

I can reduce the match to just search for '__stack_chk_guard' and that 
still works and I can put that as another expression in the grep instead 
of doing the whole nm | grep | sort | sed thing a second time. Would 
that address your concerns?

Thanks,
Andrew
Simon Glass July 2, 2024, 3:50 p.m. UTC | #3
Hi Andrew,

On Mon, 1 Jul 2024 at 16:01, Andrew Goodbody <andrew.goodbody@linaro.org> wrote:
>
> On 01/07/2024 14:57, Simon Glass wrote:
> > Hi Andrew,
> >
> > On Mon, 24 Jun 2024 at 10:29, Andrew Goodbody
> > <andrew.goodbody@linaro.org> wrote:
> >>
> >> Add the STACKPROTECTOR symbols to the script that generates the
> >> symbols that should not be removed by the use of LTO when linking
> >> a shared object. This prevents a fail to build due to link errors.
> >>
> >> https://source.denx.de/u-boot/u-boot/-/issues/35
> >>
> >> Signed-off-by: Andrew Goodbody <andrew.goodbody@linaro.org>
> >> ---
> >>
> >>   scripts/gen_ll_addressable_symbols.sh | 2 ++
> >>   1 file changed, 2 insertions(+)
> >>
> >> diff --git a/scripts/gen_ll_addressable_symbols.sh b/scripts/gen_ll_addressable_symbols.sh
> >> index d0864804aa..ebf89b04bf 100755
> >> --- a/scripts/gen_ll_addressable_symbols.sh
> >> +++ b/scripts/gen_ll_addressable_symbols.sh
> >> @@ -13,3 +13,5 @@ set -e
> >>   echo '#include <common.h>'
> >>   $@ 2>/dev/null | grep -oe '_u_boot_list_2_[a-zA-Z0-9_]*_2_[a-zA-Z0-9_]*' | \
> >>          sort -u | sed -e 's/^\(.*\)/extern char \1[];\n__ADDRESSABLE(\1);/'
> >> +$@ 2>/dev/null | grep -oe '__stack_chk_.*' | \
> >> +       sort -u | sed -e 's/^\(.*\)/extern char \1[];\n__ADDRESSABLE(\1);/'
> >> --
> >
> > It is OK to add these new ones, but here you seem to be adding lots of
> > other things also. So far we are only allowing linker lists. So can
> > you update this to allow just linker lists and stack_chk? Perhaps
> > egrep (_u_boot_list_2_|__stack_chk_) or similar?
> >
> > Regards,
> > Simon
>
> I am sorry but I do not follow you here. I am not sure what you mean by
> 'linker lists'. The script is matching on symbols from object files as
> output by nm. I have not changed that original match expression.
>
> My addition only adds the following 3 symbols to the output of
> gen_ll_addressable_symbols.sh
>
> __stack_chk_guard
> __stack_chk_fail
> __stack_chk_fail_local
>
> These all come from the same file, common/stackprot.c
>
> I can reduce the match to just search for '__stack_chk_guard' and that
> still works and I can put that as another expression in the grep instead
> of doing the whole nm | grep | sort | sed thing a second time. Would
> that address your concerns?

Ah yes that would help, thank you, just running grep once.

Regards,
Simon
diff mbox series

Patch

diff --git a/scripts/gen_ll_addressable_symbols.sh b/scripts/gen_ll_addressable_symbols.sh
index d0864804aa..ebf89b04bf 100755
--- a/scripts/gen_ll_addressable_symbols.sh
+++ b/scripts/gen_ll_addressable_symbols.sh
@@ -13,3 +13,5 @@  set -e
 echo '#include <common.h>'
 $@ 2>/dev/null | grep -oe '_u_boot_list_2_[a-zA-Z0-9_]*_2_[a-zA-Z0-9_]*' | \
 	sort -u | sed -e 's/^\(.*\)/extern char \1[];\n__ADDRESSABLE(\1);/'
+$@ 2>/dev/null | grep -oe '__stack_chk_.*' | \
+	sort -u | sed -e 's/^\(.*\)/extern char \1[];\n__ADDRESSABLE(\1);/'