-
Notifications
You must be signed in to change notification settings - Fork 14.9k
[llvm] Validate Parent object before dereference #157460
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
@llvm/pr-subscribers-llvm-binary-utilities Author: Daniel Kuts (apach301) ChangesFixes #157449 Full diff: https://github.com/llvm/llvm-project/pull/157460.diff 1 Files Affected:
diff --git a/llvm/lib/Object/Archive.cpp b/llvm/lib/Object/Archive.cpp
index 92f31c909efd4..c3111ebe0f525 100644
--- a/llvm/lib/Object/Archive.cpp
+++ b/llvm/lib/Object/Archive.cpp
@@ -472,10 +472,11 @@ Archive::Child::Child(const Archive *Parent, const char *Start, Error *Err)
return;
}
+ assert(Parent && "Parent can't be nullptr if Start is not a nullptr");
+
Header = Parent->createArchiveMemberHeader(
Start,
- Parent ? Parent->getData().size() - (Start - Parent->getData().data())
- : 0,
+ Parent->getData().size() - (Start - Parent->getData().data()),
Err);
// If we are pointed to real data, Start is not a nullptr, then there must be
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
Could you add a test case showing this problem, please? |
That was a static analyzer report, so I don't know how to run this code or have any input data. But the problem is in syntactically inconsistent code: the pointer |
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.
Sorry, I misread the change.
I looked back at history and think this was just a copy/paste error during a refactor.
Looks good, once my inline comment has been addressed.
@jh7370 are this PR is ready to merge? |
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. Do you need me to merge this for you?
Yes, I think I couldn't merge into project myself. Thank you! |
Done. Please keep an eye open for notification emails from the build bots. The failures probably won't be related to your change, but it's always possible! |
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/25/builds/11508 Here is the relevant piece of the build log for the reference
|
Fixes #157449