<div style="font-family: arial, helvetica, sans-serif; font-size: 10pt">I see the problem now. <div><br></div><div>With your patch we have: </div><div>  - asan calls doInitialization and creates Function objects (by calling M.getOrInsertFunction).</div>
<div>  - some other pass kicks in and removes those Functions as unused </div><div>  - asan calls runOnFunction and expects the Function objects to be alive, but they are not. </div><div><br></div><div>The fix is simple: move the M.getOrInsertFunction calls into runOnFunction. </div>
<div>Is that what we want?</div><div>This will at least be a compile performance degradation (though probably not significant for the majority of cases).</div><div><br></div><div>Evgeniy, you may also need to do this change in msan pass. </div>
<div><br></div><div>--kcc <br><br><div class="gmail_quote">On Wed, Nov 28, 2012 at 10:20 PM, Pedro Artigas <span dir="ltr"><<a href="mailto:partigas@apple.com" target="_blank">partigas@apple.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hello All,<br>
<br>
First, thanks for addressing the issue in AddressSanitizer, now make check with my patch passes the ASAN test, which means the patch for it worked as desired.<br>
<br>
Unfortunately the build/tools/clang test shows issues with my patch on the TSAN test (there is also an ASAN issue that I did not notice before, but seems to be orthogonal from the doFinalization one that got fixed), I am attaching the patch and the output of make test in the clang directory.<br>

<br>
<br><br>
<br>
I believe this is an issue due to the re-ordering of the call doInitialization because when I hacked the code to preserve the original ordering all tests passed, the current ordering is:<br>
<br>
ModulePasses doInitialization<br>
Module Passes run<br>
Function Passes doInitialization<br>
Function Passes run<br>
Function Passes doFinalization<br>
ModulePasses doFinalization<br>
<br>
The new ordering with my patch is:<br>
<br>
ModulePasses doInitialization<br>
Function Passes doInitialization<br>
Module Passes run<br>
Function Passes run<br>
Function Passes doFinalization<br>
ModulePasses doFinalization<br>
<br>
Note that the initialization/run/finalization are now clustered as the proposal mentioned they should be. The issue seems to be that TSAN prefers the doInitialization of the function pass to happen after the prior module pass runs. Unfortunately that ordering is not compatible with the model. Again if I hack in that order the tests pass but that is supposed to not be a legal order.<br>

<br>
Thanks for any info,<br>
<br>
Pedro<br>
<br>
<br>
On Nov 28, 2012, at 1:41 AM, Chandler Carruth <<a href="mailto:chandlerc@google.com">chandlerc@google.com</a>> wrote:<br>
<br>
> On Tue, Nov 27, 2012 at 8:50 PM, Kostya Serebryany <<a href="mailto:kcc@google.com">kcc@google.com</a>> wrote:<br>
>> Hi Pedro,<br>
>><br>
>> I managed to understand the problem with doFinalization, but I still don't<br>
>> see the problem with doInitialization.<br>
>> Could you please describe what's wrong in more detail?<br>
><br>
> I don't either. I've read both pass's doInitialization routines (both<br>
> ASan and TSan) and I'm quite convinced they work with the intended<br>
> end-goal of:<br>
><br>
> For each module compiled:<br>
> 1) all passes doInitialization routines are called exactly once with<br>
> that module passed into it.<br>
> 2) the pass manager's runOn* sequence occurs, running passes (perhaps<br>
> multiple times based on the pass manager) across the module.<br>
> 3) all passes doFinalization routines are called exactly once with<br>
> that module passed into it.<br>
><br>
> ThreadSanitizer and AddressSanitizer both use doInitialization to set<br>
> up boring per-module state necessary for the passes to operate:<br>
> 1) They register a function in the set of global ctors<br>
> 2) They add declarations for runtime library functions<br>
> 3) The create some global variables<br>
><br>
> These things seem completely fine and reasonable to do in<br>
> doInitialization, so I think we'll need some more details to<br>
> understand what is actually going wrong here.<br>
><br>
> Can you post a patch? Can you post the error messages you're seeing?<br>
<br>
<br></blockquote></div><br></div></div>