When built at -Os, gcc-12 recognizes an strlen() pattern in nolibc_strlen()
and replaces it with a jump to strlen(), which is not defined as a symbol
and breaks compilation. Worse, when the function is called strlen(), the
function is simply replaced with a jump to itself, hence becomes an
infinite loop.
One way to avoid this is to always set -ffreestanding, but the calling
code doesn't know this and there's no way (either via attributes or
pragmas) to globally enable it from include files, effectively leaving
a painful situation for the caller.
It turns out that -fno-tree-loop-distribute-patterns disables replacement
of strlen-like loops with calls to strlen and that this option is accepted
in the optimize() function attribute. Thus at least it allows us to make
sure our local definition is not replaced with a self jump. The function
only needs to be renamed back to strlen() so that the symbol exists, which
implies that nolibc_strlen() which is used on variable strings has to be
declared as a macro that points back to it before the strlen() macro is
redifined.
It was verified to produce valid code with gcc 3.4 to 12.1 at different
optimization levels, and both with constant and variable strings.
Reported-by: kernel test robot <yujie.liu(a)intel.com>
Link: https://lore.kernel.org/r/202210081618.754a77db-yujie.liu@intel.com
Fixes: 66b6f755ad45 ("rcutorture: Import a copy of nolibc")
Fixes: 96980b833a21 ("tools/nolibc/string: do not use __builtin_strlen() at -O0")
Cc: "Paul E. McKenney" <paulmck(a)kernel.org>
Signed-off-by: Willy Tarreau <w(a)1wt.eu>
---
tools/include/nolibc/string.h | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/tools/include/nolibc/string.h b/tools/include/nolibc/string.h
index bef35bee9c44..5ef8778cd16f 100644
--- a/tools/include/nolibc/string.h
+++ b/tools/include/nolibc/string.h
@@ -125,10 +125,16 @@ char *strcpy(char *dst, const char *src)
}
/* this function is only used with arguments that are not constants or when
- * it's not known because optimizations are disabled.
+ * it's not known because optimizations are disabled. Note that gcc 12
+ * recognizes an strlen() pattern and replaces it with a jump to strlen(),
+ * thus itself, hence the optimize() attribute below that's meant to disable
+ * this confusing practice.
*/
+#if defined(__GNUC__) && (__GNUC__ >= 12)
+__attribute__((optimize("no-tree-loop-distribute-patterns")))
+#endif
static __attribute__((unused))
-size_t nolibc_strlen(const char *str)
+size_t strlen(const char *str)
{
size_t len;
@@ -140,13 +146,12 @@ size_t nolibc_strlen(const char *str)
* the two branches, then will rely on an external definition of strlen().
*/
#if defined(__OPTIMIZE__)
+#define nolibc_strlen(x) strlen(x)
#define strlen(str) ({ \
__builtin_constant_p((str)) ? \
__builtin_strlen((str)) : \
nolibc_strlen((str)); \
})
-#else
-#define strlen(str) nolibc_strlen((str))
#endif
static __attribute__((unused))
--
2.35.3
This change enables to extend CFLAGS and LDFLAGS from command line, e.g.
to extend compiler checks: make USERCFLAGS=-Werror USERLDFLAGS=-static
USERCFLAGS and USERLDFLAGS are documented in
Documentation/kbuild/makefiles.rst and Documentation/kbuild/kbuild.rst
This should be backported (down to 5.10) to improve previous kernel
versions testing as well.
Cc: Shuah Khan <skhan(a)linuxfoundation.org>
Cc: stable(a)vger.kernel.org
Signed-off-by: Mickaël Salaün <mic(a)digikod.net>
Link: https://lore.kernel.org/r/20220909103901.1503436-1-mic@digikod.net
---
tools/testing/selftests/lib.mk | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/tools/testing/selftests/lib.mk b/tools/testing/selftests/lib.mk
index d44c72b3abe3..da47a0257165 100644
--- a/tools/testing/selftests/lib.mk
+++ b/tools/testing/selftests/lib.mk
@@ -119,6 +119,11 @@ endef
clean:
$(CLEAN)
+# Enables to extend CFLAGS and LDFLAGS from command line, e.g.
+# make USERCFLAGS=-Werror USERLDFLAGS=-static
+CFLAGS += $(USERCFLAGS)
+LDFLAGS += $(USERLDFLAGS)
+
# When make O= with kselftest target from main level
# the following aren't defined.
#
base-commit: 7e18e42e4b280c85b76967a9106a13ca61c16179
--
2.37.2
Enable the KASAN/KUnit integration even when the KASAN tests are
disabled, as it's useful for testing other things under KASAN.
Essentially, this reverts commit 49d9977ac909 ("kasan: check CONFIG_KASAN_KUNIT_TEST instead of CONFIG_KUNIT").
To mitigate the performance impact slightly, add a likely() to the check
for a currently running test.
There's more we can do for performance if/when it becomes more of a
problem, such as only enabling the "expect a KASAN failure" support wif
the KASAN tests are enabled, or putting the whole thing behind a "kunit
tests are running" static branch (which I do plan to do eventually).
Fixes: 49d9977ac909 ("kasan: check CONFIG_KASAN_KUNIT_TEST instead of CONFIG_KUNIT")
Signed-off-by: David Gow <davidgow(a)google.com>
---
Basically, hiding the KASAN/KUnit integration broke being able to just
pass --kconfig_add CONFIG_KASAN=y to kunit_tool to enable KASAN
integration. We didn't notice this, because usually
CONFIG_KUNIT_ALL_TESTS is enabled, which in turn enables
CONFIG_KASAN_KUNIT_TEST. However, using a separate .kunitconfig might
result in failures being missed.
Take, for example:
./tools/testing/kunit/kunit.py run --kconfig_add CONFIG_KASAN=y \
--kunitconfig drivers/gpu/drm/tests
This should run the drm tests with KASAN enabled, but even if there's a
KASAN failure (such as the one fixed by [1]), kunit_tool will report
success.
[1]: https://lore.kernel.org/dri-devel/20221019073239.3779180-1-davidgow@google.…
---
mm/kasan/kasan.h | 2 +-
mm/kasan/report.c | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h
index abbcc1b0eec5..afacef14c7f4 100644
--- a/mm/kasan/kasan.h
+++ b/mm/kasan/kasan.h
@@ -261,7 +261,7 @@ struct kasan_stack_ring {
#endif /* CONFIG_KASAN_SW_TAGS || CONFIG_KASAN_HW_TAGS */
-#if IS_ENABLED(CONFIG_KASAN_KUNIT_TEST)
+#if IS_ENABLED(CONFIG_KUNIT)
/* Used in KUnit-compatible KASAN tests. */
struct kunit_kasan_status {
bool report_found;
diff --git a/mm/kasan/report.c b/mm/kasan/report.c
index df3602062bfd..efa063b9d093 100644
--- a/mm/kasan/report.c
+++ b/mm/kasan/report.c
@@ -114,7 +114,7 @@ EXPORT_SYMBOL_GPL(kasan_restore_multi_shot);
#endif
-#if IS_ENABLED(CONFIG_KASAN_KUNIT_TEST)
+#if IS_ENABLED(CONFIG_KUNIT)
static void update_kunit_status(bool sync)
{
struct kunit *test;
@@ -122,7 +122,7 @@ static void update_kunit_status(bool sync)
struct kunit_kasan_status *status;
test = current->kunit_test;
- if (!test)
+ if (likely(!test))
return;
resource = kunit_find_named_resource(test, "kasan_status");
--
2.38.0.413.g74048e4d9e-goog