[PATCH] D108897: [lld/mac] Set branchRange a bit more carefully

[Nico Weber via Phabricator via llvm-commits]

thakis added inline comments.


================
Comment at: lld/MachO/ConcatOutputSection.cpp:178
   uint64_t stubsInRangeVA = isecEnd + maxPotentialThunks * target->thunkSize +
                             in.stubs->getSize() - branchRange;
   log("thunks = " + std::to_string(thunkMap.size()) +
----------------
int3 wrote:
> thakis wrote:
> > int3 wrote:
> > > I think the size of the stubs table is factored in here
> > The size of the stubs table yes, but not the size of of thunks needed to reach it as far as I understand. For example:
> > 
> > ```
> > f:
> >   b stubFun40
> >   b stubFun39
> >   ret
> > g:
> >   .fill X MB
> >   ret
> > stubTable:
> >   ...40 entries stubFun1, stubFun2, ...
> > ```
> > 
> > Where X is chosen so that you can jump until stubFun39 from the start of f without a thunk. That means we'll finalize f and g, and after g is finalized, call this estimateStubsInRange() function, and then process the relocations in f. `maxPotentialThunks` is 0 since there are only stub calls in this program. So we set `stubsInRangeVA` at the address of the 2nd `b` in f. But now we process the first `b`, realize it needs a thunk (since it's before `stubsInRangeVA`), which means we insert that thunk after `g` before the stubs table. But that shifts the stub table 12 bytes further back, which means the b to stubFun39 is now no longer in range, even though according to `stubsInRangeVA` still thinks it is.
> > 
> > (Right?)
> > 
> > From what I understand, that's what `maxPotentialThunks` is designed to prevent, but it doesn't count stub calls.
> oh gotcha. yeah, I think you're right
Made https://reviews.llvm.org/D108924 for this then :)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108897/new/

https://reviews.llvm.org/D108897