[PATCH] D39242: [ELF] - Stop naming relocation sections with first input section name.

Rui Ueyama via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Oct 25 08:36:48 PDT 2017


ruiu added inline comments.


================
Comment at: ELF/Writer.cpp:104-110
+  for (StringRef V : {".rel.", ".rela."}) {
+    if (!Name.startswith(V))
+      continue;
+    StringRef Prefix = V.drop_back();
+    StringRef Target = getOutputSectionName(Name.drop_front(Prefix.size()));
+    return Saver.save(Prefix + Target);
+  }
----------------
grimar wrote:
> ruiu wrote:
> > Is this the same as this?
> > 
> >   if (Name.startswith(".rel."))
> >     return Saver.save(".rel." + getOutputSectionName(Name.substr(4)));
> >   if (Name.startswith(".rela."))
> >     return Saver.save(".rela." + getOutputSectionName(Name.substr(5)));
> That was something close to what I did in the first diff. I do not think we want to do that.
> `getOutputSectionName` sometimes returns value from string saver, or for "COMMON" case it returns
> ".bss". So it is not consistent and not always returns the part of the same string.
> 
> I am not sure how much is possible to create .rela.COMMON or .rela.zdebug_ section, but I think we should not
> write code that assumes it is impossible, as it is too complicated assumption which needs additional testing probably. 
> I believe my version is much more straightforward and the fact it rarely allocates the string is not a great issue.
I just tried to unroll your loop, so there shouldn't be any semantic differences. What am I missing?


https://reviews.llvm.org/D39242





More information about the llvm-commits mailing list