-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[CIR][NFC] Fix memory corruption in type converter #160211
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
Conversation
This fixes a problem where a rogue call to converter.addConversion inside of a type conversion lambda was causing memory corruption that (luckily!) triggered a test failure on some build platforms. Near as I can tell, the unwanted addConversion call was the result of a bad rebase (by me). The code shouldn't have ever been there. Because the failure depends on the build environment, I can't find a reliable way to test this change.
@llvm/pr-subscribers-clang @llvm/pr-subscribers-clangir Author: Andy Kaylor (andykaylor) ChangesThis fixes a problem where a rogue call to converter.addConversion inside of a type conversion lambda was causing memory corruption that (luckily!) triggered a test failure on some build platforms. Near as I can tell, the unwanted addConversion call was the result of a bad rebase (by me). The code shouldn't have ever been there. Because the failure depends on the build environment, I can't find a reliable way to test this change. Full diff: https://github.com/llvm/llvm-project/pull/160211.diff 1 Files Affected:
diff --git a/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp b/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp
index 1865698838134..1596ccb6a2617 100644
--- a/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp
+++ b/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp
@@ -2381,9 +2381,6 @@ static void prepareTypeConverter(mlir::LLVMTypeConverter &converter,
}
break;
}
- converter.addConversion([&](cir::VoidType type) -> mlir::Type {
- return mlir::LLVM::LLVMVoidType::get(type.getContext());
- });
// Record has a name: lower as an identified record.
mlir::LLVM::LLVMStructType llvmStruct;
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for fixing this!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/157/builds/40137 Here is the relevant piece of the build log for the reference
|
This fixes a problem where a rogue call to converter.addConversion inside of a type conversion lambda was causing memory corruption that (luckily!) triggered a test failure on some build platforms.
Near as I can tell, the unwanted addConversion call was the result of a bad rebase (by me). The code shouldn't have ever been there. Because the failure depends on the build environment, I can't find a reliable way to test this change.