-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[LLVM][PDB] Use IsUnsigned flag for APInt correctly #131598
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-debuginfo Author: Aleksandr Korepanov (AlexK0) ChangesHello. This patch fixes an assertion that occurs in The assertion can be reproduced with the test Here is a stack trace:
Full diff: https://github.com/llvm/llvm-project/pull/131598.diff 1 Files Affected:
diff --git a/llvm/include/llvm/DebugInfo/PDB/PDBTypes.h b/llvm/include/llvm/DebugInfo/PDB/PDBTypes.h
index b6a794ad7e760..a64a2e888e9b9 100644
--- a/llvm/include/llvm/DebugInfo/PDB/PDBTypes.h
+++ b/llvm/include/llvm/DebugInfo/PDB/PDBTypes.h
@@ -510,7 +510,9 @@ struct Variant {
#define VARIANT_APSINT(Enum, NumBits, IsUnsigned) \
case PDB_VariantType::Enum: \
- return APSInt(APInt(NumBits, Value.Enum), IsUnsigned);
+ return APSInt( \
+ APInt(NumBits, static_cast<uint64_t>(Value.Enum), !IsUnsigned), \
+ IsUnsigned);
APSInt toAPSInt() const {
switch (Type) {
|
A test case should be included - if it's reachable via pdb utilities in LLVM, that'd be ideal. If not, then maybe a unit test in LLVM using the API would be do-able? @zmodem might have pointers for a suitable reviewer for PDB related things |
I didn't find a way to write a test using |
✅ With the latest revision this PR passed the C/C++ code formatter. |
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 with some nits
94135a4
to
7e354ea
Compare
Thanks! Do you have access to the "Squash and merge" button, or would you like me to do it? |
@zmodem I don't have access, could you please merge? Thanks! |
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/145/builds/5745 Here is the relevant piece of the build log for the reference
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/143/builds/6278 Here is the relevant piece of the build log for the reference
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/146/builds/2522 Here is the relevant piece of the build log for the reference
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/64/builds/2525 Here is the relevant piece of the build log for the reference
|
Hello! Sorry, it seems I broke the build in #131598 Build: https://lab.llvm.org/buildbot/#/builders/145/builds/5745 This is a compilation fix, please take a look.
Hello! Sorry, it seems I broke the build in llvm/llvm-project#131598 Build: https://lab.llvm.org/buildbot/#/builders/145/builds/5745 This is a compilation fix, please take a look.
Hello.
This patch fixes an assertion that occurs in
APInt
.The assertion can be reproduced with LLDB test
python_api/thread/TestThreadAPI.py
(it fails).Here is a stack trace: