commit 4431e54d1f80867890e340d89c4cd7584f5a0653
author James Price <jrprice@google.com> Thu Jun 27 17:45:44 2024 +0000
committer Dawn LUCI CQ <dawn-scoped@luci-project-accounts.iam.gserviceaccount.com> Thu Jun 27 17:45:44 2024 +0000
tree 97e1474e14df8cdb133790f097035f70e26f4183
parent 6d578e820b0ec229f39341be2ca8dcdbf835173f

[tint] Fix UB in FileContent copy constructor

Calling `front()` on an empty string_view is undefined behavior, and
crashes on macOS. This only affects our benchmarks.

Change-Id: I5342833bc0b0d7ad5ce494c9776c0ba4118699f1
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/196094
Reviewed-by: dan sinclair <dsinclair@chromium.org>
Commit-Queue: James Price <jrprice@google.com>
Auto-Submit: James Price <jrprice@google.com>
Commit-Queue: dan sinclair <dsinclair@chromium.org>

src/tint/utils/diagnostic/source.cc

diff --git a/src/tint/utils/diagnostic/source.cc b/src/tint/utils/diagnostic/source.cc
index 8f07d77..8b036e8 100644
--- a/src/tint/utils/diagnostic/source.cc
+++ b/src/tint/utils/diagnostic/source.cc
@@ -118,6 +118,9 @@
                                                       const std::string_view& dst_view) {
     std::vector<std::string_view> out(src_list.size());
     for (size_t i = 0; i < src_list.size(); i++) {
+        if (src_list[i].empty()) {
+            continue;
+        }
         auto offset = static_cast<size_t>(&src_list[i].front() - &src_view.front());
         auto count = src_list[i].length();
         out[i] = dst_view.substr(offset, count);

src/tint/utils/diagnostic/source_test.cc

diff --git a/src/tint/utils/diagnostic/source_test.cc b/src/tint/utils/diagnostic/source_test.cc
index b15eafb..4c9df84 100644
--- a/src/tint/utils/diagnostic/source_test.cc
+++ b/src/tint/utils/diagnostic/source_test.cc
@@ -37,8 +37,10 @@
 namespace tint {
 namespace {
 
+// Include a blank line to test that empty strings are handled correctly.
 static constexpr std::string_view kSource = R"(line one
 line two
+
 line three)";
 
 using SourceFileContentTest = testing::Test;
@@ -46,10 +48,11 @@
 TEST_F(SourceFileContentTest, Init) {
     Source::FileContent fc(kSource);
     EXPECT_EQ(fc.data, kSource);
-    ASSERT_EQ(fc.lines.size(), 3u);
+    ASSERT_EQ(fc.lines.size(), 4u);
     EXPECT_EQ(fc.lines[0], "line one");
     EXPECT_EQ(fc.lines[1], "line two");
-    EXPECT_EQ(fc.lines[2], "line three");
+    EXPECT_EQ(fc.lines[2], "");
+    EXPECT_EQ(fc.lines[3], "line three");
 }
 
 TEST_F(SourceFileContentTest, CopyInit) {
@@ -57,10 +60,11 @@
     Source::FileContent fc{*src};
     src.reset();
     EXPECT_EQ(fc.data, kSource);
-    ASSERT_EQ(fc.lines.size(), 3u);
+    ASSERT_EQ(fc.lines.size(), 4u);
     EXPECT_EQ(fc.lines[0], "line one");
     EXPECT_EQ(fc.lines[1], "line two");
-    EXPECT_EQ(fc.lines[2], "line three");
+    EXPECT_EQ(fc.lines[2], "");
+    EXPECT_EQ(fc.lines[3], "line three");
 }
 
 TEST_F(SourceFileContentTest, MoveInit) {
@@ -68,10 +72,11 @@
     Source::FileContent fc{std::move(*src)};
     src.reset();
     EXPECT_EQ(fc.data, kSource);
-    ASSERT_EQ(fc.lines.size(), 3u);
+    ASSERT_EQ(fc.lines.size(), 4u);
     EXPECT_EQ(fc.lines[0], "line one");
     EXPECT_EQ(fc.lines[1], "line two");
-    EXPECT_EQ(fc.lines[2], "line three");
+    EXPECT_EQ(fc.lines[2], "");
+    EXPECT_EQ(fc.lines[3], "line three");
 }
 
 // Line break code points