[PATCH] Disable branch folding with MemorySanitizer

[Kostya Serebryany]

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/58be25c7/attachment.html>