<div dir="ltr">Hi Andy,<div><br></div><div>That all makes perfect sense. I've suggested to the LLDB folks that they call getSymbolAddress rather than getGlobalValueAddress - if they can avoid finalizing before they move the sections their issue should just go away.</div>
<div><br></div><div>Longer term, we do want to support recompilation of modules, but I see now why simply reapplying the relocations to the existing (finalized) sections is a non-starter. I just had to personally go through the process of shooting myself in the foot to appreciate it.</div>
<div><br></div><div>Thanks, as always, for all the help. :)</div><div><br></div><div>Cheers,</div><div>Lang. </div>
</div><div class="gmail_extra"><br><br><div class="gmail_quote">On Thu, May 8, 2014 at 10:01 AM, Kaylor, Andrew <span dir="ltr"><<a href="mailto:andrew.kaylor@intel.com" target="_blank">andrew.kaylor@intel.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div lang="EN-US" link="blue" vlink="purple">
<div>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">I changed the model that MCJIT uses for module finalization when I added the multiple module support because it ended up being too complicated to expect clients
to manage it. Requiring explicit finalization was really never a good model anyway. I tried to keep clients using the older model working, but I marked a few methods as deprecated. LLDB is still using the older model.<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">As I recall, MCJIT::getGlobalValueAddress implicitly finalizes the loaded modules because of it is being used to get the address of a function, the function
needs to be executable when the pointer is returned. I believe this was needed to support the behavior of existing clients. I haven’t looked at the LLDB code in a while, but I’m guessing that this issue is caused by the way that they are doing their own
pointer replacement instead of trusting the relocation mechanism.<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">MCJIT has a method, getSymbolAddress, which will return an address without forcing finalization, but it isn’t currently exposed through the ExecutionEngine
interface. RuntimeDyld, in its public interface, exposes both getSymbolAddress, which returns the address of a symbol in local memory (which I’m guessing is what LLDB actually wants) and getSymbolLoadAddress, which returns the address that a symbol will have
in the address space in which it will be executed. Of course, the fact that it is in this interface and not MCJIT’s interface reflects the fact that we typically don’t want MCJIT clients mucking around with RuntimeDyld directly. To put a finer point on it,
we don’t want MCJIT clients trying to do the kinds of things LLDB is doing.<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">I suspect that the best path forward here is to bring LLDB up-to-date with the latest MCJIT interface and only change MCJIT if some legitimate general use case
is uncovered in the process of bringing LLDB up to date.<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">-Andy<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p>
<p class="MsoNormal"><b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif"">From:</span></b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif""> Lang Hames [mailto:<a href="mailto:lhames@gmail.com" target="_blank">lhames@gmail.com</a>]
<br>
<b>Sent:</b> Wednesday, May 07, 2014 11:26 PM<br>
<b>To:</b> Jim Grosbach<br>
<b>Cc:</b> Kaylor, Andrew; <a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a></span></p><div><div class="h5"><br>
<b>Subject:</b> Re: [llvm] r208257 - [RuntimeDyld] Make RuntimeDyldImpl::resolveExternalSymbols preserve the<u></u><u></u></div></div><p></p><div><div class="h5">
<p class="MsoNormal"><u></u> <u></u></p>
<div>
<p class="MsoNormal">Hi Andy,<u></u><u></u></p>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">Looking at the LLDB code that led me to this issue, I see it's not actually calling finalize directly, it's actually calling MCJIT::getGlobalValueAddress, but that method automatically calls finalizeLoadedModules if a value is found. I
wasn't expecting that - why should asking for a global's address should force finalization?<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">The crashes on the bots appears to be due to exactly the issue Andy raised, which I hadn't groked previously - we don't currently have any option for 'de-finalising' a section so that relocations can be re-applied, so this patch will cause
crashes on any client that is actually marking the memory read-only. Obviously I'll hold off recommitting until I've got a solution for that. :)<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">Cheers,<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">Lang.<u></u><u></u></p>
</div>
</div>
<div>
<p class="MsoNormal" style="margin-bottom:12.0pt"><u></u> <u></u></p>
<div>
<p class="MsoNormal">On Wed, May 7, 2014 at 5:38 PM, Jim Grosbach <<a href="mailto:grosbach@apple.com" target="_blank">grosbach@apple.com</a>> wrote:<u></u><u></u></p>
<div>
<p class="MsoNormal">FWIW, support for recompilation of modules was a conscious intention of the design originally in order to support tiering up levels of optimization. The implementation since then has obviously diverged from that a bit, but it’s still a
relevant design goal.<u></u><u></u></p>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">You make a good point on the section permissions. Supporting recompilation is more than just relocation handling and would require nontrivial juggling in a memory manager to do effectively.<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><span style="color:#888888"><u></u> <u></u></span></p>
</div>
<div>
<p class="MsoNormal"><span style="color:#888888">-Jim<u></u><u></u></span></p>
</div>
<div>
<div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
<div>
<div>
<p class="MsoNormal">On May 7, 2014, at 4:43 PM, Kaylor, Andrew <<a href="mailto:andrew.kaylor@intel.com" target="_blank">andrew.kaylor@intel.com</a>> wrote:<u></u><u></u></p>
</div>
<p class="MsoNormal"><br>
<br>
<u></u><u></u></p>
<div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">I’m not sure I’d agree that MCJIT is supposed to support recompilation of a module. I believe we’ve explicitly said that you can’t modify a module after it
has been added to MCJIT, so why would you recompile it? In any event, if you do recompile a module, the relocation information will be recreated. </span><u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> </span><u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">But I take it that you’re actually addressing some case where a module is moved after it has been compiled and put into an executable state. I don’t understand
this case either. Why would you move a module after it has been made executable?</span><u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> </span><u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Also, I would expect that you’ll run into problems with memory managers that set the permissions for code sections to RX after a module has been finalized.
In those cases, if you try to reapply relocations (presumably to a module that didn’t move) it will cause a protection fault. Conversely, if you allow a module to be moved after it has been finalized, don’t you run into issues with the client having to keep
track of whether or not the module is in an executable state?</span><u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> </span><u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">-Andy</span><u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> </span><u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif"">From:</span></b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif""> Lang Hames [<a href="mailto:lhames@gmail.com" target="_blank">mailto:lhames@gmail.com</a>] <br>
<b>Sent:</b> Wednesday, May 07, 2014 4:09 PM<br>
<b>To:</b> Kaylor, Andrew<br>
<b>Cc:</b> <a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
<b>Subject:</b> Re: [llvm] r208257 - [RuntimeDyld] Make RuntimeDyldImpl::resolveExternalSymbols preserve the</span><u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<div>
<p class="MsoNormal">Hi Andy,<u></u><u></u></p>
</div>
<div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal">MCJIT is supposed to support recompilation of a module, and linking multiple modules (thanks for making the latter work! :). So we need to keep external relocations around in case the module they point to is recompiled.<u></u><u></u></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal">Cheers,<u></u><u></u></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal">Lang.<u></u><u></u></p>
</div>
</div>
</div>
<div>
<p class="MsoNormal" style="margin-bottom:12.0pt"> <u></u><u></u></p>
<div>
<div>
<p class="MsoNormal">On Wed, May 7, 2014 at 3:56 PM, Kaylor, Andrew <<a href="mailto:andrew.kaylor@intel.com" target="_blank"><span style="color:purple">andrew.kaylor@intel.com</span></a>> wrote:<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">The discarding of processed relocations was an intentional choice. The relocation map can take up a pretty fair amount of memory, particularly in the case where you have a large number of modules that reference one another. Keeping the
relocation map around will cause memory bloat issues for some clients.<br>
<br>
My thinking was that once a module was finalized you can't remap its sections anymore. Do you have a use case where this is an unacceptable limitation?<br>
<br>
-Andy<u></u><u></u></p>
</div>
<div>
<div>
<p class="MsoNormal"><br>
-----Original Message-----<br>
From: <a href="mailto:llvm-commits-bounces@cs.uiuc.edu" target="_blank"><span style="color:purple">llvm-commits-bounces@cs.uiuc.edu</span></a> [mailto:<a href="mailto:llvm-commits-bounces@cs.uiuc.edu" target="_blank"><span style="color:purple">llvm-commits-bounces@cs.uiuc.edu</span></a>]
On Behalf Of Lang Hames<br>
Sent: Wednesday, May 07, 2014 3:34 PM<br>
To: <a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank"><span style="color:purple">llvm-commits@cs.uiuc.edu</span></a><br>
Subject: [llvm] r208257 - [RuntimeDyld] Make RuntimeDyldImpl::resolveExternalSymbols preserve the<br>
<br>
Author: lhames<br>
Date: Wed May 7 17:34:08 2014<br>
New Revision: 208257<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=208257&view=rev" target="_blank"><span style="color:purple">http://llvm.org/viewvc/llvm-project?rev=208257&view=rev</span></a><br>
Log:<br>
[RuntimeDyld] Make RuntimeDyldImpl::resolveExternalSymbols preserve the relocation entries it applies.<br>
<br>
Prior to this patch, RuntimeDyldImpl::resolveExternalSymbols discarded relocations for external symbols once they had been applied. This causes issues if the client calls MCJIT::finalizeLoadedModules more than once, and updates the location of any symbols in
between (e.g. by calling MCJIT::mapSectionAddress).<br>
<br>
No test case yet: None of our in-tree memory managers support moving sections around. I'll have to hack up a dummy memory manager before I can write a unit test.<br>
<br>
Fixes <rdar://problem/16764378><br>
<br>
<br>
Modified:<br>
llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp<br>
<br>
Modified: llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp?rev=208257&r1=208256&r2=208257&view=diff" target="_blank"><span style="color:purple">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp?rev=208257&r1=208256&r2=208257&view=diff</span></a><br>
==============================================================================<br>
--- llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp (original)<br>
+++ llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp Wed May<u></u><u></u></p>
</div>
</div>
<div>
<p class="MsoNormal">+++ 7 17:34:08 2014<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal" style="margin-bottom:12.0pt">@@ -620,6 +620,8 @@ void RuntimeDyldImpl::resolveRelocationL<br>
}<br>
<br>
void RuntimeDyldImpl::resolveExternalSymbols() {<br>
+ StringMap<RelocationList> ProcessedSymbols;<br>
+<br>
while (!ExternalSymbolRelocations.empty()) {<br>
StringMap<RelocationList>::iterator i = ExternalSymbolRelocations.begin();<br>
<br>
@@ -665,8 +667,20 @@ void RuntimeDyldImpl::resolveExternalSym<br>
resolveRelocationList(Relocs, Addr);<br>
}<br>
<br>
+ ProcessedSymbols[i->first()] = i->second;<br>
ExternalSymbolRelocations.erase(i);<br>
}<br>
+<br>
+ // Restore the relocation entries that were consumed in the loop above:<br>
+ //<br>
+ // FIXME: Replace the following loop with:<br>
+ // std::swap(ProcessedSymbols, ExternalSymbolRelocations)<br>
+ // once StringMap has copy and move construction.<br>
+ for (StringMap<RelocationList>::iterator I = ProcessedSymbols.begin(),<br>
+ E = ProcessedSymbols.end();<br>
+ I != E; ++I) {<br>
+ ExternalSymbolRelocations[I->first()] = I->second; }<br>
}<u></u><u></u></p>
</div>
<div>
<div>
<p class="MsoNormal"> //===----------------------------------------------------------------------===//<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank"><span style="color:purple">llvm-commits@cs.uiuc.edu</span></a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank"><span style="color:purple">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</span></a><u></u><u></u></p>
</div>
</div>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
</div>
</div>
<p class="MsoNormal"><span style="font-size:9.0pt;font-family:"Helvetica","sans-serif"">_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><u></u><u></u></span></p>
</div>
</div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
</div>
</div>
</div>
</div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
</div></div></div>
</div>
</blockquote></div><br></div>