selftests: do not macro-expand failed assertion expressions

I've stumbled over the current macro-expand behaviour of the test harness:

$ gcc -Wall -xc - <<'__EOF__'
TEST(macro) {
        int status = 0;
        ASSERT_TRUE(WIFSIGNALED(status));
}
TEST_HARNESS_MAIN
__EOF__
$ ./a.out
[==========] Running 1 tests from 1 test cases.
[ RUN      ] global.macro
<stdin>:4:global.macro:Expected 0 (0) != (((signed char) (((status) & 0x7f) + 1) >> 1) > 0) (0)
global.macro: Test terminated by assertion
[     FAIL ] global.macro
[==========] 0 / 1 tests passed.
[  FAILED  ]

With this change the output of the same test looks much more comprehensible:

[==========] Running 1 tests from 1 test cases.
[ RUN ] global.macro
<stdin>:4:global.macro:Expected 0 (0) != WIFSIGNALED(status) (0) global.macro: Test terminated by assertion
[ FAIL ] global.macro
[==========] 0 / 1 tests passed.
[ FAILED ]

The issue is very similar to the bug fixed in glibc assert(3) three years ago:
https://sourceware.org/bugzilla/show_bug.cgi?id=18604

Cc Shuah Khan <shuah at kernel.org>

Cc Kees Cook <keescook at chromium.org>

Cc Andy Lutomirski <luto at amacapital.net>

Cc Will Drewry <wad at chromium.org>

Cc linux-kselftest at vger.kernel.org

Signed-off-by Dmitry V. Levin <ldv at altlinux.org>

---
 tools/testing/selftests/kselftest_harness.h | 42 ++++++++++-----------
 1 file changed, 21 insertions(+), 21 deletions(-)

tools/testing/selftests/kselftest_harness.h +21 -22

index 6ae3730c4ee3..76d654ef3234 100644
@@ -354,7 +354,7 @@
  * ASSERT_EQ(expected, measured): expected == measured
  */
 #define ASSERT_EQ(expected, seen) \
-       __EXPECT(expected, seen, ==, 1)
+       __EXPECT(expected, #expected, seen, #seen, ==, 1)
 
 /**
  * ASSERT_NE(expected, seen)

@@ -365,7 +365,7 @@
  * ASSERT_NE(expected, measured): expected != measured
  */
 #define ASSERT_NE(expected, seen) \
-       __EXPECT(expected, seen, !=, 1)
+       __EXPECT(expected, #expected, seen, #seen, !=, 1)
 
 /**
  * ASSERT_LT(expected, seen)

@@ -376,7 +376,7 @@
  * ASSERT_LT(expected, measured): expected < measured
  */
 #define ASSERT_LT(expected, seen) \
-       __EXPECT(expected, seen, <, 1)
+       __EXPECT(expected, #expected, seen, #seen, <, 1)
 
 /**
  * ASSERT_LE(expected, seen)

@@ -387,7 +387,7 @@
  * ASSERT_LE(expected, measured): expected <= measured
  */
 #define ASSERT_LE(expected, seen) \
-       __EXPECT(expected, seen, <=, 1)
+       __EXPECT(expected, #expected, seen, #seen, <=, 1)
 
 /**
  * ASSERT_GT(expected, seen)

@@ -398,7 +398,7 @@
  * ASSERT_GT(expected, measured): expected > measured
  */
 #define ASSERT_GT(expected, seen) \
-       __EXPECT(expected, seen, >, 1)
+       __EXPECT(expected, #expected, seen, #seen, >, 1)
 
 /**
  * ASSERT_GE(expected, seen)

@@ -409,7 +409,7 @@
  * ASSERT_GE(expected, measured): expected >= measured
  */
 #define ASSERT_GE(expected, seen) \
-       __EXPECT(expected, seen, >=, 1)
+       __EXPECT(expected, #expected, seen, #seen, >=, 1)
 
 /**
  * ASSERT_NULL(seen)

@@ -419,7 +419,7 @@
  * ASSERT_NULL(measured): NULL == measured
  */
 #define ASSERT_NULL(seen) \
-       __EXPECT(NULL, seen, ==, 1)
+       __EXPECT(NULL, "NULL", seen, #seen, ==, 1)
 
 /**
  * ASSERT_TRUE(seen)

@@ -429,7 +429,7 @@
  * ASSERT_TRUE(measured): measured != 0
  */
 #define ASSERT_TRUE(seen) \
-       ASSERT_NE(0, seen)
+       __EXPECT(0, "0", seen, #seen, !=, 1)
 
 /**
  * ASSERT_FALSE(seen)

@@ -439,7 +439,7 @@
  * ASSERT_FALSE(measured): measured == 0
  */
 #define ASSERT_FALSE(seen) \
-       ASSERT_EQ(0, seen)
+       __EXPECT(0, "0", seen, #seen, ==, 1)
 
 /**
  * ASSERT_STREQ(expected, seen)

@@ -472,7 +472,7 @@
  * EXPECT_EQ(expected, measured): expected == measured
  */
 #define EXPECT_EQ(expected, seen) \
-       __EXPECT(expected, seen, ==, 0)
+       __EXPECT(expected, #expected, seen, #seen, ==, 0)
 
 /**
  * EXPECT_NE(expected, seen)

@@ -483,7 +483,7 @@
  * EXPECT_NE(expected, measured): expected != measured
  */
 #define EXPECT_NE(expected, seen) \
-       __EXPECT(expected, seen, !=, 0)
+       __EXPECT(expected, #expected, seen, #seen, !=, 0)
 
 /**
  * EXPECT_LT(expected, seen)

@@ -494,7 +494,7 @@
  * EXPECT_LT(expected, measured): expected < measured
  */
 #define EXPECT_LT(expected, seen) \
-       __EXPECT(expected, seen, <, 0)
+       __EXPECT(expected, #expected, seen, #seen, <, 0)
 
 /**
  * EXPECT_LE(expected, seen)

@@ -505,7 +505,7 @@
  * EXPECT_LE(expected, measured): expected <= measured
  */
 #define EXPECT_LE(expected, seen) \
-       __EXPECT(expected, seen, <=, 0)
+       __EXPECT(expected, #expected, seen, #seen, <=, 0)
 
 /**
  * EXPECT_GT(expected, seen)

@@ -516,7 +516,7 @@
  * EXPECT_GT(expected, measured): expected > measured
  */
 #define EXPECT_GT(expected, seen) \
-       __EXPECT(expected, seen, >, 0)
+       __EXPECT(expected, #expected, seen, #seen, >, 0)
 
 /**
  * EXPECT_GE(expected, seen)

@@ -527,7 +527,7 @@
  * EXPECT_GE(expected, measured): expected >= measured
  */
 #define EXPECT_GE(expected, seen) \
-       __EXPECT(expected, seen, >=, 0)
+       __EXPECT(expected, #expected, seen, #seen, >=, 0)
 
 /**
  * EXPECT_NULL(seen)

@@ -537,7 +537,7 @@
  * EXPECT_NULL(measured): NULL == measured
  */
 #define EXPECT_NULL(seen) \
-       __EXPECT(NULL, seen, ==, 0)
+       __EXPECT(NULL, "NULL", seen, #seen, ==, 0)
 
 /**
  * EXPECT_TRUE(seen)

@@ -547,7 +547,7 @@
  * EXPECT_TRUE(measured): 0 != measured
  */
 #define EXPECT_TRUE(seen) \
-       EXPECT_NE(0, seen)
+       __EXPECT(0, "0", seen, #seen, !=, 0)
 
 /**
  * EXPECT_FALSE(seen)

@@ -557,7 +557,7 @@
  * EXPECT_FALSE(measured): 0 == measured
  */
 #define EXPECT_FALSE(seen) \
-       EXPECT_EQ(0, seen)
+       __EXPECT(0, "0", seen, #seen, ==, 0)
 
 /**
  * EXPECT_STREQ(expected, seen)

@@ -597,7 +597,7 @@
        if (_metadata->passed && _metadata->step < 255) \
                _metadata->step++;
 
-#define __EXPECT(_expected, _seen, _t, _assert) do { \
+#define __EXPECT(_expected, _expected_str, _seen, _seen_str, _t, _assert) do { \
        /* Avoid multiple evaluation of the cases */ \
        __typeof__(_expected) __exp = (_expected); \
        __typeof__(_seen) __seen = (_seen); \

@@ -606,8 +606,8 @@
                unsigned long long __exp_print = (uintptr_t)__exp; \
                unsigned long long __seen_print = (uintptr_t)__seen; \
                __TH_LOG("Expected %s (%llu) %s %s (%llu)", \
-                        #_expected, __exp_print, #_t, \
-                        #_seen, __seen_print); \
+                        _expected_str, __exp_print, #_t, \
+                        _seen_str, __seen_print); \
                _metadata->passed = 0; \
                /* Ensure the optional handler is triggered */ \
                _metadata->trigger = 1; \
--

ldv

Quoted 35 lines · Dmitry V. Levin

I've stumbled over the current macro-expand behaviour of the test harness:

$ gcc -Wall -xc - <<'__EOF__'
TEST(macro) {
         int status = 0;
         ASSERT_TRUE(WIFSIGNALED(status));
}
TEST_HARNESS_MAIN
__EOF__
$ ./a.out
[==========] Running 1 tests from 1 test cases.
[ RUN      ] global.macro
<stdin>:4:global.macro:Expected 0 (0) != (((signed char) (((status) & 0x7f) + 1) >> 1) > 0) (0)
global.macro: Test terminated by assertion
[     FAIL ] global.macro
[==========] 0 / 1 tests passed.
[  FAILED  ]

With this change the output of the same test looks much more comprehensible:

[==========] Running 1 tests from 1 test cases.
[ RUN ] global.macro
<stdin>:4:global.macro:Expected 0 (0) != WIFSIGNALED(status) (0) global.macro: Test terminated by assertion
[ FAIL ] global.macro
[==========] 0 / 1 tests passed.
[ FAILED ]

The issue is very similar to the bug fixed in glibc assert(3) three years ago:
https://sourceware.org/bugzilla/show_bug.cgi?id=18604

Quoted 6 lines

Cc Shuah Khan <shuah at kernel.org>

Cc Kees Cook <keescook at chromium.org>

Cc Andy Lutomirski <luto at amacapital.net>

Cc Will Drewry <wad at chromium.org>

Cc linux-kselftest at vger.kernel.org

Signed-off-by Dmitry V. Levin <ldv at altlinux.org>

Yeah, good idea.

Acked-by Kees Cook <keescook at chromium.org>