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

[George Rimar via Phabricator via llvm-commits]

grimar 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);
+  }
----------------
ruiu wrote:
> 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?
I am sorry, I think I read your code wrong at first. Yes, that should be equal and your way looks better for me.


https://reviews.llvm.org/D39242