Fix the CHECK_XX family of macros to not print char* arguments as C-strings if the comparison happened as pointers.

Printing as pointers is more relevant to the result of the comparison.

PiperOrigin-RevId: 824656335
Change-Id: I3b586e633c1271c1728c961193db106e4f4e5a8b
This commit is contained in:
Samuel Benzaquen
2025-10-27 13:33:57 -07:00
committed by Copybara-Service
parent b8c46feecc
commit f0a99676ff
2 changed files with 89 additions and 8 deletions

View File

@@ -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 <typename CharT>
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<char>();
TestCharStarComparison<signed char>();
TestCharStarComparison<unsigned char>();
TestCharStarComparison<wchar_t>();
#if defined(__cpp_char8_t)
TestCharStarComparison<char8_t>();
#endif
TestCharStarComparison<char16_t>();
TestCharStarComparison<char32_t>();
}
// 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),

View File

@@ -393,10 +393,35 @@ std::enable_if_t<
std::conditional_t<std::is_signed_v<UnderlyingTypeT<T>>,
int64_t, uint64_t>>>
Detect(...);
} // namespace detect_specialization
template <typename T>
using CheckOpStreamType = decltype(detect_specialization::Detect<T>(0));
using Detected = decltype(Detect<T>(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 <typename T>
constexpr bool IsCharStarOrVoidStar() {
if constexpr (std::is_reference_v<T>) {
return IsCharStarOrVoidStar<std::remove_reference_t<T>>();
} else if constexpr (std::is_array_v<T>) {
return IsCharStarOrVoidStar<std::decay_t<T>>();
} else {
using U = std::remove_const_t<std::remove_pointer_t<T>>;
return std::is_pointer_v<T> &&
(std::is_same_v<char, U> || std::is_same_v<unsigned char, U> ||
std::is_same_v<signed char, U> || std::is_void_v<U>);
}
}
template <typename T1, typename T2,
typename U1 = detect_specialization::Detected<T1>,
typename U2 = detect_specialization::Detected<T2>>
using CheckOpStreamType =
std::conditional_t<IsCharStarOrVoidStar<U1>() && IsCharStarOrVoidStar<U2>(),
const void*, U1>;
// Build the error message string. Specify no inlining for code size.
template <typename T1, typename T2>
@@ -406,8 +431,8 @@ ABSL_ATTRIBUTE_RETURNS_NONNULL const char* absl_nonnull MakeCheckOpString(
template <typename T1, typename T2>
const char* absl_nonnull MakeCheckOpString(T1 v1, T2 v2,
const char* absl_nonnull exprtext) {
if constexpr (std::is_same_v<CheckOpStreamType<T1>, UnprintableWrapper> &&
std::is_same_v<CheckOpStreamType<T2>, UnprintableWrapper>) {
if constexpr (std::is_same_v<CheckOpStreamType<T1, T2>, UnprintableWrapper> &&
std::is_same_v<CheckOpStreamType<T2, T1>, 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 <typename T1, typename T2> \
inline constexpr const char* absl_nullable name##Impl( \
const T1& v1, const T2& v2, const char* absl_nonnull exprtext) { \
using U1 = CheckOpStreamType<T1>; \
using U2 = CheckOpStreamType<T2>; \
using U1 = CheckOpStreamType<T1, T2>; \
using U2 = CheckOpStreamType<T2, T1>; \
return ABSL_PREDICT_TRUE(v1 op v2) \
? nullptr \
: ABSL_LOG_INTERNAL_CHECK_OP_IMPL_RESULT(U1, U2, U1(v1), \