diff options
author | Emma Pilkington <emma.pilkington95@gmail.com> | 2023-11-28 13:13:05 -0500 |
---|---|---|
committer | GitHub <noreply@github.com> | 2023-11-28 13:13:05 -0500 |
commit | 6c62f7cbfb3a4dc12cee2f6c02191b83047321d9 (patch) | |
tree | d8b6a6b0821eb4b9311ce5376fc3b8d6fbc9266b | |
parent | 50b9930cfaa145145c13a89ef59b67efc9a772de (diff) |
[MsgPack] Handle Expected<T> errors in document reader (#73183)
This was causing an assert on invalid in the modified test case.
5 files changed, 34 insertions, 17 deletions
diff --git a/llvm/include/llvm/BinaryFormat/MsgPackReader.h b/llvm/include/llvm/BinaryFormat/MsgPackReader.h index 5123fb65cf09..29bab8f9691a 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 21ffa35dfb6e..11598ee24d6f 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 48d0bde139d7..0fa67c559cb2 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 b6713377d7ef..dd090b9483e2 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 25e0d3fc6ab1..0ed791c30954 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' |