[PATCH] Disable branch folding with MemorySanitizer

[Kostya Serebryany]

And we've been just informed by the mozilla folks about yet another case of
optimization being hostile to sanitizers:
hoisting a safe load out of conditional branch introduces a race which tsan
happily reports.
https://code.google.com/p/thread-sanitizer/issues/detail?id=40#c2

--kcc


On Tue, Nov 19, 2013 at 8:06 AM, Kostya Serebryany <kcc at google.com> wrote:

>
>
>
> On Tue, Nov 19, 2013 at 1:27 AM, David Blaikie <dblaikie at gmail.com> wrote:
>
>> Do we have precedence for this kind of change (where sanitizers affect
>> optimizations in arbitrary internal ways - not simply by enabling/disabling
>> certain passes)?
>>
>
> Yes. AddressSanitizer and ThreadSanitizer disable load widening that would
> create a partially out-of-bounds or a racy access.
> See lib/Analysis/MemoryDependenceAnalysis.cpp (search for
>  Attribute::SanitizeAddress and Attribute::SanitizeThread).
> This case with MemorySanitizer is slightly different because we are not
> fighting a false positive, but rather a debug-info-damaging optimization.
>
> --kcc
>
>
>> If not, does this need some deeper discussion about alternatives (is it
>> important that we be able to produce equivalent code without the sanitizers
>> enabled?)?
>>
>>
>> On Mon, Nov 18, 2013 at 7:02 AM, Evgeniy Stepanov <eugenis at google.com>wrote:
>>
>>> Branch folding optimization often leads to confusing MSan reports due to
>>> lost debug info.
>>> For example,
>>> 1: if (x < 0)
>>> 2:   if (y < 0)
>>> 3:    do_something();
>>> is transformed into something like
>>>   %0 = and i32 %y, %x
>>>   %1 = icmp slt i32 %0, 0
>>>   br i1 %1, label %if.then2, label %if.end3
>>> where all 3 instructions are associated with line 1.
>>>
>>> This patch disables folding of conditional branches in functions with
>>> sanitize_memory attribute.
>>>
>>> http://llvm-reviews.chandlerc.com/D2214
>>>
>>> Files:
>>>   lib/Transforms/Utils/SimplifyCFG.cpp
>>>   test/Transforms/SimplifyCFG/branch-fold-msan.ll
>>>
>>> Index: lib/Transforms/Utils/SimplifyCFG.cpp
>>> ===================================================================
>>> --- lib/Transforms/Utils/SimplifyCFG.cpp
>>> +++ lib/Transforms/Utils/SimplifyCFG.cpp
>>> @@ -1967,6 +1967,13 @@
>>>  bool llvm::FoldBranchToCommonDest(BranchInst *BI) {
>>>    BasicBlock *BB = BI->getParent();
>>>
>>> +  // This optimization results in confusing MemorySanitizer reports.
>>> Use of
>>> +  // uninitialized value in this branch instruction is reported with the
>>> +  // predecessor's debug location.
>>> +  if (BB->getParent()->hasFnAttribute(Attribute::SanitizeMemory) &&
>>> +      BI->isConditional())
>>> +    return false;
>>> +
>>>    Instruction *Cond = 0;
>>>    if (BI->isConditional())
>>>      Cond = dyn_cast<Instruction>(BI->getCondition());
>>> Index: test/Transforms/SimplifyCFG/branch-fold-msan.ll
>>> ===================================================================
>>> --- test/Transforms/SimplifyCFG/branch-fold-msan.ll
>>> +++ test/Transforms/SimplifyCFG/branch-fold-msan.ll
>>> @@ -0,0 +1,29 @@
>>> +; RUN: opt < %s -simplifycfg -S | FileCheck %s
>>> +
>>> +declare void @callee()
>>> +
>>> +; Test that conditional branches are not folded with sanitize_memory.
>>> +define void @caller(i32 %x, i32 %y) sanitize_memory {
>>> +; CHECK: define void @caller(i32 [[X:%.*]], i32 [[Y:%.*]])
>>> +; CHECK: icmp slt i32 {{.*}}[[X]]
>>> +; CHECK: icmp slt i32 {{.*}}[[Y]]
>>> +; CHECK: ret void
>>> +
>>> +entry:
>>> +  %cmp = icmp slt i32 %x, 0
>>> +  br i1 %cmp, label %if.then, label %if.end3
>>> +
>>> +if.then:                                          ; preds = %entry
>>> +  %cmp1 = icmp slt i32 %y, 0
>>> +  br i1 %cmp1, label %if.then2, label %if.end
>>> +
>>> +if.then2:                                         ; preds = %if.then
>>> +  call void @callee()
>>> +  br label %if.end
>>> +
>>> +if.end:                                           ; preds = %if.then2,
>>> %if.then
>>> +  br label %if.end3
>>> +
>>> +if.end3:                                          ; preds = %if.end,
>>> %entry
>>> +  ret void
>>> +}
>>>
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>
>>>
>>
>> _______________________________________________
>> 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/20131119/e643af83/attachment.html>