Skip to content

Conversation

@clubby789
Copy link
Contributor

Fixes #63477 by bailing out instead of crashing

@clubby789 clubby789 requested a review from nikic as a code owner May 2, 2025 17:08
@llvmbot llvmbot added llvm:instcombine Covers the InstCombine, InstSimplify and AggressiveInstCombine passes llvm:transforms labels May 2, 2025
@llvmbot
Copy link
Member

llvmbot commented May 2, 2025

@llvm/pr-subscribers-llvm-transforms

Author: None (clubby789)

Changes

Fixes #63477 by bailing out instead of crashing


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

2 Files Affected:

  • (modified) llvm/lib/Transforms/InstCombine/InstructionCombining.cpp (+4-2)
  • (modified) llvm/test/Transforms/InstCombine/malloc-free-mismatched.ll (+19-1)
diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
index f807f5f4519fc..43b858ed327df 100644
--- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -3302,14 +3302,16 @@ static bool isAllocSiteRemovable(Instruction *AI,
 
         if (getFreedOperand(cast<CallBase>(I), &TLI) == PI &&
             getAllocationFamily(I, &TLI) == Family) {
-          assert(Family);
+          if (!Family)
+            return false;
           Users.emplace_back(I);
           continue;
         }
 
         if (getReallocatedOperand(cast<CallBase>(I)) == PI &&
             getAllocationFamily(I, &TLI) == Family) {
-          assert(Family);
+          if (!Family)
+            return false;
           Users.emplace_back(I);
           Worklist.push_back(I);
           continue;
diff --git a/llvm/test/Transforms/InstCombine/malloc-free-mismatched.ll b/llvm/test/Transforms/InstCombine/malloc-free-mismatched.ll
index 658dbd662508c..5f31d787839e2 100644
--- a/llvm/test/Transforms/InstCombine/malloc-free-mismatched.ll
+++ b/llvm/test/Transforms/InstCombine/malloc-free-mismatched.ll
@@ -3,7 +3,7 @@
 
 define dso_local i32 @_Z6answeri(i32 %0) {
 ; CHECK-LABEL: @_Z6answeri(
-; CHECK-NEXT:    [[TMP2:%.*]] = call noalias nonnull dereferenceable(80) ptr @_Znam(i64 80) #[[ATTR2:[0-9]+]]
+; CHECK-NEXT:    [[TMP2:%.*]] = call noalias nonnull dereferenceable(80) ptr @_Znam(i64 80) #[[ATTR4:[0-9]+]]
 ; CHECK-NEXT:    call void @free(ptr [[TMP2]])
 ; CHECK-NEXT:    ret i32 42
 ;
@@ -25,11 +25,29 @@ define void @test_alloca() {
   ret void
 }
 
+; Test that missing `alloc-family` attributes don't crash LLVM
+; https://github.com/llvm/llvm-project/issues/63749
+
+define void @no_family() {
+; CHECK-LABEL: @no_family(
+; CHECK-NEXT:       [[ALLOC:%.*]] = call ptr @customalloc(i64 64)
+; CHECK-NEXT:       call void @customfree(ptr [[ALLOC]])
+; CHECK-NEXT:       ret void
+;
+  %alloc = call ptr @customalloc(i64 64)
+  call void @customfree(ptr %alloc)
+  ret void
+}
+
+
 ; Function Attrs: nobuiltin allocsize(0)
 declare dso_local nonnull ptr @_Znam(i64) #1
 
 ; Function Attrs: nounwind
 declare dso_local void @free(ptr) allockind("free") "alloc-family"="malloc"
 
+declare ptr @customalloc(i64) allockind("alloc")
+declare void @customfree(ptr allocptr) allockind("free")
+
 attributes #0 = { builtin allocsize(0) }
 attributes #1 = { nobuiltin allocsize(0) allockind("alloc,uninitialized") "alloc-family"="_Znam" }

Copy link
Contributor

@nikic nikic left a comment

Choose a reason for hiding this comment

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

LGTM

@nikic nikic changed the title Fix crash when alloc functions are missing alloc-family [InstCombine] Fix crash when alloc functions are missing alloc-family May 2, 2025
@nikic nikic merged commit b006756 into llvm:main May 3, 2025
11 checks passed
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
…y` (llvm#138310)

Fixes llvm#63477 by bailing out instead of crashing.

Co-authored-by: Jamie <[email protected]>
GeorgeARM pushed a commit to GeorgeARM/llvm-project that referenced this pull request May 7, 2025
…y` (llvm#138310)

Fixes llvm#63477 by bailing out instead of crashing.

Co-authored-by: Jamie <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

llvm:instcombine Covers the InstCombine, InstSimplify and AggressiveInstCombine passes llvm:transforms

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Crash when instcombine tries to optimize away allocation

4 participants