Skip to content

[LLVM][OpenMPOpt] Fix a crash when associated function is nullptr #66274

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Sep 14, 2023

Conversation

shiltian
Copy link
Contributor

The associated function can be a nullptr if it is an indirect call.
This causes a crash in CheckCallee which always assumes the callee
is a valid pointer.

Fix #66904.

@shiltian shiltian requested a review from a team as a code owner September 13, 2023 18:41
@shiltian shiltian requested a review from jdoerfert September 13, 2023 18:41
@llvmbot
Copy link
Member

llvmbot commented Sep 13, 2023

@llvm/pr-subscribers-llvm-transforms

Changes The associated function can be a nullptr if it is an indirect call. This causes a crash in `CheckCallee` which always assumes the callee is a valid pointer.

Fix #66904.

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

2 Files Affected:

  • (modified) llvm/lib/Transforms/IPO/OpenMPOpt.cpp (+3-1)
  • (added) llvm/test/Transforms/OpenMP/indirect_call_kernel_info_crash.ll (+42)
diff --git a/llvm/lib/Transforms/IPO/OpenMPOpt.cpp b/llvm/lib/Transforms/IPO/OpenMPOpt.cpp
index a18730ab35621ef..16bb1d37c23d4f5 100644
--- a/llvm/lib/Transforms/IPO/OpenMPOpt.cpp
+++ b/llvm/lib/Transforms/IPO/OpenMPOpt.cpp
@@ -5028,7 +5028,9 @@ struct AAKernelInfoCallSite : AAKernelInfo {
     const auto *AACE =
         A.getAAFor<AACallEdges>(*this, getIRPosition(), DepClassTy::OPTIONAL);
     if (!AACE || !AACE->getState().isValidState() || AACE->hasUnknownCallee()) {
-      CheckCallee(getAssociatedFunction(), /*NumCallees=*/1);
+      Function *F = getAssociatedFunction();
+      if (F)
+        CheckCallee(getAssociatedFunction(), /*NumCallees=*/1);
     } else {
       const auto &OptimisticEdges = AACE->getOptimisticEdges();
       for (auto *Callee : OptimisticEdges) {
diff --git a/llvm/test/Transforms/OpenMP/indirect_call_kernel_info_crash.ll b/llvm/test/Transforms/OpenMP/indirect_call_kernel_info_crash.ll
new file mode 100644
index 000000000000000..03bc31bac2034e6
--- /dev/null
+++ b/llvm/test/Transforms/OpenMP/indirect_call_kernel_info_crash.ll
@@ -0,0 +1,42 @@
+; RUN: opt -S -passes=openmp-opt < %s
+target datalayout = "e-p:64:64-p1:64:64-p2:32:32-p3:32:32-p4:64:64-p5:32:32-p6:32:32-p7:160:256:256:32-p8:128:128-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-v2048:2048-n32:64-S32-A5-G1-ni:7:8"
+target triple = "amdgcn-amd-amdhsa"
+
+%"struct.ompx::state::TeamStateTy" = type { %"struct.ompx::state::ICVStateTy", i32, i32, ptr }
+%"struct.ompx::state::ICVStateTy" = type { i32, i32, i32, i32, i32, i32, i32 }
+
+@_ZN4ompx5state9TeamStateE = internal addrspace(3) global %"struct.ompx::state::TeamStateTy" undef
+
+define amdgpu_kernel void @__omp_offloading_32_70c2e76c_main_l24() {
+  %1 = tail call i32 @__kmpc_target_init(ptr null)
+  call void @__kmpc_parallel_51(ptr null, i32 0, i32 0, i32 0, i32 0, ptr @__omp_offloading_32_70c2e76c_main_l24_omp_outlined, ptr null, ptr null, i64 0)
+  ret void
+}
+
+define void @__omp_offloading_32_70c2e76c_main_l24_omp_outlined(ptr %0) {
+  call void @__kmpc_for_static_init_4()
+  br label %2
+
+2:                                                ; preds = %2, %1
+  %3 = load ptr, ptr addrspace(1) null, align 4294967296
+  %4 = call i32 %3(i32 0)
+  store i32 %4, ptr %0, align 4
+  br label %2
+}
+
+define internal i32 @__kmpc_target_init(ptr %0) {
+  store i32 0, ptr addrspace(3) @_ZN4ompx5state9TeamStateE, align 16
+  ret i32 0
+}
+
+declare void @__kmpc_parallel_51(ptr, i32, i32, i32, i32, ptr, ptr, ptr, i64)
+
+define void @__kmpc_for_static_init_4() {
+  %1 = load i32, ptr addrspace(3) @_ZN4ompx5state9TeamStateE, align 8
+  ret void
+}
+
+!llvm.module.flags = !{!0, !1}
+
+!0 = !{i32 7, !"openmp", i32 51}
+!1 = !{i32 7, !"openmp-device", i32 51}

Copy link
Contributor

@jplehr jplehr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LG

The associated function can be a nullptr if it is an indirect call.
This causes a crash in `CheckCallee` which always assumes the callee
is a valid pointer.

Fix llvm#66904.
Copy link
Contributor

@jplehr jplehr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@shiltian shiltian merged commit 22e1df7 into llvm:main Sep 14, 2023
@shiltian shiltian deleted the shiltian/issue66094 branch September 14, 2023 00:23
searlmc1 pushed a commit to ROCm/llvm-project that referenced this pull request Sep 14, 2023
wq
The Attributor has gained support for indirect calls but it is opt-in.
This patch makes AAKernelInfoCallSite able to handle multiple potential
callees.

[LLVM][OpenMPOpt] Fix a crash when associated function is nullptr (llvm#66274)

The associated function can be a nullptr if it is an indirect call.
This causes a crash in `CheckCallee` which always assumes the callee
is a valid pointer.

Fix llvm#66904.

Change-Id: Ief6ea28456dc5c01709049153fe7e30f6aa4cb29
kstoimenov pushed a commit to kstoimenov/llvm-project that referenced this pull request Sep 14, 2023
…vm#66274)

The associated function can be a nullptr if it is an indirect call.
This causes a crash in `CheckCallee` which always assumes the callee
is a valid pointer.

Fix llvm#66904.
CheckCallee(getAssociatedFunction(), /*NumCallees=*/1);
Function *F = getAssociatedFunction();
if (F)
CheckCallee(getAssociatedFunction(), /*NumCallees=*/1);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

F

ZijunZhaoCCK pushed a commit to ZijunZhaoCCK/llvm-project that referenced this pull request Sep 19, 2023
…vm#66274)

The associated function can be a nullptr if it is an indirect call.
This causes a crash in `CheckCallee` which always assumes the callee
is a valid pointer.

Fix llvm#66904.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants