<div style="font-family: arial, helvetica, sans-serif; font-size: 10pt"><br><br><div class="gmail_quote">On Tue, Nov 20, 2012 at 4:12 PM, Kostya Serebryany <span dir="ltr"><<a href="mailto:kcc@google.com" target="_blank">kcc@google.com</a>></span> wrote:<br>
<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"><div class="im">On Tue, Nov 20, 2012 at 3:05 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"><div>On Mon, Nov 19, 2012 at 4:46 AM, Kostya Serebryany <<a href="mailto:kcc@google.com" target="_blank">kcc@google.com</a>> wrote:<br>
><br>
><br>
> On Mon, Nov 19, 2012 at 4:33 PM, Chandler Carruth <<a href="mailto:chandlerc@google.com" target="_blank">chandlerc@google.com</a>><br>
> wrote:<br>
>><br>
>> FYI, it shouldn't be moved back to a module pass. I'm just suggesting the<br>
>> logic in doFinalization should be lifted into a module pass.<br>
><br>
> You mean split the AddressSanitizer class into two? This will cause some<br>
> pain.<br>
> There are things computed at doInitialization or at runOnFunction, and then<br>
> used in doFinalization.<br>
> E.g. DynamicallyInitializedGlobals will need to be computed twice.<br>
> GlobalsCreatedByAsan will have to be replaced with some other way to mark<br>
> globals created by asan (use metadata nodes? match name strings?)<br>
<br>
</div>This is what I meant... I realize it's not trivial, but I think it's<br>
the correct thing to do.<br>
<br>
Essentially, transforms on globals themselves should be done as a<br>
module pass (much like globalopt is today a module pass), and<br>
transforms on the functions as a function pass.The global transforms<br>
are also much simpler / more constrained, and so it seems less likely<br>
for them to benefit from being a function pass.<br></blockquote><div><br></div></div><div>Makes sense. (The downside, of course, is increased code complexity. Hopefully not too much). </div><div class="im"><div><br></div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
<br>
To address your specific points:<br>
<br>
DynamicallyInitializedGlobals appears to only be created inside the<br>
finalization step, even though it is referenced from outside of it...<br>
I've no idea how the outside references can work currently. </blockquote><div><br></div></div><div>hah! This is a performance bug in asan-initialization-order mode!</div><div>(The global transform phase was done in doInitialization until recently). </div>
<div>I'll fix this with a unit test this time ;) </div><div class="im"><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">That said,<br>
computing this twice (once in the initialization step of the function<br>
pass, once in the module pass) doesn't seem terribly costly.<br></blockquote><div><br></div></div><div>Not too bad, indeed. </div><div class="im"><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
GlobalsCreatedByAsan should indeed be handled in some other way. I'm<br>
hopeful that the new attribute system will be generalized to support<br>
globals as well as functions, which would allow us to just not mark<br>
the ASan globals as instrumented. </blockquote><div><br></div></div><div>But that won't happen for at lest a couple of months, right? </div><div class="im"><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
I'd also be fine using a particular<br>
naming convention, there are already some naming convention stuff done<br>
in ASan, so it wouldn't be too weird.<br></blockquote><div><br></div></div><div>I will probably have to use names for now (e.g. __asan_gen_1234?)</div><div class="im"><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Are there any other bits that would be made particularly awkward by this split?<br></blockquote><div><br></div></div><div>Nothing awkward, just quite a few duplicated lines. </div><div class="im"><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
<br>
Is this something you folks have bandwidth for? I can look at it if not.<br>
</blockquote></div></div><br><div>Depends on what is the ETA. I'll do it, but don't promise to finish it immediately. </div></div></blockquote><div><br></div><div><br></div><div>Some progress: I've ripped of the creation of global redzones into a separate class.</div>
<div>Now I'll need to actually create a separate ModulePass.</div><div><br></div><div>--kcc </div><div> </div><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">
<div><br></div><div>This all is a bit sad. </div><div>Mainly because the behavior of doInitialization is a) not what I expected it to be and b) not at all understood (by me). </div>
<div><br></div><div>--kcc </div></div>
</blockquote></div><br></div>