[prev in list] [next in list] [prev in thread] [next in thread] 

List:       busybox
Subject:    [PATCH] Fix const pointer assignment trick on clang 9+
From:       LoveSy <lovesykun () gmail ! com>
Date:       2021-09-29 9:37:26
Message-ID: 20210929093726.24973-1-shana () zju ! edu ! cn
[Download RAW message or body]

From: YU Jincheng <shana@zju.edu.cn>

- 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 <31466456+canyie@users.noreply.github.com>
Signed-off-by: YU Jincheng <shana@zju.edu.cn>
---
 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
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic