-
Notifications
You must be signed in to change notification settings - Fork 13.6k
[lldb][TypeSystemClang] Add support for floating point template argument constants #127206
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
Changes from 2 commits
759f056
e78624d
00233b9
c9ad8b5
8ad988f
ceb86b8
7eca96d
944559f
6bf3c81
1fe6a61
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -91,7 +91,7 @@ lldb::ChildCacheState GenericBitsetFrontEnd::Update() { | |
size_t size = 0; | ||
|
||
if (auto arg = m_backend.GetCompilerType().GetIntegralTemplateArgument(0)) | ||
size = arg->value.getLimitedValue(); | ||
size = arg->value.getInt().getLimitedValue(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Technically I think these need to check that the value is an integral type as a malicious binary could crash lldb by declaring a |
||
|
||
m_elements.assign(size, ValueObjectSP()); | ||
m_first = | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1973,6 +1973,27 @@ class DWARFASTParserClang::DelayedAddObjCClassProperty { | |
ClangASTMetadata m_metadata; | ||
}; | ||
|
||
static clang::APValue MakeAPValue(CompilerType clang_type, uint64_t bit_width, | ||
uint64_t value) { | ||
bool is_signed = false; | ||
const bool is_integral = clang_type.IsIntegerOrEnumerationType(is_signed); | ||
|
||
llvm::APSInt apint(bit_width, !is_signed); | ||
apint = value; | ||
|
||
if (is_integral) | ||
return clang::APValue(apint); | ||
|
||
uint32_t count; | ||
bool is_complex; | ||
assert(clang_type.IsFloatingPointType(count, is_complex)); | ||
|
||
if (bit_width == 32) | ||
return clang::APValue(llvm::APFloat(apint.bitsToFloat())); | ||
|
||
return clang::APValue(llvm::APFloat(apint.bitsToDouble())); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Any chance this could be something like Reason being There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmmm good question The only differentiator between the different float types we have in DWARF is the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think we need to solve the problem of parsing all floating point types here. What I'm mainly interested in is seeing if there's a way to write this code such that it is automatically correct for any floating point type that we do support. I'm assuming/hoping that there is a way to get floating point semantics object out of a clang type without relying on the bitwidth... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ahh I misunderstood the suggestion. Yes, getting the semantics from the QualType makes sense here. Done in the latest commit |
||
} | ||
|
||
bool DWARFASTParserClang::ParseTemplateDIE( | ||
const DWARFDIE &die, | ||
TypeSystemClang::TemplateParameterInfos &template_param_infos) { | ||
|
@@ -2050,9 +2071,6 @@ bool DWARFASTParserClang::ParseTemplateDIE( | |
clang_type = m_ast.GetBasicType(eBasicTypeVoid); | ||
|
||
if (!is_template_template_argument) { | ||
bool is_signed = false; | ||
// Get the signed value for any integer or enumeration if available | ||
clang_type.IsIntegerOrEnumerationType(is_signed); | ||
|
||
if (name && !name[0]) | ||
name = nullptr; | ||
|
@@ -2061,11 +2079,12 @@ bool DWARFASTParserClang::ParseTemplateDIE( | |
std::optional<uint64_t> size = clang_type.GetBitSize(nullptr); | ||
if (!size) | ||
return false; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It looks like this exists only to be passed to |
||
llvm::APInt apint(*size, uval64, is_signed); | ||
|
||
template_param_infos.InsertArg( | ||
name, clang::TemplateArgument(ast, llvm::APSInt(apint, !is_signed), | ||
ClangUtil::GetQualType(clang_type), | ||
is_default_template_arg)); | ||
name, | ||
clang::TemplateArgument(ast, ClangUtil::GetQualType(clang_type), | ||
MakeAPValue(clang_type, *size, uval64), | ||
is_default_template_arg)); | ||
} else { | ||
template_param_infos.InsertArg( | ||
name, clang::TemplateArgument(ClangUtil::GetQualType(clang_type), | ||
|
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.
I just realized this is adding a clang dependency on lldb core (non-plugin) code. What would you say to making this a
Scalar
instead?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.
Ah good point. Changed to Scalar, wasn't a difficult transformation to make