[PATCH] D39242: [ELF] - Stop naming relocation sections with first input section name.
George Rimar via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Oct 25 08:40:51 PDT 2017
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
More information about the llvm-commits
mailing list