Hello,
This patch mainly intends to add Clang/LLVM support, which currently is broken.
Problem: the const pointer trick (used by the struct globals, etc.) is
technically undefined behavior. In practice, it causes problems with
an LLVM-based toolchain, since LLVM optimizes away the store to the
const pointer (despite the memory barrier), and therefore the applets
crash when they dereference the null pointers. There already was a
preprocessor define (BB_GLOBAL_CONST) in busybox to deal with this
issue, but it only applied to the ash applet. This patch contributes
the following changes:
- Replaces the hardcoded const modifier with BB_GLOBAL_CONST in the
remaining places where the const pointer trick was used (if I missed
any place please point it out);
- Moves the BB_GLOBAL_CONST define to libbb.h, so that a single
definition is accessible to the other applets;
- Whitelists the use of the const pointer trick for GCC. Clang is
explicitly ruled out (in part because it also defines __GNUC__). The
default for other toolchains is set to not have the trick enabled (at
least if they don't define __GNUC__).
The reasoning behind making it a whitelist instead of an opt-out is as
follows. The trick relies on C undefined behavior, which for non-GCC
compilers could result in miscompilations. For Clang that
miscompilation is fairly easy to detect (some of the applets
immediately crash on lunch), but in other situations the effects could
be more subtle and even have security implications. Most people will
use GCC anyway. So, if the user is using a different toolchain and is
not aware of this issue, then it's better to play it safe. If the user
is dissatisfied with the code size or performance then that person can
investigate how the toolchain is underperforming and evaluate if
enabling the trick would be safe for that toolchain or not.
Best regards,
Luís
---
diff --git a/coreutils/test.c b/coreutils/test.c
index 8d7dac025..e1d440106 100644
--- a/coreutils/test.c
+++ b/coreutils/test.c
@@ -400,8 +400,7 @@ struct test_statics {
jmp_buf leaving;
};
-/* 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 )
diff --git a/coreutils/test_ptr_hack.c b/coreutils/test_ptr_hack.c
index 5ba9dcc68..6759b2144 100644
--- a/coreutils/test_ptr_hack.c
+++ b/coreutils/test_ptr_hack.c
@@ -18,6 +18,6 @@ struct test_statics *test_ptr_to_statics;
/* gcc -combine will see through and complain */
/* Using alternative method which is more likely to break
* on weird architectures, compilers, linkers and so on */
-struct test_statics *const test_ptr_to_statics __attribute__
((section (".data")));
+struct test_statics *BB_GLOBAL_CONST test_ptr_to_statics
__attribute__ ((section (".data")));
#endif
diff --git a/include/libbb.h b/include/libbb.h
index 100d6b606..d0b9eccd9 100644
--- a/include/libbb.h
+++ b/include/libbb.h
@@ -338,10 +338,33 @@ struct BUG_off_t_size_is_misdetected {
#endif
#endif
+/* We use a trick to have more optimized code (fewer pointer reloads). E.g.:
+ * 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 cannot change.
+ *
+ * However, this relies on C undefined behavior, so we whitelist compilers
+ * where we know this isn't problematic, by using the the BB_GLOBAL_CONST
+ * preprocessor definition.
+ * If you are sure this trick also works with your toolchain you can add
+ * "-DBB_GLOBAL_CONST='const'" to CONFIG_EXTRA_CFLAGS or add your compiler to
+ * the whitelist below.
+ */
+
+#ifndef BB_GLOBAL_CONST
+# if defined(__clang__)
+# define BB_GLOBAL_CONST
+# elif defined(__GNUC__)
+# define BB_GLOBAL_CONST const
+# else
+# define BB_GLOBAL_CONST
+# endif
+#endif
+
#if defined(__GLIBC__)
/* glibc uses __errno_location() to get a ptr to errno */
/* We can just memorize it once - no multithreading in busybox :) */
-extern int *const bb_errno;
+extern int *BB_GLOBAL_CONST bb_errno;
#undef errno
#define errno (*bb_errno)
#endif
@@ -2108,7 +2131,7 @@ struct globals;
/* '*const' ptr makes gcc optimize code much better.
* Magic prevents ptr_to_globals from going into rodata.
* If you want to assign a value, use SET_PTR_TO_GLOBALS(x) */
-extern struct globals *const ptr_to_globals;
+extern struct globals *BB_GLOBAL_CONST ptr_to_globals;
/* At least gcc 3.4.6 on mipsel system needs optimization barrier */
#define barrier() __asm__ __volatile__("":::"memory")
#define SET_PTR_TO_GLOBALS(x) do { \
diff --git a/libbb/lineedit.c b/libbb/lineedit.c
index fbabc6c12..b9e9719c5 100644
--- a/libbb/lineedit.c
+++ b/libbb/lineedit.c
@@ -180,8 +180,7 @@ struct lineedit_statics {
#endif
};
-/* 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 )
diff --git a/libbb/lineedit_ptr_hack.c b/libbb/lineedit_ptr_hack.c
index dc45855d5..ac33bd409 100644
--- a/libbb/lineedit_ptr_hack.c
+++ b/libbb/lineedit_ptr_hack.c
@@ -18,6 +18,6 @@ struct lineedit_statics *lineedit_ptr_to_statics;
/* gcc -combine will see through and complain */
/* Using alternative method which is more likely to break
* on weird architectures, compilers, linkers and so on */
-struct lineedit_statics *const lineedit_ptr_to_statics __attribute__
((section (".data")));
+struct lineedit_statics *BB_GLOBAL_CONST lineedit_ptr_to_statics
__attribute__ ((section (".data")));
#endif
diff --git a/libbb/ptr_to_globals.c b/libbb/ptr_to_globals.c
index 8ba9cd154..26d7b2042 100644
--- a/libbb/ptr_to_globals.c
+++ b/libbb/ptr_to_globals.c
@@ -25,7 +25,7 @@ int *bb_errno;
/* gcc -combine will see through and complain */
/* Using alternative method which is more likely to break
* on weird architectures, compilers, linkers and so on */
-struct globals *const ptr_to_globals __attribute__ ((section (".data")));
+struct globals *BB_GLOBAL_CONST ptr_to_globals __attribute__
((section (".data")));
#ifdef __GLIBC__
int *const bb_errno __attribute__ ((section (".data")));
diff --git a/shell/ash.c b/shell/ash.c
index e3bbac9a0..3141f3812 100644
--- a/shell/ash.c
+++ b/shell/ash.c
@@ -288,19 +288,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. */
diff --git a/shell/ash_ptr_hack.c b/shell/ash_ptr_hack.c
index f69840825..af16cca27 100644
--- a/shell/ash_ptr_hack.c
+++ b/shell/ash_ptr_hack.c
@@ -22,8 +22,8 @@ struct globals_var *ash_ptr_to_globals_var;
/* gcc -combine will see through and complain */
/* Using alternative method which is more likely to break
* on weird architectures, compilers, linkers and so on */
-struct globals_misc *const ash_ptr_to_globals_misc __attribute__
((section (".data")));
-struct globals_memstack *const ash_ptr_to_globals_memstack
__attribute__ ((section (".data")));
-struct globals_var *const ash_ptr_to_globals_var __attribute__
((section (".data")));
+struct globals_misc *BB_GLOBAL_CONST ash_ptr_to_globals_misc
__attribute__ ((section (".data")));
+struct globals_memstack *BB_GLOBAL_CONST ash_ptr_to_globals_memstack
__attribute__ ((section (".data")));
+struct globals_var *BB_GLOBAL_CONST ash_ptr_to_globals_var
__attribute__ ((section (".data")));
#endif
_______________________________________________
busybox mailing list
[email protected]
http://lists.busybox.net/mailman/listinfo/busybox
