Skip to content

Commit ae570d5

Browse files
authored
[lldb][TypeSystemClang] Fix enum signedness in CompleteEnumType (#125203)
I tried using `CompleteEnumType` to replace some duplicated code in `DWARFASTParserClang::ParseEnum` but tests started failing. `CompleteEnumType` parses/attaches the child enumerators using the signedness it got from `CompilerType::IsIntegerType`. However, this would only report the correct signedness for builtin integer types (never for `clang::EnumType`s). We have a different API for that in `CompilerType::IsIntegerOrEnumerationType` which could've been used there instead. This patch calls `IsEnumerationIntegerTypeSigned` to determine signedness because we always pass an enum type into `CompleteEnumType` anyway. Based on git history this has been the case for a long time, but possibly never caused issues because `ParseEnum` was completing the definition manually instead of through `CompleteEnumType`. I couldn't find a good way to test `CompleteEnumType` on its own because it expects an enum type to be passed to it, which only gets created in `ParseEnum` (at which point we already call `CompleteEnumType`). The only other place we call `CompleteEnumType` at is in [`CompleteTypeFromDWARF`](https://github.com/llvm/llvm-project/blob/466217eb0334d467ec8e9b96c52ee988aaadc389/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp#L2260-L2262). Though I think we don't actually ever end up calling into that codepath because we eagerly complete enum definitions. Maybe we can remove that call to `CompleteEnumType` in a follow-up.
1 parent fcb1234 commit ae570d5

File tree

1 file changed

+7
-15
lines changed

1 file changed

+7
-15
lines changed

lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp

Lines changed: 7 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1019,16 +1019,7 @@ TypeSP DWARFASTParserClang::ParseEnum(const SymbolContext &sc,
10191019
// Declaration DIE is inserted into the type map in ParseTypeFromDWARF
10201020
}
10211021

1022-
1023-
if (TypeSystemClang::StartTagDeclarationDefinition(clang_type)) {
1024-
if (def_die.HasChildren()) {
1025-
bool is_signed = false;
1026-
enumerator_clang_type.IsIntegerType(is_signed);
1027-
ParseChildEnumerators(clang_type, is_signed,
1028-
type_sp->GetByteSize(nullptr).value_or(0), def_die);
1029-
}
1030-
TypeSystemClang::CompleteTagDeclarationDefinition(clang_type);
1031-
} else {
1022+
if (!CompleteEnumType(def_die, type_sp.get(), clang_type)) {
10321023
dwarf->GetObjectFile()->GetModule()->ReportError(
10331024
"DWARF DIE at {0:x16} named \"{1}\" was not able to start its "
10341025
"definition.\nPlease file a bug and attach the file at the "
@@ -2221,13 +2212,14 @@ bool DWARFASTParserClang::CompleteRecordType(const DWARFDIE &die,
22212212
bool DWARFASTParserClang::CompleteEnumType(const DWARFDIE &die,
22222213
lldb_private::Type *type,
22232214
const CompilerType &clang_type) {
2215+
assert(clang_type.IsEnumerationType());
2216+
22242217
if (TypeSystemClang::StartTagDeclarationDefinition(clang_type)) {
2225-
if (die.HasChildren()) {
2226-
bool is_signed = false;
2227-
clang_type.IsIntegerType(is_signed);
2228-
ParseChildEnumerators(clang_type, is_signed,
2218+
if (die.HasChildren())
2219+
ParseChildEnumerators(clang_type,
2220+
clang_type.IsEnumerationIntegerTypeSigned(),
22292221
type->GetByteSize(nullptr).value_or(0), die);
2230-
}
2222+
22312223
TypeSystemClang::CompleteTagDeclarationDefinition(clang_type);
22322224
}
22332225
return (bool)clang_type;

0 commit comments

Comments
 (0)