pw_unit_test: Do not print contents of unknown objects

Objects may contain uninitialized data (e.g. std::optional), which
should not be accessed, even just to print the bytes.

Fixes: b/303664953
Change-Id: I5427109fefde8c3f2df0a6d02065e55ac9648e7b
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/174911
Commit-Queue: Auto-Submit <auto-submit@pigweed-service-accounts.iam.gserviceaccount.com>
Pigweed-Auto-Submit: Wyatt Hepler <hepler@google.com>
Reviewed-by: Taylor Cramer <cramertj@google.com>
Presubmit-Verified: CQ Bot Account <pigweed-scoped@luci-project-accounts.iam.gserviceaccount.com>

2 files changed, 32 insertions, 21 deletions

diff --git a/pw_unit_test/framework_test.cc b/pw_unit_test/framework_test.cc
index b57a64294..2e1fe90c4 100644
--- a/pw_unit_test/framework_test.cc
+++ b/pw_unit_test/framework_test.cc
@@ -283,13 +283,13 @@ TEST(TestSuiteTearDown, MakeSureItRan) {
   EXPECT_EQ(SetUpAndTearDown::value, 8);
 }
 
-TEST(UnknownTypeToString, SmallObjectDisplaysFullContents) {
+TEST(UnknownTypeToString, SmallObject) {
   struct {
     char a = 0xa1;
   } object;
 
   StringBuffer<64> expected;
-  expected << "<1-byte object at 0x" << &object << " | a1>";
+  expected << "<1-byte object at 0x" << &object << '>';
   ASSERT_EQ(OkStatus(), expected.status());
 
   StringBuffer<64> actual;
@@ -298,14 +298,13 @@ TEST(UnknownTypeToString, SmallObjectDisplaysFullContents) {
   EXPECT_STREQ(expected.c_str(), actual.c_str());
 }
 
-TEST(UnknownTypeToString, MaxSizeToDisplayFullContents) {
+TEST(UnknownTypeToString, NineByteObject) {
   struct {
     char a[9] = {1, 2, 3, 4, 5, 6, 7, 8, 9};
   } object;
 
   StringBuffer<64> expected;
-  expected << "<9-byte object at 0x" << &object
-           << " | 01 02 03 04 05 06 07 08 09>";
+  expected << "<9-byte object at 0x" << &object << '>';
   ASSERT_EQ(OkStatus(), expected.status());
 
   StringBuffer<64> actual;
@@ -314,14 +313,13 @@ TEST(UnknownTypeToString, MaxSizeToDisplayFullContents) {
   EXPECT_STREQ(expected.c_str(), actual.c_str());
 }
 
-TEST(UnknownTypeToString, TruncatedContents) {
+TEST(UnknownTypeToString, TenByteObject) {
   struct {
     char a[10] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10};
   } object;
 
   StringBuffer<72> expected;
-  expected << "<10-byte object at 0x" << &object
-           << " | 01 02 03 04 05 06 07 08 …>";
+  expected << "<10-byte object at 0x" << &object << '>';
   ASSERT_EQ(OkStatus(), expected.status());
 
   StringBuffer<72> actual;
diff --git a/pw_unit_test/public/pw_unit_test/internal/framework.h b/pw_unit_test/public/pw_unit_test/internal/framework.h
index 25682e680..fce2293d1 100644
--- a/pw_unit_test/public/pw_unit_test/internal/framework.h
+++ b/pw_unit_test/public/pw_unit_test/internal/framework.h
@@ -164,23 +164,36 @@ namespace string {
 template <typename T>
 StatusWithSize UnknownTypeToString(const T& value, span<char> buffer) {
   StringBuilder sb(buffer);
-  sb << '<' << sizeof(value) << "-byte object at 0x" << &value << " |";
+  sb << '<' << sizeof(value) << "-byte object at 0x" << &value;
 
-  // Always show the first 8 bytes of the object.
-  constexpr size_t kBytesToPrint = std::min(sizeof(value), size_t{8});
+  // How many bytes of the object to print.
+  //
+  // WARNING: Printing the contents of an object may be undefined behavior!
+  // Accessing unintialized memory is undefined behavior, and objects sometimes
+  // contiain uninitialized regions, such as padding bytes or unalloacted
+  // storage (e.g. std::optional). kPrintMaybeUnintializedBytes MUST stay at 0,
+  // except when changed locally to help with debugging.
+  constexpr size_t kPrintMaybeUnintializedBytes = 0;
 
-  // reinterpret_cast to std::byte is permitted by C++'s type aliasing rules.
-  const std::byte* bytes = reinterpret_cast<const std::byte*>(&value);
+  constexpr size_t kBytesToPrint =
+      std::min(sizeof(value), kPrintMaybeUnintializedBytes);
 
-  for (size_t i = 0; i < kBytesToPrint; ++i) {
-    sb << ' ' << bytes[i];
-  }
+  if (kBytesToPrint != 0u) {
+    sb << " |";
+
+    // reinterpret_cast to std::byte is permitted by C++'s type aliasing rules.
+    const std::byte* bytes = reinterpret_cast<const std::byte*>(&value);
 
-  // If there's just one more byte, output it. Otherwise, output ellipsis.
-  if (sizeof(value) == kBytesToPrint + 1) {
-    sb << ' ' << bytes[sizeof(value) - 1];
-  } else if (sizeof(value) > kBytesToPrint) {
-    sb << " …";
+    for (size_t i = 0; i < kBytesToPrint; ++i) {
+      sb << ' ' << bytes[i];
+    }
+
+    // If there's just one more byte, output it. Otherwise, output ellipsis.
+    if (sizeof(value) == kBytesToPrint + 1) {
+      sb << ' ' << bytes[sizeof(value) - 1];
+    } else if (sizeof(value) > kBytesToPrint) {
+      sb << " …";
+    }
   }
 
   sb << '>';