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]> --- 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
