<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Tue, Jun 14, 2016 at 2:22 PM, Simon Atanasyan <span dir="ltr"><<a href="mailto:simon@atanasyan.com" target="_blank">simon@atanasyan.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">atanasyan added a comment.<br>
<span class=""><br>
In <a href="http://reviews.llvm.org/D21297#457595" rel="noreferrer" target="_blank">http://reviews.llvm.org/D21297#457595</a>, @rafael wrote:<br>
<br>
> It sounds like we should have another expression. Say<br>
>  R_GOT_ADDEND_OFF. With that it would be easier to write code that<br>
><br>
</span>> - Creates a got entry for S+A instead of just S. If the addend is 0, it uses the existing GotIndex/GotPltIndex. Otherwise we need a map. I would suggest starting with just a (sym, addend). We can try optimizing it afterwards.<br>
> - When handling the expression,  we would have<br>
<span class="">><br>
>   case R_GOT_ADDEND_OFFSET: return Body.getGotOffset<ELFT>(A);<br>
><br>
>   And the implementation of getGotOffset uses the hash table in the Got section if the addend is not 0.<br>
<br>
<br>
</span>So you suggest to hide the fact that it is MIPS specific stuff. Ok, I will rename related functions and expressions. Now I use a similar approach for the R_MIPS_GOT_LOCAL expression so it will be rather easy.<br></blockquote><div><br></div><div>I'd name it R_MIPS_GOT_ADDEND_OFFSET or something like this as it's actually MIPS-specific. This should help readers who are not familiar (and not interested in) MIPS.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span class=""><br>
> This should allow handling both preemptable and non preemptable<br>
<br>
>  symbols uniformly, no?<br>
<br>
<br>
</span>I'm not sure. We still need to write GOT entries for preemptable and non preemptable symbols to the different parts of the GOT. Probably we can use a single expression type in both cases but check symbol type in more than one place.<br>
<span class=""><br>
> It also looks like this code would be cleaner if we used<br>
<br>
>  section+offsets instead of symbols is the Relocation vector. I will<br>
<br>
>  give that a try.<br>
<br>
<br>
</span>What will be a section for preemptable symbol comes from DSO?<br>
<span class=""><br>
> > On my test base I have never seen a GOT relocation against preemptible symbol with addend, but I am going to add `fatal` call to catch such case.<br>
<br>
><br>
<br>
><br>
<br>
> What does gold/bfd do in that case?<br>
<br>
<br>
</span>BFD and Gold handle (Symbol,Addend) pairs for any type of symbols. So they are ready to handle GOT relocations against preemptable symbols and non-zero addends. So your approach covers this case too.<br>
<br>
I will try to re-write the patch and send update for review.<br>
<div class="HOEnZb"><div class="h5"><br>
<br>
Repository:<br>
  rL LLVM<br>
<br>
<a href="http://reviews.llvm.org/D21297" rel="noreferrer" target="_blank">http://reviews.llvm.org/D21297</a><br>
<br>
<br>
<br>
</div></div></blockquote></div><br></div></div>