[debugger] Resolve extern variables
Implements reading extern global variables and static structure members
defined externally. This requires converting the extern value
declaration to a real variable to get its location, and then evaluating
that.
This requires a new member is added to Value that reflects the
DW_AT_external DWARF tag. Symbol reading is updated accordingly.
This function is added to EvalContext::GetVariableValue() which handles
the global extern variable case. This function is changed to take a
Valur and not a Variable so both Variables and static DataMembers can be
passed to it.
A special code path is added to resolving structure members that detects
extern static members and resolve the variable using the above function.
Array symbols are updated to allow unknown array sizes. These can be
encountered when dealing with extern data. There is not much to do with
this type, but we can at least represent it without errors.
Documentation is added to array_type.h on this case.
Skips printing static data members when printing structs. This decision
is documented in format.cc. We can reevaluate and add in the future if
desired.
Change-Id: I65a7dda98664f8fccb1e943cfb9420fcdf885654
diff --git a/src/developer/debug/zxdb/expr/eval_context.h b/src/developer/debug/zxdb/expr/eval_context.h
index 3e5072e..8f5afbe 100644
--- a/src/developer/debug/zxdb/expr/eval_context.h
+++ b/src/developer/debug/zxdb/expr/eval_context.h
@@ -20,6 +20,7 @@
class PrettyTypeManager;
class Symbol;
class SymbolDataProvider;
+class Value;
class Variable;
// Interface used by expression evaluation to communicate with the outside world. This provides
@@ -55,9 +56,14 @@
// If the EvalContext is destroyed before the data is ready, the callback will not be issued.
virtual void GetNamedValue(const ParsedIdentifier& identifier, ValueCallback cb) const = 0;
- // Like GetNamedValue() but takes an already-identified Variable. In this case the Symbol of the
- // callback will just be the input |variable|.
- virtual void GetVariableValue(fxl::RefPtr<Variable> variable, ValueCallback cb) const = 0;
+ // Like GetNamedValue() but takes an already-identified Variable.
+ //
+ // This will handle extern variables and will resolve them. In this case the ValueCallback's
+ // variable will be the resolved extern one. Otherwise it will be the input Value.
+ //
+ // The value is normally a Variable but it can also be an extern DataMember (which will transform
+ // into a Variable when the extern is resolved).
+ virtual void GetVariableValue(fxl::RefPtr<Value> variable, ValueCallback cb) const = 0;
// Attempts to resolve a type that is a declaration (is_declaration() is set on the type) by
// looking up a non-declaration type with the same name.
diff --git a/src/developer/debug/zxdb/expr/eval_context_impl.cc b/src/developer/debug/zxdb/expr/eval_context_impl.cc
index d54303e..cb7eb7f 100644
--- a/src/developer/debug/zxdb/expr/eval_context_impl.cc
+++ b/src/developer/debug/zxdb/expr/eval_context_impl.cc
@@ -137,7 +137,18 @@
});
}
-void EvalContextImpl::GetVariableValue(fxl::RefPtr<Variable> var, ValueCallback cb) const {
+void EvalContextImpl::GetVariableValue(fxl::RefPtr<Value> input_val, ValueCallback cb) const {
+ fxl::RefPtr<Variable> var;
+ if (input_val->is_external()) {
+ // Convert extern Variables and DataMembers to the actual variable memory.
+ if (Err err = ResolveExternValue(input_val, &var); err.has_error())
+ return cb(err, nullptr);
+ } else {
+ // Everything else should be a variable.
+ var = RefPtrTo(input_val->AsVariable());
+ FXL_DCHECK(var);
+ }
+
// Need to explicitly take a reference to the type.
fxl::RefPtr<Type> type = RefPtrTo(var->type().Get()->AsType());
if (!type)
@@ -261,6 +272,27 @@
return locations[0];
}
+Err EvalContextImpl::ResolveExternValue(const fxl::RefPtr<Value>& input_value,
+ fxl::RefPtr<Variable>* resolved) const {
+ FXL_DCHECK(input_value->is_external());
+
+ FindNameOptions options(FindNameOptions::kNoKinds);
+ options.find_vars = true;
+
+ // Passing a null block in the FindNameContext will bypass searching the current scope and
+ // "this" object and instead only search global names. This is what we want since the extern
+ // Value name will be fully qualified.
+ FindNameContext context = GetFindNameContext();
+ context.block = nullptr;
+
+ FoundName found = FindName(context, options, ToParsedIdentifier(input_value->GetIdentifier()));
+ if (!found || !found.variable())
+ return Err("Extern variable '%s' not found.", input_value->GetFullName().c_str());
+
+ *resolved = std::move(found.variable_ref());
+ return Err();
+}
+
void EvalContextImpl::DoResolve(FoundName found, ValueCallback cb) const {
if (found.kind() == FoundName::kVariable) {
// Simple variable resolution.
@@ -268,8 +300,14 @@
return;
}
- // Object variable resolution: Get the value of of the |this| variable.
+ // Everything below here is an object variable resolution.
FXL_DCHECK(found.kind() == FoundName::kMemberVariable);
+
+ // Static ("external") data members don't require a "this" pointer.
+ if (found.member().data_member()->is_external())
+ return GetVariableValue(RefPtrTo(found.member().data_member()), std::move(cb));
+
+ // Get the value of of the |this| variable to resolve.
GetVariableValue(
found.object_ptr_ref(), [weak_this = weak_factory_.GetWeakPtr(), found, cb = std::move(cb)](
ErrOrValue value, fxl::RefPtr<Symbol> symbol) mutable {
diff --git a/src/developer/debug/zxdb/expr/eval_context_impl.h b/src/developer/debug/zxdb/expr/eval_context_impl.h
index 80afc5e..4bc8713 100644
--- a/src/developer/debug/zxdb/expr/eval_context_impl.h
+++ b/src/developer/debug/zxdb/expr/eval_context_impl.h
@@ -25,17 +25,16 @@
class SymbolDataProvider;
class Variable;
-// An implementation of EvalContext that integrates with the DWARF symbol
-// system. It will provide the values of variables currently in scope.
+// An implementation of EvalContext that integrates with the DWARF symbol system. It will provide
+// the values of variables currently in scope.
//
-// This object is reference counted since it requires asynchronous operations
-// in some cases. This means it can outlive the scope in which it was invoked
-// (say if the thread was resumed or the process was killed).
+// This object is reference counted since it requires asynchronous operations in some cases. This
+// means it can outlive the scope in which it was invoked (say if the thread was resumed or the
+// process was killed).
//
-// Generally the creator of this context will be something representing that
-// context in the running program like a stack frame. This frame should call
-// DisownContext() when it is destroyed to ensure that evaluation does not use
-// any invalid context.
+// Generally the creator of this context will be something representing that context in the running
+// program like a stack frame. This frame should call DisownContext() when it is destroyed to ensure
+// that evaluation does not use any invalid context.
class EvalContextImpl : public EvalContext {
public:
// All of the input pointers can be null:
@@ -61,7 +60,7 @@
// EvalContext implementation.
ExprLanguage GetLanguage() const override;
void GetNamedValue(const ParsedIdentifier& name, ValueCallback cb) const override;
- void GetVariableValue(fxl::RefPtr<Variable> variable, ValueCallback cb) const override;
+ void GetVariableValue(fxl::RefPtr<Value> variable, ValueCallback cb) const override;
fxl::RefPtr<Type> ResolveForwardDefinition(const Type* type) const override;
fxl::RefPtr<Type> GetConcreteType(const Type* type) const override;
fxl::RefPtr<SymbolDataProvider> GetDataProvider() override;
@@ -72,8 +71,12 @@
private:
struct ResolutionState;
- // Computes the value of the given variable and issues the callback (possibly
- // asynchronously, possibly not).
+ // Converts an extern value to a real Variable by looking the name up in the index.
+ Err ResolveExternValue(const fxl::RefPtr<Value>& input_value,
+ fxl::RefPtr<Variable>* resolved) const;
+
+ // Computes the value of the given variable and issues the callback (possibly asynchronously,
+ // possibly not).
void DoResolve(FoundName found, ValueCallback cb) const;
// Callback for when the dwarf_eval_ has completed evaluation.
@@ -88,8 +91,8 @@
SymbolContext symbol_context_;
fxl::RefPtr<SymbolDataProvider> data_provider_; // Possibly null.
- // Innermost block of the current context. May be null if there is none
- // (this means you won't get any local variable lookups).
+ // Innermost block of the current context. May be null if there is none (this means you won't get
+ // any local variable lookups).
fxl::RefPtr<const CodeBlock> block_;
// Language extracted from the code block.
diff --git a/src/developer/debug/zxdb/expr/eval_context_impl_unittest.cc b/src/developer/debug/zxdb/expr/eval_context_impl_unittest.cc
index b2a24e6..1c85119 100644
--- a/src/developer/debug/zxdb/expr/eval_context_impl_unittest.cc
+++ b/src/developer/debug/zxdb/expr/eval_context_impl_unittest.cc
@@ -344,6 +344,53 @@
EXPECT_EQ(ExprValue(kValue), result1.value.value());
}
+// Tests that externs are resolved by GetVariableValue(). This requires using the index.
+TEST_F(EvalContextImplTest, ExternVariable) {
+ // Offset from beginning of the module of the data.
+ constexpr uint8_t kRelativeValAddress = 0x99;
+ const char kValName[] = "val";
+
+ // The non-extern declaration for the variable (0, 0 means always valid). The little-endian
+ // module-relative address follows DW_OP_addr in the expression.
+ auto real_variable = MakeUint64VariableForTest(
+ kValName, 0, 0, {llvm::dwarf::DW_OP_addr, kRelativeValAddress, 0, 0, 0, 0, 0, 0, 0});
+
+ // A reference to the same variable, marked "external" with no location.
+ auto extern_variable = fxl::MakeRefCounted<Variable>(DwarfTag::kVariable);
+ extern_variable->set_assigned_name(kValName);
+ extern_variable->set_is_external(true);
+ extern_variable->set_type(MakeUint64Type());
+
+ constexpr uint64_t kLoadAddress = 0x1000000;
+ constexpr uint64_t kAbsoluteValAddress = kLoadAddress + kRelativeValAddress;
+
+ // Need to have a module for the variable to be relative to and to have an index.
+ ProcessSymbolsTestSetup setup;
+ auto module_symbols = fxl::MakeRefCounted<MockModuleSymbols>("mod.so");
+ SymbolContext symbol_context(kLoadAddress);
+ setup.InjectModule("mod1", "1234", kLoadAddress, module_symbols);
+
+ // Index the non-extern variable.
+ auto& root = module_symbols->index().root(); // Root of the index for module.
+ TestIndexedSymbol indexed_def(module_symbols.get(), &root, kValName, real_variable);
+
+ // Set the value for the non-extern variable in the mocked memory.
+ constexpr uint64_t kValValue = 0x0102030405060708;
+ provider()->AddMemory(kAbsoluteValAddress, {8, 7, 6, 5, 4, 3, 2, 1});
+
+ auto context = fxl::MakeRefCounted<EvalContextImpl>(setup.process().GetWeakPtr(), symbol_context,
+ provider(), MakeCodeBlock());
+
+ // Resolving the extern variable should give the value that the non-extern one points to.
+ ValueResult result;
+ GetVariableValue(context, extern_variable, GetValueAsync::kQuitLoop, &result);
+ loop().Run();
+ ASSERT_TRUE(result.called);
+ ASSERT_TRUE(result.value.ok());
+ EXPECT_EQ(ExprValue(kValValue), result.value.value());
+ EXPECT_EQ(static_cast<const Symbol*>(real_variable.get()), result.symbol.get());
+}
+
// This is a larger test that runs the EvalContext through ExprNode.Eval.
TEST_F(EvalContextImplTest, NodeIntegation) {
constexpr uint64_t kValue = 12345678;
diff --git a/src/developer/debug/zxdb/expr/format.cc b/src/developer/debug/zxdb/expr/format.cc
index b24cbfa..d7a8f72 100644
--- a/src/developer/debug/zxdb/expr/format.cc
+++ b/src/developer/debug/zxdb/expr/format.cc
@@ -283,15 +283,13 @@
if (!member)
continue;
- // TODO(brettw) handle static data members here.
-
// Save the first member's name to be the name of the whole enum, even if there are no data
// members. Normally there will be exactly one.
if (enum_name.empty())
enum_name = member->GetAssignedName();
// In the error case, still append a child so that the child can have the error associated with
- // it.
+ // it. Note that Rust enums are never static so we can use the synchronous variant.
node->children().push_back(std::make_unique<FormatNode>(
member->GetAssignedName(), ResolveNonstaticMember(eval_context, node->value(), member)));
}
@@ -385,6 +383,16 @@
if (member->artificial())
continue; // Skip compiler-generated data.
+ // Skip static data members. This could potentially be revisited. This generally gives
+ // duplicated and uninteresting data in the view, and the user can still explicitly type the
+ // name if desired.
+ //
+ // To implement we should probably append a FormatNode with a lambda that gets the right
+ // value. It can be asynchronously expanded layer. That way this function doesn't need to
+ // handle any asynchronous state.
+ if (member->is_external())
+ continue;
+
node->children().push_back(std::make_unique<FormatNode>(
member->GetAssignedName(), ResolveNonstaticMember(eval_context, node->value(), member)));
}
@@ -537,12 +545,18 @@
if (!array)
return false;
+ if (!array->num_elts()) {
+ // Unknown array size, see ArrayType header for what this means. Nothing to do in this case.
+ node->SetDescribedError(Err("Array with unknown size."));
+ return true;
+ }
+
auto value_type = eval_context->GetConcreteType(array->value_type());
if (!value_type)
return false;
if (IsCharacterType(eval_context, value_type.get())) {
- size_t length = array->num_elts();
+ size_t length = *array->num_elts();
bool truncated = false;
if (length > options.max_array_size) {
length = options.max_array_size;
@@ -550,7 +564,8 @@
}
FormatCharArrayNode(node, value_type, node->value().data().data(), length, true, truncated);
} else {
- FormatArrayNode(node, node->value(), array->num_elts(), options, eval_context, std::move(cb));
+ FormatArrayNode(node, node->value(), *array->num_elts(), options, eval_context,
+ std::move(cb));
}
return true;
}
diff --git a/src/developer/debug/zxdb/expr/format_unittest.cc b/src/developer/debug/zxdb/expr/format_unittest.cc
index 589aa85..ce31e0b 100644
--- a/src/developer/debug/zxdb/expr/format_unittest.cc
+++ b/src/developer/debug/zxdb/expr/format_unittest.cc
@@ -286,6 +286,28 @@
SyncTreeTypeDesc(pair_value, opts));
}
+TEST_F(FormatTest, StructStatic) {
+ // Currently we don't output static struct members so this test validates that this case is
+ // handled as expected. This may be changed in the future if we change the policy on statics.
+ auto extern_member = fxl::MakeRefCounted<DataMember>("static_one", MakeInt32Type(), 0);
+ extern_member->set_is_external(true);
+ auto regular_member = fxl::MakeRefCounted<DataMember>("regular_one", MakeInt32Type(), 0);
+
+ auto collection = fxl::MakeRefCounted<Collection>(DwarfTag::kStructureType);
+ collection->set_assigned_name("Collection");
+ collection->set_data_members({LazySymbol(extern_member), LazySymbol(regular_member)});
+
+ // The collection is just the single non-external int32.
+ constexpr uint8_t kRegularValue = 42;
+ ExprValue value(collection, {kRegularValue, 0, 0, 0});
+
+ FormatOptions opts;
+ EXPECT_EQ(
+ " = Collection, \n"
+ " regular_one = int32_t, 42\n",
+ SyncTreeTypeDesc(value, opts));
+}
+
TEST_F(FormatTest, Struct_Anon) {
// Test an anonymous struct. Clang will generate structs with no names for things like closures.
// This struct has no members.
diff --git a/src/developer/debug/zxdb/expr/mock_eval_context.cc b/src/developer/debug/zxdb/expr/mock_eval_context.cc
index 34b4c82f..b7e29c5 100644
--- a/src/developer/debug/zxdb/expr/mock_eval_context.cc
+++ b/src/developer/debug/zxdb/expr/mock_eval_context.cc
@@ -16,7 +16,10 @@
MockEvalContext::~MockEvalContext() = default;
-void MockEvalContext::AddVariable(const std::string& name, ExprValue v) { values_[name] = v; }
+void MockEvalContext::AddVariable(const std::string& name, ExprValue v) {
+ values_by_name_[name] = v;
+}
+void MockEvalContext::AddVariable(const Value* key, ExprValue v) { values_by_symbol_[key] = v; }
void MockEvalContext::AddType(fxl::RefPtr<Type> type) { types_[type->GetFullName()] = type; }
@@ -28,15 +31,23 @@
void MockEvalContext::GetNamedValue(const ParsedIdentifier& ident, ValueCallback cb) const {
// Can ignore the symbol output for this test, it's not needed by the
// expression evaluation system.
- auto found = values_.find(ident.GetFullName());
- if (found == values_.end())
- cb(Err("Not found"), nullptr);
- else
+ auto found = values_by_name_.find(ident.GetFullName());
+ if (found == values_by_name_.end()) {
+ cb(Err("MockEvalContext::GetVariableValue '%s' not found.", ident.GetFullName().c_str()),
+ nullptr);
+ } else {
cb(found->second, nullptr);
+ }
}
-void MockEvalContext::GetVariableValue(fxl::RefPtr<Variable> variable, ValueCallback cb) const {
- cb(Err("Not found"), nullptr);
+void MockEvalContext::GetVariableValue(fxl::RefPtr<Value> variable, ValueCallback cb) const {
+ auto found = values_by_symbol_.find(variable.get());
+ if (found == values_by_symbol_.end()) {
+ cb(Err("MockEvalContext::GetVariableValue '%s' not found.", variable->GetFullName().c_str()),
+ nullptr);
+ } else {
+ cb(found->second, nullptr);
+ }
}
fxl::RefPtr<Type> MockEvalContext::ResolveForwardDefinition(const Type* type) const {
diff --git a/src/developer/debug/zxdb/expr/mock_eval_context.h b/src/developer/debug/zxdb/expr/mock_eval_context.h
index 5203d3d..1d15ddd 100644
--- a/src/developer/debug/zxdb/expr/mock_eval_context.h
+++ b/src/developer/debug/zxdb/expr/mock_eval_context.h
@@ -24,8 +24,10 @@
void set_language(ExprLanguage lang) { language_ = lang; }
- // Adds the given mocked variable with the given name and value.
+ // Adds the given mocked variable with the given name and value. If using the "Value" variant, the
+ // checked thing is the actual pointer value, not the name.
void AddVariable(const std::string& name, ExprValue v);
+ void AddVariable(const Value* key, ExprValue v);
// Adds a definition for the given mocked type for returning from ResolveForwardDefinition() and
// GetConcreteType().
@@ -37,7 +39,7 @@
// EvalContext implementation.
ExprLanguage GetLanguage() const override { return language_; }
void GetNamedValue(const ParsedIdentifier& ident, ValueCallback cb) const override;
- void GetVariableValue(fxl::RefPtr<Variable> variable, ValueCallback cb) const override;
+ void GetVariableValue(fxl::RefPtr<Value> variable, ValueCallback cb) const override;
fxl::RefPtr<Type> ResolveForwardDefinition(const Type* type) const override;
fxl::RefPtr<Type> GetConcreteType(const Type* type) const override;
fxl::RefPtr<SymbolDataProvider> GetDataProvider() override;
@@ -47,7 +49,8 @@
private:
fxl::RefPtr<MockSymbolDataProvider> data_provider_;
- std::map<std::string, ExprValue> values_;
+ std::map<std::string, ExprValue> values_by_name_;
+ std::map<const Value*, ExprValue> values_by_symbol_;
std::map<std::string, fxl::RefPtr<Type>> types_;
std::map<uint64_t, Location> locations_;
ExprLanguage language_ = ExprLanguage::kC;
diff --git a/src/developer/debug/zxdb/expr/pretty_type.cc b/src/developer/debug/zxdb/expr/pretty_type.cc
index 8580e5c..79ebb97 100644
--- a/src/developer/debug/zxdb/expr/pretty_type.cc
+++ b/src/developer/debug/zxdb/expr/pretty_type.cc
@@ -39,7 +39,7 @@
// EvalContext implementation. Everything except GetNamedValue() passes through to the impl_.
ExprLanguage GetLanguage() const { return impl_->GetLanguage(); }
void GetNamedValue(const ParsedIdentifier& name, ValueCallback cb) const;
- void GetVariableValue(fxl::RefPtr<Variable> variable, ValueCallback cb) const {
+ void GetVariableValue(fxl::RefPtr<Value> variable, ValueCallback cb) const {
return impl_->GetVariableValue(std::move(variable), std::move(cb));
}
fxl::RefPtr<Type> ResolveForwardDefinition(const Type* type) const {
diff --git a/src/developer/debug/zxdb/expr/pretty_type_manager_unittest.cc b/src/developer/debug/zxdb/expr/pretty_type_manager_unittest.cc
index 4b075da..11fb125f 100644
--- a/src/developer/debug/zxdb/expr/pretty_type_manager_unittest.cc
+++ b/src/developer/debug/zxdb/expr/pretty_type_manager_unittest.cc
@@ -151,7 +151,9 @@
fit::defer_callback([&called, loop = &loop()]() { called = true, loop->QuitNow(); }));
ASSERT_TRUE(called); // Current error case is sync.
- EXPECT_EQ("Not found", bool_node.err().msg());
+ EXPECT_EQ(
+ "MockEvalContext::GetVariableValue 'vector_bool_printer_not_implemented_yet' not found.",
+ bool_node.err().msg());
// Since this is an error, it should have no children.
ASSERT_EQ(0u, bool_node.children().size());
diff --git a/src/developer/debug/zxdb/expr/resolve_collection.cc b/src/developer/debug/zxdb/expr/resolve_collection.cc
index 4d76739..bf54334 100644
--- a/src/developer/debug/zxdb/expr/resolve_collection.cc
+++ b/src/developer/debug/zxdb/expr/resolve_collection.cc
@@ -151,7 +151,18 @@
// static data members that may require a memory fetch.
void DoResolveMember(const fxl::RefPtr<EvalContext>& context, const ExprValue& base,
const FoundMember& member, EvalCallback cb) {
- // TODO(brettw) resolve extern data members here.
+ FXL_DCHECK(member.data_member());
+ if (member.data_member()->is_external()) {
+ // A forward-declared static member.
+ return context->GetVariableValue(
+ RefPtrTo(member.data_member()),
+ [cb = std::move(cb)](ErrOrValue value, fxl::RefPtr<Symbol>) mutable {
+ // Discard the symbol since it's not needed in this path.
+ cb(std::move(value));
+ });
+ }
+
+ // Normal nonstatic resolution is synchronous.
cb(DoResolveNonstaticMember(context, base, member));
}
diff --git a/src/developer/debug/zxdb/expr/resolve_collection_unittest.cc b/src/developer/debug/zxdb/expr/resolve_collection_unittest.cc
index e057994..ee532fb 100644
--- a/src/developer/debug/zxdb/expr/resolve_collection_unittest.cc
+++ b/src/developer/debug/zxdb/expr/resolve_collection_unittest.cc
@@ -226,6 +226,45 @@
EXPECT_EQ(forward_decl.get(), result.value().type());
}
+TEST_F(ResolveCollectionTest, ExternStaticMember) {
+ // This test doesn't do an end-to-end resolution of the EvalContextImpl resolving extern variables
+ // since that requires a lot of setup and is tested by the EvalContextImpl unit tests. Instead
+ // this test only tests the resolve_collection code and validates that the extern variable was
+ // detected and the right EvalContext function was called.
+ const char kName[] = "member_name";
+
+ // External data member.
+ auto extern_member = fxl::MakeRefCounted<DataMember>(kName, MakeInt32Type(), 0);
+ extern_member->set_is_external(true);
+
+ // Collection with the member.
+ auto collection = fxl::MakeRefCounted<Collection>(DwarfTag::kClassType);
+ extern_member->set_parent(collection);
+
+ collection->set_assigned_name("Collection");
+ collection->set_data_members({LazySymbol(extern_member)});
+
+ // The collection needs no storage since the member is static.
+ ExprValue collection_value(collection, {});
+
+ auto mock_eval_context = fxl::MakeRefCounted<MockEvalContext>();
+ ExprValue expected(42);
+ mock_eval_context->AddVariable(extern_member.get(), expected);
+
+ bool called = false;
+ ResolveMember(mock_eval_context, collection_value, ParsedIdentifier(kName),
+ [&called, expected](ErrOrValue result) {
+ called = true;
+
+ EXPECT_FALSE(result.has_error());
+ EXPECT_EQ(expected, result.value());
+ });
+ EXPECT_TRUE(called);
+
+ // Clear parent to eliminate circular refcount.
+ extern_member->set_parent(nullptr);
+}
+
TEST_F(ResolveCollectionTest, BadMemberArgs) {
const DataMember* a_data;
const DataMember* b_data;
diff --git a/src/developer/debug/zxdb/symbols/array_type.cc b/src/developer/debug/zxdb/symbols/array_type.cc
index d3dc940..ff66fe2 100644
--- a/src/developer/debug/zxdb/symbols/array_type.cc
+++ b/src/developer/debug/zxdb/symbols/array_type.cc
@@ -4,13 +4,15 @@
#include "src/developer/debug/zxdb/symbols/array_type.h"
+#include "src/developer/debug/zxdb/symbols/arch.h"
#include "src/lib/fxl/strings/string_printf.h"
namespace zxdb {
-ArrayType::ArrayType(fxl::RefPtr<Type> value_type, size_t num_elts)
+ArrayType::ArrayType(fxl::RefPtr<Type> value_type, std::optional<size_t> num_elts)
: Type(DwarfTag::kArrayType), value_type_(std::move(value_type)), num_elts_(num_elts) {
- set_byte_size(num_elts * value_type_->byte_size());
+ if (num_elts_)
+ set_byte_size(*num_elts_ * value_type_->byte_size());
}
ArrayType::~ArrayType() = default;
@@ -23,7 +25,7 @@
}
std::string ArrayType::ComputeFullNameOfNestedArray(const std::string& outer_dims) const {
- std::string elt_count = fxl::StringPrintf("[%zu]", num_elts_);
+ std::string elt_count = num_elts_ ? fxl::StringPrintf("[%zu]", *num_elts_) : "[]";
if (const ArrayType* inner_array = value_type_->AsArrayType()) {
// Special-case nested arrays.
return inner_array->ComputeFullNameOfNestedArray(outer_dims + elt_count);
diff --git a/src/developer/debug/zxdb/symbols/array_type.h b/src/developer/debug/zxdb/symbols/array_type.h
index a794758..84cc2e6 100644
--- a/src/developer/debug/zxdb/symbols/array_type.h
+++ b/src/developer/debug/zxdb/symbols/array_type.h
@@ -5,44 +5,51 @@
#ifndef SRC_DEVELOPER_DEBUG_ZXDB_SYMBOLS_ARRAY_TYPE_H_
#define SRC_DEVELOPER_DEBUG_ZXDB_SYMBOLS_ARRAY_TYPE_H_
+#include <optional>
+
#include "src/developer/debug/zxdb/symbols/type.h"
namespace zxdb {
-// Represents an array. An array is similar to a pointer but we specifically
-// know its an array and know its length.
+// Represents an array. An array is similar to a pointer but we specifically know that it is an
+// array and often know its length. Not much can be done with arrays with unknown lengths.
//
-// DWARF says an array *may* have a length, but in practice Clang and GCC
-// both define int[] as a pointer. Therefore, we require arrays to have
-// known lengths.
+// DWARF says an array *may* have a length. Clang and GCC define int[] as a pointer so we expect
+// all "real" arrays to have a length.
+//
+// The case that may not have lengths are extern definitions that refer to arrays. For example:
+//
+// extern const char kFoo[];
+//
+// will be marked as an "external" variable with an array type with no length. Resolving the
+// extern to getting the real variable definition will give an array type with a real length.
class ArrayType final : public Type {
public:
const ArrayType* AsArrayType() const override;
const Type* value_type() const { return value_type_.get(); }
- size_t num_elts() const { return num_elts_; }
+ std::optional<size_t> num_elts() const { return num_elts_; }
private:
FRIEND_REF_COUNTED_THREAD_SAFE(ArrayType);
FRIEND_MAKE_REF_COUNTED(ArrayType);
- // The actual type (rather than a LazySymbol) is passed to this constructor
- // because all Types expect to have their size set as a member, and we can't
- // compute the size of an array without knowing the size of the contained
- // elements.
- ArrayType(fxl::RefPtr<Type> value_type, size_t num_elts);
+ // The actual type (rather than a LazySymbol) is passed to this constructor because all Types
+ // expect to have their size set as a member, and we can't compute the size of an array without
+ // knowing the size of the contained elements.
+ ArrayType(fxl::RefPtr<Type> value_type, std::optional<size_t> num_elts);
~ArrayType() override;
std::string ComputeFullName() const override;
- // Normally array names are the contained type with a "[...]" on the end,
- // but nested array dimensions work in the other direction, so it will look
- // like "array[outer][inner]". This function takes a previously computed
- // substring for what should be "[outer]" and creates the final type name.
+ // Normally array names are the contained type with a "[...]" on the end, but nested array
+ // dimensions work in the other direction, so it will look like "array[outer][inner]". This
+ // function takes a previously computed substring for what should be "[outer]" and creates the
+ // final type name.
std::string ComputeFullNameOfNestedArray(const std::string& outer_dims) const;
const fxl::RefPtr<Type> value_type_;
- const size_t num_elts_;
+ const std::optional<size_t> num_elts_;
};
} // namespace zxdb
diff --git a/src/developer/debug/zxdb/symbols/data_member.h b/src/developer/debug/zxdb/symbols/data_member.h
index 97e45bd..b759ac2 100644
--- a/src/developer/debug/zxdb/symbols/data_member.h
+++ b/src/developer/debug/zxdb/symbols/data_member.h
@@ -20,7 +20,8 @@
// Symbol overrides.
const DataMember* AsDataMember() const;
- // The byte offset from the containing class or struct of this data member.
+ // The byte offset from the containing class or struct of this data member. This is only valid
+ // if !is_external() -- see the base class' Value::is_external().
uint32_t member_location() const { return member_location_; }
void set_member_location(uint32_t m) { member_location_ = m; }
diff --git a/src/developer/debug/zxdb/symbols/dwarf_symbol_factory.cc b/src/developer/debug/zxdb/symbols/dwarf_symbol_factory.cc
index 651758e..4a928928 100644
--- a/src/developer/debug/zxdb/symbols/dwarf_symbol_factory.cc
+++ b/src/developer/debug/zxdb/symbols/dwarf_symbol_factory.cc
@@ -134,10 +134,10 @@
return VariableLocation(std::move(entries));
}
-// Extracts the subrange size from an array subrange DIE. Puts the result in *size and returns true
-// on success, false on failure.
-bool ReadArraySubrange(llvm::DWARFContext* context, const llvm::DWARFDie& subrange_die,
- uint64_t* size) {
+// Extracts the subrange size from an array subrange DIE. Returns the value on success, nullopt on
+// failure.
+std::optional<size_t> ReadArraySubrange(llvm::DWARFContext* context,
+ const llvm::DWARFDie& subrange_die) {
// Extract the DW_AT_count attribute which Clang generates, and DW_AT_upper_bound which GCC
// generated.
DwarfDieDecoder range_decoder(context, subrange_die.getDwarfUnit());
@@ -149,13 +149,11 @@
range_decoder.AddUnsignedConstant(llvm::dwarf::DW_AT_upper_bound, &upper_bound);
if (!range_decoder.Decode(subrange_die) || (!count && !upper_bound))
- return false;
+ return std::nullopt;
if (count)
- *size = *count;
- else
- *size = *upper_bound;
- return true;
+ return static_cast<size_t>(*count);
+ return static_cast<size_t>(*upper_bound);
}
} // namespace
@@ -419,13 +417,10 @@
// Find all subranges stored in the declaration order. More than one means a multi-dimensional
// array.
- std::vector<uint64_t> subrange_sizes;
+ std::vector<std::optional<size_t>> subrange_sizes;
for (const llvm::DWARFDie& child : die) {
- if (child.getTag() == llvm::dwarf::DW_TAG_subrange_type) {
- subrange_sizes.push_back(0);
- if (!ReadArraySubrange(symbols_->context(), child, &subrange_sizes.back()))
- return fxl::MakeRefCounted<Symbol>();
- }
+ if (child.getTag() == llvm::dwarf::DW_TAG_subrange_type)
+ subrange_sizes.push_back(ReadArraySubrange(symbols_->context(), child));
}
// Require a subrange with a count in it. If we find cases where this isn't the case, we could add
@@ -586,6 +581,9 @@
llvm::Optional<bool> artificial;
decoder.AddBool(llvm::dwarf::DW_AT_artificial, &artificial);
+ llvm::Optional<bool> external;
+ decoder.AddBool(llvm::dwarf::DW_AT_external, &external);
+
llvm::Optional<uint64_t> member_offset;
decoder.AddUnsignedConstant(llvm::dwarf::DW_AT_data_member_location, &member_offset);
@@ -599,6 +597,8 @@
result->set_type(MakeLazy(type));
if (artificial)
result->set_artificial(*artificial);
+ if (external)
+ result->set_is_external(*external);
if (member_offset)
result->set_member_location(static_cast<uint32_t>(*member_offset));
return result;
@@ -923,6 +923,9 @@
llvm::DWARFDie type;
decoder.AddReference(llvm::dwarf::DW_AT_type, &type);
+ llvm::Optional<bool> external;
+ decoder.AddBool(llvm::dwarf::DW_AT_external, &external);
+
llvm::Optional<bool> artificial;
decoder.AddBool(llvm::dwarf::DW_AT_artificial, &artificial);
@@ -950,6 +953,8 @@
variable->set_assigned_name(*name);
if (type)
variable->set_type(MakeLazy(type));
+ if (external)
+ variable->set_is_external(*external);
if (artificial)
variable->set_artificial(*artificial);
variable->set_location(std::move(location));
diff --git a/src/developer/debug/zxdb/symbols/value.h b/src/developer/debug/zxdb/symbols/value.h
index 9b1905f..6f778ba 100644
--- a/src/developer/debug/zxdb/symbols/value.h
+++ b/src/developer/debug/zxdb/symbols/value.h
@@ -25,9 +25,20 @@
// Symbol::GetAssignedName().
void set_assigned_name(const std::string& n) { assigned_name_ = n; }
+ // May be incomplete for is_external() values.
const LazySymbol& type() const { return type_; }
void set_type(const LazySymbol& t) { type_ = t; }
+ // External (DW_AT_external) data members are for static struct data members and extern global
+ // variables. They don't have a location. To find these members, look up the full name in the
+ // symbol index to get the actual non-external definition.
+ //
+ // External values might also have different type information. An example is external arrays which
+ // won't have a length, but the real definition will have the length. When dealing with external
+ // data members, always use the type from the real definition.
+ bool is_external() const { return is_external_; }
+ void set_is_external(bool e) { is_external_ = e; }
+
// Artificial values are ones generated by the compiler that don't appear in the source. The most
// common example is "this" parameters to functions. Other examples are GCC-generated "__func__"
// variables and the discriminant data member in a rust enum.
@@ -45,6 +56,8 @@
private:
std::string assigned_name_;
LazySymbol type_;
+
+ bool is_external_ = false;
bool artificial_ = false;
};
diff --git a/src/developer/debug/zxdb/symbols/variable_test_support.h b/src/developer/debug/zxdb/symbols/variable_test_support.h
index 7c2fac0..6bedd11 100644
--- a/src/developer/debug/zxdb/symbols/variable_test_support.h
+++ b/src/developer/debug/zxdb/symbols/variable_test_support.h
@@ -21,6 +21,7 @@
// auto var = MakeVariableForTest(
// "var", my_type, 0x1000, 0x2000, { llvm::dwarf::DW_OP_reg0 });
//
+// Use 0's for the begin_ip_range and end_ip_range for a variable that's always valid.
fxl::RefPtr<Variable> MakeVariableForTest(const std::string& name, fxl::RefPtr<Type> type,
uint64_t begin_ip_range, uint64_t end_ip_range,
std::vector<uint8_t> location_expression);
