diff --git a/absl/log/check_test_impl.inc b/absl/log/check_test_impl.inc index 495f85a2..47af1dd0 100644 --- a/absl/log/check_test_impl.inc +++ b/absl/log/check_test_impl.inc @@ -255,6 +255,60 @@ TEST(CHECKTest, TestBinaryChecksWithStringComparison) { ABSL_TEST_CHECK_LT(a, "b"); } +TEST(CHECKDeathTest, CheckWithCharStarAndStringPrintsTheCharStar) { + std::string str = "B"; + + // When the comparison happens as strings, then we print the CharT* as a + // string. + EXPECT_DEATH(ABSL_TEST_CHECK_EQ("A", str), + R"re(Check failed: \"A\" == str \(A vs. B\))re"); +} + +#if defined(GTEST_USES_SIMPLE_RE) && GTEST_USES_SIMPLE_RE +#define POINTER_VALUE_RE R"re(\w*)re" +#else +#define POINTER_VALUE_RE R"re((0x)*[0-9a-fA-F]*)re" +#endif + +#define POINTER_VS_POINTER_RE(lhs, rhs) "\\(" lhs " vs. " rhs "\\)" + +template +void TestCharStarComparison() { + // When the comparison happens as pointers, we only print the pointer and not + // interpret it as a C-String because it might not be. + // Leave the CharTs uninitialized to trigger ASan/MSan failures if we actually + // read the pointers. + CharT* p1 = new CharT; + CharT* p2 = new CharT; + EXPECT_DEATH(ABSL_TEST_CHECK_EQ(p1, p2), + R"re(Check failed: p1 == p2 )re" POINTER_VS_POINTER_RE( + POINTER_VALUE_RE, POINTER_VALUE_RE)); + CharT as_array[10]; + EXPECT_DEATH(ABSL_TEST_CHECK_EQ(p1, as_array), + R"re(Check failed: p1 == as_array )re" POINTER_VS_POINTER_RE( + POINTER_VALUE_RE, POINTER_VALUE_RE)); + + const void* as_void = as_array; + EXPECT_DEATH(ABSL_TEST_CHECK_EQ(as_void, p2), + R"re(Check failed: as_void == p2 )re" POINTER_VS_POINTER_RE( + POINTER_VALUE_RE, POINTER_VALUE_RE)); + + delete p1; + delete p2; +} + +TEST(CHECKDeathTest, CheckWithCharStarStringification) { + TestCharStarComparison(); + TestCharStarComparison(); + TestCharStarComparison(); + TestCharStarComparison(); +#if defined(__cpp_char8_t) + TestCharStarComparison(); +#endif + TestCharStarComparison(); + TestCharStarComparison(); +} + // For testing using CHECK*() on anonymous enums. enum { CASE_A, CASE_B }; @@ -339,9 +393,11 @@ TEST(CHECKDeathTest, TestNullValuesAreReportedCleanly) { a = "xx"; EXPECT_DEATH(ABSL_TEST_CHECK_EQ(a, b), - "Check failed: a == b \\(xx vs. \\(null\\)\\)"); + R"re(Check failed: a == b )re" POINTER_VS_POINTER_RE( + POINTER_VALUE_RE, "\\(null\\)")); EXPECT_DEATH(ABSL_TEST_CHECK_EQ(b, a), - "Check failed: b == a \\(\\(null\\) vs. xx\\)"); + R"re(Check failed: b == a )re" POINTER_VS_POINTER_RE( + "\\(null\\)", POINTER_VALUE_RE)); std::nullptr_t n{}; EXPECT_DEATH(ABSL_TEST_CHECK_NE(n, nullptr), diff --git a/absl/log/internal/check_op.h b/absl/log/internal/check_op.h index 532b37ca..9bf908e8 100644 --- a/absl/log/internal/check_op.h +++ b/absl/log/internal/check_op.h @@ -393,10 +393,35 @@ std::enable_if_t< std::conditional_t>, int64_t, uint64_t>>> Detect(...); -} // namespace detect_specialization template -using CheckOpStreamType = decltype(detect_specialization::Detect(0)); +using Detected = decltype(Detect(0)); +} // namespace detect_specialization + +// If the comparison will happen as pointers, decay `char*` arguments to `void*` +// when printing them. There is no evidence that they are a NULL terminated +// C-String so printing them as such could lead to UB, and more importantly we +// compared pointers so showing the pointers is a better result. +template +constexpr bool IsCharStarOrVoidStar() { + if constexpr (std::is_reference_v) { + return IsCharStarOrVoidStar>(); + } else if constexpr (std::is_array_v) { + return IsCharStarOrVoidStar>(); + } else { + using U = std::remove_const_t>; + return std::is_pointer_v && + (std::is_same_v || std::is_same_v || + std::is_same_v || std::is_void_v); + } +} + +template , + typename U2 = detect_specialization::Detected> +using CheckOpStreamType = + std::conditional_t() && IsCharStarOrVoidStar(), + const void*, U1>; // Build the error message string. Specify no inlining for code size. template @@ -406,8 +431,8 @@ ABSL_ATTRIBUTE_RETURNS_NONNULL const char* absl_nonnull MakeCheckOpString( template const char* absl_nonnull MakeCheckOpString(T1 v1, T2 v2, const char* absl_nonnull exprtext) { - if constexpr (std::is_same_v, UnprintableWrapper> && - std::is_same_v, UnprintableWrapper>) { + if constexpr (std::is_same_v, UnprintableWrapper> && + std::is_same_v, UnprintableWrapper>) { // No sense printing " (UNPRINTABLE vs. UNPRINTABLE)" return exprtext; } else { @@ -462,8 +487,8 @@ ABSL_LOG_INTERNAL_DEFINE_MAKE_CHECK_OP_STRING_EXTERN(const void* absl_nonnull); template \ inline constexpr const char* absl_nullable name##Impl( \ const T1& v1, const T2& v2, const char* absl_nonnull exprtext) { \ - using U1 = CheckOpStreamType; \ - using U2 = CheckOpStreamType; \ + using U1 = CheckOpStreamType; \ + using U2 = CheckOpStreamType; \ return ABSL_PREDICT_TRUE(v1 op v2) \ ? nullptr \ : ABSL_LOG_INTERNAL_CHECK_OP_IMPL_RESULT(U1, U2, U1(v1), \