From 4c3c28a6269956781e07d97ab03d2572ce43d3cb Mon Sep 17 00:00:00 2001
From: David Neto <[email protected]>
Date: Tue, 23 Jun 2026 15:02:29 -0700
Subject: [PATCH] [wire]: client: Fix indexing when copying utf16 messages

When doing a deep copy of compilation info, the code assumed
that either every message had a DawnCompilationMessageUtf16, or none
of them did.

Also guard against dup dawn message in shader compilation info chain.

Fixed: 523731236
Change-Id: Ib39cc9bcc75a82eab704d039c97866d56a6a6964
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/318975
Commit-Queue: David Neto <[email protected]>
Reviewed-by: Loko Kung <[email protected]>
---

diff --git a/src/dawn/tests/unittests/wire/WireShaderModuleTests.cpp b/src/dawn/tests/unittests/wire/WireShaderModuleTests.cpp
index 22de924..d485853 100644
--- a/src/dawn/tests/unittests/wire/WireShaderModuleTests.cpp
+++ b/src/dawn/tests/unittests/wire/WireShaderModuleTests.cpp
@@ -25,6 +25,7 @@
 // OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
+#include <array>
 #include <memory>
 
 #include "dawn/wire/WireClient.h"
@@ -130,6 +131,118 @@
     });
 }
 
+TEST_P(WireShaderModuleTests, GetCompilationInfoMixedUseOfDawnCompilationMessages) {
+    // Verify bookkeeping for the DawnCompilationMessageUtf16 instances.
+    // An earlier version of the feature code was incorrectly indexing the
+    // utf16 vector. It assumed the utf16 chained message for the i'th message
+    // would appear in the i'th slot on the utf16 vector. Construct a case where
+    // that is not true.
+    wgpu::DawnCompilationMessageUtf16 utf16 = {{nullptr, 30, 32, 34}};
+    wgpu::CompilationMessage message0 = {
+        .nextInChain = nullptr, .lineNum = 0, .linePos = 0, .offset = 0, .length = 0};
+    wgpu::CompilationMessage message1 = {
+        .nextInChain = &utf16, .lineNum = 0, .linePos = 0, .offset = 0, .length = 0};
+    std::array<wgpu::CompilationMessage, 2> messages = {message0, message1};
+    wgpu::CompilationInfo compilationInfo = {nullptr, 2, messages.data()};
+
+    GetCompilationInfo();
+
+    EXPECT_CALL(api, OnShaderModuleGetCompilationInfo(apiShaderModule, _))
+        .WillOnce(InvokeWithoutArgs([&] {
+            api.CallShaderModuleGetCompilationInfoCallback(
+                apiShaderModule, WGPUCompilationInfoRequestStatus_Success,
+                reinterpret_cast<const WGPUCompilationInfo*>(&compilationInfo));
+        }));
+    FlushClient();
+    FlushFutures();
+
+    ExpectWireCallbacksWhen([&](auto& mockCb) {
+        EXPECT_CALL(
+            mockCb,
+            Call(wgpu::CompilationInfoRequestStatus::Success,
+                 MatchesLambda([&](const wgpu::CompilationInfo* info) -> bool {
+                     if (info->messageCount != compilationInfo.messageCount) {
+                         return false;
+                     }
+                     const wgpu::CompilationMessage* msg0 = &info->messages[0];
+                     EXPECT_EQ(msg0->nextInChain, nullptr);
+
+                     // SAFETY: index into std::array 'messages' with 2 elements.
+                     const wgpu::CompilationMessage* msg1 = DAWN_UNSAFE_BUFFERS(&info->messages[1]);
+                     EXPECT_NE(msg1->nextInChain, nullptr);
+                     EXPECT_EQ(msg1->nextInChain->sType, wgpu::SType::DawnCompilationMessageUtf16);
+                     const auto* utf16_1 =
+                         reinterpret_cast<const wgpu::DawnCompilationMessageUtf16*>(
+                             msg1->nextInChain);
+
+                     // The client should always be copying the data returned from
+                     // the server so the memory addresses should never be equal.
+                     EXPECT_NE(utf16_1, &utf16);
+
+                     return utf16_1->linePos == utf16.linePos && utf16_1->offset == utf16.offset &&
+                            utf16_1->length == utf16.length;
+                 })))
+            .Times(1);
+
+        FlushCallbacks();
+    });
+}
+
+TEST_P(WireShaderModuleTests, GetCompilationInfoDuplicateDawnMessageStructIsDropped) {
+    // Setup a message that has two DawnCompilationMessageUtf16 structs chained to it
+    // This is invalid. The implementation drops the second one.
+    // crbug.com/523731236
+    wgpu::DawnCompilationMessageUtf16 secondUtf16 = {{nullptr, 30, 32, 34}};
+    wgpu::DawnCompilationMessageUtf16 firstUtf16 = {{&secondUtf16, 20, 22, 24}};
+    wgpu::CompilationMessage message = {
+        .nextInChain = &firstUtf16, .lineNum = 0, .linePos = 0, .offset = 0, .length = 0};
+    wgpu::CompilationInfo compilationInfo = {nullptr, 1, &message};
+
+    GetCompilationInfo();
+
+    EXPECT_CALL(api, OnShaderModuleGetCompilationInfo(apiShaderModule, _))
+        .WillOnce(InvokeWithoutArgs([&] {
+            api.CallShaderModuleGetCompilationInfoCallback(
+                apiShaderModule, WGPUCompilationInfoRequestStatus_Success,
+                reinterpret_cast<const WGPUCompilationInfo*>(&compilationInfo));
+        }));
+    FlushClient();
+    FlushFutures();
+
+    ExpectWireCallbacksWhen([&](auto& mockCb) {
+        EXPECT_CALL(mockCb, Call(wgpu::CompilationInfoRequestStatus::Success,
+                                 MatchesLambda([&](const wgpu::CompilationInfo* info) -> bool {
+                                     if (info->messageCount != compilationInfo.messageCount) {
+                                         return false;
+                                     }
+                                     const wgpu::CompilationMessage* infoMessage =
+                                         &info->messages[0];
+                                     EXPECT_NE(infoMessage->message.length, WGPU_STRLEN);
+                                     EXPECT_NE(infoMessage->nextInChain, nullptr);
+                                     EXPECT_EQ(infoMessage->nextInChain->sType,
+                                               wgpu::SType::DawnCompilationMessageUtf16);
+                                     const auto* utf16 =
+                                         reinterpret_cast<const wgpu::DawnCompilationMessageUtf16*>(
+                                             infoMessage->nextInChain);
+
+                                     // The client should always be copying the data returned from
+                                     // the server so the memory addresses should never be equal.
+                                     EXPECT_NE(utf16, &firstUtf16);
+                                     EXPECT_NE(utf16, &secondUtf16);
+                                     // The chain ends after the first struct.
+                                     EXPECT_EQ(utf16->nextInChain, nullptr)
+                                         << " " << &firstUtf16 << " " << &secondUtf16;
+
+                                     return utf16->linePos == firstUtf16.linePos &&
+                                            utf16->offset == firstUtf16.offset &&
+                                            utf16->length == firstUtf16.length;
+                                 })))
+            .Times(1);
+
+        FlushCallbacks();
+    });
+}
+
 // Test that calling GetCompilationInfo then disconnecting the wire calls the callback with
 // instance dropped.
 TEST_P(WireShaderModuleTests, GetCompilationInfoBeforeDisconnect) {
diff --git a/src/dawn/wire/client/ShaderModule.cpp b/src/dawn/wire/client/ShaderModule.cpp
index 4b9cd80..c835f24 100644
--- a/src/dawn/wire/client/ShaderModule.cpp
+++ b/src/dawn/wire/client/ShaderModule.cpp
@@ -80,29 +80,31 @@
             // Iterate the message chain for extensions that we want to handle.
             WGPUChainedStruct** tail = &mShader->mMessages[i].nextInChain;
             WGPUChainedStruct* chain = DAWN_UNSAFE_TODO(info->messages[i]).nextInChain;
+            // Guard against duplicates, to avoid a reallocation on the destination vector.
+            // Duplicate structs of the same type are not valid in the first place, so don't
+            // do much to try to recover or error out.
+            bool seenDawnCompilationMessageUtf16 = false;
             while (chain != nullptr) {
                 switch (chain->sType) {
                     case WGPUSType_DawnCompilationMessageUtf16: {
-                        mShader->mUtf16s.push_back(
-                            *reinterpret_cast<const WGPUDawnCompilationMessageUtf16*>(chain));
-                        *tail = &mShader->mUtf16s[i].chain;
+                        if (!seenDawnCompilationMessageUtf16) {
+                            seenDawnCompilationMessageUtf16 = true;
+                            mShader->mUtf16s.push_back(
+                                *reinterpret_cast<const WGPUDawnCompilationMessageUtf16*>(chain));
+                            *tail = &(mShader->mUtf16s.back().chain);
+                            tail = &((*tail)->next);
+                        }
                         break;
                     }
                     default:
                         break;
                 }
 
-                // Update the tail if we added one, and go to the next chain.
-                if (*tail) {
-                    tail = &(*tail)->next;
-                }
                 chain = chain->next;
             }
 
             // Ensure that the tail is pointing to nothing else.
-            if (*tail) {
-                **tail = {nullptr, WGPUSType(0)};
-            }
+            *tail = nullptr;
         }
         mShader->mCompilationInfo = {nullptr, mShader->mMessages.size(), mShader->mMessages.data()};
 

diff --git a/src/dawn/tests/unittests/wire/WireShaderModuleTests.cpp b/src/dawn/tests/unittests/wire/WireShaderModuleTests.cpp
index 22de924..d485853 100644
--- a/src/dawn/tests/unittests/wire/WireShaderModuleTests.cpp
+++ b/src/dawn/tests/unittests/wire/WireShaderModuleTests.cpp
@@ -25,6 +25,7 @@
 // OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
+#include <array>
 #include <memory>
 
 #include "dawn/wire/WireClient.h"
@@ -130,6 +131,118 @@
     });
 }
 
+TEST_P(WireShaderModuleTests, GetCompilationInfoMixedUseOfDawnCompilationMessages) {
+    // Verify bookkeeping for the DawnCompilationMessageUtf16 instances.
+    // An earlier version of the feature code was incorrectly indexing the
+    // utf16 vector. It assumed the utf16 chained message for the i'th message
+    // would appear in the i'th slot on the utf16 vector. Construct a case where
+    // that is not true.
+    wgpu::DawnCompilationMessageUtf16 utf16 = {{nullptr, 30, 32, 34}};
+    wgpu::CompilationMessage message0 = {
+        .nextInChain = nullptr, .lineNum = 0, .linePos = 0, .offset = 0, .length = 0};
+    wgpu::CompilationMessage message1 = {
+        .nextInChain = &utf16, .lineNum = 0, .linePos = 0, .offset = 0, .length = 0};
+    std::array<wgpu::CompilationMessage, 2> messages = {message0, message1};
+    wgpu::CompilationInfo compilationInfo = {nullptr, 2, messages.data()};
+
+    GetCompilationInfo();
+
+    EXPECT_CALL(api, OnShaderModuleGetCompilationInfo(apiShaderModule, _))
+        .WillOnce(InvokeWithoutArgs([&] {
+            api.CallShaderModuleGetCompilationInfoCallback(
+                apiShaderModule, WGPUCompilationInfoRequestStatus_Success,
+                reinterpret_cast<const WGPUCompilationInfo*>(&compilationInfo));
+        }));
+    FlushClient();
+    FlushFutures();
+
+    ExpectWireCallbacksWhen([&](auto& mockCb) {
+        EXPECT_CALL(
+            mockCb,
+            Call(wgpu::CompilationInfoRequestStatus::Success,
+                 MatchesLambda([&](const wgpu::CompilationInfo* info) -> bool {
+                     if (info->messageCount != compilationInfo.messageCount) {
+                         return false;
+                     }
+                     const wgpu::CompilationMessage* msg0 = &info->messages[0];
+                     EXPECT_EQ(msg0->nextInChain, nullptr);
+
+                     // SAFETY: index into std::array 'messages' with 2 elements.
+                     const wgpu::CompilationMessage* msg1 = DAWN_UNSAFE_BUFFERS(&info->messages[1]);
+                     EXPECT_NE(msg1->nextInChain, nullptr);
+                     EXPECT_EQ(msg1->nextInChain->sType, wgpu::SType::DawnCompilationMessageUtf16);
+                     const auto* utf16_1 =
+                         reinterpret_cast<const wgpu::DawnCompilationMessageUtf16*>(
+                             msg1->nextInChain);
+
+                     // The client should always be copying the data returned from
+                     // the server so the memory addresses should never be equal.
+                     EXPECT_NE(utf16_1, &utf16);
+
+                     return utf16_1->linePos == utf16.linePos && utf16_1->offset == utf16.offset &&
+                            utf16_1->length == utf16.length;
+                 })))
+            .Times(1);
+
+        FlushCallbacks();
+    });
+}
+
+TEST_P(WireShaderModuleTests, GetCompilationInfoDuplicateDawnMessageStructIsDropped) {
+    // Setup a message that has two DawnCompilationMessageUtf16 structs chained to it
+    // This is invalid. The implementation drops the second one.
+    // crbug.com/523731236
+    wgpu::DawnCompilationMessageUtf16 secondUtf16 = {{nullptr, 30, 32, 34}};
+    wgpu::DawnCompilationMessageUtf16 firstUtf16 = {{&secondUtf16, 20, 22, 24}};
+    wgpu::CompilationMessage message = {
+        .nextInChain = &firstUtf16, .lineNum = 0, .linePos = 0, .offset = 0, .length = 0};
+    wgpu::CompilationInfo compilationInfo = {nullptr, 1, &message};
+
+    GetCompilationInfo();
+
+    EXPECT_CALL(api, OnShaderModuleGetCompilationInfo(apiShaderModule, _))
+        .WillOnce(InvokeWithoutArgs([&] {
+            api.CallShaderModuleGetCompilationInfoCallback(
+                apiShaderModule, WGPUCompilationInfoRequestStatus_Success,
+                reinterpret_cast<const WGPUCompilationInfo*>(&compilationInfo));
+        }));
+    FlushClient();
+    FlushFutures();
+
+    ExpectWireCallbacksWhen([&](auto& mockCb) {
+        EXPECT_CALL(mockCb, Call(wgpu::CompilationInfoRequestStatus::Success,
+                                 MatchesLambda([&](const wgpu::CompilationInfo* info) -> bool {
+                                     if (info->messageCount != compilationInfo.messageCount) {
+                                         return false;
+                                     }
+                                     const wgpu::CompilationMessage* infoMessage =
+                                         &info->messages[0];
+                                     EXPECT_NE(infoMessage->message.length, WGPU_STRLEN);
+                                     EXPECT_NE(infoMessage->nextInChain, nullptr);
+                                     EXPECT_EQ(infoMessage->nextInChain->sType,
+                                               wgpu::SType::DawnCompilationMessageUtf16);
+                                     const auto* utf16 =
+                                         reinterpret_cast<const wgpu::DawnCompilationMessageUtf16*>(
+                                             infoMessage->nextInChain);
+
+                                     // The client should always be copying the data returned from
+                                     // the server so the memory addresses should never be equal.
+                                     EXPECT_NE(utf16, &firstUtf16);
+                                     EXPECT_NE(utf16, &secondUtf16);
+                                     // The chain ends after the first struct.
+                                     EXPECT_EQ(utf16->nextInChain, nullptr)
+                                         << " " << &firstUtf16 << " " << &secondUtf16;
+
+                                     return utf16->linePos == firstUtf16.linePos &&
+                                            utf16->offset == firstUtf16.offset &&
+                                            utf16->length == firstUtf16.length;
+                                 })))
+            .Times(1);
+
+        FlushCallbacks();
+    });
+}
+
 // Test that calling GetCompilationInfo then disconnecting the wire calls the callback with
 // instance dropped.
 TEST_P(WireShaderModuleTests, GetCompilationInfoBeforeDisconnect) {