Read-only mirrors. Development, issues and pull requests are on GitHub.

summaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
authorMartin Hořeňovský2024-10-27 17:08:47 +0100
committerMartin Hořeňovský2024-10-27 20:27:03 +0100
commite26028880790addff43cc5546de1e39214d86a04 (patch)
tree0d3dede7149aa56198e6b0ecb0f8752eb005eeb7
parent7c2e1fb1b210cf1710549fd8b2df37d700b90178 (diff)
downloadcatch3-e26028880790addff43cc5546de1e39214d86a04.tar.gz
Allow disabling use of __builtin_constant_p in internal macros
Turns out that even in GCC, the expression in `__builtin_cosntant_p` can end up evaluated and side-effects executed. To allow users to work around this bug, I added a configuration option to disable its use in internal macros. Related to #2925
-rw-r--r--BUILD.bazel2
-rw-r--r--CMake/CatchConfigOptions.cmake1
-rw-r--r--docs/configuration.md9
-rw-r--r--src/catch2/catch_user_config.hpp.in9
-rw-r--r--src/catch2/internal/catch_compiler_capabilities.hpp70
5 files changed, 60 insertions, 31 deletions
diff --git a/BUILD.bazel b/BUILD.bazel
index c51bf57e..d96c7fa7 100644
--- a/BUILD.bazel
+++ b/BUILD.bazel
@@ -56,6 +56,8 @@ expand_template(
"#cmakedefine CATCH_CONFIG_WCHAR": "",
"#cmakedefine CATCH_CONFIG_WINDOWS_CRTDBG": "",
"#cmakedefine CATCH_CONFIG_WINDOWS_SEH": "",
+ "#cmakedefine CATCH_CONFIG_USE_BUILTIN_CONSTANT_P": "",
+ "#cmakedefine CATCH_CONFIG_NO_USE_BUILTIN_CONSTANT_P": "",
},
template = "src/catch2/catch_user_config.hpp.in",
)
diff --git a/CMake/CatchConfigOptions.cmake b/CMake/CatchConfigOptions.cmake
index 6eae220d..a2f2870b 100644
--- a/CMake/CatchConfigOptions.cmake
+++ b/CMake/CatchConfigOptions.cmake
@@ -44,6 +44,7 @@ set(_OverridableOptions
"WINDOWS_SEH"
"GETENV"
"EXPERIMENTAL_STATIC_ANALYSIS_SUPPORT"
+ "USE_BUILTIN_CONSTANT_P"
)
foreach(OptionName ${_OverridableOptions})
diff --git a/docs/configuration.md b/docs/configuration.md
index 8a3ddfab..f268ba50 100644
--- a/docs/configuration.md
+++ b/docs/configuration.md
@@ -158,11 +158,14 @@ by using `_NO_` in the macro, e.g. `CATCH_CONFIG_NO_CPP17_UNCAUGHT_EXCEPTIONS`.
CATCH_CONFIG_ANDROID_LOGWRITE // Use android's logging system for debug output
CATCH_CONFIG_GLOBAL_NEXTAFTER // Use nextafter{,f,l} instead of std::nextafter
CATCH_CONFIG_GETENV // System has a working `getenv`
+ CATCH_CONFIG_USE_BUILTIN_CONSTANT_P // Use __builtin_constant_p to trigger warnings
> [`CATCH_CONFIG_ANDROID_LOGWRITE`](https://github.com/catchorg/Catch2/issues/1743) and [`CATCH_CONFIG_GLOBAL_NEXTAFTER`](https://github.com/catchorg/Catch2/pull/1739) were introduced in Catch2 2.10.0
> `CATCH_CONFIG_GETENV` was [introduced](https://github.com/catchorg/Catch2/pull/2562) in Catch2 3.2.0
+> `CATCH_CONFIG_USE_BUILTIN_CONSTANT_P` was introduced in Catch2 vX.Y.Z
+
Currently Catch enables `CATCH_CONFIG_WINDOWS_SEH` only when compiled with MSVC, because some versions of MinGW do not have the necessary Win32 API support.
`CATCH_CONFIG_POSIX_SIGNALS` is on by default, except when Catch is compiled under `Cygwin`, where it is disabled by default (but can be force-enabled by defining `CATCH_CONFIG_POSIX_SIGNALS`).
@@ -183,6 +186,12 @@ With the exception of `CATCH_CONFIG_EXPERIMENTAL_REDIRECT`,
these toggles can be disabled by using `_NO_` form of the toggle,
e.g. `CATCH_CONFIG_NO_WINDOWS_SEH`.
+`CATCH_CONFIG_USE_BUILTIN_CONSTANT_P` is ON by default for Clang and GCC
+(but as far as possible, not for other compilers masquerading for these
+two). However, it can cause bugs where the enclosed code is evaluated, even
+though it should not be, e.g. in [#2925](https://github.com/catchorg/Catch2/issues/2925).
+
+
### `CATCH_CONFIG_FAST_COMPILE`
This compile-time flag speeds up compilation of assertion macros by ~20%,
by disabling the generation of assertion-local try-catch blocks for
diff --git a/src/catch2/catch_user_config.hpp.in b/src/catch2/catch_user_config.hpp.in
index 10d61937..3acda688 100644
--- a/src/catch2/catch_user_config.hpp.in
+++ b/src/catch2/catch_user_config.hpp.in
@@ -178,6 +178,15 @@
#endif
+#cmakedefine CATCH_CONFIG_USE_BUILTIN_CONSTANT_P
+#cmakedefine CATCH_CONFIG_NO_USE_BUILTIN_CONSTANT_P
+
+#if defined( CATCH_CONFIG_USE_BUILTIN_CONSTANT_P ) && \
+ defined( CATCH_CONFIG_NO_USE_BUILTIN_CONSTANT_P )
+# error Cannot force USE_BUILTIN_CONSTANT_P to both ON and OFF
+#endif
+
+
// ------
// Simple toggle defines
// their value is never used and they cannot be overridden
diff --git a/src/catch2/internal/catch_compiler_capabilities.hpp b/src/catch2/internal/catch_compiler_capabilities.hpp
index 7f09da51..d544b00c 100644
--- a/src/catch2/internal/catch_compiler_capabilities.hpp
+++ b/src/catch2/internal/catch_compiler_capabilities.hpp
@@ -62,7 +62,7 @@
# define CATCH_INTERNAL_SUPPRESS_SHADOW_WARNINGS \
_Pragma( "GCC diagnostic ignored \"-Wshadow\"" )
-# define CATCH_INTERNAL_IGNORE_BUT_WARN(...) (void)__builtin_constant_p(__VA_ARGS__)
+# define CATCH_INTERNAL_CONFIG_USE_BUILTIN_CONSTANT_P
#endif
@@ -86,35 +86,13 @@
// clang-cl defines _MSC_VER as well as __clang__, which could cause the
// start/stop internal suppression macros to be double defined.
#if defined(__clang__) && !defined(_MSC_VER)
-
+# define CATCH_INTERNAL_CONFIG_USE_BUILTIN_CONSTANT_P
# define CATCH_INTERNAL_START_WARNINGS_SUPPRESSION _Pragma( "clang diagnostic push" )
# define CATCH_INTERNAL_STOP_WARNINGS_SUPPRESSION _Pragma( "clang diagnostic pop" )
-
#endif // __clang__ && !_MSC_VER
#if defined(__clang__)
-// As of this writing, IBM XL's implementation of __builtin_constant_p has a bug
-// which results in calls to destructors being emitted for each temporary,
-// without a matching initialization. In practice, this can result in something
-// like `std::string::~string` being called on an uninitialized value.
-//
-// For example, this code will likely segfault under IBM XL:
-// ```
-// REQUIRE(std::string("12") + "34" == "1234")
-// ```
-//
-// Similarly, NVHPC's implementation of `__builtin_constant_p` has a bug which
-// results in calls to the immediately evaluated lambda expressions to be
-// reported as unevaluated lambdas.
-// https://developer.nvidia.com/nvidia_bug/3321845.
-//
-// Therefore, `CATCH_INTERNAL_IGNORE_BUT_WARN` is not implemented.
-# if !defined(__ibmxl__) && !defined(__CUDACC__) && !defined( __NVCOMPILER )
-# define CATCH_INTERNAL_IGNORE_BUT_WARN(...) (void)__builtin_constant_p(__VA_ARGS__) /* NOLINT(cppcoreguidelines-pro-type-vararg, hicpp-vararg) */
-# endif
-
-
# define CATCH_INTERNAL_SUPPRESS_GLOBALS_WARNINGS \
_Pragma( "clang diagnostic ignored \"-Wexit-time-destructors\"" ) \
_Pragma( "clang diagnostic ignored \"-Wglobal-constructors\"")
@@ -139,6 +117,27 @@
#endif // __clang__
+// As of this writing, IBM XL's implementation of __builtin_constant_p has a bug
+// which results in calls to destructors being emitted for each temporary,
+// without a matching initialization. In practice, this can result in something
+// like `std::string::~string` being called on an uninitialized value.
+//
+// For example, this code will likely segfault under IBM XL:
+// ```
+// REQUIRE(std::string("12") + "34" == "1234")
+// ```
+//
+// Similarly, NVHPC's implementation of `__builtin_constant_p` has a bug which
+// results in calls to the immediately evaluated lambda expressions to be
+// reported as unevaluated lambdas.
+// https://developer.nvidia.com/nvidia_bug/3321845.
+//
+// Therefore, `CATCH_INTERNAL_IGNORE_BUT_WARN` is not implemented.
+#if defined( __ibmxl__ ) || defined( __CUDACC__ ) || defined( __NVCOMPILER )
+# define CATCH_INTERNAL_CONFIG_NO_USE_BUILTIN_CONSTANT_P
+#endif
+
+
////////////////////////////////////////////////////////////////////////////////
// We know some environments not to support full POSIX signals
@@ -362,6 +361,22 @@
#endif
+// The goal of this macro is to avoid evaluation of the arguments, but
+// still have the compiler warn on problems inside...
+#if defined( CATCH_INTERNAL_CONFIG_USE_BUILTIN_CONSTANT_P ) && \
+ !defined( CATCH_INTERNAL_CONFIG_NO_USE_BUILTIN_CONSTANT_P ) && !defined(CATCH_CONFIG_USE_BUILTIN_CONSTANT_P)
+#define CATCH_CONFIG_USE_BUILTIN_CONSTANT_P
+#endif
+
+#if defined( CATCH_CONFIG_USE_BUILTIN_CONSTANT_P ) && \
+ !defined( CATCH_CONFIG_NO_USE_BUILTIN_CONSTANT_P )
+# define CATCH_INTERNAL_IGNORE_BUT_WARN( ... ) \
+ (void)__builtin_constant_p( __VA_ARGS__ ) /* NOLINT(cppcoreguidelines-pro-type-vararg, \
+ hicpp-vararg) */
+#else
+# define CATCH_INTERNAL_IGNORE_BUT_WARN( ... )
+#endif
+
// Even if we do not think the compiler has that warning, we still have
// to provide a macro that can be used by the code.
#if !defined(CATCH_INTERNAL_START_WARNINGS_SUPPRESSION)
@@ -398,13 +413,6 @@
# define CATCH_INTERNAL_SUPPRESS_SHADOW_WARNINGS
#endif
-
-// The goal of this macro is to avoid evaluation of the arguments, but
-// still have the compiler warn on problems inside...
-#if !defined(CATCH_INTERNAL_IGNORE_BUT_WARN)
-# define CATCH_INTERNAL_IGNORE_BUT_WARN(...)
-#endif
-
#if defined(__APPLE__) && defined(__apple_build_version__) && (__clang_major__ < 10)
# undef CATCH_INTERNAL_SUPPRESS_UNUSED_TEMPLATE_WARNINGS
#elif defined(__clang__) && (__clang_major__ < 5)