toolchain/wrapper: move -ztext to the toolchain wrapper
cover toolchain/wrapper: move -ztext to the toolchain wrapper
From: "Yann E. MORIN" <[email protected]>
Hello All!
Following a discussion with Thomas , we were both left wondering why -ztext was not handled in the toolchain wrapper, like the other similar flags are.
This small series is an attempt at making it so.
Regards,
Yann E. MORIN.
The following changes since commit 81e6541efe054df635947d0c143818f8cf7cb048
package/gnuradio: fix build with fmt >= 11.x (2024-08-12 22:50:39 +0200)
are available as patches in this mail series,
for you to apply patches up to 8402ba3c5551505159a155c5cf0fc5b58c6a3614
toolchain/wrapper: move -ztext from LDFLAGS to toolchain wrapper (2024-08-13 12:03:55 +0200)
----------------------------------------------------------------
Yann E. MORIN (3):
toolchain/wrapper: check unsafe paths earlier toolchain/wrapper: check we did not add more args than expected toolchain/wrapper: move -ztext from LDFLAGS to toolchain wrapper
package/Makefile.in | 19 -------------- toolchain/toolchain-wrapper.c | 57 +++++++++++++++++++++++++----------------- toolchain/toolchain-wrapper.mk | 9 +++++++ 3 files changed, 43 insertions(+), 42 deletions(-)
--
____________ .-----------------.--------------------: _ :------------------. | Yann E. MORIN | Real-Time Embedded | __/ ) | /"\ ASCII RIBBON | | | Software Designer | _/ - /' | \ / CAMPAIGN | | +33 638.411.245 '--------------------: (_ `--, | X AGAINST | | yann.morin (at) orange.com |_=" ,--' | / \ HTML MAIL |
'--------------------------------------:______/_____:------------------'
____________________________________________________________________________________________________________
Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration, Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci.
This message and its attachments may contain confidential or privileged information that may be protected by law; they should not be distributed, used or copied without authorisation.
If you have received this email in error, please notify the sender and delete this message and its attachments. As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified. Thank you.
_______________________________________________
buildroot mailing list
[email protected]
https://lists.buildroot.org/mailman/listinfo/buildroot
PATCH 1/3 toolchain/wrapper: check unsafe paths earlier
From: "Yann E. MORIN" <[email protected]>
We currently check for unsafe paths right between adding our arguments, and adding the one passed from the command line. This not very consistent.
Unsafe paths can only come from the command line, as we are not adding any of our own (hopefully, we know better!), so we can run the check as early as possible.
Move the check very early, but not before we handle --help.
--- toolchain/toolchain-wrapper.c | 44 +++++++++++++++++------------------ 1 file changed, 22 insertions(+), 22 deletions(-)
toolchain/toolchain-wrapper.c +22 -22
@@ -321,6 +321,28 @@ int main(int argc, char **argv) } } +/* Check for unsafe library and header paths */ +for (i =1 ; i < argc; i++) { +const struct str_len_s *opt; +for (opt=unsafe_opts; opt->str; opt++ ) { +/* Skip any non-unsafe option. */ +if (strncmp (argv[i], opt->str, opt->len)) +continue ; + +/* Handle both cases: +* - path is a separate argument, +* - path is concatenated with option. +*/ +if (argv[i][opt->len] =='\0' ) { + i++; +if (i == argc) +break ; +check_unsafe_path (argv[i-1 ], argv[i],0 ); + }else +check_unsafe_path (argv[i], argv[i] + opt->len,1 ); + } + } +#ifdef BR_CCACHE ret =snprintf (ccache_path,sizeof (ccache_path),"%s/bin/ccache" , absbasedir);if (ret >=sizeof (ccache_path)) {
@@ -474,28 +496,6 @@ int main(int argc, char **argv)#endif } -/* Check for unsafe library and header paths */ -for (i =1 ; i < argc; i++) { -const struct str_len_s *opt; -for (opt=unsafe_opts; opt->str; opt++ ) { -/* Skip any non-unsafe option. */ -if (strncmp (argv[i], opt->str, opt->len)) -continue ; - -/* Handle both cases: -* - path is a separate argument, -* - path is concatenated with option. -*/ -if (argv[i][opt->len] =='\0' ) { - i++; -if (i == argc) -break ; -check_unsafe_path (argv[i-1 ], argv[i],0 ); - }else -check_unsafe_path (argv[i], argv[i] + opt->len,1 ); - } - } -/* append forward args */ memcpy (cur, &argv[1 ],sizeof (char *) * (argc -1 )); cur += argc -1 ;
Signature · 18 lines
2.34.1 ____________________________________________________________________________________________________________ Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration, Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci. This message and its attachments may contain confidential or privileged information that may be protected by law; they should not be distributed, used or copied without authorisation. If you have received this email in error, please notify the sender and delete this message and its attachments. As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified. Thank you. _______________________________________________ buildroot mailing list [email protected] https://lists.buildroot.org/mailman/listinfo/buildroot
Re: [Buildroot] [PATCH 1/3] toolchain/wrapper: check unsafe paths earlier
Quoted 11 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
<[email protected]> writes:
From: "Yann E. MORIN" <[email protected]>
We currently check for unsafe paths right between adding our arguments, and adding the one passed from the command line. This not very consistent.Unsafe paths can only come from the command line, as we are not adding any of our own (hopefully, we know better!), so we can run the check as early as possible.
Quoted 6 lines
Move the check very early, but not before we handle --help.
Signed-off-by Yann E. MORIN <[email protected]>Cc Romain Naour <[email protected]>Cc Giulio Benetti <[email protected]>Cc Thomas Petazzoni <[email protected]>
Committed, thanks.
Signature · 5 lines
Bye, Peter Korsgaard _______________________________________________ buildroot mailing list [email protected] https://lists.buildroot.org/mailman/listinfo/buildroot
Re: [Buildroot] [PATCH 1/3] toolchain/wrapper: check unsafe paths earlier
In reply to [email protected]Quoted 10 lines · On 13/08/2024 12:04, [email protected] wrote:
From: "Yann E. MORIN" <[email protected]>
We currently check for unsafe paths right between adding our arguments, and adding the one passed from the command line. This not very consistent.
Unsafe paths can only come from the command line, as we are not adding any of our own (hopefully, we know better!), so we can run the check as early as possible.
Quoted 6 lines
Move the check very early, but not before we handle --help.
Signed-off-by Yann E. MORIN <[email protected]>Cc Romain Naour <[email protected]>Cc Giulio Benetti <[email protected]>Cc Thomas Petazzoni <[email protected]>
Applied to 2025.02.x, thanks.
Regards,
Arnout
Quoted 56 lines
--- toolchain/toolchain-wrapper.c | 44 +++++++++++++++++------------------ 1 file changed, 22 insertions(+), 22 deletions(-)toolchain/toolchain-wrapper.c +22
-15@@ -321,6 +321,28 @@ int main(int argc, char **argv) } } +/* Check for unsafe library and header paths */ +for (i =1 ; i < argc; i++) { +const struct str_len_s *opt; +for (opt=unsafe_opts; opt->str; opt++ ) { +/* Skip any non-unsafe option. */ +if (strncmp (argv[i], opt->str, opt->len)) +continue ; + +/* Handle both cases: +* - path is a separate argument, +* - path is concatenated with option. +*/ +if (argv[i][opt->len] =='\0' ) { + i++; +if (i == argc) +break ; +check_unsafe_path (argv[i-1 ], argv[i],0 ); + }else +check_unsafe_path (argv[i], argv[i] + opt->len,1 ); + } + } +#ifdef BR_CCACHE ret =snprintf (ccache_path,sizeof (ccache_path),"%s/bin/ccache" , absbasedir);if (ret >=sizeof (ccache_path)) {@@ -474,28 +496,6 @@ int main(int argc, char **argv)#endif } -/* Check for unsafe library and header paths */ -for (i =1 ; i < argc; i++) { -const struct str_len_s *opt; -for (opt=unsafe_opts; opt->str; opt++ ) { -/* Skip any non-unsafe option. */ -if (strncmp (argv[i], opt->str, opt->len)) -continue ; - -/* Handle both cases: -* - path is a separate argument, -* - path is concatenated with option. -*/ -if (argv[i][opt->len] =='\0' ) { - i++; -if (i == argc)
toolchain/toolchain-wrapper.c int main(int argc, char **argv) · line 501
Quoted 10 lines
toolchain/toolchain-wrapper.c +0
-7-break ; -check_unsafe_path (argv[i-1 ], argv[i],0 ); - }else -check_unsafe_path (argv[i], argv[i] + opt->len,1 ); - } - } -/* append forward args */ memcpy (cur, &argv[1 ],sizeof (char *) * (argc -1 )); cur += argc -1 ;
_______________________________________________
buildroot mailing list
[email protected]
https://lists.buildroot.org/mailman/listinfo/buildroot
PATCH 2/3 toolchain/wrapper: check we did not add more args than expected
In reply to [email protected]From: "Yann E. MORIN" <[email protected]>
We have a hard-coded constant that defines how many expected args we may conditionally add at most, but it is very easy to miss updating that when adding new conditional args.
Add a check that we did not overshoot the allowance.
Ideally, we would have a nice way to add to, and extend the *args array dynamically, but this would be quite costly, while the wrapper is a hot path to the compiler. So, this test is a better solution in the end: it is simple and cheap.
Note that this would only trigger at runtime when all conditional args are added, which can only happen in special conditions, so local testing might not always catch it.
--- toolchain/toolchain-wrapper.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)
toolchain/toolchain-wrapper.c +9 -2
@@ -240,7 +240,7 @@ bool parse_source_date_epoch_from_env(void)int main (int argc,char **argv) { -char **args, **cur, **exec_args; +char **args, **cur, **exec_args, **cond_args;char *relbasedir, *absbasedir;char *progpath = argv[0 ];char *basename;
@@ -365,7 +365,7 @@ int main(int argc, char **argv)/* start with predefined args */ memcpy (cur, predef_args,sizeof (predef_args)); - cur +=sizeof (predef_args) /sizeof (predef_args[0 ]); + cond_args = cur +=sizeof (predef_args) /sizeof (predef_args[0 ]);#ifdef BR_FLOAT_ABI /* add float abi if not overridden in args */
@@ -496,6 +496,13 @@ int main(int argc, char **argv)#endif } +/* Check that we did not add more conditional args than we expected */ +if ((cur-cond_args) >EXCLUSIVE_ARGS ) { + errno =E2BIG ; +perror (__FILE__": Not enough EXCLUSIVE_ARGS" ); +return 3 ; + } +/* append forward args */ memcpy (cur, &argv[1 ],sizeof (char *) * (argc -1 )); cur += argc -1 ;
Signature · 18 lines
2.34.1 ____________________________________________________________________________________________________________ Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration, Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci. This message and its attachments may contain confidential or privileged information that may be protected by law; they should not be distributed, used or copied without authorisation. If you have received this email in error, please notify the sender and delete this message and its attachments. As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified. Thank you. _______________________________________________ buildroot mailing list [email protected] https://lists.buildroot.org/mailman/listinfo/buildroot
Re: [Buildroot] [PATCH 2/3] toolchain/wrapper: check we did not add more args than expected
Quoted 7 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
<[email protected]> writes:
From: "Yann E. MORIN" <[email protected]>
We have a hard-coded constant that defines how many expected args we may conditionally add at most, but it is very easy to miss updating that when adding new conditional args.
Quoted 6 lines
Add a check that we did not overshoot the allowance.
Ideally, we would have a nice way to add to, and extend the *args array dynamically, but this would be quite costly, while the wrapper is a hot path to the compiler. So, this test is a better solution in the end: it is simple and cheap.
Costly? It would just be a realloc call, and the list of argument pointers is not very long, so I doubt it would be noticable compared to all the argument parsing and finally running the real compiler.
In fact, I think it would make more sense to get rid of our EXCLUSIVE_ARGS constant and just allocate room for E.G. 1024 arguments and then just realloc in the special cases where that is not enough.
I've sent a small series doing that here:
https://patchwork.ozlabs.org/project/buildroot/patch/[email protected]/
Signature · 5 lines
Bye, Peter Korsgaard _______________________________________________ buildroot mailing list [email protected] https://lists.buildroot.org/mailman/listinfo/buildroot
PATCH 3/3 toolchain/wrapper: move -ztext from LDFLAGS to toolchain wrapper
In reply to [email protected]From: "Yann E. MORIN" <[email protected]>
Passing linker flags via LDFLAGS is notoriously fragile: packages either use LDFLAGS with the gcc frontend, or directly with ld. This means that care must be taken to only pass flags that are recognised by both gcc and ld, which usually proves a bit challenging, as bd035872be99 (package/Makefile.in: Use "-z text" instead of "-Wl, -z, text") demonstrates (later ammended by 3a39e706a91a).
It turns out that we already have a better, robust way of passing LDFLAGS via the gcc frotned: the toolchain wrapper, which we already use to pass relro for example.
We can revert back to using -Wl,-z,text, as we're sure whis is going to be a call to the gcc frontend (the wrapper is not used for ld), which makes it consistent with the other similar args for -z relro and -z now.
--- package/Makefile.in | 19 ------------------- toolchain/toolchain-wrapper.c | 6 +++++- toolchain/toolchain-wrapper.mk | 9 +++++++++ 3 files changed, 14 insertions(+), 20 deletions(-)
package/Makefile.in +0 -19
@@ -149,25 +149,6 @@ endif TARGET_LDFLAGS = $(call qstrip,$(BR2_TARGET_LDFLAGS)) -# musl's dynamic loader doesn't support DT_TEXTREL, which results in a runtime -# crash if it gets used. The "-z text" linker option issues a build-time error -# when DT_TEXREL is used, so we capture the problem earlier. -# -# See also: https://www.openwall.com/lists/musl/2020/09/25/4 -# -# NOTE: We're using "-ztext" instead of "-Wl,-z,text" here, because some -# packages pass TARGET_LDFLAGS directly to ld rather than gcc, and ld doesn't -# support -Wl,[...]. -z is supported by both gcc and clang, so it probably -# won't cause us problems. -# -# We're using "-ztext" instead of "-z text" here, because some buildsystems -# (like scons, for gpsd) will reorder and/or drop LDFLAGS, causing a lone -# "-z" to be passed and the "text" keyword to be dropped otherwise. Both -# gcc and ld supports that, so it probably won't cause us problems. -ifeq ($(BR2_TOOLCHAIN_USES_MUSL):$(BR2_STATIC_LIBS),y:) -TARGET_LDFLAGS += -ztext -endif - # By design, _FORTIFY_SOURCE requires gcc optimization to be enabled. # Therefore, we need to pass _FORTIFY_SOURCE and the optimization level # through the same mechanism, i.e currently through CFLAGS. Passing
toolchain/toolchain-wrapper.c +5 -1
@@ -51,10 +51,11 @@ static char _date_[sizeof("-D__DATE__=\"MMM DD YYYY\"")];* -Wno-builtin-macro-redefined * -Wl,-z,now * -Wl,-z,relro +* -Wl,-z,text * -fPIE * -pie */ -#define EXCLUSIVE_ARGS 10 +#define EXCLUSIVE_ARGS 11 static char *predef_args[] = {#ifdef BR_CCACHE
@@ -486,6 +487,9 @@ int main(int argc, char **argv)break ; }if (i == argc) { +#ifdef BR2_ZTEXT + *cur++ ="-Wl,-z,text" ; +#endif /* https://wiki.gentoo.org/wiki/Hardened/Toolchain#Mark_Read-Only_Appropriate_Sections */ #ifdef BR2_RELRO_PARTIAL *cur++ ="-Wl,-z,relro" ;
toolchain/toolchain-wrapper.mk +9 -0
@@ -83,6 +83,15 @@ else ifeq ($(BR2_RELRO_FULL),y) TOOLCHAIN_WRAPPER_ARGS += -DBR2_RELRO_FULL endif +# musl's dynamic loader doesn't support DT_TEXTREL, which results in a runtime +# crash if it gets used. The "-z text" linker option issues a build-time error +# when DT_TEXREL is used, so we capture the problem earlier. +# +# See also: https://www.openwall.com/lists/musl/2020/09/25/4 +ifeq ($(BR2_TOOLCHAIN_USES_MUSL):$(BR2_STATIC_LIBS),y:) +TOOLCHAIN_WRAPPER_ARGS += -DBR2_ZTEXT +endif + define TOOLCHAIN_WRAPPER_BUILD $(HOSTCC) $(HOST_CFLAGS) $(TOOLCHAIN_WRAPPER_ARGS) \ -s -Wl,--hash-style=$(TOOLCHAIN_WRAPPER_HASH_STYLE) \
Signature · 17 lines
2.34.1 ____________________________________________________________________________________________________________ Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration, Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci. This message and its attachments may contain confidential or privileged information that may be protected by law; they should not be distributed, used or copied without authorisation. If you have received this email in error, please notify the sender and delete this message and its attachments. As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified. Thank you. _______________________________________________ buildroot mailing list [email protected] https://lists.buildroot.org/mailman/listinfo/buildroot
Re: [Buildroot] [PATCH 3/3] toolchain/wrapper: move -ztext from LDFLAGS to toolchain wrapper
All,
On 2024-08-13 12:04 +0200, [email protected] spake thusly:
Quoted 17 lines
From: "Yann E. MORIN" <[email protected]>
Passing linker flags via LDFLAGS is notoriously fragile: packages either use LDFLAGS with the gcc frontend, or directly with ld. This means that care must be taken to only pass flags that are recognised by both gcc and ld, which usually proves a bit challenging, as
bd035872be99(package/Makefile.in: Use "-z text" instead of "-Wl, -z, text") demonstrates (later ammended by3a39e706a91a).It turns out that we already have a better, robust way of passing LDFLAGS via the gcc frotned: the toolchain wrapper, which we already use to pass relro for example.
We can revert back to using -Wl,-z,text, as we're sure whis is going to be a call to the gcc frontend (the wrapper is not used for ld), which makes it consistent with the other similar args for -z relro and -z now.
Quoted 5 lines
Signed-off-by Yann E. MORIN <[email protected]>Cc J. Neuschäfer <[email protected]>Cc Romain Naour <[email protected]>Cc Giulio Benetti <[email protected]>Cc Thomas Petazzoni <[email protected]>
Markus provided their tested-by tag in another thread:
https://lore.kernel.org/buildroot/CAGt4E5u3UeGDLp_D1SgedQgXvcN2mGxSF3ExX+e0KKqDMMTVzw@mail.gmail.com/
Regards,
Yann E. MORIN.
Signature · 23 lines
____________ .-----------------.--------------------: _ :------------------. | Yann E. MORIN | Real-Time Embedded | __/ ) | /"\ ASCII RIBBON | | | Software Designer | _/ - /' | \ / CAMPAIGN | | +33 638.411.245 '--------------------: (_ `--, | X AGAINST | | yann.morin (at) orange.com |_=" ,--' | / \ HTML MAIL | '--------------------------------------:______/_____:------------------' ____________________________________________________________________________________________________________ Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration, Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci. This message and its attachments may contain confidential or privileged information that may be protected by law; they should not be distributed, used or copied without authorisation. If you have received this email in error, please notify the sender and delete this message and its attachments. As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified. Thank you. _______________________________________________ buildroot mailing list [email protected] https://lists.buildroot.org/mailman/listinfo/buildroot
Re: [Buildroot] [PATCH 3/3] toolchain/wrapper: move -ztext from LDFLAGS to toolchain wrapper
In reply to [email protected]On Tue, 13 Aug 2024 12:04:31 +0200, [email protected] wrote:
${quote_head}
Applied to master, thanks!
Regards,
Arnout
${quote_tail}
_______________________________________________
buildroot mailing list
[email protected]
https://lists.buildroot.org/mailman/listinfo/buildroot
Quoted 7 lines · On Tue, Aug 13, 2024 at 12:04:28PM +0200, [email protected] wrote:
From: "Yann E. MORIN" <[email protected]>
Hello All!
Following a discussion with Thomas , we were both left wondering why -ztext was not handled in the toolchain wrapper, like the other similar flags are.
Simple answer: I didn't know about the toolchain wrapper at the time :-)
Quoted 1 line
This small series is an attempt at making it so.
Thanks for taking care of this!
-- jn
_______________________________________________
buildroot mailing list
[email protected]
https://lists.buildroot.org/mailman/listinfo/buildroot
Hi all,
Quoted 9 lines · J. Neuschäfer via buildroot
Quoted 7 lines · On Tue, Aug 13, 2024 at 12:04:28PM +0200, [email protected] wrote:
From: "Yann E. MORIN" <[email protected]>
Hello All!
Following a discussion with Thomas , we were both left wondering why -ztext was not handled in the toolchain wrapper, like the other similar flags are.
I am curious what the plan is with this series. On the one hand it is looking like this new approach is the approach that should have been taken all along (handle "-ztext" in the wrapper). On the other hand, it doesn't seem to have been applied anywhere that I can find.
Will it be brought in at some point?
Thanks,
-Markus
_______________________________________________
buildroot mailing list
[email protected]
https://lists.buildroot.org/mailman/listinfo/buildroot