This is an archive of the discontinued LLVM Phabricator instance.

Differential D79392

[SanitizerCoverage] Replace the unconditional store with a load, then a conditional store.
ClosedPublic

Authored by pratyai on May 4 2020, 9:17 PM.

Details

Reviewers
vitalybuka
kcc
Commits
rG08032e7192d4: [SanitizerCoverage] Replace the unconditional store with a load, then a…

Diff Detail

Event Timeline

pratyai created this revision.May 4 2020, 9:17 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 4 2020, 9:17 PM
Herald added subscribers: llvm-commits, hiraditya. · View Herald Transcript
pratyai added reviewers: vitalybuka, kcc.May 4 2020, 9:32 PM
Harbormaster failed remote builds in B55734: Diff 262002!May 4 2020, 10:36 PM
vitalybuka accepted this revision.May 5 2020, 1:23 AM
vitalybuka added inline comments.
llvm/test/Instrumentation/SanitizerCoverage/inline-bool-flag.ll
3

you can use env var for that
FILECHECK_OPTS="--dump-input=fail" ninja check-all

This revision is now accepted and ready to land.May 5 2020, 1:23 AM
vitalybuka added inline comments.May 5 2020, 1:38 AM
llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
951

This should be the opposite?
I'll fix this.

vitalybuka added inline comments.May 5 2020, 1:40 AM
llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
951

Now:

if (Load) {
  Store
}

should be :

if (!Load) {
  Store
}
pratyai marked an inline comment as done.May 5 2020, 2:16 AM
pratyai added inline comments.
llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
951

My bad; fixing.

vitalybuka updated this revision to Diff 262038.May 5 2020, 2:22 AM

fix branch

pratyai updated this revision to Diff 262037.May 5 2020, 2:23 AM
pratyai updated this revision to Diff 262039.May 5 2020, 2:25 AM
In D79392#2019807, @vitalybuka wrote:

fix branch

Oops; sorry; didn't notice your updates :(

Closed by commit rG08032e7192d4: [SanitizerCoverage] Replace the unconditional store with a load, then a… (authored by pratyai, committed by vitalybuka). · Explain WhyMay 5 2020, 2:39 AM
This revision was automatically updated to reflect the committed changes.
Harbormaster failed remote builds in B55755: Diff 262038!May 5 2020, 3:12 AM
Harbormaster failed remote builds in B55756: Diff 262039!

Revision Contents

PathSize
llvm/
lib/
Transforms/
Instrumentation/
6 lines
test/
Instrumentation/
SanitizerCoverage/
6 lines

Diff 262037

llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp

Show First 20 Lines • Show All 939 Lines • ▼ Show 20 Lines if (Options.Inline8bitCounters) {
auto Store = IRB.CreateStore(Inc, CounterPtr); auto Store = IRB.CreateStore(Inc, CounterPtr);
SetNoSanitizeMetadata(Load); SetNoSanitizeMetadata(Load);
SetNoSanitizeMetadata(Store); SetNoSanitizeMetadata(Store);
} }
if (Options.InlineBoolFlag) { if (Options.InlineBoolFlag) {
auto FlagPtr = IRB.CreateGEP( auto FlagPtr = IRB.CreateGEP(
FunctionBoolArray->getValueType(), FunctionBoolArray, FunctionBoolArray->getValueType(), FunctionBoolArray,
{ConstantInt::get(IntptrTy, 0), ConstantInt::get(IntptrTy, Idx)}); {ConstantInt::get(IntptrTy, 0), ConstantInt::get(IntptrTy, Idx)});
auto Store = IRB.CreateStore(ConstantInt::getTrue(Int1Ty), FlagPtr); auto Load = IRB.CreateLoad(Int1Ty, FlagPtr);
auto ThenTerm = SplitBlockAndInsertIfThen(IRB.CreateNot(Load), &*IP, false);
IRBuilder<> ThenIRB(ThenTerm);
auto Store = ThenIRB.CreateStore(ConstantInt::getTrue(Int1Ty), FlagPtr);
vitalybukaUnsubmitted
Not Done
ReplyInline Actions

This should be the opposite?
I'll fix this.

vitalybuka: This should be the opposite? I'll fix this.
vitalybukaUnsubmitted
Not Done
ReplyInline Actions

Now:

if (Load) {
  Store
}

should be :

if (!Load) {
  Store
}
vitalybuka: Now: ``` if (Load) { Store } ``` should be : ``` if (!Load) { Store } ```
pratyaiAuthorUnsubmitted
Done
ReplyInline Actions

My bad; fixing.

pratyai: My bad; fixing.
SetNoSanitizeMetadata(Load);
SetNoSanitizeMetadata(Store); SetNoSanitizeMetadata(Store);
} }
if (Options.StackDepth && IsEntryBB && !IsLeafFunc) { if (Options.StackDepth && IsEntryBB && !IsLeafFunc) {
// Check stack depth. If it's the deepest so far, record it. // Check stack depth. If it's the deepest so far, record it.
Module *M = F.getParent(); Module *M = F.getParent();
Function *GetFrameAddr = Intrinsic::getDeclaration( Function *GetFrameAddr = Intrinsic::getDeclaration(
M, Intrinsic::frameaddress, M, Intrinsic::frameaddress,
IRB.getInt8PtrTy(M->getDataLayout().getAllocaAddrSpace())); IRB.getInt8PtrTy(M->getDataLayout().getAllocaAddrSpace()));
▲ Show 20 Lines • Show All 59 Lines • Show Last 20 Lines

llvm/test/Instrumentation/SanitizerCoverage/inline-bool-flag.ll

; Test -sanitizer-coverage-inline-bool-flag=1 ; Test -sanitizer-coverage-inline-bool-flag=1
; RUN: opt < %s -sancov -sanitizer-coverage-level=1 -sanitizer-coverage-inline-bool-flag=1 -S | FileCheck %s ; RUN: opt < %s -sancov -sanitizer-coverage-level=1 -sanitizer-coverage-inline-bool-flag=1 -S | FileCheck %s
; RUN: opt < %s -passes='module(sancov-module)' -sanitizer-coverage-level=1 -sanitizer-coverage-inline-bool-flag=1 -S | FileCheck %s ; RUN: opt < %s -passes='module(sancov-module)' -sanitizer-coverage-level=1 -sanitizer-coverage-inline-bool-flag=1 -S | FileCheck %s
vitalybukaUnsubmitted
Not Done
ReplyInline Actions

you can use env var for that
FILECHECK_OPTS="--dump-input=fail" ninja check-all

vitalybuka: you can use env var for that FILECHECK_OPTS="--dump-input=fail" ninja check-all
target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64" target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64"
target triple = "x86_64-unknown-linux-gnu" target triple = "x86_64-unknown-linux-gnu"
define void @foo() { define void @foo() {
entry: entry:
; CHECK: @__sancov_gen_ = private global [1 x i1] zeroinitializer, section "__sancov_bools", comdat($foo), align 1, !associated !0 ; CHECK: @__sancov_gen_ = private global [1 x i1] zeroinitializer, section "__sancov_bools", comdat($foo), align 1, !associated !0
; CHECK: %0 = load i1, i1* getelementptr inbounds ([1 x i1], [1 x i1]* @__sancov_gen_, i64 0, i64 0), align 1, !nosanitize !1
; CHECK: %1 = xor i1 %0, true
; CHECK: br i1 %1, label %2, label %3
; CHECK: 2: ; preds = %entry
; CHECK: store i1 true, i1* getelementptr inbounds ([1 x i1], [1 x i1]* @__sancov_gen_, i64 0, i64 0), align 1, !nosanitize !1 ; CHECK: store i1 true, i1* getelementptr inbounds ([1 x i1], [1 x i1]* @__sancov_gen_, i64 0, i64 0), align 1, !nosanitize !1
; CHECK: br label %3
; CHECK: 3: ; preds = %entry, %2
ret void ret void
} }
; CHECK: call void @__sanitizer_cov_bool_flag_init(i1* bitcast (i1** @__start___sancov_bools to i1*), i1* bitcast (i1** @__stop___sancov_bools to i1*)) ; CHECK: call void @__sanitizer_cov_bool_flag_init(i1* bitcast (i1** @__start___sancov_bools to i1*), i1* bitcast (i1** @__stop___sancov_bools to i1*))