[InstCombine][asan] Don't speculate loads before select ptr

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. Pull Request: #100773
llvm · Jul 27, 2024 · 6c404f7 · 6c404f7
1 parent 0fce1ba
commit 6c404f7
Show file tree

Hide file tree

Showing 8 changed files with 174 additions and 23 deletions.
diff --git a/llvm/lib/Analysis/Loads.cpp b/llvm/lib/Analysis/Loads.cpp
@@ -378,8 +378,12 @@ bool llvm::isSafeToLoadUnconditionally(Value *V, Align Alignment, const APInt &S
   // If DT is not specified we can't make context-sensitive query
   const Instruction* CtxI = DT ? ScanFrom : nullptr;
   if (isDereferenceableAndAlignedPointer(V, Alignment, Size, DL, CtxI, AC, DT,
-                                         TLI))
-    return true;
+                                         TLI)) {
+    // With sanitizers `Dereferenceable` is not always enough for unconditional
+    // load.
+    if (!ScanFrom || !suppressSpeculativeLoadForSanitizers(*ScanFrom))
+      return true;
+  }
 
   if (!ScanFrom)
     return false;

diff --git a/llvm/test/Transforms/InstCombine/load.ll b/llvm/test/Transforms/InstCombine/load.ll
@@ -56,9 +56,11 @@ define i32 @test5(i1 %C) {
   ret i32 %Z
 }
 
+; FIXME: Constants should be allowed for this optimization.
 define i32 @test5_asan(i1 %C) sanitize_address {
 ; CHECK-LABEL: @test5_asan(
-; CHECK-NEXT:    [[Z:%.*]] = select i1 [[C:%.*]], i32 42, i32 47
+; CHECK-NEXT:    [[Y:%.*]] = select i1 [[C:%.*]], ptr @X, ptr @X2
+; CHECK-NEXT:    [[Z:%.*]] = load i32, ptr [[Y]], align 4
 ; CHECK-NEXT:    ret i32 [[Z]]
 ;
   %Y = select i1 %C, ptr @X, ptr @X2		; <ptr> [#uses=1]

diff --git a/llvm/test/Transforms/InstCombine/ptr-replace-alloca.ll b/llvm/test/Transforms/InstCombine/ptr-replace-alloca.ll
@@ -430,7 +430,9 @@ entry:
 define i8 @select_diff_addrspace_remove_alloca_asan(i1 %cond, ptr %p) sanitize_address {
 ; CHECK-LABEL: @select_diff_addrspace_remove_alloca_asan(
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    ret i8 0
+; CHECK-NEXT:    [[GEP2:%.*]] = select i1 [[COND:%.*]], ptr addrspace(1) getelementptr inbounds (i8, ptr addrspace(1) @g2, i64 4), ptr addrspace(1) getelementptr inbounds (i8, ptr addrspace(1) @g2, i64 6)
+; CHECK-NEXT:    [[LOAD:%.*]] = load i8, ptr addrspace(1) [[GEP2]], align 1
+; CHECK-NEXT:    ret i8 [[LOAD]]
 ;
 entry:
   %alloca = alloca [32 x i8]

diff --git a/llvm/test/Transforms/InstCombine/select-load.ll 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
+}
diff --git a/llvm/test/Transforms/InstCombine/strnlen-2.ll b/llvm/test/Transforms/InstCombine/strnlen-2.ll
@@ -38,9 +38,14 @@ define i64 @fold_strnlen_s3_s5_1(i1 %C) {
   ret i64 %len
 }
 
+; FIXME: Constants should be allowed for this optimization.
 define i64 @fold_strnlen_s3_s5_1_asan(i1 %C) sanitize_address {
 ; CHECK-LABEL: @fold_strnlen_s3_s5_1_asan(
-; CHECK-NEXT:    ret i64 1
+; CHECK-NEXT:    [[PTR:%.*]] = select i1 [[C:%.*]], ptr @s3, ptr @s6
+; CHECK-NEXT:    [[STRNLEN_CHAR0:%.*]] = load i8, ptr [[PTR]], align 1
+; CHECK-NEXT:    [[STRNLEN_CHAR0CMP:%.*]] = icmp ne i8 [[STRNLEN_CHAR0]], 0
+; CHECK-NEXT:    [[LEN:%.*]] = zext i1 [[STRNLEN_CHAR0CMP]] to i64
+; CHECK-NEXT:    ret i64 [[LEN]]
 ;
   %ptr = select i1 %C, ptr @s3, ptr @s6
 

diff --git a/llvm/test/Transforms/SROA/phi-and-select.ll b/llvm/test/Transforms/SROA/phi-and-select.ll
@@ -348,13 +348,26 @@ entry:
 define i32 @test9_asan(i32 %b, ptr %ptr) sanitize_address {
 ; Same as @test8 but for a select rather than a PHI node.
 ;
-; CHECK-LABEL: @test9_asan(
-; CHECK-NEXT:  entry:
-; CHECK-NEXT:    store i32 0, ptr [[PTR:%.*]], align 4
-; CHECK-NEXT:    [[TEST:%.*]] = icmp ne i32 [[B:%.*]], 0
-; CHECK-NEXT:    [[LOADED_SROA_SPECULATE_LOAD_FALSE:%.*]] = load i32, ptr [[PTR]], align 4
-; CHECK-NEXT:    [[LOADED_SROA_SPECULATED:%.*]] = select i1 [[TEST]], i32 undef, i32 [[LOADED_SROA_SPECULATE_LOAD_FALSE]]
-; CHECK-NEXT:    ret i32 [[LOADED_SROA_SPECULATED]]
+; CHECK-PRESERVE-CFG-LABEL: @test9_asan(
+; CHECK-PRESERVE-CFG-NEXT:  entry:
+; CHECK-PRESERVE-CFG-NEXT:    [[F:%.*]] = alloca float, align 4
+; CHECK-PRESERVE-CFG-NEXT:    store i32 0, ptr [[PTR:%.*]], align 4
+; CHECK-PRESERVE-CFG-NEXT:    [[TEST:%.*]] = icmp ne i32 [[B:%.*]], 0
+; CHECK-PRESERVE-CFG-NEXT:    [[SELECT:%.*]] = select i1 [[TEST]], ptr [[F]], ptr [[PTR]]
+; CHECK-PRESERVE-CFG-NEXT:    [[LOADED:%.*]] = load i32, ptr [[SELECT]], align 4
+; CHECK-PRESERVE-CFG-NEXT:    ret i32 [[LOADED]]
+;
+; CHECK-MODIFY-CFG-LABEL: @test9_asan(
+; CHECK-MODIFY-CFG-NEXT:  entry:
+; CHECK-MODIFY-CFG-NEXT:    store i32 0, ptr [[PTR:%.*]], align 4
+; CHECK-MODIFY-CFG-NEXT:    [[TEST:%.*]] = icmp ne i32 [[B:%.*]], 0
+; CHECK-MODIFY-CFG-NEXT:    [[LOADED_ELSE_VAL:%.*]] = load i32, ptr [[PTR]], align 4
+; CHECK-MODIFY-CFG-NEXT:    br i1 [[TEST]], label [[ENTRY_THEN:%.*]], label [[ENTRY_CONT:%.*]]
+; CHECK-MODIFY-CFG:       entry.then:
+; CHECK-MODIFY-CFG-NEXT:    br label [[ENTRY_CONT]]
+; CHECK-MODIFY-CFG:       entry.cont:
+; CHECK-MODIFY-CFG-NEXT:    [[LOADED:%.*]] = phi i32 [ undef, [[ENTRY_THEN]] ], [ [[LOADED_ELSE_VAL]], [[ENTRY:%.*]] ]
+; CHECK-MODIFY-CFG-NEXT:    ret i32 [[LOADED]]
 ;
 entry:
   %f = alloca float

diff --git a/llvm/test/Transforms/SROA/phi-with-duplicate-pred.ll b/llvm/test/Transforms/SROA/phi-with-duplicate-pred.ll
@@ -55,19 +55,20 @@ cleanup7:                                         ; preds = %cleanup
 define void @f2_hwasan(i1 %c1) sanitize_hwaddress {
 ; CHECK-LABEL: @f2_hwasan(
 ; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[E:%.*]] = alloca i16, align 1
 ; CHECK-NEXT:    br i1 [[C1:%.*]], label [[IF_THEN:%.*]], label [[IF_ELSE:%.*]]
 ; CHECK:       if.then:
 ; CHECK-NEXT:    br label [[CLEANUP:%.*]]
 ; CHECK:       cleanup:
-; CHECK-NEXT:    [[G_0_SROA_SPECULATE_LOAD_CLEANUP:%.*]] = load i16, ptr @a, align 1
 ; CHECK-NEXT:    switch i32 2, label [[CLEANUP7:%.*]] [
 ; CHECK-NEXT:      i32 0, label [[LBL1:%.*]]
 ; CHECK-NEXT:      i32 2, label [[LBL1]]
 ; CHECK-NEXT:    ]
 ; CHECK:       if.else:
 ; CHECK-NEXT:    br label [[LBL1]]
 ; CHECK:       lbl1:
-; CHECK-NEXT:    [[G_0_SROA_SPECULATED:%.*]] = phi i16 [ [[G_0_SROA_SPECULATE_LOAD_CLEANUP]], [[CLEANUP]] ], [ [[G_0_SROA_SPECULATE_LOAD_CLEANUP]], [[CLEANUP]] ], [ undef, [[IF_ELSE]] ]
+; CHECK-NEXT:    [[G_0:%.*]] = phi ptr [ @a, [[CLEANUP]] ], [ @a, [[CLEANUP]] ], [ [[E]], [[IF_ELSE]] ]
+; CHECK-NEXT:    [[TMP0:%.*]] = load i16, ptr [[G_0]], align 1
 ; CHECK-NEXT:    unreachable
 ; CHECK:       cleanup7:
 ; CHECK-NEXT:    ret void

diff --git a/llvm/test/Transforms/SROA/select-load.ll b/llvm/test/Transforms/SROA/select-load.ll
@@ -59,13 +59,36 @@ entry:
 
 ; Sanitizer will break optimization.
 define void @test_multiple_loads_select_asan(i1 %cmp) sanitize_address {
-; CHECK-LABEL: @test_multiple_loads_select_asan(
-; CHECK-NEXT:  entry:
-; CHECK-NEXT:    [[ADDR_I8_SROA_SPECULATED:%.*]] = select i1 [[CMP:%.*]], ptr undef, ptr undef
-; CHECK-NEXT:    call void @foo_i8(ptr [[ADDR_I8_SROA_SPECULATED]])
-; CHECK-NEXT:    [[ADDR_I32_SROA_SPECULATED:%.*]] = select i1 [[CMP]], ptr undef, ptr undef
-; CHECK-NEXT:    call void @foo_i32(ptr [[ADDR_I32_SROA_SPECULATED]])
-; CHECK-NEXT:    ret void
+; CHECK-PRESERVE-CFG-LABEL: @test_multiple_loads_select_asan(
+; CHECK-PRESERVE-CFG-NEXT:  entry:
+; CHECK-PRESERVE-CFG-NEXT:    [[ARGS_SROA_0:%.*]] = alloca ptr, align 8
+; CHECK-PRESERVE-CFG-NEXT:    [[ARGS_SROA_1:%.*]] = alloca ptr, align 8
+; CHECK-PRESERVE-CFG-NEXT:    [[SEL_SROA_SEL:%.*]] = select i1 [[CMP:%.*]], ptr [[ARGS_SROA_1]], ptr [[ARGS_SROA_0]]
+; CHECK-PRESERVE-CFG-NEXT:    [[ADDR_I8:%.*]] = load ptr, ptr [[SEL_SROA_SEL]], align 8
+; CHECK-PRESERVE-CFG-NEXT:    call void @foo_i8(ptr [[ADDR_I8]])
+; CHECK-PRESERVE-CFG-NEXT:    [[ADDR_I32:%.*]] = load ptr, ptr [[SEL_SROA_SEL]], align 8
+; CHECK-PRESERVE-CFG-NEXT:    call void @foo_i32(ptr [[ADDR_I32]])
+; CHECK-PRESERVE-CFG-NEXT:    ret void
+;
+; CHECK-MODIFY-CFG-LABEL: @test_multiple_loads_select_asan(
+; CHECK-MODIFY-CFG-NEXT:  entry:
+; CHECK-MODIFY-CFG-NEXT:    br i1 [[CMP:%.*]], label [[ENTRY_THEN:%.*]], label [[ENTRY_ELSE:%.*]]
+; CHECK-MODIFY-CFG:       entry.then:
+; CHECK-MODIFY-CFG-NEXT:    br label [[ENTRY_CONT:%.*]]
+; CHECK-MODIFY-CFG:       entry.else:
+; CHECK-MODIFY-CFG-NEXT:    br label [[ENTRY_CONT]]
+; CHECK-MODIFY-CFG:       entry.cont:
+; CHECK-MODIFY-CFG-NEXT:    [[ADDR_I8:%.*]] = phi ptr [ undef, [[ENTRY_THEN]] ], [ undef, [[ENTRY_ELSE]] ]
+; CHECK-MODIFY-CFG-NEXT:    call void @foo_i8(ptr [[ADDR_I8]])
+; CHECK-MODIFY-CFG-NEXT:    br i1 [[CMP]], label [[ENTRY_CONT_THEN:%.*]], label [[ENTRY_CONT_ELSE:%.*]]
+; CHECK-MODIFY-CFG:       entry.cont.then:
+; CHECK-MODIFY-CFG-NEXT:    br label [[ENTRY_CONT_CONT:%.*]]
+; CHECK-MODIFY-CFG:       entry.cont.else:
+; CHECK-MODIFY-CFG-NEXT:    br label [[ENTRY_CONT_CONT]]
+; CHECK-MODIFY-CFG:       entry.cont.cont:
+; CHECK-MODIFY-CFG-NEXT:    [[ADDR_I32:%.*]] = phi ptr [ undef, [[ENTRY_CONT_THEN]] ], [ undef, [[ENTRY_CONT_ELSE]] ]
+; CHECK-MODIFY-CFG-NEXT:    call void @foo_i32(ptr [[ADDR_I32]])
+; CHECK-MODIFY-CFG-NEXT:    ret void
 ;
 entry:
   %args = alloca [2 x %st.args], align 16
@@ -436,13 +459,13 @@ define void @load_of_select_with_noundef_nonnull(ptr %buffer, i1 %b) {
 ; CHECK-PRESERVE-CFG-LABEL: @load_of_select_with_noundef_nonnull(
 ; CHECK-PRESERVE-CFG-NEXT:    [[UB_PTR:%.*]] = alloca ptr, align 8
 ; CHECK-PRESERVE-CFG-NEXT:    [[SELECT_PTR:%.*]] = select i1 [[B:%.*]], ptr [[BUFFER:%.*]], ptr [[UB_PTR]]
-; CHECK-PRESERVE-CFG-NEXT:    [[LOAD_PTR:%.*]] = load ptr, ptr [[SELECT_PTR]], align 8, !nonnull !1, !noundef !1
+; CHECK-PRESERVE-CFG-NEXT:    [[LOAD_PTR:%.*]] = load ptr, ptr [[SELECT_PTR]], align 8, !nonnull [[META1:![0-9]+]], !noundef [[META1]]
 ; CHECK-PRESERVE-CFG-NEXT:    ret void
 ;
 ; CHECK-MODIFY-CFG-LABEL: @load_of_select_with_noundef_nonnull(
 ; CHECK-MODIFY-CFG-NEXT:    br i1 [[B:%.*]], label [[DOTTHEN:%.*]], label [[DOTCONT:%.*]]
 ; CHECK-MODIFY-CFG:       .then:
-; CHECK-MODIFY-CFG-NEXT:    [[LOAD_PTR_THEN_VAL:%.*]] = load ptr, ptr [[BUFFER:%.*]], align 8, !nonnull !2, !noundef !2
+; CHECK-MODIFY-CFG-NEXT:    [[LOAD_PTR_THEN_VAL:%.*]] = load ptr, ptr [[BUFFER:%.*]], align 8, !nonnull [[META2:![0-9]+]], !noundef [[META2]]
 ; CHECK-MODIFY-CFG-NEXT:    br label [[DOTCONT]]
 ; CHECK-MODIFY-CFG:       .cont:
 ; CHECK-MODIFY-CFG-NEXT:    [[LOAD_PTR:%.*]] = phi ptr [ [[LOAD_PTR_THEN_VAL]], [[DOTTHEN]] ], [ undef, [[TMP0:%.*]] ]