[llvm][MetadataLoader] Make sure we correctly load DW_APPLE_ENUM_KIND from bitcode #132374
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… from bitcode This was pointed out in llvm#124752 (comment) There was not test that roundtrips this attribute through LLVM bitcode, so this was never caught.
|
@llvm/pr-subscribers-debuginfo Author: Michael Buch (Michael137) ChangesThis was pointed out in There was not test that roundtrips this attribute through LLVM bitcode, so this was never caught. Full diff: https://github.com/llvm/llvm-project/pull/132374.diff 2 Files Affected:
diff --git a/llvm/lib/Bitcode/Reader/MetadataLoader.cpp b/llvm/lib/Bitcode/Reader/MetadataLoader.cpp
index 1baf0a9214e00..12794d3346e3f 100644
--- a/llvm/lib/Bitcode/Reader/MetadataLoader.cpp
+++ b/llvm/lib/Bitcode/Reader/MetadataLoader.cpp
@@ -1704,8 +1704,8 @@ Error MetadataLoader::MetadataLoaderImpl::parseOneMetadata(
}
}
- if (Record.size() > 25 && Record[25] != dwarf::DW_APPLE_ENUM_KIND_invalid)
- EnumKind = Record[25];
+ if (Record.size() > 24 && Record[24] != dwarf::DW_APPLE_ENUM_KIND_invalid)
+ EnumKind = Record[24];
DICompositeType *CT = nullptr;
if (Identifier)
diff --git a/llvm/test/DebugInfo/AArch64/DW_AT_APPLE_enum_kind.ll b/llvm/test/DebugInfo/AArch64/DW_AT_APPLE_enum_kind.ll
index 399d80c778072..4f219a7e7b12e 100644
--- a/llvm/test/DebugInfo/AArch64/DW_AT_APPLE_enum_kind.ll
+++ b/llvm/test/DebugInfo/AArch64/DW_AT_APPLE_enum_kind.ll
@@ -1,5 +1,6 @@
-; RUN: llc < %s -filetype=obj -o %t
-; RUN: llvm-dwarfdump -v %t | FileCheck %s
+; RUN: clang++ %s -c -g -emit-llvm -o %t.bc
+; RUN: llc %t.bc -filetype=obj -o %t.o
+; RUN: llvm-dwarfdump -v %t.o | FileCheck %s
; C++ source to regenerate:
; enum __attribute__((enum_extensibility(open))) OpenEnum {
|
|
@MaskRay are these failures related to your recent refactorings? If so, could you revert? |
|
FWIW this patch looks good to me. |
| @@ -1,5 +1,6 @@ | |||
| ; RUN: llc < %s -filetype=obj -o %t | |||
| ; RUN: llvm-dwarfdump -v %t | FileCheck %s | |||
| ; RUN: clang++ %s -c -g -emit-llvm -o %t.bc | |||
There was a problem hiding this comment.
an LLVM test can't/shouldn't depend on clang. Can you check in the bitcode file instead?
There was a problem hiding this comment.
Sounds good
Instead of checking in bitcode i roundtrip this file through llvm-dis/llvm-as instead. Which tests the same codepath
| @@ -1,5 +1,7 @@ | |||
| ; RUN: llc < %s -filetype=obj -o %t | |||
| ; RUN: llvm-dwarfdump -v %t | FileCheck %s | |||
| ; | |||
| ; RUN: llvm-as < %s | llvm-dis | llvm-as | llvm-dis | FileCheck %s --check-prefix=CHECK-METADATA | |||
There was a problem hiding this comment.
Can you hoist the round-trip part into tests/IR or BitCode?
adrian-prantl
approved these changes
Mar 21, 2025
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/59/builds/14740 Here is the relevant piece of the build log for the reference |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This was pointed out in #124752 (comment)
There was no test that roundtrips this attribute through LLVM bitcode, so this was never caught.