[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