On Sat, Oct 9, 2021 at 8:20 PM LoveSy <[email protected]> wrote: > > From: YU Jincheng <[email protected]> > > - This can act as memory barrier in clang to avoid > read before assign of a const ptr > > Signed-off-by: LoveSy <[email protected]>
git tree commit hook requires something of the form "Name Surname" in the signed-off lines. Can you use a name of this format? I propose the attached patch. Please test that it works on clang 9. I assume we will need to massage every SET_PTR_TO_GLOBALS(xzalloc(sz)) to something like XZALLOC_PTR_TO_GLOBALS(sz) too? The bug may be occurring there as well, right?
From 5156b245536ce0f07165793f07c63fd9fa5dd3b7 Mon Sep 17 00:00:00 2001 From: YU Jincheng <[email protected]> Date: Sun, 10 Oct 2021 02:19:51 +0800 Subject: [PATCH] Make const ptr assign as function call in clang - This can act as memory barrier in clang to avoid read before assign of a const ptr Signed-off-by: LoveSy <[email protected]> Signed-off-by: Denys Vlasenko <[email protected]> --- coreutils/test.c | 2 +- include/libbb.h | 21 +++++++++++++++------ libbb/Kbuild.src | 1 + libbb/appletlib.c | 2 +- libbb/const_hack.c | 16 ++++++++++++++++ libbb/lineedit.c | 2 +- shell/ash.c | 6 +++--- 7 files changed, 38 insertions(+), 12 deletions(-) create mode 100644 libbb/const_hack.c diff --git a/coreutils/test.c b/coreutils/test.c index fc956724b..a914c7490 100644 --- a/coreutils/test.c +++ b/coreutils/test.c @@ -446,7 +446,7 @@ extern struct test_statics *BB_GLOBAL_CONST test_ptr_to_statics; #define leaving (S.leaving ) #define INIT_S() do { \ - ASSIGN_CONST_PTR(test_ptr_to_statics, xzalloc(sizeof(S))); \ + XZALLOC_CONST_PTR(&test_ptr_to_statics, sizeof(S)); \ } while (0) #define DEINIT_S() do { \ free(group_array); \ diff --git a/include/libbb.h b/include/libbb.h index 296417dae..a340f27d2 100644 --- a/include/libbb.h +++ b/include/libbb.h @@ -2280,6 +2280,7 @@ extern const char bb_PATH_root_path[] ALIGN1; /* BB_PATH_ROOT_PATH */ extern const int const_int_0; //extern const int const_int_1; + /* This struct is deliberately not defined. */ /* See docs/keep_data_small.txt */ struct globals; @@ -2304,23 +2305,31 @@ static ALWAYS_INLINE void* not_const_pp(const void *p) ); return pp; } +# define ASSIGN_CONST_PTR(pptr, v) do { \ + *(void**)not_const_pp(pptr) = (void*)(v); \ + barrier(); \ +} while (0) +/* XZALLOC_CONST_PTR() is an out-of-line function to prevent + * clang from reading pointer before it is assigned. + */ +void XZALLOC_CONST_PTR(const void *pptr, size_t size) FAST_FUNC; #else -static ALWAYS_INLINE void* not_const_pp(const void *p) { return (void*)p; } -#endif - -#define ASSIGN_CONST_PTR(p, v) do { \ - *(void**)not_const_pp(&p) = (void*)(v); \ +# define ASSIGN_CONST_PTR(pptr, v) do { \ + *(void**)(pptr) = (void*)(v); \ /* At least gcc 3.4.6 on mipsel needs optimization barrier */ \ barrier(); \ } while (0) +# define XZALLOC_CONST_PTR(pptr, size) ASSIGN_CONST_PTR(pptr, xzalloc(size)) +#endif -#define SET_PTR_TO_GLOBALS(x) ASSIGN_CONST_PTR(ptr_to_globals, x) +#define SET_PTR_TO_GLOBALS(x) ASSIGN_CONST_PTR(&ptr_to_globals, x) #define FREE_PTR_TO_GLOBALS() do { \ if (ENABLE_FEATURE_CLEAN_UP) { \ free(ptr_to_globals); \ } \ } while (0) + /* You can change LIBBB_DEFAULT_LOGIN_SHELL, but don't use it, * use bb_default_login_shell and following defines. * If you change LIBBB_DEFAULT_LOGIN_SHELL, diff --git a/libbb/Kbuild.src b/libbb/Kbuild.src index 676300801..2fa239857 100644 --- a/libbb/Kbuild.src +++ b/libbb/Kbuild.src @@ -24,6 +24,7 @@ lib-y += chomp.o lib-y += compare_string_array.o lib-y += concat_path_file.o lib-y += concat_subpath_file.o +lib-y += const_hack.o lib-y += copy_file.o lib-y += copyfd.o lib-y += crc32.o diff --git a/libbb/appletlib.c b/libbb/appletlib.c index bf26c99e9..e8c308467 100644 --- a/libbb/appletlib.c +++ b/libbb/appletlib.c @@ -247,7 +247,7 @@ void lbb_prepare(const char *applet IF_FEATURE_INDIVIDUAL(, char **argv)) { #ifdef bb_cached_errno_ptr - ASSIGN_CONST_PTR(bb_errno, get_perrno()); + ASSIGN_CONST_PTR(&bb_errno, get_perrno()); #endif applet_name = applet; diff --git a/libbb/const_hack.c b/libbb/const_hack.c new file mode 100644 index 000000000..9575e6d67 --- /dev/null +++ b/libbb/const_hack.c @@ -0,0 +1,16 @@ +/* vi: set sw=4 ts=4: */ +/* + * Trick to assign a const ptr with barrier for clang + * + * Copyright (C) 2021 by YU Jincheng <[email protected]> + * + * Licensed under GPLv2 or later, see file LICENSE in this source tree. + */ +#include "libbb.h" + +#if defined(__clang_major__) && __clang_major__ >= 9 +void FAST_FUNC XZALLOC_CONST_PTR(const void *pptr, size_t size) +{ + ASSIGN_CONST_PTR(pptr, xzalloc(size)); +} +#endif diff --git a/libbb/lineedit.c b/libbb/lineedit.c index 3c87abcf9..9960448ec 100644 --- a/libbb/lineedit.c +++ b/libbb/lineedit.c @@ -214,7 +214,7 @@ extern struct lineedit_statics *BB_GLOBAL_CONST lineedit_ptr_to_statics; #define delbuf (S.delbuf ) #define INIT_S() do { \ - ASSIGN_CONST_PTR(lineedit_ptr_to_statics, xzalloc(sizeof(S))); \ + XZALLOC_CONST_PTR(&lineedit_ptr_to_statics, sizeof(S)); \ } while (0) static void deinit_S(void) diff --git a/shell/ash.c b/shell/ash.c index 199975191..2d3cc8a61 100644 --- a/shell/ash.c +++ b/shell/ash.c @@ -504,7 +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 { \ - ASSIGN_CONST_PTR(ash_ptr_to_globals_misc, xzalloc(sizeof(G_misc))); \ + XZALLOC_CONST_PTR(&ash_ptr_to_globals_misc, sizeof(G_misc)); \ savestatus = -1; \ curdir = nullstr; \ physdir = nullstr; \ @@ -1582,7 +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 { \ - ASSIGN_CONST_PTR(ash_ptr_to_globals_memstack, xzalloc(sizeof(G_memstack))); \ + XZALLOC_CONST_PTR(&ash_ptr_to_globals_memstack, sizeof(G_memstack)); \ g_stackp = &stackbase; \ g_stacknxt = stackbase.space; \ g_stacknleft = MINSIZE; \ @@ -2213,7 +2213,7 @@ extern struct globals_var *BB_GLOBAL_CONST ash_ptr_to_globals_var; #endif #define INIT_G_var() do { \ unsigned i; \ - ASSIGN_CONST_PTR(ash_ptr_to_globals_var, xzalloc(sizeof(G_var))); \ + XZALLOC_CONST_PTR(&ash_ptr_to_globals_var, 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.30.0
_______________________________________________ busybox mailing list [email protected] http://lists.busybox.net/mailman/listinfo/busybox
