[PATCH] [yaml2obj] ELF Relocations Support

Simon Atanasyan simon at atanasyan.com
Tue Apr 8 21:40:32 PDT 2014



================
Comment at: tools/yaml2obj/yaml2elf.cpp:268-269
@@ +267,4 @@
+    } else {
+      errs() << "error: Unknown section type.\n";
+      return false;
+    }
----------------
Sean Silva wrote:
> This is actually unreachable, right? If we get here then someone has extended the ELFYAML::Section hierarchy without adding a case here which I think is a programming error.
Agreed. The `llvm_unreachable` is better here.

================
Comment at: test/Object/yaml2obj-elf-rel.yaml:40
@@ +39,3 @@
+  Relocations:
+    - !Relocation
+      Offset: 0x1
----------------
Sean Silva wrote:
> Is this per-relocation "!Relocation" tag needed? Doesn't the "!Relocations" on the section cover this?
The "!Relocation" tag is optional. I will remove it from the test case to make it more clear.

================
Comment at: lib/Object/ELFYAML.cpp:582-591
@@ +581,12 @@
+    } else if (auto S = dyn_cast<ELFYAML::RelocationSection>(Section.get())) {
+      IO.mapTag("!Relocation", true);
+      MappingTraits<ELFYAML::RelocationSection>::mapping(IO, *S);
+    } else
+      IO.setError("Unknown section type");
+  } else {
+    if (IO.mapTag("!Section", true) || IO.mapTag("tag:yaml.org,2002:map")) {
+      Section.reset(new ELFYAML::ContentSection());
+      MappingTraits<ELFYAML::ContentSection>::mapping(
+          IO, static_cast<ELFYAML::ContentSection &>(*Section));
+    } else if (IO.mapTag("!Relocations")) {
+      Section.reset(new ELFYAML::RelocationSection());
----------------
Sean Silva wrote:
> Is there a reason that on one side there is "!Relocation" and on the other "!Relocations" (plural)?
Good catch. It is just a typo. The section's type name should be "!Relocations" in both cases.


http://reviews.llvm.org/D3302






More information about the llvm-commits mailing list