[PATCH] D35944: [ELF] Disable relocation validation when targeting weak undefined symbols

Dave Bozier via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jul 28 07:40:58 PDT 2017


davidb added inline comments.


================
Comment at: ELF/InputSection.cpp:731-733
+      bool UndefinedWeak = (Rel.Sym->isUndefined() || Rel.Sym->isLazy()) &&
+                           !Rel.Sym->isLocal() && Rel.Sym->symbol()->isWeak();
+      Target->relocateOne(BufLoc, Type, TargetVA, !UndefinedWeak);
----------------
davidb wrote:
> jhenderson wrote:
> > jhenderson wrote:
> > > ruiu wrote:
> > > > I wonder if you really have to change this many places to handle weak undefined symbols. If you don't call `relocateOne` if `UndefinedWeak` is true, doesn't it work?
> > > I'm happy to do that. The only reason I did it this way was to match ld.bfd and gold's behaviour in this situation, but I don't think it really matters (the call should never be executed anyway).
> > I've looked at making a version of this change that does what you suggested. However, this alternative behaviour breaks the guarantee provided by the ELF specification, that undefined weak symbols are treated as having a value of zero (ignoring the ARM special case). I'm concerned that there might be some issue with a corner case that relies on this behaviour.
> > 
> > The end result of this alternative change would be that either zero or the addend would be left in place in the instruction (depending on whether RELA or REL relocations are used), as opposed to zero or a value relative to zero (depending on whether we are dealing with an absolute or relative relocation).
> The change as it currently stands doesn't necessarily guarantee the result of the relocation patch is zero? 
> 
> Can't you just enforce TargetVA to 0 when invoking relocateOne?
With the ARM special case, are you referring to PC-biasing?


https://reviews.llvm.org/D35944





More information about the llvm-commits mailing list