[PATCH] Fix non-determinism causing assertion failure (PR22750)

Dario Domizioli dario.domizioli at gmail.com
Tue Mar 3 10:22:16 PST 2015


Thanks David!

The test case is actually pretty reliable (I'm surprised myself). Five
scopes and twenty-five declarations seem to be enough to trigger the
unstable sort behaviour, at least with the std::sort in glibc.
Still, it is a maintenance burden if the metadata format changes, so I
defer to your judgement.

Would a comment like this be OK?

    // Stable sort to preserve the order of appearance of imported entities.
    // This is to avoid out-of-order processing of interdependent
declarations
    // within the same scope, e.g. { namespace A = base; namespace B = A; }

(I'm rebasing the patch and rebuilding at the moment)

Cheers,
    Dario Domizioli
    SN Systems - Sony Computer Entertainment Group




On 3 March 2015 at 17:33, David Blaikie <dblaikie at gmail.com> wrote:

> I think this sounds reasonable - though, yeah, it is perhaps a bit subtle
> that the order of entries in the imported entities list must be such that
> there are no forward references. I'm kind of OK with that.
>
> A comment on the sort explaining why it needs to be stable wouldn't go
> astray.
>
> & I'd probably skip the test case - testing for non-determinism is a bit
> of a lost cause. We regularly fix non-determinism bugs without
> corresponding test cases.
>
> On Tue, Mar 3, 2015 at 8:13 AM, Dario Domizioli <dario.domizioli at gmail.com
> > wrote:
>
>> Hello LLVM,
>>
>> I have found and filed PR22750, and I've looked into fixing it.
>>
>> Basically, it's an assertion failure during debug info generation, and
>> its cause is the interaction of two factors:
>> 1) When generating a DW_TAG_imported_declaration DIE which imports
>> another imported declaration, the code in
>> lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp asserts that the second
>> imported declaration must already have a DIE (line 661).
>> 2) There is a non-determinism in the order in which imported declarations
>> within the same scope are processed.
>>
>> Because of the non-determinism (2), it is possible that an imported
>> declaration is processed before another one it depends on, breaking the
>> assumption in (1).
>>
>> I have tracked down the source of the non-determinism.
>> The imported declaration DIDescriptors are sorted by scope in
>> DwarfDebug::beginModule(), in lib/CodeGen/AsmPrinter/DwarfDebug.cpp:480;
>> however that sort is not a stable_sort, therefore the order of the
>> declarations within the same scope is not preserved.
>>
>> The attached patch changes the std::sort to a std::stable_sort and it
>> fixes the problem.
>>
>> I'm not 100% sure this is the right solution, even if it works. My
>> question is: would it be better to relax the assumption in (1), e.g. by
>> making the code create the dependency DIE on demand?
>>
>> Cheers,
>>     Dario Domizioli
>>     SN Systems - Sony Computer Entertainment Group
>>
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150303/71ee54e6/attachment.html>


More information about the llvm-commits mailing list