From c6e72c4fdeaee8f858c4f9399390ca98242abf29 Mon Sep 17 00:00:00 2001 From: Vitaly Buka Date: Sat, 18 Jan 2025 12:23:12 -0800 Subject: [PATCH 1/2] [NFC][YAML] Add `IO::error()` Pull Request: https://github.com/llvm/llvm-project/pull/123475 --- llvm/include/llvm/Support/YAMLTraits.h | 4 +++- llvm/lib/Support/YAMLTraits.cpp | 2 ++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/llvm/include/llvm/Support/YAMLTraits.h b/llvm/include/llvm/Support/YAMLTraits.h index eca26e90845bf..e707a445012b5 100644 --- a/llvm/include/llvm/Support/YAMLTraits.h +++ b/llvm/include/llvm/Support/YAMLTraits.h @@ -819,6 +819,7 @@ class IO { virtual NodeKind getNodeKind() = 0; virtual void setError(const Twine &) = 0; + virtual std::error_code error() = 0; virtual void setAllowUnknownKeys(bool Allow); template @@ -1448,7 +1449,7 @@ class Input : public IO { ~Input() override; // Check if there was an syntax or semantic error during parsing. - std::error_code error(); + std::error_code error() override; private: bool outputting() const override; @@ -1631,6 +1632,7 @@ class Output : public IO { void scalarTag(std::string &) override; NodeKind getNodeKind() override; void setError(const Twine &message) override; + std::error_code error() override; bool canElideEmptySequence() override; // These are only used by operator<<. They could be private diff --git a/llvm/lib/Support/YAMLTraits.cpp b/llvm/lib/Support/YAMLTraits.cpp index f326422138488..28642e004c4f4 100644 --- a/llvm/lib/Support/YAMLTraits.cpp +++ b/llvm/lib/Support/YAMLTraits.cpp @@ -750,6 +750,8 @@ void Output::scalarTag(std::string &Tag) { void Output::setError(const Twine &message) { } +std::error_code Output::error() { return {}; } + bool Output::canElideEmptySequence() { // Normally, with an optional key/value where the value is an empty sequence, // the whole key/value can be not written. But, that produces wrong yaml From 50ab70c2e1812eb7d06f07b250fb93cb06691341 Mon Sep 17 00:00:00 2001 From: Vitaly Buka Date: Thu, 16 Jan 2025 20:07:02 -0800 Subject: [PATCH 2/2] [YAML] Don't validate `Fill::Size` after error Size is required, so we don't know if it's in uninitialized state after the previous error. Triggers msan on llvm/test/tools/yaml2obj/ELF/custom-fill.yaml Pull Request: https://github.com/llvm/llvm-project/pull/123280 --- llvm/lib/ObjectYAML/ELFYAML.cpp | 4 +- llvm/lib/ObjectYAML/MachOYAML.cpp | 5 ++- llvm/test/ObjectYAML/MachO/section_data.yaml | 41 +++++++++++++++++++ llvm/test/tools/yaml2obj/ELF/custom-fill.yaml | 5 ++- 4 files changed, 51 insertions(+), 4 deletions(-) diff --git a/llvm/lib/ObjectYAML/ELFYAML.cpp b/llvm/lib/ObjectYAML/ELFYAML.cpp index 7e94d01a97153..24f426a9aa1f7 100644 --- a/llvm/lib/ObjectYAML/ELFYAML.cpp +++ b/llvm/lib/ObjectYAML/ELFYAML.cpp @@ -1747,7 +1747,9 @@ void MappingTraits>::mapping( std::string MappingTraits>::validate( IO &io, std::unique_ptr &C) { if (const auto *F = dyn_cast(C.get())) { - if (F->Pattern && F->Pattern->binary_size() != 0 && !F->Size) + // Can't check the `Size`, as it's required and may be left uninitialized by + // previous error. + if (!io.error() && F->Pattern && F->Pattern->binary_size() != 0 && !F->Size) return "\"Size\" can't be 0 when \"Pattern\" is not empty"; return ""; } diff --git a/llvm/lib/ObjectYAML/MachOYAML.cpp b/llvm/lib/ObjectYAML/MachOYAML.cpp index 4857b5911ff2e..b7eda97c22ae0 100644 --- a/llvm/lib/ObjectYAML/MachOYAML.cpp +++ b/llvm/lib/ObjectYAML/MachOYAML.cpp @@ -346,7 +346,10 @@ void MappingTraits::mapping(IO &IO, std::string MappingTraits::validate(IO &IO, MachOYAML::Section &Section) { - if (Section.content && Section.size < Section.content->binary_size()) + // Can't check the `size`, as it's required and may be left uninitialized by + // previous error. + if (!IO.error() && Section.content && + Section.size < Section.content->binary_size()) return "Section size must be greater than or equal to the content size"; return ""; } diff --git a/llvm/test/ObjectYAML/MachO/section_data.yaml b/llvm/test/ObjectYAML/MachO/section_data.yaml index 87c5bc803ee1a..a2d9a3b7e1675 100644 --- a/llvm/test/ObjectYAML/MachO/section_data.yaml +++ b/llvm/test/ObjectYAML/MachO/section_data.yaml @@ -159,3 +159,44 @@ LoadCommands: reserved2: 0x00000000 reserved3: 0x00000000 content: AA + +## Case 4: Don't validate if size is missing. +# RUN: not yaml2obj --docnum=4 %s -o %t1 2>&1 | FileCheck %s --check-prefix=CASE4 --implicit-check-not=error: +# CASE4: error: missing required key 'size' +# CASE4: error: failed to parse YAML + +--- !mach-o +FileHeader: + magic: 0xFEEDFACF + cputype: 0x01000007 + cpusubtype: 0x00000003 + filetype: 0x00000001 + ncmds: 1 + sizeofcmds: 232 + flags: 0x00002000 + reserved: 0x00000000 +LoadCommands: + - cmd: LC_SEGMENT_64 + cmdsize: 232 + segname: '' + vmaddr: 0 + vmsize: 4 + fileoff: 392 + filesize: 4 + maxprot: 7 + initprot: 7 + nsects: 1 + flags: 0 + Sections: + - sectname: __data + segname: __DATA + addr: 0x0000000000000000 + content: AA + offset: 0x00000188 + align: 2 + reloff: 0x00000000 + nreloc: 0 + flags: 0x00000000 + reserved1: 0x00000000 + reserved2: 0x00000000 + reserved3: 0x00000000 diff --git a/llvm/test/tools/yaml2obj/ELF/custom-fill.yaml b/llvm/test/tools/yaml2obj/ELF/custom-fill.yaml index d770fdb982532..cdb9a97889ac1 100644 --- a/llvm/test/tools/yaml2obj/ELF/custom-fill.yaml +++ b/llvm/test/tools/yaml2obj/ELF/custom-fill.yaml @@ -156,9 +156,10 @@ Sections: Pattern: "BB" ## Check that the "Size" field is mandatory. -# RUN: not yaml2obj --docnum=5 2>&1 %s | FileCheck %s --check-prefix=NOSIZE +# RUN: not yaml2obj --docnum=5 2>&1 %s | FileCheck %s --check-prefix=NOSIZE --implicit-check-not=error: -## NOSIZE: error: missing required key 'Size' +# NOSIZE: error: missing required key 'Size' +# NOSIZE: error: failed to parse YAML --- !ELF FileHeader: