<div style="font-family: arial, helvetica, sans-serif; font-size: 10pt"><br><br><div class="gmail_quote">On Mon, Nov 19, 2012 at 4:33 PM, Chandler Carruth <span dir="ltr"><<a href="mailto:chandlerc@google.com" target="_blank">chandlerc@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><p dir="ltr">FYI, it shouldn't be moved back to a module pass. I'm just suggesting the logic in doFinalization should be lifted into a module pass.</p>
</blockquote><div>You mean split the AddressSanitizer class into two? This will cause some pain. </div><div>There are things computed at doInitialization or at runOnFunction, and then used in doFinalization. </div><div>E.g. DynamicallyInitializedGlobals will need to be computed twice. </div>
<div>GlobalsCreatedByAsan will have to be replaced with some other way to mark globals created by asan (use metadata nodes? match name strings?)</div><div><br></div><div><br></div><div><br></div><div><br></div><div><br></div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">
<div class="gmail_quote">On Nov 19, 2012 4:22 AM, "Kostya Serebryany" <<a href="mailto:kcc@google.com" target="_blank">kcc@google.com</a>> wrote:<br type="attribution"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div style="font-family:arial,helvetica,sans-serif;font-size:10pt"><br><br><div class="gmail_quote">On Mon, Nov 19, 2012 at 4:08 PM, Chandler Carruth <span dir="ltr"><<a href="mailto:chandlerc@google.com" target="_blank">chandlerc@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">FWIW, similar to Evan's comments, I would be interested to see what<br>
variation of the patch would satisfy the immediate needs of ASan,<br>
mostly because I don't want to block this on the fixing the much<br>
deeper issues w/ the pass manager.<br></blockquote><div><br></div><div>I can move asan back to ModulePass if that helps.</div><div>It was a ModulePass until recently, I changed it to FunctionPass because it simplified using ScalarEvolution, which is a FunctionPass too. </div>
<div>Apparently, this chage was wrong, although I still not see why :( </div><div>(We still did not apply the patch with ScalarEvolution, so moving asan back to FunctionPass should not break anything)</div><div><br></div>
<div>--kcc </div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
However, having looked at the *specific* case of ASan, I think it is<br>
likely playing fast and loose with the finalization step, and that<br>
step should be moved to a module pass. Are the issues you're seeing<br>
strictly a consequence of the finalization step? If so, I can work on<br>
splitting it into a module pass to help unblock you.<br>
<br>
<br>
Everything below is trying to get a better understanding of the<br>
underlying issues, and so may not be relevant in the short-term...<br>
<div><br>
On Sun, Nov 18, 2012 at 3:37 PM, Nick Lewycky <<a href="mailto:nicholas@mxc.ca" target="_blank">nicholas@mxc.ca</a>> wrote:<br>
> Pedro Artigas wrote:<br>
>><br>
>> Hello evan,<br>
>><br>
>> I believe that there should be no ordering assumption between any<br>
>> doFinalization/doInitialization calls, that is what got us in trouble in the<br>
>> first place. The current assumption is that doFinalization of<br>
>> AddressSanitizer is run before runOnModule of the printer which is, AFAIK,<br>
>> an unreasonable assumption and a current implementation detail.<br>
><br>
><br>
> Hi Pedro, I think I'm responsible for the way that ASAN uses<br>
> doInitialization/doFinalization, by suggesting it in review when the pass<br>
> was added.<br>
><br>
> My understanding is that function passes are expected to do things to the<br>
> module in their init/finalize functions that they can't do in runOnFunction<br>
> -- in this case, creating and then cleaning up the function declarations.<br>
> Trying to do that in runOnFunction would break parallel runs of function<br>
> passes.<br>
<br>
</div>My perspective on the init/finalize functions is somewhat different:<br>
0) They should have a common interface across all passes: function,<br>
module, and BB. I think the current differences should be fixed.<br>
1) They should get a handle to the module being processed.<br>
2) They should be called exactly once per module, no matter how many<br>
times the pass appears in the optimization pipeline.<br>
3) The init variant should be called before any pass's 'runOnFoo'<br>
method is called, and the finalize after every runOnFoo.<br>
<br>
The goal here is to allow once-per-module setup and teardown of common<br>
infrastructure used in a pass. It is *not* for the purpose of doing<br>
transformations of entities within the pass, however merely adding<br>
declarations of functions (or even adding entirely new functions)<br>
seems an excellent use of the init.<br>
<br>
I think ASan's finalize is actually misbehaving here, as it is really<br>
doing a transformation that should be its own module pass, run after<br>
globalopt and friends.<br>
<div><br>
> I didn't think of what would happen if you put the asm printer in the same<br>
> passmanager. You're right that it doesn't work, but it's not immediately<br>
> obvious to me that this is asan's fault, or the user's fault for putting a<br>
> per-function asm printer inside the same pass manager.<br>
<br>
</div>Forgive my ignorance, but what problem are you envisioning here, and<br>
how can I observe it? I'd like to understand it fully to see in what<br>
ways my thoughts above would or wouldn't interact with it, and I'm<br>
just not sufficiently familiar w/ the asm printer infrastructure...<br>
<div><div>_______________________________________________<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><br>
</div></div></blockquote></div><br></div>
</blockquote></div>
</div></div></blockquote></div><br></div>