Thanks for your attention to this. Your patches can be approached by `-BB_GLOBAL_CONST=""` now after patch 1f925038ab9c6bd8f6b3cd40ed7aab0ef10d898e. > But this is not a fix See https://bugs.busybox.net/show_bug.cgi?id=14231
This trick reduces _kilobytes_ of the binary size.and we should try to keep it. On Fri, Oct 8, 2021 at 12:44 AM Khem Raj <[email protected]> wrote: > > > > On 10/6/21 11:20 PM, YU Jincheng wrote: > > Does this path have any problem? > > > > From: YU Jincheng <[email protected]> > > > > - clang 9+ will load the const pointer first before the const > > > > pointer assignment trick and thus cause null pointer defer. > > > > - This patch creates a shadow variable to prevent the above from > > > > happening for clang 9+. > > > > - Also, this patch applies `BB_GLOBAL_CONST` to all variables > > > > using the same trick, allowing archs or toolchains having the > > > > same issue to bypass this trick correctly. > > > > This patch fixes https://bugs.busybox.net/show_bug.cgi?id=14231 > > > > and https://bugs.busybox.net/show_bug.cgi?id=14236 > > > > Co-authored-by: canyie <[email protected]> > > > > Signed-off-by: YU Jincheng <[email protected]> > > > > We have been carrying a fix for similar problem here > https://github.com/kraj/meta-clang/blob/master/recipes-core/busybox/busybox/0001-Turn-ptr_to_globals-and-bb_errno-to-be-non-const.patch > > does this patch fixes your issue as well ? > > > --- > > > > coreutils/test.c | 7 ++----- > > > > include/libbb.h | 34 +++++++++++++++++++++++++++++----- > > > > libbb/appletlib.c | 3 +-- > > > > libbb/lineedit.c | 7 ++----- > > > > shell/ash.c | 23 +++-------------------- > > > > 5 files changed, 37 insertions(+), 37 deletions(-) > > > > diff --git a/coreutils/test.c b/coreutils/test.c > > > > index 7c6574334..3a91de407 100644 > > > > --- a/coreutils/test.c > > > > +++ b/coreutils/test.c > > > > @@ -435,7 +435,7 @@ struct test_statics { > > > > }; > > > > /* See test_ptr_hack.c */ > > > > -extern struct test_statics *const test_ptr_to_statics; > > > > +extern struct test_statics *BB_GLOBAL_CONST test_ptr_to_statics; > > > > #define S (*test_ptr_to_statics) > > > > #define args (S.args ) > > > > @@ -445,10 +445,7 @@ extern struct test_statics *const test_ptr_to_statics; > > > > #define bash_test2 (S.bash_test2 ) > > > > #define leaving (S.leaving ) > > > > -#define INIT_S() do { \ > > > > - (*(struct > > test_statics**)not_const_pp(&test_ptr_to_statics)) = xzalloc(sizeof(S)); \ > > > > - barrier(); \ > > > > -} while (0) > > > > +#define INIT_S() ASSIGN_CONST_PTR(test_ptr_to_statics, xzalloc(sizeof(S))) > > > > #define DEINIT_S() do { \ > > > > free(group_array); \ > > > > free(test_ptr_to_statics); \ > > > > diff --git a/include/libbb.h b/include/libbb.h > > > > index dfcaa05ec..cdc8bd271 100644 > > > > --- a/include/libbb.h > > > > +++ b/include/libbb.h > > > > @@ -365,10 +365,24 @@ struct BUG_off_t_size_is_misdetected { > > > > #endif > > > > #endif > > > > +/* We use a trick to have more optimized code (fewer pointer reloads > > > > + * and reduce binary size by a few kilobytes) like: > > > > + * ash.c: extern struct globals *const ash_ptr_to_globals; > > > > + * ash_ptr_hack.c: struct globals *ash_ptr_to_globals; > > > > + * This way, compiler in ash.c knows the pointer can not change. > > > > + * > > > > + * However, this may break on weird arches or toolchains. In this case, > > > > + * set "-DBB_GLOBAL_CONST=''" in CONFIG_EXTRA_CFLAGS to disable > > > > + * this optimization. > > > > + */ > > > > +#ifndef BB_GLOBAL_CONST > > > > +# define BB_GLOBAL_CONST const > > > > +#endif > > > > + > > > > #if defined(errno) > > > > /* If errno is a define, assume it's "define errno (*__errno_location())" > > > > * and we will cache it's result in this variable */ > > > > -extern int *const bb_errno; > > > > +extern int *BB_GLOBAL_CONST bb_errno; > > > > #undef errno > > > > #define errno (*bb_errno) > > > > #define bb_cached_errno_ptr 1 > > > > @@ -2284,16 +2298,26 @@ static ALWAYS_INLINE void* not_const_pp(const > > void *p) > > > > ); > > > > return pp; > > > > } > > > > +#ifndef ASSIGN_CONST_PTR > > > > +#define ASSIGN_CONST_PTR(p, v) \ > > > > +_Pragma("clang diagnostic push"); \ > > > > +_Pragma("clang diagnostic ignored \"-Wshadow\""); \ > > > > + __typeof__(p) p = ({*(void**)not_const_pp(&p) = (void*)(v);}); > > > > +_Pragma("clang diagnostic pop") > > > > +#endif > > > > #else > > > > static ALWAYS_INLINE void* not_const_pp(const void *p) { return (void*)p; } > > > > +#ifndef ASSIGN_CONST_PTR > > > > +#define ASSIGN_CONST_PTR(p, v) do { \ > > > > + (*(void**)not_const_pp(&p)) = (void*)(v); \ > > > > + barrier(); \ > > > > +} while (0) > > > > +#endif > > > > #endif > > > > /* At least gcc 3.4.6 on mipsel system needs optimization barrier */ > > > > #define barrier() __asm__ __volatile__("":::"memory") > > > > -#define SET_PTR_TO_GLOBALS(x) do { \ > > > > - (*(struct globals**)not_const_pp(&ptr_to_globals)) = > > (void*)(x); \ > > > > - barrier(); \ > > > > -} while (0) > > > > +#define SET_PTR_TO_GLOBALS(x) ASSIGN_CONST_PTR(ptr_to_globals, x) > > > > #define FREE_PTR_TO_GLOBALS() do { \ > > > > if (ENABLE_FEATURE_CLEAN_UP) { \ > > > > diff --git a/libbb/appletlib.c b/libbb/appletlib.c > > > > index 14be33603..8b4c0f0e7 100644 > > > > --- a/libbb/appletlib.c > > > > +++ b/libbb/appletlib.c > > > > @@ -247,8 +247,7 @@ void lbb_prepare(const char *applet > > > > IF_FEATURE_INDIVIDUAL(, char **argv)) > > > > { > > > > #ifdef bb_cached_errno_ptr > > > > - (*(int **)not_const_pp(&bb_errno)) = get_perrno(); > > > > - barrier(); > > > > + ASSIGN_CONST_PTR(bb_errno, get_perrno()); > > > > #endif > > > > applet_name = applet; > > > > diff --git a/libbb/lineedit.c b/libbb/lineedit.c > > > > index a7a3ee103..73194282c 100644 > > > > --- a/libbb/lineedit.c > > > > +++ b/libbb/lineedit.c > > > > @@ -192,7 +192,7 @@ struct lineedit_statics { > > > > }; > > > > /* See lineedit_ptr_hack.c */ > > > > -extern struct lineedit_statics *const lineedit_ptr_to_statics; > > > > +extern struct lineedit_statics *BB_GLOBAL_CONST lineedit_ptr_to_statics; > > > > #define S (*lineedit_ptr_to_statics) > > > > #define state (S.state ) > > > > @@ -213,10 +213,7 @@ extern struct lineedit_statics *const > > lineedit_ptr_to_statics; > > > > #define newdelflag (S.newdelflag ) > > > > #define delbuf (S.delbuf ) > > > > -#define INIT_S() do { \ > > > > - (*(struct > > lineedit_statics**)not_const_pp(&lineedit_ptr_to_statics)) = > > xzalloc(sizeof(S)); \ > > > > - barrier(); \ > > > > -} while (0) > > > > +#define INIT_S() ASSIGN_CONST_PTR(lineedit_ptr_to_statics, > > xzalloc(sizeof(S))) > > > > static void deinit_S(void) > > > > { > > > > diff --git a/shell/ash.c b/shell/ash.c > > > > index 4bc4f55d0..c8a20ab94 100644 > > > > --- a/shell/ash.c > > > > +++ b/shell/ash.c > > > > @@ -303,20 +303,6 @@ typedef long arith_t; > > > > # error "Do not even bother, ash will not run on NOMMU machine" > > > > #endif > > > > -/* We use a trick to have more optimized code (fewer pointer reloads): > > > > - * ash.c: extern struct globals *const ash_ptr_to_globals; > > > > - * ash_ptr_hack.c: struct globals *ash_ptr_to_globals; > > > > - * This way, compiler in ash.c knows the pointer can not change. > > > > - * > > > > - * However, this may break on weird arches or toolchains. In this case, > > > > - * set "-DBB_GLOBAL_CONST=''" in CONFIG_EXTRA_CFLAGS to disable > > > > - * this optimization. > > > > - */ > > > > -#ifndef BB_GLOBAL_CONST > > > > -# define BB_GLOBAL_CONST const > > > > -#endif > > > > - > > > > - > > > > /* ============ Hash table sizes. Configurable. */ > > > > #define VTABSIZE 39 > > > > @@ -518,8 +504,7 @@ extern struct globals_misc *BB_GLOBAL_CONST > > ash_ptr_to_globals_misc; > > > > #define random_gen (G_misc.random_gen ) > > > > #define backgndpid (G_misc.backgndpid ) > > > > #define INIT_G_misc() do { \ > > > > - (*(struct > > globals_misc**)not_const_pp(&ash_ptr_to_globals_misc)) = > > xzalloc(sizeof(G_misc)); \ > > > > - barrier(); \ > > > > + ASSIGN_CONST_PTR(ash_ptr_to_globals_misc, > > xzalloc(sizeof(G_misc))); \ > > > > savestatus = -1; \ > > > > curdir = nullstr; \ > > > > physdir = nullstr; \ > > > > @@ -1597,8 +1582,7 @@ extern struct globals_memstack *BB_GLOBAL_CONST > > ash_ptr_to_globals_memstack; > > > > #define g_stacknleft (G_memstack.g_stacknleft) > > > > #define stackbase (G_memstack.stackbase ) > > > > #define INIT_G_memstack() do { \ > > > > - (*(struct > > globals_memstack**)not_const_pp(&ash_ptr_to_globals_memstack)) = > > xzalloc(sizeof(G_memstack)); \ > > > > - barrier(); \ > > > > + ASSIGN_CONST_PTR(ash_ptr_to_globals_memstack, > > xzalloc(sizeof(G_memstack))); \ > > > > g_stackp = &stackbase; \ > > > > g_stacknxt = stackbase.space; \ > > > > g_stacknleft = MINSIZE; \ > > > > @@ -2229,8 +2213,7 @@ extern struct globals_var *BB_GLOBAL_CONST > > ash_ptr_to_globals_var; > > > > #endif > > > > #define INIT_G_var() do { \ > > > > unsigned i; \ > > > > - (*(struct > > globals_var**)not_const_pp(&ash_ptr_to_globals_var)) = > > xzalloc(sizeof(G_var)); \ > > > > - barrier(); \ > > > > + ASSIGN_CONST_PTR(ash_ptr_to_globals_var, > > xzalloc(sizeof(G_var))); \ > > > > for (i = 0; i < ARRAY_SIZE(varinit_data); i++) { \ > > > > varinit[i].flags = varinit_data[i].flags; \ > > > > varinit[i].var_text = > > varinit_data[i].var_text; \ > > > > -- > > > > 2.25.1 > > > > > > _______________________________________________ > > busybox mailing list > > [email protected] > > http://lists.busybox.net/mailman/listinfo/busybox > > _______________________________________________ busybox mailing list [email protected] http://lists.busybox.net/mailman/listinfo/busybox
