[llvm] [YAML] Don't validate `Fill::Size` after error (PR #123280)

Vitaly Buka via llvm-commits llvm-commits at lists.llvm.org
Thu Jan 23 09:51:40 PST 2025


https://github.com/vitalybuka updated https://github.com/llvm/llvm-project/pull/123280

>From c6e72c4fdeaee8f858c4f9399390ca98242abf29 Mon Sep 17 00:00:00 2001
From: Vitaly Buka <vitalybuka at google.com>
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 eca26e90845bf6..e707a445012b51 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 <typename T>
@@ -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 f326422138488c..28642e004c4f43 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 <vitalybuka at google.com>
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 7e94d01a971534..24f426a9aa1f7c 100644
--- a/llvm/lib/ObjectYAML/ELFYAML.cpp
+++ b/llvm/lib/ObjectYAML/ELFYAML.cpp
@@ -1747,7 +1747,9 @@ void MappingTraits<std::unique_ptr<ELFYAML::Chunk>>::mapping(
 std::string MappingTraits<std::unique_ptr<ELFYAML::Chunk>>::validate(
     IO &io, std::unique_ptr<ELFYAML::Chunk> &C) {
   if (const auto *F = dyn_cast<ELFYAML::Fill>(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 4857b5911ff2ef..b7eda97c22ae04 100644
--- a/llvm/lib/ObjectYAML/MachOYAML.cpp
+++ b/llvm/lib/ObjectYAML/MachOYAML.cpp
@@ -346,7 +346,10 @@ void MappingTraits<MachOYAML::Section>::mapping(IO &IO,
 std::string
 MappingTraits<MachOYAML::Section>::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 87c5bc803ee1a2..a2d9a3b7e1675b 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 d770fdb9825327..cdb9a97889ac12 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:



More information about the llvm-commits mailing list