[llvm-branch-commits] [InstCombine][asan] Don't speculate loads before `select ptr` (PR #100773)

[via llvm-branch-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-llvm-transforms

Author: Vitaly Buka (vitalybuka)

<details>
<summary>Changes</summary>

Even if memory is valid from `llvm` point of view,
e.g. local alloca, sanitizers have API for user
specific memory annotations.

This annotations can be used to track size of the
local object, e.g. inline vector like may prevent
accessed beyond the current vector size.

So valid programs should not access those parts of
alloca before checking preconditions.

Fixes #<!-- -->100639.


---
Full diff: https://github.com/llvm/llvm-project/pull/100773.diff


2 Files Affected:

- (modified) llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp (+2-2) 
- (added) llvm/test/Transforms/InstCombine/select-load.ll (+101) 


``````````diff
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp b/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
index 1661fa564c65c..ff2ea5d682142 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
@@ -1042,8 +1042,8 @@ Instruction *InstCombinerImpl::visitLoadInst(LoadInst &LI) {
   }
 
   // None of the following transforms are legal for volatile/ordered atomic
-  // loads.  Most of them do apply for unordered atomics.
-  if (!LI.isUnordered()) return nullptr;
+  // loads and sanitizers.  Most of them do apply for unordered atomics.
+  if (mustSuppressSpeculation(LI)) return nullptr;
 
   // load(gep null, ...) -> unreachable
   // load null/undef -> unreachable
diff --git a/llvm/test/Transforms/InstCombine/select-load.ll b/llvm/test/Transforms/InstCombine/select-load.ll
new file mode 100644
index 0000000000000..36883423aea36
--- /dev/null
+++ b/llvm/test/Transforms/InstCombine/select-load.ll
@@ -0,0 +1,101 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt -passes=instcombine -S < %s | FileCheck %s
+
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-grtev4-linux-gnu"
+
+define i32 @test_plain(i1 %f) {
+; CHECK-LABEL: @test_plain(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[A:%.*]] = alloca i32, align 8
+; CHECK-NEXT:    [[B:%.*]] = alloca i32, align 8
+; CHECK-NEXT:    [[A_VAL:%.*]] = load i32, ptr [[A]], align 8
+; CHECK-NEXT:    [[B_VAL:%.*]] = load i32, ptr [[B]], align 8
+; CHECK-NEXT:    [[L:%.*]] = select i1 [[F:%.*]], i32 [[A_VAL]], i32 [[B_VAL]]
+; CHECK-NEXT:    ret i32 [[L]]
+;
+entry:
+  %a = alloca i32, align 8
+  %b = alloca i32, align 8
+  %sel = select i1 %f, ptr %a, ptr %b
+  %l = load i32, ptr %sel, align 8
+  ret i32 %l
+}
+
+; Don't speculate as the condition may control which memory is valid from
+; sanitizer perspective.
+define i32 @test_asan(i1 %f) sanitize_address {
+; CHECK-LABEL: @test_asan(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[A:%.*]] = alloca i32, align 8
+; CHECK-NEXT:    [[B:%.*]] = alloca i32, align 8
+; CHECK-NEXT:    [[SEL:%.*]] = select i1 [[F:%.*]], ptr [[A]], ptr [[B]]
+; CHECK-NEXT:    [[L:%.*]] = load i32, ptr [[SEL]], align 8
+; CHECK-NEXT:    ret i32 [[L]]
+;
+entry:
+  %a = alloca i32, align 8
+  %b = alloca i32, align 8
+  %sel = select i1 %f, ptr %a, ptr %b
+  %l = load i32, ptr %sel, align 8
+  ret i32 %l
+}
+
+
+; Don't speculate as the condition may control which memory is valid from
+; sanitizer perspective.
+define i32 @test_hwasan(i1 %f) sanitize_hwaddress {
+; CHECK-LABEL: @test_hwasan(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[A:%.*]] = alloca i32, align 8
+; CHECK-NEXT:    [[B:%.*]] = alloca i32, align 8
+; CHECK-NEXT:    [[SEL:%.*]] = select i1 [[F:%.*]], ptr [[A]], ptr [[B]]
+; CHECK-NEXT:    [[L:%.*]] = load i32, ptr [[SEL]], align 8
+; CHECK-NEXT:    ret i32 [[L]]
+;
+entry:
+  %a = alloca i32, align 8
+  %b = alloca i32, align 8
+  %sel = select i1 %f, ptr %a, ptr %b
+  %l = load i32, ptr %sel, align 8
+  ret i32 %l
+}
+
+; Don't speculate as the condition may control which memory is valid from
+; sanitizer perspective.
+define i32 @test_tsan(i1 %f) sanitize_thread {
+; CHECK-LABEL: @test_tsan(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[A:%.*]] = alloca i32, align 8
+; CHECK-NEXT:    [[B:%.*]] = alloca i32, align 8
+; CHECK-NEXT:    [[SEL:%.*]] = select i1 [[F:%.*]], ptr [[A]], ptr [[B]]
+; CHECK-NEXT:    [[L:%.*]] = load i32, ptr [[SEL]], align 8
+; CHECK-NEXT:    ret i32 [[L]]
+;
+entry:
+  %a = alloca i32, align 8
+  %b = alloca i32, align 8
+  %sel = select i1 %f, ptr %a, ptr %b
+  %l = load i32, ptr %sel, align 8
+  ret i32 %l
+}
+
+; Msan just propagates shadow, even if speculated load accesses uninitialized
+; value, instrumentation will select shadow of the desired value anyway.
+define i32 @test_msan(i1 %f) sanitize_memory {
+; CHECK-LABEL: @test_msan(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[A:%.*]] = alloca i32, align 8
+; CHECK-NEXT:    [[B:%.*]] = alloca i32, align 8
+; CHECK-NEXT:    [[A_VAL:%.*]] = load i32, ptr [[A]], align 8
+; CHECK-NEXT:    [[B_VAL:%.*]] = load i32, ptr [[B]], align 8
+; CHECK-NEXT:    [[L:%.*]] = select i1 [[F:%.*]], i32 [[A_VAL]], i32 [[B_VAL]]
+; CHECK-NEXT:    ret i32 [[L]]
+;
+entry:
+  %a = alloca i32, align 8
+  %b = alloca i32, align 8
+  %sel = select i1 %f, ptr %a, ptr %b
+  %l = load i32, ptr %sel, align 8
+  ret i32 %l
+}

``````````

</details>


https://github.com/llvm/llvm-project/pull/100773