Skip to content

[lldb] Fix dynamic type resolution for types parsed from declarations #137974

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

Merged
merged 1 commit into from
May 2, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions lldb/source/Plugins/ExpressionParser/Clang/ClangASTMetadata.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,22 @@

using namespace lldb_private;

std::optional<bool> ClangASTMetadata::GetIsDynamicCXXType() const {
switch (m_is_dynamic_cxx) {
case 0:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we make these enum cases for readability? Don't feel strongly either way.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that's necessary given that the only one who needs to know about them are these two short functions. And then I would be able to do the bool+1 trick :P

return std::nullopt;
case 1:
return false;
case 2:
return true;
}
llvm_unreachable("Invalid m_is_dynamic_cxx value");
}

void ClangASTMetadata::SetIsDynamicCXXType(std::optional<bool> b) {
m_is_dynamic_cxx = b ? *b + 1 : 0;
}

void ClangASTMetadata::Dump(Stream *s) {
lldb::user_id_t uid = GetUserID();

Expand Down
15 changes: 9 additions & 6 deletions lldb/source/Plugins/ExpressionParser/Clang/ClangASTMetadata.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,19 +12,22 @@
#include "lldb/Core/dwarf.h"
#include "lldb/lldb-defines.h"
#include "lldb/lldb-enumerations.h"
#include "lldb/lldb-private-enumerations.h"

namespace lldb_private {

class ClangASTMetadata {
public:
ClangASTMetadata()
: m_user_id(0), m_union_is_user_id(false), m_union_is_isa_ptr(false),
m_has_object_ptr(false), m_is_self(false), m_is_dynamic_cxx(true),
m_is_forcefully_completed(false) {}
m_has_object_ptr(false), m_is_self(false),
m_is_forcefully_completed(false) {
SetIsDynamicCXXType(std::nullopt);
}

bool GetIsDynamicCXXType() const { return m_is_dynamic_cxx; }
std::optional<bool> GetIsDynamicCXXType() const;

void SetIsDynamicCXXType(bool b) { m_is_dynamic_cxx = b; }
void SetIsDynamicCXXType(std::optional<bool> b);

void SetUserID(lldb::user_id_t user_id) {
m_user_id = user_id;
Expand Down Expand Up @@ -101,8 +104,8 @@ class ClangASTMetadata {
uint64_t m_isa_ptr;
};

bool m_union_is_user_id : 1, m_union_is_isa_ptr : 1, m_has_object_ptr : 1,
m_is_self : 1, m_is_dynamic_cxx : 1, m_is_forcefully_completed : 1;
unsigned m_union_is_user_id : 1, m_union_is_isa_ptr : 1, m_has_object_ptr : 1,
m_is_self : 1, m_is_dynamic_cxx : 2, m_is_forcefully_completed : 1;
};

} // namespace lldb_private
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1837,7 +1837,8 @@ DWARFASTParserClang::ParseStructureLikeDIE(const SymbolContext &sc,

ClangASTMetadata metadata;
metadata.SetUserID(die.GetID());
metadata.SetIsDynamicCXXType(dwarf->ClassOrStructIsVirtual(die));
if (!attrs.is_forward_declaration)
metadata.SetIsDynamicCXXType(dwarf->ClassOrStructIsVirtual(die));

TypeSystemClang::TemplateParameterInfos template_param_infos;
if (ParseTemplateParameterInfos(die, template_param_infos)) {
Expand Down
18 changes: 11 additions & 7 deletions lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3657,13 +3657,17 @@ bool TypeSystemClang::IsPossibleDynamicType(lldb::opaque_compiler_type_t type,
bool success;
if (cxx_record_decl->isCompleteDefinition())
success = cxx_record_decl->isDynamicClass();
else if (std::optional<ClangASTMetadata> metadata =
GetMetadata(cxx_record_decl))
success = metadata->GetIsDynamicCXXType();
else if (GetType(pointee_qual_type).GetCompleteType())
success = cxx_record_decl->isDynamicClass();
else
success = false;
else {
std::optional<ClangASTMetadata> metadata = GetMetadata(cxx_record_decl);
std::optional<bool> is_dynamic =
metadata ? metadata->GetIsDynamicCXXType() : std::nullopt;
if (is_dynamic)
success = *is_dynamic;
else if (GetType(pointee_qual_type).GetCompleteType())
success = cxx_record_decl->isDynamicClass();
else
success = false;
}

if (success)
set_dynamic_pointee_type(pointee_qual_type);
Expand Down
2 changes: 1 addition & 1 deletion lldb/test/API/lang/cpp/dynamic-value/Makefile
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
CXX_SOURCES := pass-to-base.cpp anonymous-b.cpp
CXX_SOURCES := pass-to-base.cpp anonymous-b.cpp forward-a.cpp

include Makefile.rules
12 changes: 12 additions & 0 deletions lldb/test/API/lang/cpp/dynamic-value/TestDynamicValue.py
Original file line number Diff line number Diff line change
Expand Up @@ -266,3 +266,15 @@ def examine_value_object_of_this_ptr(
contained_b_static_addr = int(contained_b_static.GetValue(), 16)

self.assertLess(contained_b_addr, contained_b_static_addr)

@no_debug_info_test
def test_from_forward_decl(self):
"""Test fetching C++ dynamic values forward-declared types. It's
imperative that this is a separate test so that we don't end up parsing
a definition of A from somewhere else."""
self.build()
lldbutil.run_to_name_breakpoint(self, "take_A")
self.expect(
"frame var -d run-target --ptr-depth=1 --show-types a",
substrs=["(B *) a", "m_b_value = 10"],
)
2 changes: 2 additions & 0 deletions lldb/test/API/lang/cpp/dynamic-value/a.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,6 @@ class A {

A *make_anonymous_B();

A *take_A(A *a);

#endif
3 changes: 3 additions & 0 deletions lldb/test/API/lang/cpp/dynamic-value/forward-a.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
class A;

A *take_A(A *a) { return a; }
2 changes: 2 additions & 0 deletions lldb/test/API/lang/cpp/dynamic-value/pass-to-base.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -45,5 +45,7 @@ main (int argc, char **argv)

myB.doSomething(*make_anonymous_B());

take_A(&myB);

return 0;
}
Loading