| Message ID | 20240624092900.2108175-1-andrew.goodbody@linaro.org |
|---|---|
| State | Changes Requested |
| Delegated to: | Simon Glass |
| Headers | show |
| Series | sandbox: Fix LTO to work with STACKPROTECTOR | expand |
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
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
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 --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);/'
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(+)