[libcxx] [test] Fix odr_signature tests with optimizations enabled

[mstorsjo]

If optimization is enabled, the inline f() function actually gets inlined, meaning that the functions tu1() and tu2() trivially return 1 and 2, instead of actually referencing the potentially linker deduplicated function f(), which is what the test tries to test.

Therefore, this test previously actually failed to test what it was supposed to test, if optimization was enabled.

Mark the inline functions with TEST_NOINLINE to make sure that they don't get inlined even with optimizations enabled.

Also update the TODO comments to explain why we have an XFAIL for msvc mode here.

This avoids these tests unexpectedly passing if building in msvc mode, with optimizations enabled
(-DLIBCXX_TEST_PARAMS="optimization=speed").

[llvmbot]

@llvm/pr-subscribers-libcxx

Author: Martin Storsjö (mstorsjo)

Changes

If optimization is enabled, the inline f() function actually gets inlined, meaning that the functions tu1() and tu2() trivially return 1 and 2, instead of actually referencing the potentially linker deduplicated function f(), which is what the test tries to test.

Therefore, this test previously actually failed to test what it was supposed to test, if optimization was enabled.

Mark the inline functions with __attribute__((noinline)) to make sure that they don't get inlined even with optimizations enabled.

Also update the TODO comments to explain why we have an XFAIL for msvc mode here.

This avoids these tests unexpectedly passing if building in msvc mode, with optimizations enabled
(-DLIBCXX_TEST_PARAMS="optimization=speed").

---

Full diff: https://github.com/llvm/llvm-project/pull/144317.diff

2 Files Affected:

• (modified) libcxx/test/libcxx/odr_signature.exceptions.sh.cpp (+3-3)
• (modified) libcxx/test/libcxx/odr_signature.hardening.sh.cpp (+5-5)

diff --git a/libcxx/test/libcxx/odr_signature.exceptions.sh.cpp b/libcxx/test/libcxx/odr_signature.exceptions.sh.cpp
index 6bf60b5e82d3c..8bc7f6cffbd0c 100644
--- a/libcxx/test/libcxx/odr_signature.exceptions.sh.cpp
+++ b/libcxx/test/libcxx/odr_signature.exceptions.sh.cpp
@@ -6,7 +6,7 @@
 //
 //===----------------------------------------------------------------------===//
 
-// TODO: Investigate
+// ABI tags have no effect in MSVC mode.
 // XFAIL: msvc
 
 // Test that we encode whether exceptions are supported in an ABI tag to avoid
@@ -21,14 +21,14 @@
 // -fno-exceptions
 #ifdef TU1
 #  include <__config>
-_LIBCPP_HIDE_FROM_ABI inline int f() { return 1; }
+_LIBCPP_HIDE_FROM_ABI __attribute__((noinline)) inline int f() { return 1; }
 int tu1() { return f(); }
 #endif // TU1
 
 // -fexceptions
 #ifdef TU2
 #  include <__config>
-_LIBCPP_HIDE_FROM_ABI inline int f() { return 2; }
+_LIBCPP_HIDE_FROM_ABI __attribute__((noinline)) inline int f() { return 2; }
 int tu2() { return f(); }
 #endif // TU2
 
diff --git a/libcxx/test/libcxx/odr_signature.hardening.sh.cpp b/libcxx/test/libcxx/odr_signature.hardening.sh.cpp
index 0dc280bf28182..e2ea6bf4a597a 100644
--- a/libcxx/test/libcxx/odr_signature.hardening.sh.cpp
+++ b/libcxx/test/libcxx/odr_signature.hardening.sh.cpp
@@ -6,7 +6,7 @@
 //
 //===----------------------------------------------------------------------===//
 
-// TODO: Investigate
+// ABI tags have no effect in MSVC mode.
 // XFAIL: msvc
 
 // Test that we encode the hardening mode in an ABI tag to avoid ODR violations
@@ -24,28 +24,28 @@
 // fast hardening mode
 #ifdef TU1
 #  include <__config>
-_LIBCPP_HIDE_FROM_ABI inline int f() { return 1; }
+_LIBCPP_HIDE_FROM_ABI __attribute__((noinline)) inline int f() { return 1; }
 int tu1() { return f(); }
 #endif // TU1
 
 // extensive hardening mode
 #ifdef TU2
 #  include <__config>
-_LIBCPP_HIDE_FROM_ABI inline int f() { return 2; }
+_LIBCPP_HIDE_FROM_ABI __attribute__((noinline)) inline int f() { return 2; }
 int tu2() { return f(); }
 #endif // TU2
 
 // debug hardening mode
 #ifdef TU3
 #  include <__config>
-_LIBCPP_HIDE_FROM_ABI inline int f() { return 3; }
+_LIBCPP_HIDE_FROM_ABI __attribute__((noinline)) inline int f() { return 3; }
 int tu3() { return f(); }
 #endif // TU3
 
 // No hardening
 #ifdef TU4
 #  include <__config>
-_LIBCPP_HIDE_FROM_ABI inline int f() { return 4; }
+_LIBCPP_HIDE_FROM_ABI __attribute__((noinline)) inline int f() { return 4; }
 int tu4() { return f(); }
 #endif // TU4
 

[philnik777]

We should probably document that we're not providing our per-TU guarantees on MSVC?

[mstorsjo]

I guess we should do that somewhere, yes.

[philnik777]

Let's use TEST_NOINLINE instead.

[mstorsjo]

Ok, can do. These test files don't include anything before the #ifdef TU*, where we manually include <__config>, does it disturb something of that setup if we include "test_macros.h" globally before this?

[philnik777]

I'm not sure why it's this way in the first place TBH. I don't think it'll make a difference.

[mstorsjo]

Ok, changed to TEST_NOINLINE with an include of "test_macros.h" now.