-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[OMPIRBuilder] Fix use of uninitialized variable. #145883
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
The code in OpenMPIRBuilder::getTargetEntryUniqueInfo calls ID.getDevice() even when getUniqueID has failed and ID is un-initialized. This caused a sanitizer fail for me in llvm#145026. Fix it by giving a default value to ID. The value chosen is the same as used in OpenMPToLLVMIRTranslation.cpp.
@llvm/pr-subscribers-flang-openmp Author: Abid Qadeer (abidh) ChangesThe code in Full diff: https://github.com/llvm/llvm-project/pull/145883.diff 1 Files Affected:
diff --git a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
index d4f95be083a47..85451b1233f96 100644
--- a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -9846,7 +9846,7 @@ void OffloadEntriesInfoManager::getTargetRegionEntryFnName(
TargetRegionEntryInfo
OpenMPIRBuilder::getTargetEntryUniqueInfo(FileIdentifierInfoCallbackTy CallBack,
StringRef ParentName) {
- sys::fs::UniqueID ID;
+ sys::fs::UniqueID ID(0xdeadf17e, 0);
auto FileIDInfo = CallBack();
uint64_t FileID = 0;
std::error_code EC = sys::fs::getUniqueID(std::get<0>(FileIDInfo), ID);
|
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.
LGTM, but maybe consider not using a fixed value but compute one using a hash function like in #131646.
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.
LGTM, asking for a hash was more a nit (in the light that other places do the same)
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/138/builds/15180 Here is the relevant piece of the build log for the reference
|
… clause. (#145889) llvm/llvm-project#145026 was reverted because it failed a sanitizer test. That issue has been fixed in llvm/llvm-project#145883.
The code in `OpenMPIRBuilder::getTargetEntryUniqueInfo` calls `ID.getDevice()` even when `getUniqueID` has failed and ID is un-initialized. This caused a sanitizer fail for me in llvm#145026. Fix it by giving a default value to `ID`. The value chosen is the same as used in `OpenMPToLLVMIRTranslation.cpp`.
…lvm#145889) llvm#145026 was reverted because it failed a sanitizer test. That issue has been fixed in llvm#145883.
The code in
OpenMPIRBuilder::getTargetEntryUniqueInfo
callsID.getDevice()
even whengetUniqueID
has failed and ID is un-initialized. This caused a sanitizer fail for me in #145026. Fix it by giving a default value toID
. The value chosen is the same as used inOpenMPToLLVMIRTranslation.cpp
.