[llvm] 6c62f7c - [MsgPack] Handle Expected<T> errors in document reader (#73183)

via llvm-commits llvm-commits at lists.llvm.org
Tue Nov 28 10:13:09 PST 2023


Author: Emma Pilkington
Date: 2023-11-28T13:13:05-05:00
New Revision: 6c62f7cbfb3a4dc12cee2f6c02191b83047321d9

URL: https://github.com/llvm/llvm-project/commit/6c62f7cbfb3a4dc12cee2f6c02191b83047321d9
DIFF: https://github.com/llvm/llvm-project/commit/6c62f7cbfb3a4dc12cee2f6c02191b83047321d9.diff

LOG: [MsgPack] Handle Expected<T> errors in document reader (#73183)

This was causing an assert on invalid in the modified test case.

Added: 
    

Modified: 
    llvm/include/llvm/BinaryFormat/MsgPackReader.h
    llvm/lib/BinaryFormat/MsgPackDocument.cpp
    llvm/lib/Target/AMDGPU/Utils/AMDGPUPALMetadata.cpp
    llvm/test/tools/llvm-readobj/ELF/note-amd-invalid-v3.test
    llvm/test/tools/llvm-readobj/ELF/note-amdgpu-invalid.s

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/BinaryFormat/MsgPackReader.h b/llvm/include/llvm/BinaryFormat/MsgPackReader.h
index 5123fb65cf0951b..29bab8f9691afa9 100644
--- a/llvm/include/llvm/BinaryFormat/MsgPackReader.h
+++ b/llvm/include/llvm/BinaryFormat/MsgPackReader.h
@@ -18,7 +18,12 @@
 ///  msgpack::Reader MPReader(input);
 ///  msgpack::Object Obj;
 ///
-///  while (MPReader.read(Obj)) {
+///  while (true) {
+///    Expected<bool> ReadObj = MPReader.read(&Obj);
+///    if (!ReadObj)
+///      // Handle error...
+///    if (!ReadObj.get())
+///      break; // Reached end of input
 ///    switch (Obj.Kind) {
 ///    case msgpack::Type::Int:
 //       // Use Obj.Int

diff  --git a/llvm/lib/BinaryFormat/MsgPackDocument.cpp b/llvm/lib/BinaryFormat/MsgPackDocument.cpp
index 21ffa35dfb6eeb8..11598ee24d6f21c 100644
--- a/llvm/lib/BinaryFormat/MsgPackDocument.cpp
+++ b/llvm/lib/BinaryFormat/MsgPackDocument.cpp
@@ -143,7 +143,13 @@ bool Document::readFromBlob(
     // On to next element (or key if doing a map key next).
     // Read the value.
     Object Obj;
-    if (!MPReader.read(Obj)) {
+    Expected<bool> ReadObj = MPReader.read(Obj);
+    if (!ReadObj) {
+      // FIXME: Propagate the Error to the caller.
+      consumeError(ReadObj.takeError());
+      return false;
+    }
+    if (!ReadObj.get()) {
       if (Multi && Stack.size() == 1) {
         // OK to finish here as we've just done a top-level element with Multi
         break;

diff  --git a/llvm/lib/Target/AMDGPU/Utils/AMDGPUPALMetadata.cpp b/llvm/lib/Target/AMDGPU/Utils/AMDGPUPALMetadata.cpp
index 48d0bde139d7728..0fa67c559cb2916 100644
--- a/llvm/lib/Target/AMDGPU/Utils/AMDGPUPALMetadata.cpp
+++ b/llvm/lib/Target/AMDGPU/Utils/AMDGPUPALMetadata.cpp
@@ -83,7 +83,6 @@ bool AMDGPUPALMetadata::setFromLegacyBlob(StringRef Blob) {
 
 // Set PAL metadata from msgpack blob.
 bool AMDGPUPALMetadata::setFromMsgPackBlob(StringRef Blob) {
-  msgpack::Reader Reader(Blob);
   return MsgPackDoc.readFromBlob(Blob, /*Multi=*/false);
 }
 

diff  --git a/llvm/test/tools/llvm-readobj/ELF/note-amd-invalid-v3.test b/llvm/test/tools/llvm-readobj/ELF/note-amd-invalid-v3.test
index b6713377d7ef4eb..dd090b9483e2901 100644
--- a/llvm/test/tools/llvm-readobj/ELF/note-amd-invalid-v3.test
+++ b/llvm/test/tools/llvm-readobj/ELF/note-amd-invalid-v3.test
@@ -9,16 +9,15 @@
 # LLVM-NEXT:    NoteSection {
 # LLVM-NEXT:      Name: .note.nt_amdgpu_metadata
 # LLVM-NEXT:      Offset: 0x40
-# LLVM-NEXT:      Size: 0x28
+# LLVM-NEXT:      Size: 0x38
 # LLVM-NEXT:      Note {
 # LLVM-NEXT:        Owner: AMDGPU
-# LLVM-NEXT:        Data size: 0x11
+# LLVM-NEXT:        Data size: 0x24
 # LLVM-NEXT:        Type: NT_AMDGPU_METADATA (AMDGPU Metadata)
 # LLVM-NEXT:        AMDGPU Metadata: Invalid AMDGPU Metadata
 # LLVM-NEXT:  ---
-# LLVM-NEXT:  0:               0
-# LLVM-NEXT:  amdhsa.kernels:
-# LLVM-NEXT:    - 0
+# LLVM-NEXT: amdhsa.kernels:
+# LLVM-NEXT:  - .name:           test_kernel
 # LLVM-NEXT:  ...
 # LLVM-EMPTY:
 # LLVM-NEXT:      }
@@ -27,13 +26,12 @@
 
 # GNU:      Displaying notes found in: .note.nt_amdgpu_metadata
 # GNU-NEXT:   Owner                Data size        Description
-# GNU-NEXT:   AMDGPU               0x00000011       NT_AMDGPU_METADATA (AMDGPU Metadata)
+# GNU-NEXT:   AMDGPU               0x00000024       NT_AMDGPU_METADATA (AMDGPU Metadata)
 # GNU-NEXT:     AMDGPU Metadata:
 # GNU-NEXT:         Invalid AMDGPU Metadata
 # GNU-NEXT: ---
-# GNU-NEXT: 0:               0
 # GNU-NEXT: amdhsa.kernels:
-# GNU-NEXT:   - 0
+# GNU-NEXT:  - .name:           test_kernel
 # GNU-NEXT: ...
 
 --- !ELF
@@ -48,4 +46,4 @@ Sections:
       - Name: AMDGPU
         Type: NT_AMDGPU_METADATA
         ## Desc contains 'amdhsa.kernels' without valid entries.
-        Desc: '82ae616d646873612e6b65726e656c7391'
+        Desc: '81ae616d646873612e6b65726e656c739181a52e6e616d65ab746573745f6b65726e656c'

diff  --git a/llvm/test/tools/llvm-readobj/ELF/note-amdgpu-invalid.s b/llvm/test/tools/llvm-readobj/ELF/note-amdgpu-invalid.s
index 25e0d3fc6ab1cce..0ed791c30954e8d 100644
--- a/llvm/test/tools/llvm-readobj/ELF/note-amdgpu-invalid.s
+++ b/llvm/test/tools/llvm-readobj/ELF/note-amdgpu-invalid.s
@@ -26,6 +26,8 @@
 # GNU-NEXT:   Owner                Data size 	Description
 # GNU-NEXT:   AMDGPU               0x00000003	NT_AMDGPU_METADATA (AMDGPU Metadata)
 # GNU-NEXT:    description data: 12 34 56
+# GNU-NEXT:   AMDGPU               0x00000003	NT_AMDGPU_METADATA (AMDGPU Metadata)
+# GNU-NEXT:    description data: ab cd ef
 # GNU-EMPTY:
 
 # LLVM:      Notes [
@@ -57,7 +59,7 @@
 # LLVM-NEXT:  NoteSection {
 # LLVM-NEXT:    Name: .note.bar
 # LLVM-NEXT:    Offset: 0x128
-# LLVM-NEXT:    Size: 0x18
+# LLVM-NEXT:    Size: 0x30
 # LLVM-NEXT:    Note {
 # LLVM-NEXT:      Owner: AMDGPU
 # LLVM-NEXT:      Data size: 0x3
@@ -66,6 +68,14 @@
 # LLVM-NEXT:        0000: 123456                               |.4V|
 # LLVM-NEXT:      )
 # LLVM-NEXT:    }
+# LLVM-NEXT:    Note {
+# LLVM-NEXT:      Owner: AMDGPU
+# LLVM-NEXT:      Data size: 0x3
+# LLVM-NEXT:      Type: NT_AMDGPU_METADATA (AMDGPU Metadata)
+# LLVM-NEXT:      Description data (
+# LLVM-NEXT:        0000: ABCDEF                               |...|
+# LLVM-NEXT:      )
+# LLVM-NEXT:    }
 # LLVM-NEXT:  }
 # LLVM-NEXT:]
 
@@ -87,7 +97,6 @@ Sections:
       - Name: AMDGPU
         Type: NT_AMDGPU_METADATA
         Desc: '123456'
-      # TODO: https://bugs.llvm.org/show_bug.cgi?id=49034
-      # - Name: AMDGPU
-      #   Type: NT_AMDGPU_METADATA
-      #    Desc: 'abcdef'
+      - Name: AMDGPU
+        Type: NT_AMDGPU_METADATA
+        Desc: 'abcdef'


        


More information about the llvm-commits mailing list