[llvm-commits] Thread Sanitizer seems to have the dual issue

[Evgeniy Stepanov]

On Thu, Nov 29, 2012 at 10:14 AM, Kostya Serebryany <kcc at google.com> wrote:

> I see the problem now.
>
> With your patch we have:
>   - asan calls doInitialization and creates Function objects (by calling
> M.getOrInsertFunction).
>   - some other pass kicks in and removes those Functions as unused
>   - asan calls runOnFunction and expects the Function objects to be alive,
> but they are not.
>
> The fix is simple: move the M.getOrInsertFunction calls into
> runOnFunction.
>

There are plans to run function passes in parallel in the future, right? If
so, is it possible for a function to disappear while we instrument some
other function? In general, is FunctionPass allowed to peek at other
functions while in runOnFunction?



> Is that what we want?
> This will at least be a compile performance degradation (though probably
> not significant for the majority of cases).
>
> Evgeniy, you may also need to do this change in msan pass.
>
> --kcc
>
> On Wed, Nov 28, 2012 at 10:20 PM, Pedro Artigas <partigas at apple.com>wrote:
>
>> Hello All,
>>
>> 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.
>>
>> 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.
>>
>>
>>
>>
>> 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:
>>
>> ModulePasses doInitialization
>> Module Passes run
>> Function Passes doInitialization
>> Function Passes run
>> Function Passes doFinalization
>> ModulePasses doFinalization
>>
>> The new ordering with my patch is:
>>
>> ModulePasses doInitialization
>> Function Passes doInitialization
>> Module Passes run
>> Function Passes run
>> Function Passes doFinalization
>> ModulePasses doFinalization
>>
>> 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.
>>
>> Thanks for any info,
>>
>> Pedro
>>
>>
>> On Nov 28, 2012, at 1:41 AM, Chandler Carruth <chandlerc at google.com>
>> wrote:
>>
>> > On Tue, Nov 27, 2012 at 8:50 PM, Kostya Serebryany <kcc at google.com>
>> wrote:
>> >> Hi Pedro,
>> >>
>> >> I managed to understand the problem with doFinalization, but I still
>> don't
>> >> see the problem with doInitialization.
>> >> Could you please describe what's wrong in more detail?
>> >
>> > I don't either. I've read both pass's doInitialization routines (both
>> > ASan and TSan) and I'm quite convinced they work with the intended
>> > end-goal of:
>> >
>> > For each module compiled:
>> > 1) all passes doInitialization routines are called exactly once with
>> > that module passed into it.
>> > 2) the pass manager's runOn* sequence occurs, running passes (perhaps
>> > multiple times based on the pass manager) across the module.
>> > 3) all passes doFinalization routines are called exactly once with
>> > that module passed into it.
>> >
>> > ThreadSanitizer and AddressSanitizer both use doInitialization to set
>> > up boring per-module state necessary for the passes to operate:
>> > 1) They register a function in the set of global ctors
>> > 2) They add declarations for runtime library functions
>> > 3) The create some global variables
>> >
>> > These things seem completely fine and reasonable to do in
>> > doInitialization, so I think we'll need some more details to
>> > understand what is actually going wrong here.
>> >
>> > Can you post a patch? Can you post the error messages you're seeing?
>>
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20121129/40571878/attachment.html>