Skip to content

Commit 6c62f7c

Browse files
authored
[MsgPack] Handle Expected<T> errors in document reader (#73183)
This was causing an assert on invalid in the modified test case.
1 parent 50b9930 commit 6c62f7c

File tree

5 files changed

+34
-17
lines changed

5 files changed

+34
-17
lines changed

llvm/include/llvm/BinaryFormat/MsgPackReader.h

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,12 @@
1818
/// msgpack::Reader MPReader(input);
1919
/// msgpack::Object Obj;
2020
///
21-
/// while (MPReader.read(Obj)) {
21+
/// while (true) {
22+
/// Expected<bool> ReadObj = MPReader.read(&Obj);
23+
/// if (!ReadObj)
24+
/// // Handle error...
25+
/// if (!ReadObj.get())
26+
/// break; // Reached end of input
2227
/// switch (Obj.Kind) {
2328
/// case msgpack::Type::Int:
2429
// // Use Obj.Int

llvm/lib/BinaryFormat/MsgPackDocument.cpp

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,13 @@ bool Document::readFromBlob(
143143
// On to next element (or key if doing a map key next).
144144
// Read the value.
145145
Object Obj;
146-
if (!MPReader.read(Obj)) {
146+
Expected<bool> ReadObj = MPReader.read(Obj);
147+
if (!ReadObj) {
148+
// FIXME: Propagate the Error to the caller.
149+
consumeError(ReadObj.takeError());
150+
return false;
151+
}
152+
if (!ReadObj.get()) {
147153
if (Multi && Stack.size() == 1) {
148154
// OK to finish here as we've just done a top-level element with Multi
149155
break;

llvm/lib/Target/AMDGPU/Utils/AMDGPUPALMetadata.cpp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,6 @@ bool AMDGPUPALMetadata::setFromLegacyBlob(StringRef Blob) {
8383

8484
// Set PAL metadata from msgpack blob.
8585
bool AMDGPUPALMetadata::setFromMsgPackBlob(StringRef Blob) {
86-
msgpack::Reader Reader(Blob);
8786
return MsgPackDoc.readFromBlob(Blob, /*Multi=*/false);
8887
}
8988

llvm/test/tools/llvm-readobj/ELF/note-amd-invalid-v3.test

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -9,16 +9,15 @@
99
# LLVM-NEXT: NoteSection {
1010
# LLVM-NEXT: Name: .note.nt_amdgpu_metadata
1111
# LLVM-NEXT: Offset: 0x40
12-
# LLVM-NEXT: Size: 0x28
12+
# LLVM-NEXT: Size: 0x38
1313
# LLVM-NEXT: Note {
1414
# LLVM-NEXT: Owner: AMDGPU
15-
# LLVM-NEXT: Data size: 0x11
15+
# LLVM-NEXT: Data size: 0x24
1616
# LLVM-NEXT: Type: NT_AMDGPU_METADATA (AMDGPU Metadata)
1717
# LLVM-NEXT: AMDGPU Metadata: Invalid AMDGPU Metadata
1818
# LLVM-NEXT: ---
19-
# LLVM-NEXT: 0: 0
20-
# LLVM-NEXT: amdhsa.kernels:
21-
# LLVM-NEXT: - 0
19+
# LLVM-NEXT: amdhsa.kernels:
20+
# LLVM-NEXT: - .name: test_kernel
2221
# LLVM-NEXT: ...
2322
# LLVM-EMPTY:
2423
# LLVM-NEXT: }
@@ -27,13 +26,12 @@
2726

2827
# GNU: Displaying notes found in: .note.nt_amdgpu_metadata
2928
# GNU-NEXT: Owner Data size Description
30-
# GNU-NEXT: AMDGPU 0x00000011 NT_AMDGPU_METADATA (AMDGPU Metadata)
29+
# GNU-NEXT: AMDGPU 0x00000024 NT_AMDGPU_METADATA (AMDGPU Metadata)
3130
# GNU-NEXT: AMDGPU Metadata:
3231
# GNU-NEXT: Invalid AMDGPU Metadata
3332
# GNU-NEXT: ---
34-
# GNU-NEXT: 0: 0
3533
# GNU-NEXT: amdhsa.kernels:
36-
# GNU-NEXT: - 0
34+
# GNU-NEXT: - .name: test_kernel
3735
# GNU-NEXT: ...
3836

3937
--- !ELF
@@ -48,4 +46,4 @@ Sections:
4846
- Name: AMDGPU
4947
Type: NT_AMDGPU_METADATA
5048
## Desc contains 'amdhsa.kernels' without valid entries.
51-
Desc: '82ae616d646873612e6b65726e656c7391'
49+
Desc: '81ae616d646873612e6b65726e656c739181a52e6e616d65ab746573745f6b65726e656c'

llvm/test/tools/llvm-readobj/ELF/note-amdgpu-invalid.s

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@
2626
# GNU-NEXT: Owner Data size Description
2727
# GNU-NEXT: AMDGPU 0x00000003 NT_AMDGPU_METADATA (AMDGPU Metadata)
2828
# GNU-NEXT: description data: 12 34 56
29+
# GNU-NEXT: AMDGPU 0x00000003 NT_AMDGPU_METADATA (AMDGPU Metadata)
30+
# GNU-NEXT: description data: ab cd ef
2931
# GNU-EMPTY:
3032

3133
# LLVM: Notes [
@@ -57,7 +59,7 @@
5759
# LLVM-NEXT: NoteSection {
5860
# LLVM-NEXT: Name: .note.bar
5961
# LLVM-NEXT: Offset: 0x128
60-
# LLVM-NEXT: Size: 0x18
62+
# LLVM-NEXT: Size: 0x30
6163
# LLVM-NEXT: Note {
6264
# LLVM-NEXT: Owner: AMDGPU
6365
# LLVM-NEXT: Data size: 0x3
@@ -66,6 +68,14 @@
6668
# LLVM-NEXT: 0000: 123456 |.4V|
6769
# LLVM-NEXT: )
6870
# LLVM-NEXT: }
71+
# LLVM-NEXT: Note {
72+
# LLVM-NEXT: Owner: AMDGPU
73+
# LLVM-NEXT: Data size: 0x3
74+
# LLVM-NEXT: Type: NT_AMDGPU_METADATA (AMDGPU Metadata)
75+
# LLVM-NEXT: Description data (
76+
# LLVM-NEXT: 0000: ABCDEF |...|
77+
# LLVM-NEXT: )
78+
# LLVM-NEXT: }
6979
# LLVM-NEXT: }
7080
# LLVM-NEXT:]
7181

@@ -87,7 +97,6 @@ Sections:
8797
- Name: AMDGPU
8898
Type: NT_AMDGPU_METADATA
8999
Desc: '123456'
90-
# TODO: https://bugs.llvm.org/show_bug.cgi?id=49034
91-
# - Name: AMDGPU
92-
# Type: NT_AMDGPU_METADATA
93-
# Desc: 'abcdef'
100+
- Name: AMDGPU
101+
Type: NT_AMDGPU_METADATA
102+
Desc: 'abcdef'

0 commit comments

Comments
 (0)