https://github.com/llvmbot created https://github.com/llvm/llvm-project/pull/178378
Backport f2deb793af7b9543180bba4b9402dcc7cc7a0c2e Requested by: @nikic >From 5851b544177c8f9dc3562294a499f2d4e15a3533 Mon Sep 17 00:00:00 2001 From: Nikita Popov <[email protected]> Date: Mon, 26 Jan 2026 09:47:30 +0100 Subject: [PATCH] [IRMover] Use signature for exact definition (#177381) It is possible for optimizations to modify attributes on exact definitions. In particular, DeadArgumentElimination may find that a certain argument is dead, and replace arguments in calls with `poison`. This requires dropping the `noundef` attribute on the argument. When ThinLTO import is performed, the destination module already has a declaration for the function, and the definition is not imported (e.g. because it is noinline), we currently simply retain the original declaration. This is incorrect if call with poison arguments were imported, as the calls become immediate UB. There was a previous attempt to address this in https://reviews.llvm.org/D139209. What that patch did was to fix up the attributes of the declaration after the fact, dropping UB implying attributes that are not present on the definition. It was reverted because it made an incorrect assumption that the signature between the declaration and definition must match. In this PR, I propose to fix the issue in a different way: If the source module holds an exact definition (which are the ones that can be, in limited ways, modified by optimizations), then even if we don't import the definition, we should still import a declaration based on it. This ensures that we respect any dropped attributes. (It's probably also useful for optimization purposes, because we also see more inferred attributes.) Fixes https://github.com/llvm/llvm-project/issues/58976. (cherry picked from commit f2deb793af7b9543180bba4b9402dcc7cc7a0c2e) --- llvm/lib/Linker/IRMover.cpp | 10 +++++ .../Inputs/attr_fixup_dae_noundef.ll | 19 ++++++++++ .../FunctionImport/attr_fixup_dae_noundef.ll | 38 +++++++++++++++++++ 3 files changed, 67 insertions(+) create mode 100644 llvm/test/Transforms/FunctionImport/Inputs/attr_fixup_dae_noundef.ll create mode 100644 llvm/test/Transforms/FunctionImport/attr_fixup_dae_noundef.ll diff --git a/llvm/lib/Linker/IRMover.cpp b/llvm/lib/Linker/IRMover.cpp index f215f39f41bfb..03c807ead9dfd 100644 --- a/llvm/lib/Linker/IRMover.cpp +++ b/llvm/lib/Linker/IRMover.cpp @@ -943,6 +943,16 @@ Expected<Constant *> IRLinker::linkGlobalValueProto(GlobalValue *SGV, GlobalValue *NewGV; if (DGV && !ShouldLink) { NewGV = DGV; + + // If the source is an exact definition, optimizations may have modified + // its attributes, e.g. by dropping noundef attributes when replacing + // arguments with poison. In this case, it is important for correctness + // that we use the signature from the exact definition. + if (isa<Function>(SGV) && DGV->isDeclaration() && + SGV->hasExactDefinition() && !DoneLinkingBodies) { + NewGV = copyGlobalValueProto(SGV, /*ForDefinition=*/false); + NeedsRenaming = true; + } } else { // If we are done linking global value bodies (i.e. we are performing // metadata linking), don't link in the global value due to this diff --git a/llvm/test/Transforms/FunctionImport/Inputs/attr_fixup_dae_noundef.ll b/llvm/test/Transforms/FunctionImport/Inputs/attr_fixup_dae_noundef.ll new file mode 100644 index 0000000000000..9fd8665ee1922 --- /dev/null +++ b/llvm/test/Transforms/FunctionImport/Inputs/attr_fixup_dae_noundef.ll @@ -0,0 +1,19 @@ +; This file contains the post-"deadargelim" IR. + +define void @outer(i32 noundef %arg) { + ; The parameter was originally `noundef %arg`, changed to `poison` by "deadargelim". + call void @inner(i32 poison) + call void @inner2(i32 poison) + ret void +} + +; %arg was originally `noundef`, removed by "deadargelim". +define void @inner(i32 %arg) #0 { + ret void +} + +define void @inner2(i32 %arg) #0 { + ret void +} + +attributes #0 = { noinline } diff --git a/llvm/test/Transforms/FunctionImport/attr_fixup_dae_noundef.ll b/llvm/test/Transforms/FunctionImport/attr_fixup_dae_noundef.ll new file mode 100644 index 0000000000000..d1679860fce86 --- /dev/null +++ b/llvm/test/Transforms/FunctionImport/attr_fixup_dae_noundef.ll @@ -0,0 +1,38 @@ +; Test to ensure that if an exact definition is imported, it is used in favor +; of an already-present declaration. Exact definitions from the same module may +; be optimized together. Thus care must be taken when importing only a subset +; of the definitions from a module (because other referenced definitions from +; that module may have been changed by the optimizer and may no longer match +; declarations already present in the module being imported into). + +; Generate bitcode and index, and run the function import. +; `Inputs/attr_fixup_dae_noundef.ll` contains the post-"Dead Argument Elimination" IR, which +; removed the `noundef` from `@inner`. +; RUN: opt -module-summary %p/Inputs/attr_fixup_dae_noundef.ll -o %t.inputs.attr_fixup_dae_noundef.bc +; RUN: opt -module-summary %s -o %t.main.bc +; RUN: llvm-lto -thinlto -o %t.summary %t.main.bc %t.inputs.attr_fixup_dae_noundef.bc +; RUN: opt -passes=function-import -summary-file %t.summary.thinlto.bc %t.main.bc -S 2>&1 \ +; RUN: | FileCheck %s + +define void @main() { + call void @outer(i32 noundef 1) + call void @inner(i32 noundef 1) + ; This call is UB due to signature mismatch with the actual definition. + ; Make sure it does not lead to a crash. + call void @inner2() + ret void +} + +; `@outer` should get imported. +; CHECK: define available_externally void @outer(i32 noundef %arg) +; CHECK-NEXT: call void @inner(i32 poison) +declare void @outer(i32 noundef) + +; Because `@inner` is `noinline`, the definition should not be important. +; However, we should create a new declaration from the definition, which does +; not have the `noundef` attribute. +; CHECK: declare void @inner(i32) +declare void @inner(i32 noundef) + +; CHECK: declare void @inner2(i32) +declare void @inner2() _______________________________________________ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
