[llvm-branch-commits] [llvm] [InstCombine][asan] Don't speculate loads before `select ptr` (PR #100773)
Vitaly Buka via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Fri Jul 26 17:54:49 PDT 2024
https://github.com/vitalybuka updated https://github.com/llvm/llvm-project/pull/100773
>From 6c404f78e56a3376aebd3be261086ba42c71f571 Mon Sep 17 00:00:00 2001
From: Vitaly Buka <vitalybuka at google.com>
Date: Fri, 26 Jul 2024 09:18:32 -0700
Subject: [PATCH] [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: https://github.com/llvm/llvm-project/pull/100773
---
llvm/lib/Analysis/Loads.cpp | 8 +-
llvm/test/Transforms/InstCombine/load.ll | 4 +-
.../InstCombine/ptr-replace-alloca.ll | 4 +-
.../Transforms/InstCombine/select-load.ll | 101 ++++++++++++++++++
llvm/test/Transforms/InstCombine/strnlen-2.ll | 7 +-
llvm/test/Transforms/SROA/phi-and-select.ll | 27 +++--
.../SROA/phi-with-duplicate-pred.ll | 5 +-
llvm/test/Transforms/SROA/select-load.ll | 41 +++++--
8 files changed, 174 insertions(+), 23 deletions(-)
create mode 100644 llvm/test/Transforms/InstCombine/select-load.ll
diff --git a/llvm/lib/Analysis/Loads.cpp b/llvm/lib/Analysis/Loads.cpp
index 1704f0db4c599..a88469ab81a8c 100644
--- 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
index 032f9098e1093..6c087aa87845f 100644
--- 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
index 00f8f50cb4f02..a1d10c274c465 100644
--- 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
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
+}
diff --git a/llvm/test/Transforms/InstCombine/strnlen-2.ll b/llvm/test/Transforms/InstCombine/strnlen-2.ll
index 36da6df20528c..9c042409a43f3 100644
--- 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
index d237b437b9ce0..616617b079828 100644
--- 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
index 38ee760c82527..76e00a95e2caf 100644
--- a/llvm/test/Transforms/SROA/phi-with-duplicate-pred.ll
+++ b/llvm/test/Transforms/SROA/phi-with-duplicate-pred.ll
@@ -55,11 +55,11 @@ 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]]
@@ -67,7 +67,8 @@ define void @f2_hwasan(i1 %c1) sanitize_hwaddress {
; 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
index 0ebc8d71b83f6..9de765071b535 100644
--- 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:%.*]] ]
More information about the llvm-branch-commits
mailing list