[PATCH] [obj2yaml/yaml2obj] Add section group support.

Shankar Kalpathi Easwaran shankarke at gmail.com
Fri Feb 20 07:36:31 PST 2015


REPOSITORY
  rL LLVM

================
Comment at: include/llvm/Object/ELFYAML.h:88
@@ -82,2 +87,3 @@
   StringRef Link;
+  Optional<StringRef> Info;
   llvm::yaml::Hex64 AddressAlign;
----------------
atanasyan wrote:
> Can we remove now the `Info` member field from the `RelocationSection` class?
Good catch. Fixed.

================
Comment at: lib/Object/ELFYAML.cpp:526
@@ +525,3 @@
+  IO.mapOptional("Info", group.Info);
+  IO.mapOptional("Members", group.Members);
+}
----------------
atanasyan wrote:
> Is it possible/correct to get or create a group section with missed `Members` list? Should we made this field mandatory?
Yes we need to make it mandatory. I was thinking if we would want to create invalid tests cases using these tools.

================
Comment at: tools/obj2yaml/elf2yaml.cpp:306
@@ +305,3 @@
+  ELFYAML::SectionOrType s;
+  for (int i = 0; i < count; i++) {
+    if (groupMembers[i] == llvm::ELF::GRP_COMDAT) {
----------------
atanasyan wrote:
> I would write this loop with the `if/else` statement. It's a bit shorter and emphasizes the fact that we have two kinds of group section members:
> ```
> for (int i = 0; i < count; i++) {
>   if (groupMembers[i] == llvm::ELF::GRP_COMDAT) {
>     s.sectionNameOrType = "GRP_COMDAT";
>   } else {
>     const Elf_Shdr *sHdr = Obj.getSection(groupMembers[i]);
>     ErrorOr<StringRef> sectionName = Obj.getSectionName(sHdr);
>     if (std::error_code ec = sectionName.getError())
>       return ec;
>     s.sectionNameOrType = *sectionName;
>   }
>   S->Members.push_back(s);
> }
> ```
Sure.

================
Comment at: tools/yaml2obj/yaml2elf.cpp:392
@@ +391,3 @@
+  if (Section.Type != llvm::ELF::SHT_GROUP) {
+    errs() << "error: Invalid relocation section type.\n";
+    return false;
----------------
atanasyan wrote:
> Why does this error message mention the relocation section type?
Copy/paste error. Will fix.

http://reviews.llvm.org/D7781

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list