[Lldb-commits] [lldb] 8f63cf5 - [lldb][NFC] Cleanup ValueObject construction code
Author: Raphael Isemann
Date: 2021-02-23T09:39:18+01:00
New Revision: 8f63cf5da3c098f5f16a1055d6e13d4bcf399a27
URL:
https://github.com/llvm/llvm-project/commit/8f63cf5da3c098f5f16a1055d6e13d4bcf399a27
DIFF:
https://github.com/llvm/llvm-project/commit/8f63cf5da3c098f5f16a1055d6e13d4bcf399a27.diff
LOG: [lldb][NFC] Cleanup ValueObject construction code
Just code cleanup for ValueObject constructors:
* Use default member initializers where possible.
* Doxygenify the comments for membersa nd constructors where needed.
* Delete the default constructor which isn't defined.
* Initialize the bitfields via a utility struct instead of doing this in the
different constructors.
Reviewed By: JDevlieghere
Differential Revision: https://reviews.llvm.org/D97199
Added:
Modified:
lldb/include/lldb/Core/ValueObject.h
lldb/source/Core/ValueObject.cpp
lldb/source/Core/ValueObjectSyntheticFilter.cpp
Removed:
diff --git a/lldb/include/lldb/Core/ValueObject.h
b/lldb/include/lldb/Core/ValueObject.h
index a665e7afa0ca..e8c7defa330c 100644
--- a/lldb/include/lldb/Core/ValueObject.h
+++ b/lldb/include/lldb/Core/ValueObject.h
@@ -420,7 +420,9 @@ class ValueObject : public UserID {
return (GetBitfieldBitSize() != 0) || (GetBitfieldBitOffset() != 0);
}
- virtual bool IsArrayItemForPointer() { return m_is_array_item_for_pointer; }
+ virtual bool IsArrayItemForPointer() {
+return m_flags.m_is_array_item_for_pointer;
+ }
virtual const char *GetValueAsCString();
@@ -744,7 +746,9 @@ class ValueObject : public UserID {
AddressType GetAddressTypeOfChildren();
- void SetHasCompleteType() { m_did_calculate_complete_objc_class_type = true;
}
+ void SetHasCompleteType() {
+m_flags.m_did_calculate_complete_objc_class_type = true;
+ }
/// Find out if a ValueObject might have children.
///
@@ -815,76 +819,95 @@ class ValueObject : public UserID {
};
// Classes that inherit from ValueObject can see and modify these
- ValueObject
- *m_parent; // The parent value object, or nullptr if this has no parent
- ValueObject *m_root; // The root of the hierarchy for this ValueObject (or
- // nullptr if never calculated)
- EvaluationPoint m_update_point; // Stores both the stop id and the full
- // context at which this value was last
- // updated. When we are asked to update the value object, we check whether
- // the context & stop id are the same before updating.
- ConstString m_name; // The name of this object
- DataExtractor
- m_data; // A data extractor that can be used to extract the value.
+
+ /// The parent value object, or nullptr if this has no parent.
+ ValueObject *m_parent = nullptr;
+ /// The root of the hierarchy for this ValueObject (or nullptr if never
+ /// calculated).
+ ValueObject *m_root = nullptr;
+ /// Stores both the stop id and the full context at which this value was last
+ /// updated. When we are asked to update the value object, we check whether
+ /// the context & stop id are the same before updating.
+ EvaluationPoint m_update_point;
+ /// The name of this object.
+ ConstString m_name;
+ /// A data extractor that can be used to extract the value.
+ DataExtractor m_data;
Value m_value;
- Status
- m_error; // An error object that can describe any errors that occur when
- // updating values.
- std::string m_value_str; // Cached value string that will get cleared if/when
- // the value is updated.
- std::string m_old_value_str; // Cached old value string from the last time
the
- // value was gotten
- std::string m_location_str; // Cached location string that will get cleared
- // if/when the value is updated.
- std::string m_summary_str; // Cached summary string that will get cleared
- // if/when the value is updated.
- std::string m_object_desc_str; // Cached result of the "object printer".
This
- //
diff ers from the summary
- // in that the summary is consed up by us, the object_desc_string is builtin.
-
- CompilerType m_override_type; // If the type of the value object should be
-// overridden, the type to impose.
-
- ValueObjectManager *m_manager; // This object is managed by the root object
- // (any ValueObject that gets created
- // without a parent.) The manager gets passed through all the generations of
- // dependent objects, and will keep the whole cluster of objects alive as
- // long as a shared pointer to any of them has been handed out. Shared
- // pointers to value objects must always be made with the GetSP method.
+ /// An error object that can describe any errors that occur when updating
+ /// values.
[Lldb-commits] [PATCH] D97199: [lldb][NFC] Cleanup ValueObject construction code
This revision was automatically updated to reflect the committed changes.
Closed by commit rG8f63cf5da3c0: [lldb][NFC] Cleanup ValueObject construction
code (authored by teemperor).
Herald added a subscriber: lldb-commits.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D97199/new/
https://reviews.llvm.org/D97199
Files:
lldb/include/lldb/Core/ValueObject.h
lldb/source/Core/ValueObject.cpp
lldb/source/Core/ValueObjectSyntheticFilter.cpp
Index: lldb/source/Core/ValueObjectSyntheticFilter.cpp
===
--- lldb/source/Core/ValueObjectSyntheticFilter.cpp
+++ lldb/source/Core/ValueObjectSyntheticFilter.cpp
@@ -190,7 +190,7 @@
// children count for a synthetic VO that might indeed happen, so we need
// to tell the upper echelons that they need to come back to us asking for
// children
-m_children_count_valid = false;
+m_flags.m_children_count_valid = false;
{
std::lock_guard guard(m_child_mutex);
m_synthetic_children_cache.clear();
Index: lldb/source/Core/ValueObject.cpp
===
--- lldb/source/Core/ValueObject.cpp
+++ lldb/source/Core/ValueObject.cpp
@@ -77,26 +77,10 @@
// ValueObject constructor
ValueObject::ValueObject(ValueObject &parent)
: UserID(++g_value_obj_uid), // Unique identifier for every value object
- m_parent(&parent), m_root(nullptr),
- m_update_point(parent.GetUpdatePoint()), m_name(), m_data(), m_value(),
- m_error(), m_value_str(), m_old_value_str(), m_location_str(),
- m_summary_str(), m_object_desc_str(), m_manager(parent.GetManager()),
- m_children(), m_synthetic_children(), m_dynamic_value(nullptr),
- m_synthetic_value(nullptr), m_deref_valobj(nullptr),
- m_format(eFormatDefault), m_last_format(eFormatDefault),
- m_last_format_mgr_revision(0), m_type_summary_sp(), m_type_format_sp(),
- m_synthetic_children_sp(), m_user_id_of_forced_summary(),
- m_address_type_of_ptr_or_ref_children(eAddressTypeInvalid),
- m_value_checksum(),
- m_preferred_display_language(lldb::eLanguageTypeUnknown),
- m_language_flags(0), m_value_is_valid(false), m_value_did_change(false),
- m_children_count_valid(false), m_old_value_valid(false),
- m_is_deref_of_parent(false), m_is_array_item_for_pointer(false),
- m_is_bitfield_for_scalar(false), m_is_child_at_offset(false),
- m_is_getting_summary(false),
- m_did_calculate_complete_objc_class_type(false),
- m_is_synthetic_children_generated(
- parent.m_is_synthetic_children_generated) {
+ m_parent(&parent), m_update_point(parent.GetUpdatePoint()),
+ m_manager(parent.GetManager()) {
+ m_flags.m_is_synthetic_children_generated =
+ parent.m_flags.m_is_synthetic_children_generated;
m_data.SetByteOrder(parent.GetDataExtractor().GetByteOrder());
m_data.SetAddressByteSize(parent.GetDataExtractor().GetAddressByteSize());
m_manager->ManageObject(this);
@@ -107,25 +91,8 @@
ValueObjectManager &manager,
AddressType child_ptr_or_ref_addr_type)
: UserID(++g_value_obj_uid), // Unique identifier for every value object
- m_parent(nullptr), m_root(nullptr), m_update_point(exe_scope), m_name(),
- m_data(), m_value(), m_error(), m_value_str(), m_old_value_str(),
- m_location_str(), m_summary_str(), m_object_desc_str(),
- m_manager(&manager), m_children(), m_synthetic_children(),
- m_dynamic_value(nullptr), m_synthetic_value(nullptr),
- m_deref_valobj(nullptr), m_format(eFormatDefault),
- m_last_format(eFormatDefault), m_last_format_mgr_revision(0),
- m_type_summary_sp(), m_type_format_sp(), m_synthetic_children_sp(),
- m_user_id_of_forced_summary(),
- m_address_type_of_ptr_or_ref_children(child_ptr_or_ref_addr_type),
- m_value_checksum(),
- m_preferred_display_language(lldb::eLanguageTypeUnknown),
- m_language_flags(0), m_value_is_valid(false), m_value_did_change(false),
- m_children_count_valid(false), m_old_value_valid(false),
- m_is_deref_of_parent(false), m_is_array_item_for_pointer(false),
- m_is_bitfield_for_scalar(false), m_is_child_at_offset(false),
- m_is_getting_summary(false),
- m_did_calculate_complete_objc_class_type(false),
- m_is_synthetic_children_generated(false) {
+ m_update_point(exe_scope), m_manager(&manager),
+ m_address_type_of_ptr_or_ref_children(child_ptr_or_ref_addr_type) {
if (exe_scope) {
TargetSP target_sp(exe_scope->CalculateTarget());
if (target_sp) {
@@ -170,9 +137,9 @@
// Save the old value using swap to avoid a string copy which also will
// clear our m_value_str
if (m_value_str.empty()) {
- m_old_value_valid = false;
+ m_flags.m_old_value_valid = false;
} else {
- m_old_value_valid = true;
+ m_flags.
[Lldb-commits] [PATCH] D97205: [lldb][NFC] Don't inherit from UserID in ValueObject
teemperor updated this revision to Diff 325708.
teemperor added a comment.
- Rebased.
- Add a `GetID()` call where we relied on the `==` overload before.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D97205/new/
https://reviews.llvm.org/D97205
Files:
lldb/include/lldb/Core/ValueObject.h
lldb/source/Core/ValueObject.cpp
lldb/source/DataFormatters/TypeSynthetic.cpp
Index: lldb/source/DataFormatters/TypeSynthetic.cpp
===
--- lldb/source/DataFormatters/TypeSynthetic.cpp
+++ lldb/source/DataFormatters/TypeSynthetic.cpp
@@ -128,7 +128,7 @@
ValueObject &backend)
: SyntheticChildrenFrontEnd(backend), m_python_class(pclass),
m_wrapper_sp(), m_interpreter(nullptr) {
- if (backend == LLDB_INVALID_UID)
+ if (backend.GetID() == LLDB_INVALID_UID)
return;
TargetSP target_sp = backend.GetTargetSP();
Index: lldb/source/Core/ValueObject.cpp
===
--- lldb/source/Core/ValueObject.cpp
+++ lldb/source/Core/ValueObject.cpp
@@ -76,9 +76,8 @@
// ValueObject constructor
ValueObject::ValueObject(ValueObject &parent)
-: UserID(++g_value_obj_uid), // Unique identifier for every value object
- m_parent(&parent), m_update_point(parent.GetUpdatePoint()),
- m_manager(parent.GetManager()) {
+: m_parent(&parent), m_update_point(parent.GetUpdatePoint()),
+ m_manager(parent.GetManager()), m_id(++g_value_obj_uid) {
m_flags.m_is_synthetic_children_generated =
parent.m_flags.m_is_synthetic_children_generated;
m_data.SetByteOrder(parent.GetDataExtractor().GetByteOrder());
@@ -90,9 +89,9 @@
ValueObject::ValueObject(ExecutionContextScope *exe_scope,
ValueObjectManager &manager,
AddressType child_ptr_or_ref_addr_type)
-: UserID(++g_value_obj_uid), // Unique identifier for every value object
- m_update_point(exe_scope), m_manager(&manager),
- m_address_type_of_ptr_or_ref_children(child_ptr_or_ref_addr_type) {
+: m_update_point(exe_scope), m_manager(&manager),
+ m_address_type_of_ptr_or_ref_children(child_ptr_or_ref_addr_type),
+ m_id(++g_value_obj_uid) {
if (exe_scope) {
TargetSP target_sp(exe_scope->CalculateTarget());
if (target_sp) {
Index: lldb/include/lldb/Core/ValueObject.h
===
--- lldb/include/lldb/Core/ValueObject.h
+++ lldb/include/lldb/Core/ValueObject.h
@@ -102,7 +102,7 @@
/// Shared Pointer to the contained ValueObject,
/// just do so by calling GetSP() on the contained object.
-class ValueObject : public UserID {
+class ValueObject {
public:
enum GetExpressionPathFormat {
eGetExpressionPathFormatDereferencePointers = 1,
@@ -457,6 +457,9 @@
ConstString GetName() const;
+ /// Returns a unique uid for this ValueObject.
+ lldb::user_id_t GetID() const { return m_id.GetID(); }
+
virtual lldb::ValueObjectSP GetChildAtIndex(size_t idx, bool can_create);
// this will always create the children if necessary
@@ -885,6 +888,9 @@
uint64_t m_language_flags = 0;
+ /// Unique identifier for every value object.
+ UserID m_id;
+
// Utility class for initializing all bitfields in ValueObject's
constructors.
// FIXME: This could be done via default initializers once we have C++20.
struct Bitflags {
Index: lldb/source/DataFormatters/TypeSynthetic.cpp
===
--- lldb/source/DataFormatters/TypeSynthetic.cpp
+++ lldb/source/DataFormatters/TypeSynthetic.cpp
@@ -128,7 +128,7 @@
ValueObject &backend)
: SyntheticChildrenFrontEnd(backend), m_python_class(pclass),
m_wrapper_sp(), m_interpreter(nullptr) {
- if (backend == LLDB_INVALID_UID)
+ if (backend.GetID() == LLDB_INVALID_UID)
return;
TargetSP target_sp = backend.GetTargetSP();
Index: lldb/source/Core/ValueObject.cpp
===
--- lldb/source/Core/ValueObject.cpp
+++ lldb/source/Core/ValueObject.cpp
@@ -76,9 +76,8 @@
// ValueObject constructor
ValueObject::ValueObject(ValueObject &parent)
-: UserID(++g_value_obj_uid), // Unique identifier for every value object
- m_parent(&parent), m_update_point(parent.GetUpdatePoint()),
- m_manager(parent.GetManager()) {
+: m_parent(&parent), m_update_point(parent.GetUpdatePoint()),
+ m_manager(parent.GetManager()), m_id(++g_value_obj_uid) {
m_flags.m_is_synthetic_children_generated =
parent.m_flags.m_is_synthetic_children_generated;
m_data.SetByteOrder(parent.GetDataExtractor().GetByteOrder());
@@ -90,9 +89,9 @@
ValueObject::ValueObject(ExecutionContextScope *exe_scope,
ValueObjectManager &manager,
AddressType child_ptr_or_ref_addr_type)
-
[Lldb-commits] [lldb] d77e3c6 - [lldb][NFC] Don't inherit from UserID in ValueObject
Author: Raphael Isemann
Date: 2021-02-23T10:15:42+01:00
New Revision: d77e3c6aec2916fdf7b8ab0ca08c550230244695
URL:
https://github.com/llvm/llvm-project/commit/d77e3c6aec2916fdf7b8ab0ca08c550230244695
DIFF:
https://github.com/llvm/llvm-project/commit/d77e3c6aec2916fdf7b8ab0ca08c550230244695.diff
LOG: [lldb][NFC] Don't inherit from UserID in ValueObject
ValueObject inherits from UserID which is just a bad idea:
* The inheritance gives ValueObject some member functions that are at best
misleading (such as `Clear()` which doesn't clear any value beside `id`).
* It allows passing ValueObject to the overloaded operators for UserID (such as
`==` or `<<` which won't actually compare or print anything in the
ValueObject).
* It exposes the `SetID` and `Clear` which both allow users to change the
internal id value.
Similar to D91699 which did the same for Process
Reviewed By: #lldb, JDevlieghere
Differential Revision: https://reviews.llvm.org/D97205
Added:
Modified:
lldb/include/lldb/Core/ValueObject.h
lldb/source/Core/ValueObject.cpp
lldb/source/DataFormatters/TypeSynthetic.cpp
Removed:
diff --git a/lldb/include/lldb/Core/ValueObject.h
b/lldb/include/lldb/Core/ValueObject.h
index e8c7defa330c..308e0795efe6 100644
--- a/lldb/include/lldb/Core/ValueObject.h
+++ b/lldb/include/lldb/Core/ValueObject.h
@@ -102,7 +102,7 @@ class TypeSummaryOptions;
/// Shared Pointer to the contained ValueObject,
/// just do so by calling GetSP() on the contained object.
-class ValueObject : public UserID {
+class ValueObject {
public:
enum GetExpressionPathFormat {
eGetExpressionPathFormatDereferencePointers = 1,
@@ -457,6 +457,9 @@ class ValueObject : public UserID {
ConstString GetName() const;
+ /// Returns a unique id for this ValueObject.
+ lldb::user_id_t GetID() const { return m_id.GetID(); }
+
virtual lldb::ValueObjectSP GetChildAtIndex(size_t idx, bool can_create);
// this will always create the children if necessary
@@ -885,6 +888,9 @@ class ValueObject : public UserID {
uint64_t m_language_flags = 0;
+ /// Unique identifier for every value object.
+ UserID m_id;
+
// Utility class for initializing all bitfields in ValueObject's
constructors.
// FIXME: This could be done via default initializers once we have C++20.
struct Bitflags {
diff --git a/lldb/source/Core/ValueObject.cpp
b/lldb/source/Core/ValueObject.cpp
index 8f1d4e8cc7e5..6f736c3edfb7 100644
--- a/lldb/source/Core/ValueObject.cpp
+++ b/lldb/source/Core/ValueObject.cpp
@@ -76,9 +76,8 @@ static user_id_t g_value_obj_uid = 0;
// ValueObject constructor
ValueObject::ValueObject(ValueObject &parent)
-: UserID(++g_value_obj_uid), // Unique identifier for every value object
- m_parent(&parent), m_update_point(parent.GetUpdatePoint()),
- m_manager(parent.GetManager()) {
+: m_parent(&parent), m_update_point(parent.GetUpdatePoint()),
+ m_manager(parent.GetManager()), m_id(++g_value_obj_uid) {
m_flags.m_is_synthetic_children_generated =
parent.m_flags.m_is_synthetic_children_generated;
m_data.SetByteOrder(parent.GetDataExtractor().GetByteOrder());
@@ -90,9 +89,9 @@ ValueObject::ValueObject(ValueObject &parent)
ValueObject::ValueObject(ExecutionContextScope *exe_scope,
ValueObjectManager &manager,
AddressType child_ptr_or_ref_addr_type)
-: UserID(++g_value_obj_uid), // Unique identifier for every value object
- m_update_point(exe_scope), m_manager(&manager),
- m_address_type_of_ptr_or_ref_children(child_ptr_or_ref_addr_type) {
+: m_update_point(exe_scope), m_manager(&manager),
+ m_address_type_of_ptr_or_ref_children(child_ptr_or_ref_addr_type),
+ m_id(++g_value_obj_uid) {
if (exe_scope) {
TargetSP target_sp(exe_scope->CalculateTarget());
if (target_sp) {
diff --git a/lldb/source/DataFormatters/TypeSynthetic.cpp
b/lldb/source/DataFormatters/TypeSynthetic.cpp
index 75388a93cc64..ba806ca7cfbc 100644
--- a/lldb/source/DataFormatters/TypeSynthetic.cpp
+++ b/lldb/source/DataFormatters/TypeSynthetic.cpp
@@ -128,7 +128,7 @@ ScriptedSyntheticChildren::FrontEnd::FrontEnd(std::string
pclass,
ValueObject &backend)
: SyntheticChildrenFrontEnd(backend), m_python_class(pclass),
m_wrapper_sp(), m_interpreter(nullptr) {
- if (backend == LLDB_INVALID_UID)
+ if (backend.GetID() == LLDB_INVALID_UID)
return;
TargetSP target_sp = backend.GetTargetSP();
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D97205: [lldb][NFC] Don't inherit from UserID in ValueObject
This revision was automatically updated to reflect the committed changes.
Closed by commit rGd77e3c6aec29: [lldb][NFC] Don't inherit from UserID in
ValueObject (authored by teemperor).
Herald added a subscriber: lldb-commits.
Changed prior to commit:
https://reviews.llvm.org/D97205?vs=325708&id=325709#toc
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D97205/new/
https://reviews.llvm.org/D97205
Files:
lldb/include/lldb/Core/ValueObject.h
lldb/source/Core/ValueObject.cpp
lldb/source/DataFormatters/TypeSynthetic.cpp
Index: lldb/source/DataFormatters/TypeSynthetic.cpp
===
--- lldb/source/DataFormatters/TypeSynthetic.cpp
+++ lldb/source/DataFormatters/TypeSynthetic.cpp
@@ -128,7 +128,7 @@
ValueObject &backend)
: SyntheticChildrenFrontEnd(backend), m_python_class(pclass),
m_wrapper_sp(), m_interpreter(nullptr) {
- if (backend == LLDB_INVALID_UID)
+ if (backend.GetID() == LLDB_INVALID_UID)
return;
TargetSP target_sp = backend.GetTargetSP();
Index: lldb/source/Core/ValueObject.cpp
===
--- lldb/source/Core/ValueObject.cpp
+++ lldb/source/Core/ValueObject.cpp
@@ -76,9 +76,8 @@
// ValueObject constructor
ValueObject::ValueObject(ValueObject &parent)
-: UserID(++g_value_obj_uid), // Unique identifier for every value object
- m_parent(&parent), m_update_point(parent.GetUpdatePoint()),
- m_manager(parent.GetManager()) {
+: m_parent(&parent), m_update_point(parent.GetUpdatePoint()),
+ m_manager(parent.GetManager()), m_id(++g_value_obj_uid) {
m_flags.m_is_synthetic_children_generated =
parent.m_flags.m_is_synthetic_children_generated;
m_data.SetByteOrder(parent.GetDataExtractor().GetByteOrder());
@@ -90,9 +89,9 @@
ValueObject::ValueObject(ExecutionContextScope *exe_scope,
ValueObjectManager &manager,
AddressType child_ptr_or_ref_addr_type)
-: UserID(++g_value_obj_uid), // Unique identifier for every value object
- m_update_point(exe_scope), m_manager(&manager),
- m_address_type_of_ptr_or_ref_children(child_ptr_or_ref_addr_type) {
+: m_update_point(exe_scope), m_manager(&manager),
+ m_address_type_of_ptr_or_ref_children(child_ptr_or_ref_addr_type),
+ m_id(++g_value_obj_uid) {
if (exe_scope) {
TargetSP target_sp(exe_scope->CalculateTarget());
if (target_sp) {
Index: lldb/include/lldb/Core/ValueObject.h
===
--- lldb/include/lldb/Core/ValueObject.h
+++ lldb/include/lldb/Core/ValueObject.h
@@ -102,7 +102,7 @@
/// Shared Pointer to the contained ValueObject,
/// just do so by calling GetSP() on the contained object.
-class ValueObject : public UserID {
+class ValueObject {
public:
enum GetExpressionPathFormat {
eGetExpressionPathFormatDereferencePointers = 1,
@@ -457,6 +457,9 @@
ConstString GetName() const;
+ /// Returns a unique id for this ValueObject.
+ lldb::user_id_t GetID() const { return m_id.GetID(); }
+
virtual lldb::ValueObjectSP GetChildAtIndex(size_t idx, bool can_create);
// this will always create the children if necessary
@@ -885,6 +888,9 @@
uint64_t m_language_flags = 0;
+ /// Unique identifier for every value object.
+ UserID m_id;
+
// Utility class for initializing all bitfields in ValueObject's
constructors.
// FIXME: This could be done via default initializers once we have C++20.
struct Bitflags {
Index: lldb/source/DataFormatters/TypeSynthetic.cpp
===
--- lldb/source/DataFormatters/TypeSynthetic.cpp
+++ lldb/source/DataFormatters/TypeSynthetic.cpp
@@ -128,7 +128,7 @@
ValueObject &backend)
: SyntheticChildrenFrontEnd(backend), m_python_class(pclass),
m_wrapper_sp(), m_interpreter(nullptr) {
- if (backend == LLDB_INVALID_UID)
+ if (backend.GetID() == LLDB_INVALID_UID)
return;
TargetSP target_sp = backend.GetTargetSP();
Index: lldb/source/Core/ValueObject.cpp
===
--- lldb/source/Core/ValueObject.cpp
+++ lldb/source/Core/ValueObject.cpp
@@ -76,9 +76,8 @@
// ValueObject constructor
ValueObject::ValueObject(ValueObject &parent)
-: UserID(++g_value_obj_uid), // Unique identifier for every value object
- m_parent(&parent), m_update_point(parent.GetUpdatePoint()),
- m_manager(parent.GetManager()) {
+: m_parent(&parent), m_update_point(parent.GetUpdatePoint()),
+ m_manager(parent.GetManager()), m_id(++g_value_obj_uid) {
m_flags.m_is_synthetic_children_generated =
parent.m_flags.m_is_synthetic_children_generated;
m_data.SetByteOrder(parent.GetDataExtractor().GetByteOrder());
@@ -90,9
[Lldb-commits] [lldb] 03310c1 - [lldb][NFC] Give CompilerType's IsArrayType/IsVectorType/IsBlockPointerType out-parameters default values
Author: Raphael Isemann
Date: 2021-02-23T11:15:31+01:00
New Revision: 03310c1e952d0bf7aa84b8ed06809aa7ea1deb9b
URL:
https://github.com/llvm/llvm-project/commit/03310c1e952d0bf7aa84b8ed06809aa7ea1deb9b
DIFF:
https://github.com/llvm/llvm-project/commit/03310c1e952d0bf7aa84b8ed06809aa7ea1deb9b.diff
LOG: [lldb][NFC] Give CompilerType's
IsArrayType/IsVectorType/IsBlockPointerType out-parameters default values
We already do this for most functions that have out-parameters, so let's do
the same here and avoid all the `nullptr, nullptr, nullptr` in every call.
Added:
Modified:
lldb/include/lldb/Symbol/CompilerType.h
lldb/source/Core/ValueObject.cpp
lldb/source/DataFormatters/VectorType.cpp
lldb/source/Plugins/ABI/ARM/ABISysV_arm.cpp
lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
lldb/source/Plugins/Language/CPlusPlus/LibCxxBitset.cpp
lldb/source/Symbol/CompilerType.cpp
Removed:
diff --git a/lldb/include/lldb/Symbol/CompilerType.h
b/lldb/include/lldb/Symbol/CompilerType.h
index 5a0e8e57200d..0ad05a27570e 100644
--- a/lldb/include/lldb/Symbol/CompilerType.h
+++ b/lldb/include/lldb/Symbol/CompilerType.h
@@ -71,10 +71,12 @@ class CompilerType {
bool IsValid() const { return m_type != nullptr && m_type_system != nullptr;
}
- bool IsArrayType(CompilerType *element_type, uint64_t *size,
- bool *is_incomplete) const;
+ bool IsArrayType(CompilerType *element_type = nullptr,
+ uint64_t *size = nullptr,
+ bool *is_incomplete = nullptr) const;
- bool IsVectorType(CompilerType *element_type, uint64_t *size) const;
+ bool IsVectorType(CompilerType *element_type = nullptr,
+uint64_t *size = nullptr) const;
bool IsArrayOfScalarType() const;
@@ -110,7 +112,8 @@ class CompilerType {
bool IsFunctionPointerType() const;
- bool IsBlockPointerType(CompilerType *function_pointer_type_ptr) const;
+ bool
+ IsBlockPointerType(CompilerType *function_pointer_type_ptr = nullptr) const;
bool IsIntegerType(bool &is_signed) const;
diff --git a/lldb/source/Core/ValueObject.cpp
b/lldb/source/Core/ValueObject.cpp
index 6f736c3edfb7..8f90ae873160 100644
--- a/lldb/source/Core/ValueObject.cpp
+++ b/lldb/source/Core/ValueObject.cpp
@@ -914,7 +914,7 @@ ValueObject::ReadPointedString(lldb::DataBufferSP
&buffer_sp, Status &error,
if (is_array) {
// We have an array
uint64_t array_size = 0;
- if (compiler_type.IsArrayType(nullptr, &array_size, nullptr)) {
+ if (compiler_type.IsArrayType(nullptr, &array_size)) {
cstr_len = array_size;
if (cstr_len > max_length) {
capped_data = true;
@@ -1608,9 +1608,7 @@ ValueObject::GetTypeInfo(CompilerType
*pointee_or_element_compiler_type) {
bool ValueObject::IsPointerType() { return GetCompilerType().IsPointerType(); }
-bool ValueObject::IsArrayType() {
- return GetCompilerType().IsArrayType(nullptr, nullptr, nullptr);
-}
+bool ValueObject::IsArrayType() { return GetCompilerType().IsArrayType(); }
bool ValueObject::IsScalarType() { return GetCompilerType().IsScalarType(); }
diff --git a/lldb/source/DataFormatters/VectorType.cpp
b/lldb/source/DataFormatters/VectorType.cpp
index cc24bb1de428..11371918830b 100644
--- a/lldb/source/DataFormatters/VectorType.cpp
+++ b/lldb/source/DataFormatters/VectorType.cpp
@@ -219,7 +219,7 @@ class VectorTypeSyntheticFrontEnd : public
SyntheticChildrenFrontEnd {
m_parent_format = m_backend.GetFormat();
CompilerType parent_type(m_backend.GetCompilerType());
CompilerType element_type;
-parent_type.IsVectorType(&element_type, nullptr);
+parent_type.IsVectorType(&element_type);
m_child_type = ::GetCompilerTypeForFormat(m_parent_format, element_type,
parent_type.GetTypeSystem());
m_num_children = ::CalculateNumChildren(parent_type, m_child_type);
diff --git a/lldb/source/Plugins/ABI/ARM/ABISysV_arm.cpp
b/lldb/source/Plugins/ABI/ARM/ABISysV_arm.cpp
index 8a9be3fb67c7..3e544e0483a7 100644
--- a/lldb/source/Plugins/ABI/ARM/ABISysV_arm.cpp
+++ b/lldb/source/Plugins/ABI/ARM/ABISysV_arm.cpp
@@ -1617,7 +1617,7 @@ ValueObjectSP ABISysV_arm::GetReturnValueObjectImpl(
thread.GetRegisterContext()->ReadRegisterAsUnsigned(r0_reg_info, 0) &
UINT32_MAX;
value.GetScalar() = ptr;
- } else if (compiler_type.IsVectorType(nullptr, nullptr)) {
+ } else if (compiler_type.IsVectorType()) {
if (IsArmHardFloat(thread) && (*byte_size == 8 || *byte_size == 16)) {
is_vfp_candidate = true;
vfp_byte_size = 8;
@@ -1704,7 +1704,7 @@ ValueObjectSP ABISysV_arm::GetReturnValueObjectImpl(
if (homogeneous_count > 0 && homogeneous_count <= 4) {
llvm::Optional base_byte_size =
base_type.GetByteSize(&thread);
-if (base_type.IsVectorType(
[Lldb-commits] [PATCH] D96460: [LLDB] Arm64/Linux Add MTE and Pointer Authentication registers
DavidSpickett added inline comments. Comment at: lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp:276 +if (opt_regsets & eRegsetEnableMTE) + m_mte_regset_enabled = true; } omjavaid wrote: > DavidSpickett wrote: > > (like earlier) would it be ok to remove the if? > > ``` > > m_mte_regset_enabled = opt_regsets & eRegsetEnableMTE; > > ``` > I think here as well if do write eveytime regardless of true/false we might > end up refreshing the whole data set contained in cache against a > RegisterInfoPOSIX_arm64 object. What do you think? I assume you mean the cpu cache not any lldb register cache, I don't see a need to care that much about it here. The reason to always assign is so that there's no implication that there's a 3rd state. Like "false and not enabled" "true and enabled" "false and enabled" (which makes no sense but types wise, it's possible). Granted it's a drop in the bucket vs the rest of the state we're handling here so I'll leave it up to you. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96460/new/ https://reviews.llvm.org/D96460 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D96463: [LLDB] Arm64/Linux test case for MTE and Pointer Authentication regset
DavidSpickett added inline comments.
Comment at:
lldb/test/API/commands/register/register/aarch64_dynamic_regset/TestArm64DynamicRegsets.py:45
+
+self.expect("register read ffr", substrs=[p_regs_value])
+
omjavaid wrote:
> DavidSpickett wrote:
> > Do we need an ffr read if we have a read/write below?
> Yes.
> There are two values which are being verified:
> First we verify value that was written by our assembly code.
> After that we use debugger to write and read back values.
Ok but then should the second r/w be of a different value?
program writes A - we read A, so the read works
we write A - we read A, was that our A or the program's A?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D96463/new/
https://reviews.llvm.org/D96463
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] bda83ba - [lldb][NFC] Clean up ValueObject comments
Author: Raphael Isemann
Date: 2021-02-23T12:02:36+01:00
New Revision: bda83ba0d296303dff02d262cab73dc984cda3c1
URL:
https://github.com/llvm/llvm-project/commit/bda83ba0d296303dff02d262cab73dc984cda3c1
DIFF:
https://github.com/llvm/llvm-project/commit/bda83ba0d296303dff02d262cab73dc984cda3c1.diff
LOG: [lldb][NFC] Clean up ValueObject comments
* Remove commented out code.
* Doxygenify comments that serve as documentation.
* Use the LLVM comment style where possible.
Added:
Modified:
lldb/include/lldb/Core/ValueObject.h
lldb/include/lldb/Core/ValueObjectCast.h
lldb/include/lldb/Core/ValueObjectChild.h
lldb/include/lldb/Core/ValueObjectConstResult.h
lldb/include/lldb/Core/ValueObjectConstResultImpl.h
lldb/include/lldb/Core/ValueObjectDynamicValue.h
lldb/include/lldb/Core/ValueObjectList.h
lldb/include/lldb/Core/ValueObjectMemory.h
lldb/include/lldb/Core/ValueObjectSyntheticFilter.h
lldb/include/lldb/Core/ValueObjectVariable.h
Removed:
diff --git a/lldb/include/lldb/Core/ValueObject.h
b/lldb/include/lldb/Core/ValueObject.h
index 308e0795efe66..2042baae97ed5 100644
--- a/lldb/include/lldb/Core/ValueObject.h
+++ b/lldb/include/lldb/Core/ValueObject.h
@@ -121,55 +121,62 @@ class ValueObject {
};
enum ExpressionPathScanEndReason {
-eExpressionPathScanEndReasonEndOfString = 1, // out of data to parse
-eExpressionPathScanEndReasonNoSuchChild, // child element not
found
-eExpressionPathScanEndReasonNoSuchSyntheticChild, // (synthetic) child
- // element not found
-eExpressionPathScanEndReasonEmptyRangeNotAllowed, // [] only allowed for
- // arrays
-eExpressionPathScanEndReasonDotInsteadOfArrow, // . used when -> should be
- // used
-eExpressionPathScanEndReasonArrowInsteadOfDot, // -> used when . should be
- // used
-eExpressionPathScanEndReasonFragileIVarNotAllowed, // ObjC ivar expansion
- // not allowed
-eExpressionPathScanEndReasonRangeOperatorNotAllowed, // [] not allowed by
- // options
-eExpressionPathScanEndReasonRangeOperatorInvalid, // [] not valid on
objects
- // other than scalars,
- // pointers or arrays
-eExpressionPathScanEndReasonArrayRangeOperatorMet, // [] is good for
arrays,
- // but I cannot parse it
-eExpressionPathScanEndReasonBitfieldRangeOperatorMet, // [] is good for
- // bitfields, but I
- // cannot parse after
- // it
-eExpressionPathScanEndReasonUnexpectedSymbol, // something is malformed in
- // the expression
-eExpressionPathScanEndReasonTakingAddressFailed, // impossible to apply &
- // operator
-eExpressionPathScanEndReasonDereferencingFailed, // impossible to apply *
- // operator
-eExpressionPathScanEndReasonRangeOperatorExpanded, // [] was expanded into
a
- // VOList
-eExpressionPathScanEndReasonSyntheticValueMissing, // getting the synthetic
- // children failed
+/// Out of data to parse.
+eExpressionPathScanEndReasonEndOfString = 1,
+/// Child element not found.
+eExpressionPathScanEndReasonNoSuchChild,
+/// (Synthetic) child element not found.
+eExpressionPathScanEndReasonNoSuchSyntheticChild,
+/// [] only allowed for arrays.
+eExpressionPathScanEndReasonEmptyRangeNotAllowed,
+/// . used when -> should be used.
+eExpressionPathScanEndReasonDotInsteadOfArrow,
+/// -> used when . should be used.
+eExpressionPathScanEndReasonArrowInsteadOfDot,
+/// ObjC ivar expansion not allowed.
+eExpressionPathScanEndReasonFragileIVarNotAllowed,
+/// [] not allowed by options.
+eExpressionPathScanEndReasonRangeOperatorNotAllowed,
+/// [] not valid on objects other than scalars, pointers or arrays.
+eExpressionPathScanEndReasonRangeOperatorInvalid,
+/// [] is good for arrays, but I cannot parse it.
+eExpressionPathScanEndReasonArrayRangeOperatorMet,
+/// [] is good for bitfields, but I cannot parse after it.
+eExpressionPathScanEndReasonBitfieldRangeOperatorMet,
+/// Som
[Lldb-commits] [lldb] bea2d5e - [lldb][NFC] Remove unused ValueObject::LogValueObject functions
Author: Raphael Isemann
Date: 2021-02-23T12:10:40+01:00
New Revision: bea2d5e47867687c8d2f95bd70ed9a77d19eeb6e
URL:
https://github.com/llvm/llvm-project/commit/bea2d5e47867687c8d2f95bd70ed9a77d19eeb6e
DIFF:
https://github.com/llvm/llvm-project/commit/bea2d5e47867687c8d2f95bd70ed9a77d19eeb6e.diff
LOG: [lldb][NFC] Remove unused ValueObject::LogValueObject functions
Those functions aren't called anywhere. For debugging purposes we usually
have Dump() methods (which already exist in some semi-functional form in
ValueObject).
Added:
Modified:
lldb/include/lldb/Core/ValueObject.h
lldb/source/Core/ValueObject.cpp
Removed:
diff --git a/lldb/include/lldb/Core/ValueObject.h
b/lldb/include/lldb/Core/ValueObject.h
index 2042baae97ed..9fc9d6da5360 100644
--- a/lldb/include/lldb/Core/ValueObject.h
+++ b/lldb/include/lldb/Core/ValueObject.h
@@ -656,10 +656,6 @@ class ValueObject {
CreateValueObjectFromData(llvm::StringRef name, const DataExtractor &data,
const ExecutionContext &exe_ctx, CompilerType
type);
- void LogValueObject(Log *log);
-
- void LogValueObject(Log *log, const DumpValueObjectOptions &options);
-
lldb::ValueObjectSP Persist();
/// Returns true if this is a char* or a char[] if it is a char* and
diff --git a/lldb/source/Core/ValueObject.cpp
b/lldb/source/Core/ValueObject.cpp
index 8f90ae873160..ec6e95410dac 100644
--- a/lldb/source/Core/ValueObject.cpp
+++ b/lldb/source/Core/ValueObject.cpp
@@ -2617,21 +2617,6 @@ ValueObjectSP
ValueObject::GetValueForExpressionPath_Impl(
}
}
-void ValueObject::LogValueObject(Log *log) {
- if (log)
-return LogValueObject(log, DumpValueObjectOptions(*this));
-}
-
-void ValueObject::LogValueObject(Log *log,
- const DumpValueObjectOptions &options) {
- if (log) {
-StreamString s;
-Dump(s, options);
-if (s.GetSize())
- log->PutCString(s.GetData());
- }
-}
-
void ValueObject::Dump(Stream &s) { Dump(s, DumpValueObjectOptions(*this)); }
void ValueObject::Dump(Stream &s, const DumpValueObjectOptions &options) {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D97165: [lldb] Add deref support and tests to shared_ptr synthetic
teemperor accepted this revision.
teemperor added a comment.
This revision is now accepted and ready to land.
This LGTM modulo a missing nullptr check. Thanks for fixing this!
Also we probably should investigate the `strong=` summary differences. I would
have blamed the fake constructors we are creating in LLDB, but as this isn't
running any expressions this might us just reading the wrong values?
Comment at: lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp:407
+auto ptr_sp =
+valobj_sp->GetChildMemberWithName(ConstString("__ptr_"), true);
+Status status;
`ptr_sp` should have a nullptr check as `__ptr_` might be missing because we
screw up some debug info parsing, or someone renamed the member or idk. In any
case, this shouldn't crash.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D97165/new/
https://reviews.llvm.org/D97165
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 2f75363 - [lldb] [test] Un-XFAIL a test that no longer fail on FreeBSD
Author: Michał Górny
Date: 2021-02-23T14:35:34+01:00
New Revision: 2f75363a9e138429d5c80ca0a541247a5bc70614
URL:
https://github.com/llvm/llvm-project/commit/2f75363a9e138429d5c80ca0a541247a5bc70614
DIFF:
https://github.com/llvm/llvm-project/commit/2f75363a9e138429d5c80ca0a541247a5bc70614.diff
LOG: [lldb] [test] Un-XFAIL a test that no longer fail on FreeBSD
Added:
Modified:
lldb/test/API/lang/c/conflicting-symbol/TestConflictingSymbol.py
Removed:
diff --git a/lldb/test/API/lang/c/conflicting-symbol/TestConflictingSymbol.py
b/lldb/test/API/lang/c/conflicting-symbol/TestConflictingSymbol.py
index 926e9f625f5b..7044667dfb39 100644
--- a/lldb/test/API/lang/c/conflicting-symbol/TestConflictingSymbol.py
+++ b/lldb/test/API/lang/c/conflicting-symbol/TestConflictingSymbol.py
@@ -19,7 +19,6 @@ def setUp(self):
lldbutil.mkdir_p(self.getBuildArtifact("Two"))
@expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24489")
-@expectedFailureAll(oslist=["freebsd"], bugnumber="llvm.org/pr48416")
def test_conflicting_symbols(self):
self.build()
exe = self.getBuildArtifact("a.out")
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 6c06b0a - [lldb] [test] Un-XFAIL TestBuiltinTrap on FreeBSD/aarch64
Author: Michał Górny Date: 2021-02-23T14:35:34+01:00 New Revision: 6c06b0aa5a5534a370532c287cf35a96eeb2146e URL: https://github.com/llvm/llvm-project/commit/6c06b0aa5a5534a370532c287cf35a96eeb2146e DIFF: https://github.com/llvm/llvm-project/commit/6c06b0aa5a5534a370532c287cf35a96eeb2146e.diff LOG: [lldb] [test] Un-XFAIL TestBuiltinTrap on FreeBSD/aarch64 Added: Modified: lldb/test/API/linux/builtin_trap/TestBuiltinTrap.py Removed: diff --git a/lldb/test/API/linux/builtin_trap/TestBuiltinTrap.py b/lldb/test/API/linux/builtin_trap/TestBuiltinTrap.py index 604b84605fbc..bc67de6fa28e 100644 --- a/lldb/test/API/linux/builtin_trap/TestBuiltinTrap.py +++ b/lldb/test/API/linux/builtin_trap/TestBuiltinTrap.py @@ -23,7 +23,7 @@ def setUp(self): # gcc generates incorrect linetable @expectedFailureAll(archs="arm", compiler="gcc", triple=".*-android") -@expectedFailureAll(archs=['aarch64'], oslist=no_match(['linux'])) +@expectedFailureAll(archs=['aarch64'], oslist=no_match(['freebsd', 'linux'])) @skipIfWindows def test_with_run_command(self): """Test that LLDB handles a function with __builtin_trap correctly.""" ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D97281: [lldb][AArch64] Add class for managing memory tags
DavidSpickett created this revision.
Herald added subscribers: danielkiss, kristof.beyls, mgorny.
DavidSpickett requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
This adds the MemoryTagManager class and a specialisation
of that class for AArch64 MTE tags. It provides a generic
interface for various tagging operations.
Adding/removing tags, diffing tagged pointers, etc.
Later patches will use this manager to handle memory tags
in generic code in both lldb and lldb-server.
Since it will be used in both, the base class header is in
lldb/Target.
(MemoryRegionInfo is another example of this pattern)
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D97281
Files:
lldb/include/lldb/Target/MemoryTagManager.h
lldb/source/Plugins/Process/Utility/CMakeLists.txt
lldb/source/Plugins/Process/Utility/MemoryTagManagerAArch64MTE.cpp
lldb/source/Plugins/Process/Utility/MemoryTagManagerAArch64MTE.h
lldb/unittests/Process/Utility/CMakeLists.txt
lldb/unittests/Process/Utility/MemoryTagManagerAArch64MTETest.cpp
Index: lldb/unittests/Process/Utility/MemoryTagManagerAArch64MTETest.cpp
===
--- /dev/null
+++ lldb/unittests/Process/Utility/MemoryTagManagerAArch64MTETest.cpp
@@ -0,0 +1,166 @@
+//===-- MemoryTagManagerAArch64MTETest.cpp ===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "Plugins/Process/Utility/MemoryTagManagerAArch64MTE.h"
+#include "llvm/Testing/Support/Error.h"
+#include "gtest/gtest.h"
+
+using namespace lldb_private;
+
+TEST(MemoryTagManagerAArch64MTETest, UnpackTags) {
+ MemoryTagManagerAArch64MTE handler;
+
+ // Error for insufficient tag data
+ std::vector input;
+ ASSERT_THAT_EXPECTED(
+ handler.UnpackTags(input, 2),
+ llvm::FailedWithMessage(
+ "Packed tag data size does not match expected number of tags. "
+ "Expected 2 tag(s) for 2 granules, got 0 tag(s)."));
+
+ // This is out of the valid tag range
+ input.push_back(0x1f);
+ ASSERT_THAT_EXPECTED(
+ handler.UnpackTags(input, 1),
+ llvm::FailedWithMessage(
+ "Found tag 0x1f which is > max MTE tag value of 0xf."));
+
+ // MTE tags are 1 per byte
+ input.pop_back();
+ input.push_back(0xe);
+ input.push_back(0xf);
+
+ std::vector expected{0xe, 0xf};
+
+ llvm::Expected> got = handler.UnpackTags(input, 2);
+ ASSERT_THAT_EXPECTED(got, llvm::Succeeded());
+ ASSERT_THAT(expected, testing::ContainerEq(*got));
+}
+
+TEST(MemoryTagManagerAArch64MTETest, GetLogicalTag) {
+ MemoryTagManagerAArch64MTE handler;
+
+ // Set surrounding bits to check shift is correct
+ ASSERT_EQ((lldb::addr_t)0, handler.GetLogicalTag(0xe0e0));
+ // Max tag value
+ ASSERT_EQ((lldb::addr_t)0xf, handler.GetLogicalTag(0x0f00));
+ ASSERT_EQ((lldb::addr_t)2, handler.GetLogicalTag(0x0200));
+}
+
+TEST(MemoryTagManagerAArch64MTETest, RemoveLogicalTag) {
+ MemoryTagManagerAArch64MTE handler;
+
+ // Should not remove surrounding bits
+ ASSERT_EQ((lldb::addr_t)0xf0f0,
+handler.RemoveLogicalTag(0xfef0));
+ // Untagged pointers are unchanged
+ ASSERT_EQ((lldb::addr_t)0x1034567812345678,
+handler.RemoveLogicalTag(0x1034567812345678));
+}
+
+TEST(MemoryTagManagerAArch64MTETest, SetLogicalTag) {
+ MemoryTagManagerAArch64MTE handler;
+
+ // Error if tag > mte range
+ ASSERT_THAT_EXPECTED(handler.SetLogicalTag(0x0, 0x1f),
+ llvm::FailedWithMessage(
+ "MTE logical tag value is > MTE max value of 0xf."));
+
+ // Surrounding bits should be unchanged
+ llvm::Expected got =
+ handler.SetLogicalTag(0xe0e0, 0xf);
+ ASSERT_THAT_EXPECTED(got, llvm::Succeeded());
+ ASSERT_EQ(0xefe0, *got);
+
+ // Old tag removed first, does not change the result
+ llvm::Expected g1 =
+ handler.SetLogicalTag(0x, 0xa);
+ ASSERT_THAT_EXPECTED(g1, llvm::Succeeded());
+ llvm::Expected g2 =
+ handler.SetLogicalTag(0x0f00, 0xa);
+ ASSERT_THAT_EXPECTED(g2, llvm::Succeeded());
+ ASSERT_EQ(*g1, *g2);
+
+ // Should roundtrip with the other methods
+ lldb::addr_t tag = 0xe;
+ lldb::addr_t addr = 0x1011;
+ llvm::Expected set = handler.SetLogicalTag(addr, tag);
+ ASSERT_THAT_EXPECTED(set, llvm::Succeeded());
+
+ ASSERT_EQ(tag, handler.GetLogicalTag(*set));
+ ASSERT_EQ(addr, handler.RemoveLogicalTag(*set));
+}
+
+TEST(MemoryTagManagerAArch64MTETest, CopyLogicalTag) {
+ MemoryTagManagerAArch64MTE handler;
+
+ // Round trips
+ lldb::addr_t addr = 0x;
+ ASSERT_EQ(addr, handler.CopyLogicalTag(addr,
[Lldb-commits] [PATCH] D97281: [lldb][AArch64] Add class for managing memory tags
DavidSpickett added reviewers: labath, omjavaid. DavidSpickett added a comment. Herald added a subscriber: JDevlieghere. Omair, I've taken one of your suggestions on the class name from the other review. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97281/new/ https://reviews.llvm.org/D97281 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D97282: [lldb][AArch64] Add memory-tagging qSupported feature
DavidSpickett created this revision.
Herald added subscribers: danielkiss, kristof.beyls, krytarowski.
DavidSpickett requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
This feature "memory-tagging+" indicates that lldb-server
supports memory tagging packets. (added in a later patch)
Checking that the system supports MTE requires reading
CPU features which would mean target/OS specific code in
GDBRemoteCommunicationServerCommon.
To avoid this, I've enabled the feature for all AArch64 Linux
at compile time. This is the only platform that has a chance
of supporting tagging. We need to check for tagging per memory
region in any case, so you'll get an error either way if it's
not enabled.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D97282
Files:
lldb/include/lldb/Target/Process.h
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
@@ -239,6 +239,8 @@
friend class GDBRemoteCommunicationClient;
friend class GDBRemoteRegisterContext;
+ bool SupportsMemoryTagging() override;
+
/// Broadcaster event bits definitions.
enum {
eBroadcastBitAsyncContinue = (1 << 0),
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -2762,6 +2762,10 @@
return 0;
}
+bool ProcessGDBRemote::SupportsMemoryTagging() {
+ return m_gdb_comm.GetMemoryTaggingSupported();
+}
+
Status ProcessGDBRemote::WriteObjectFile(
std::vector entries) {
Status error;
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
@@ -848,6 +848,9 @@
response.PutCString(";qXfer:auxv:read+");
response.PutCString(";qXfer:libraries-svr4:read+");
#endif
+#if defined(__linux__) && defined(__aarch64__)
+ response.PutCString(";memory-tagging+");
+#endif
return SendPacketNoLock(response.GetString());
}
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
@@ -446,6 +446,8 @@
bool GetSharedCacheInfoSupported();
+ bool GetMemoryTaggingSupported();
+
/// Use qOffsets to query the offset used when relocating the target
/// executable. If successful, the returned structure will contain at least
/// one value in the offsets field.
@@ -558,6 +560,7 @@
LazyBool m_supports_jGetSharedCacheInfo;
LazyBool m_supports_QPassSignals;
LazyBool m_supports_error_string_reply;
+ LazyBool m_supports_memory_tagging;
bool m_supports_qProcessInfoPID : 1, m_supports_qfProcessInfo : 1,
m_supports_qUserName : 1, m_supports_qGroupName : 1,
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -89,6 +89,7 @@
m_supports_jGetSharedCacheInfo(eLazyBoolCalculate),
m_supports_QPassSignals(eLazyBoolCalculate),
m_supports_error_string_reply(eLazyBoolCalculate),
+ m_supports_memory_tagging(eLazyBoolCalculate),
m_supports_qProcessInfoPID(true), m_supports_qfProcessInfo(true),
m_supports_qUserName(true), m_supports_qGroupName(true),
m_supports_qThreadStopInfo(true), m_supports_z0(true),
@@ -342,6 +343,7 @@
m_supports_augmented_libraries_svr4_read = eLazyBoolNo;
m_supports_qXfer_features_read = eLazyBoolNo;
m_supports_qXfer_memory_map_read = eLazyBoolNo;
+ m_supports_memory_tagging = eLazyBoolNo;
m_max_packet_size = UINT64_MAX; // It's supposed to always be there, but if
// not, we assume no limit
@@ -433,6 +435,9 @@
else
m_supports_QPassSignals = eLazyBoolNo;
+if (::strstr(response_cstr, "memory-tagging+"))
+ m_supports_memory_tagging = eLazy
[Lldb-commits] [PATCH] D97282: [lldb][AArch64] Add memory-tagging qSupported feature
DavidSpickett added a comment. This feature name matches the one GDB is planning to use: https://sourceware.org/pipermail/gdb-patches/2021-January/175514.html Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97282/new/ https://reviews.llvm.org/D97282 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D95601: [lldb][AArch64] Add MTE memory tag reading to lldb-server
DavidSpickett updated this revision to Diff 325767.
DavidSpickett added a comment.
Split the patches into smaller changes. (see stack)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D95601/new/
https://reviews.llvm.org/D95601
Files:
lldb/include/lldb/Host/common/NativeProcessProtocol.h
lldb/include/lldb/Host/linux/Ptrace.h
lldb/include/lldb/Utility/StringExtractorGDBRemote.h
lldb/packages/Python/lldbsuite/test/decorators.py
lldb/source/Host/common/NativeProcessProtocol.cpp
lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.h
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
lldb/source/Utility/StringExtractorGDBRemote.cpp
lldb/test/API/tools/lldb-server/memory-tagging/Makefile
lldb/test/API/tools/lldb-server/memory-tagging/TestGdbRemoteMemoryTagging.py
lldb/test/API/tools/lldb-server/memory-tagging/main.c
Index: lldb/test/API/tools/lldb-server/memory-tagging/main.c
===
--- /dev/null
+++ lldb/test/API/tools/lldb-server/memory-tagging/main.c
@@ -0,0 +1,48 @@
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+void print_result(char *ptr) {
+ printf("buffer: %p\n", ptr);
+ // Wait for lldb-server to stop us
+ while (1) {
+ }
+}
+
+int main(int argc, char const *argv[]) {
+ if (prctl(PR_SET_TAGGED_ADDR_CTRL,
+PR_TAGGED_ADDR_ENABLE | PR_MTE_TCF_SYNC |
+// Allow all tags to be generated by the addg
+// instruction __arm_mte_increment_tag produces.
+(0x << PR_MTE_TAG_SHIFT),
+0, 0, 0)) {
+print_result(NULL);
+ }
+
+ size_t page_size = sysconf(_SC_PAGESIZE);
+ char *buf = mmap(0, sysconf(_SC_PAGESIZE), PROT_READ | PROT_WRITE | PROT_MTE,
+ MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+ if (buf == MAP_FAILED)
+print_result(NULL);
+
+ // Set incrementing tags until end of the page
+ char *tagged_ptr = buf;
+ // This intrinsic treats the addresses as if they were untagged
+ while (__arm_mte_ptrdiff(tagged_ptr, buf) < page_size) {
+// This sets the allocation tag
+__arm_mte_set_tag(tagged_ptr);
+// Set the tag of the next granule (hence +16) to the next
+// tag value. Returns a new pointer with the new logical tag.
+// Tag values wrap at 0xF so it'll cycle.
+tagged_ptr = __arm_mte_increment_tag(tagged_ptr + 16, 1);
+ }
+
+ print_result(buf);
+
+ return 0;
+}
Index: lldb/test/API/tools/lldb-server/memory-tagging/TestGdbRemoteMemoryTagging.py
===
--- /dev/null
+++ lldb/test/API/tools/lldb-server/memory-tagging/TestGdbRemoteMemoryTagging.py
@@ -0,0 +1,102 @@
+import gdbremote_testcase
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestGdbRemoteMemoryTagging(gdbremote_testcase.GdbRemoteTestCaseBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def check_qmemtags_response(self, body, expected):
+self.test_sequence.add_log_lines(["read packet: $qMemTags:{}#00".format(body),
+ "send packet: ${}#00".format(expected),
+ ],
+ True)
+self.expect_gdbremote_sequence()
+
+@skipUnlessArch("aarch64")
+@skipUnlessPlatform(["linux"])
+@skipUnlessAArch64MTELinuxCompiler
+def test_qmemtags_packets(self):
+""" Test that qMemTags packets are parsed correctly and/or rejected. """
+
+self.build()
+self.set_inferior_startup_launch()
+procs = self.prep_debug_monitor_and_inferior()
+
+# Run the process
+self.test_sequence.add_log_lines(
+[
+# Start running after initial stop
+"read packet: $c#63",
+ # Match the address of the MTE page
+{"type": "output_match", "regex": self.maybe_strict_output_regex(r"buffer: (.+)\r\n"),
+ "capture": {1: "buffer"}},
+# Now stop the inferior
+"read packet: {}".format(chr(3)),
+# And wait for the stop notification
+{"direction": "send", "regex": r"^\$T[0-9a-fA-F]{2}thread:[0-9a-fA-F]+;"}],
+True)
+
+# Run the packet stream
+context = self.expect_gdbremote_sequence()
+self.assertIsNotNone(context)
+
+# Grab the address
+buf_address = context.get("buffer")
+self.assertIsNotNone(buf_address)
+
+
[Lldb-commits] [PATCH] D95602: [lldb][AArch64] Add MTE memory tag reading for lldb
DavidSpickett updated this revision to Diff 325769.
DavidSpickett added a comment.
Split into smaller changes. Tag read command is now a further change.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D95602/new/
https://reviews.llvm.org/D95602
Files:
lldb/include/lldb/Core/Architecture.h
lldb/include/lldb/Target/Process.h
lldb/source/Plugins/Architecture/AArch64/ArchitectureAArch64.cpp
lldb/source/Plugins/Architecture/AArch64/ArchitectureAArch64.h
lldb/source/Plugins/Architecture/AArch64/CMakeLists.txt
lldb/source/Plugins/Architecture/CMakeLists.txt
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
lldb/source/Target/Process.cpp
lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
Index: lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
===
--- lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
+++ lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
@@ -657,3 +657,68 @@
EXPECT_EQ(llvm::None, GetQOffsets("TextSeg=0x1234"));
EXPECT_EQ(llvm::None, GetQOffsets("TextSeg=12345678123456789"));
}
+
+static void
+check_qmemtags(TestClient &client, MockServer &server, size_t read_len,
+ const char *packet, llvm::StringRef response,
+ llvm::Optional> expected_tag_data) {
+ const auto &ReadMemoryTags = [&](size_t len, const char *packet,
+ llvm::StringRef response) {
+std::future result = std::async(std::launch::async, [&] {
+ return client.ReadMemoryTags(0xDEF0, read_len, 1);
+});
+
+HandlePacket(server, packet, response);
+return result.get();
+ };
+
+ auto result = ReadMemoryTags(0, packet, response);
+ if (expected_tag_data) {
+ASSERT_TRUE(result);
+llvm::ArrayRef expected(*expected_tag_data);
+llvm::ArrayRef got = result->GetData();
+ASSERT_THAT(expected, testing::ContainerEq(got));
+ } else {
+ASSERT_FALSE(result);
+ }
+}
+
+TEST_F(GDBRemoteCommunicationClientTest, ReadMemoryTags) {
+ // Zero length reads are valid
+ check_qmemtags(client, server, 0, "qMemTags:def0,0:1", "m",
+ std::vector{});
+
+ // The client layer does not check the length of the received data.
+ // All we need is the "m" and for the decode to use all of the chars
+ check_qmemtags(client, server, 32, "qMemTags:def0,20:1", "m09",
+ std::vector{0x9});
+
+ // Zero length response is fine as long as the "m" is present
+ check_qmemtags(client, server, 0, "qMemTags:def0,0:1", "m",
+ std::vector{});
+
+ // Normal responses
+ check_qmemtags(client, server, 16, "qMemTags:def0,10:1", "m66",
+ std::vector{0x66});
+ check_qmemtags(client, server, 32, "qMemTags:def0,20:1", "m0102",
+ std::vector{0x1, 0x2});
+
+ // Empty response is an error
+ check_qmemtags(client, server, 17, "qMemTags:def0,11:1", "", llvm::None);
+ // Usual error response
+ check_qmemtags(client, server, 17, "qMemTags:def0,11:1", "E01", llvm::None);
+ // Leading m missing
+ check_qmemtags(client, server, 17, "qMemTags:def0,11:1", "01", llvm::None);
+ // Anything other than m is an error
+ check_qmemtags(client, server, 17, "qMemTags:def0,11:1", "z01", llvm::None);
+ // Decoding tag data doesn't use all the chars in the packet
+ check_qmemtags(client, server, 32, "qMemTags:def0,20:1", "m09zz", llvm::None);
+ // Data that is not hex bytes
+ check_qmemtags(client, server, 32, "qMemTags:def0,20:1", "mhello",
+ llvm::None);
+ // Data is not a complete hex char
+ check_qmemtags(client, server, 32, "qMemTags:def0,20:1", "m9", llvm::None);
+ // Data has a trailing hex char
+ check_qmemtags(client, server, 32, "qMemTags:def0,20:1", "m01020",
+ llvm::None);
+}
Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -6023,3 +6023,70 @@
return false;
}
+
+llvm::Expected
+Process::GetMemoryTagManager(lldb::addr_t addr, lldb::addr_t end_addr) {
+ Architecture *arch = GetTarget().GetArchitecturePlugin();
+ const MemoryTagManager *tag_manager =
+ arch ? arch->GetMemoryTagManager() : nullptr;
+ if (!arch || !tag_manager) {
+return llvm::createStringError(
+llvm::inconvertibleErrorCode(),
+"This architecture does not support memory tagging",
+GetPluginName().GetCString());
+ }
+
+ if (!SupportsMemoryTagging()) {
+return llvm::createStringError(llvm::inconvertibleErrorCode(),
+ "Process does not support memory tagging");
+ }
+
+ ptrdi
[Lldb-commits] [PATCH] D97284: [lldb][AArch64] Add MTE CPU feature test predicate
DavidSpickett created this revision.
Herald added subscribers: danielkiss, kristof.beyls.
DavidSpickett requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
Change the logic slightly so that the feature can
be anywhere in the list.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D97284
Files:
lldb/packages/Python/lldbsuite/test/lldbtest.py
Index: lldb/packages/Python/lldbsuite/test/lldbtest.py
===
--- lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -1269,7 +1269,7 @@
return True
return False
-def isAArch64SVE(self):
+def hasLinuxCPUInfoFeature(self, feature):
triple = self.dbg.GetSelectedPlatform().GetTriple()
# TODO other platforms, please implement this function
@@ -1283,14 +1283,18 @@
else:
cpuinfo_path = "/proc/cpuinfo"
-try:
-f = open(cpuinfo_path, 'r')
-cpuinfo = f.read()
-f.close()
-except:
+with open(cpuinfo_path, 'r') as f:
+for line in f.readlines():
+if line.startswith("Features"):
+features = line.split(':')[1].split()
+return feature in features
return False
-return " sve " in cpuinfo
+def isAArch64SVE(self):
+return self.hasLinuxCPUInfoFeature("sve")
+
+def isAArch64MTE(self):
+return self.hasLinuxCPUInfoFeature("mte")
def hasLinuxVmFlags(self):
""" Check that the target machine has "VmFlags" lines in
Index: lldb/packages/Python/lldbsuite/test/lldbtest.py
===
--- lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -1269,7 +1269,7 @@
return True
return False
-def isAArch64SVE(self):
+def hasLinuxCPUInfoFeature(self, feature):
triple = self.dbg.GetSelectedPlatform().GetTriple()
# TODO other platforms, please implement this function
@@ -1283,14 +1283,18 @@
else:
cpuinfo_path = "/proc/cpuinfo"
-try:
-f = open(cpuinfo_path, 'r')
-cpuinfo = f.read()
-f.close()
-except:
+with open(cpuinfo_path, 'r') as f:
+for line in f.readlines():
+if line.startswith("Features"):
+features = line.split(':')[1].split()
+return feature in features
return False
-return " sve " in cpuinfo
+def isAArch64SVE(self):
+return self.hasLinuxCPUInfoFeature("sve")
+
+def isAArch64MTE(self):
+return self.hasLinuxCPUInfoFeature("mte")
def hasLinuxVmFlags(self):
""" Check that the target machine has "VmFlags" lines in
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D97284: [lldb][AArch64] Add MTE CPU feature test predicate
DavidSpickett added a reviewer: omjavaid. DavidSpickett added a comment. Herald added a subscriber: JDevlieghere. I think we both have something like this in our changes. Yours will probably land sooner but here it is anyway. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97284/new/ https://reviews.llvm.org/D97284 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D97285: [lldb][AArch64] Add "memory tag read" command
DavidSpickett created this revision.
Herald added subscribers: danielkiss, kristof.beyls, mgorny.
DavidSpickett requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
This new command looks much like "memory read"
and mirrors its basic behaviour.
(lldb) memory tag read new_buf_ptr new_buf_ptr+32
Logical tag: 0x9
Allocation tags:
[0x900f7ffa000, 0x900f7ffa010): 0x9
[0x900f7ffa010, 0x900f7ffa020): 0x0
Important proprties:
- The end address is optional and defaults to reading 1 tag if ommitted
- It is an error to try to read tags if the architecture or process doesn't
support it, or if the range asked for is not tagged.
- It is an error to read an inverted range (end < begin) (logical tags are
removed for this check so you can pass tagged addresses here)
- The range will be expanded to fit the tagging granule, so you can get more
tags than simply (end-begin)/granule size. Whatever you get back will always
cover the original range.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D97285
Files:
lldb/source/Commands/CMakeLists.txt
lldb/source/Commands/CommandObjectMemory.cpp
lldb/source/Commands/CommandObjectMemoryTag.cpp
lldb/source/Commands/CommandObjectMemoryTag.h
lldb/test/API/functionalities/memory/tag/Makefile
lldb/test/API/functionalities/memory/tag/TestMemoryTag.py
lldb/test/API/functionalities/memory/tag/main.cpp
lldb/test/API/linux/aarch64/mte_tag_read/Makefile
lldb/test/API/linux/aarch64/mte_tag_read/TestAArch64LinuxMTEMemoryTagRead.py
lldb/test/API/linux/aarch64/mte_tag_read/main.c
Index: lldb/test/API/linux/aarch64/mte_tag_read/main.c
===
--- /dev/null
+++ lldb/test/API/linux/aarch64/mte_tag_read/main.c
@@ -0,0 +1,50 @@
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+int main(int argc, char const *argv[]) {
+ // We assume that the test runner has checked we're on an MTE system
+
+ if (prctl(PR_SET_TAGGED_ADDR_CTRL,
+PR_TAGGED_ADDR_ENABLE | PR_MTE_TCF_SYNC |
+// Allow all tags to be generated by the addg
+// instruction __arm_mte_increment_tag produces.
+(0x << PR_MTE_TAG_SHIFT),
+0, 0, 0)) {
+return 1;
+ }
+
+ size_t page_size = sysconf(_SC_PAGESIZE);
+
+ // Allocate memory with MTE
+ char *buf = mmap(0, sysconf(_SC_PAGESIZE), PROT_READ | PROT_WRITE | PROT_MTE,
+ MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+ if (buf == MAP_FAILED)
+return 1;
+
+ // And without MTE
+ char *non_mte_buf = mmap(0, sysconf(_SC_PAGESIZE), PROT_READ | PROT_WRITE,
+ MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+ if (non_mte_buf == MAP_FAILED)
+return 1;
+
+ // Set incrementing tags until end of the page
+ char *tagged_ptr = buf;
+ while (__arm_mte_ptrdiff(tagged_ptr, buf) < page_size) {
+__arm_mte_set_tag(tagged_ptr);
+// 16 byte granules, tags will wrap at 0xF
+tagged_ptr = __arm_mte_increment_tag(tagged_ptr + 16, 1);
+ }
+
+ // Tag the original pointer with 9
+ buf = __arm_mte_create_random_tag(buf, ~(1 << 9));
+ // A different tag so that buf_alt_tag > buf if you don't handle the tag
+ char *buf_alt_tag = __arm_mte_create_random_tag(buf, ~(1 << 10));
+
+ // Breakpoint here
+ return 0;
+}
Index: lldb/test/API/linux/aarch64/mte_tag_read/TestAArch64LinuxMTEMemoryTagRead.py
===
--- /dev/null
+++ lldb/test/API/linux/aarch64/mte_tag_read/TestAArch64LinuxMTEMemoryTagRead.py
@@ -0,0 +1,116 @@
+"""
+Test "memory tag read" command on AArch64 Linux with MTE.
+"""
+
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class AArch64LinuxMTEMemoryTagReadTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+NO_DEBUG_INFO_TESTCASE = True
+
+@skipUnlessArch("aarch64")
+@skipUnlessPlatform(["linux"])
+@skipUnlessAArch64MTELinuxCompiler
+def test_mte_tag_read(self):
+if not self.isAArch64MTE():
+self.skipTest('Target must support MTE.')
+
+self.build()
+self.runCmd("file " + self.getBuildArtifact("a.out"), CURRENT_EXECUTABLE_SET)
+
+lldbutil.run_break_set_by_file_and_line(self, "main.c",
+line_number('main.c', '// Breakpoint here'),
+num_expected_locations=1)
+
+self.runCmd("run", RUN_SUCCEEDED)
+
+if self.process().GetState() == lldb.eStateExited:
+self.fail("Test program failed to run.")
+
+self.expect("thread list", STOPPED_DUE_TO_BREAKPOINT,
+substrs=['stopped',
+ 'stop reason = breakpoint'])
+
+# Argument validation
+self.expect("memory tag read",
+substrs=["error: wrong number of arguments; expected at least , "
+
[Lldb-commits] [PATCH] D97285: [lldb][AArch64] Add "memory tag read" command
DavidSpickett added reviewers: omjavaid, labath. DavidSpickett added a comment. This command is described in the RFC under the name "mtag showatag" (https://lists.llvm.org/pipermail/lldb-dev/2020-August/016402.html). The change of naming was suggested in feedback and I think it does fit better with the other commands so I went with it. The output is very basic and could certainly be compacted but this is just the initial implementation. There is also the option to have this command highlight matches/mismatches. This would reduce (maybe remove) the need for a "memory tag check" command. I think that would be better done as a follow up if so. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97285/new/ https://reviews.llvm.org/D97285 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D95601: [lldb][AArch64] Add memory tag reading to lldb-server
DavidSpickett added inline comments.
Comment at: lldb/test/API/tools/lldb-server/memory-tagging/main.c:36
+ // This intrinsic treats the addresses as if they were untagged
+ while (__arm_mte_ptrdiff(tagged_ptr, buf) < page_size) {
+// This sets the allocation tag
See
https://developer.arm.com/documentation/101028/0012/10--Memory-tagging-intrinsics
for in depth descriptions.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D95601/new/
https://reviews.llvm.org/D95601
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D97285: [lldb][AArch64] Add "memory tag read" command
DavidSpickett added inline comments. Comment at: lldb/test/API/linux/aarch64/mte_tag_read/main.c:46 + // A different tag so that buf_alt_tag > buf if you don't handle the tag + char *buf_alt_tag = __arm_mte_create_random_tag(buf, ~(1 << 10)); + https://developer.arm.com/documentation/101028/0012/10--Memory-tagging-intrinsics for descriptions of these functions. I probably need to add some more comments here for the unfamiliar. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97285/new/ https://reviews.llvm.org/D97285 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D97285: [lldb][AArch64] Add "memory tag read" command
DavidSpickett updated this revision to Diff 325775.
DavidSpickett added a comment.
- Add some comments to explain the intrinsics used.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D97285/new/
https://reviews.llvm.org/D97285
Files:
lldb/source/Commands/CMakeLists.txt
lldb/source/Commands/CommandObjectMemory.cpp
lldb/source/Commands/CommandObjectMemoryTag.cpp
lldb/source/Commands/CommandObjectMemoryTag.h
lldb/test/API/functionalities/memory/tag/Makefile
lldb/test/API/functionalities/memory/tag/TestMemoryTag.py
lldb/test/API/functionalities/memory/tag/main.cpp
lldb/test/API/linux/aarch64/mte_tag_read/Makefile
lldb/test/API/linux/aarch64/mte_tag_read/TestAArch64LinuxMTEMemoryTagRead.py
lldb/test/API/linux/aarch64/mte_tag_read/main.c
Index: lldb/test/API/linux/aarch64/mte_tag_read/main.c
===
--- /dev/null
+++ lldb/test/API/linux/aarch64/mte_tag_read/main.c
@@ -0,0 +1,54 @@
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+int main(int argc, char const *argv[]) {
+ // We assume that the test runner has checked we're on an MTE system
+
+ if (prctl(PR_SET_TAGGED_ADDR_CTRL,
+PR_TAGGED_ADDR_ENABLE | PR_MTE_TCF_SYNC |
+// Allow all tags to be generated by the addg
+// instruction __arm_mte_increment_tag produces.
+(0x << PR_MTE_TAG_SHIFT),
+0, 0, 0)) {
+return 1;
+ }
+
+ size_t page_size = sysconf(_SC_PAGESIZE);
+
+ // Allocate memory with MTE
+ char *buf = mmap(0, sysconf(_SC_PAGESIZE), PROT_READ | PROT_WRITE | PROT_MTE,
+ MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+ if (buf == MAP_FAILED)
+return 1;
+
+ // And without MTE
+ char *non_mte_buf = mmap(0, sysconf(_SC_PAGESIZE), PROT_READ | PROT_WRITE,
+ MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+ if (non_mte_buf == MAP_FAILED)
+return 1;
+
+ // Set incrementing tags until end of the page
+ char *tagged_ptr = buf;
+ // This ignores tag bits when subtracting the addresses
+ while (__arm_mte_ptrdiff(tagged_ptr, buf) < page_size) {
+// Set the allocation tag for this location
+__arm_mte_set_tag(tagged_ptr);
+// + 16 for 16 byte granules
+// Earlier we allowed all tag values, so this will give us an
+// incrementing pattern 0-0xF wrapping back to 0.
+tagged_ptr = __arm_mte_increment_tag(tagged_ptr + 16, 1);
+ }
+
+ // Tag the original pointer with 9
+ buf = __arm_mte_create_random_tag(buf, ~(1 << 9));
+ // A different tag so that buf_alt_tag > buf if you don't handle the tag
+ char *buf_alt_tag = __arm_mte_create_random_tag(buf, ~(1 << 10));
+
+ // Breakpoint here
+ return 0;
+}
Index: lldb/test/API/linux/aarch64/mte_tag_read/TestAArch64LinuxMTEMemoryTagRead.py
===
--- /dev/null
+++ lldb/test/API/linux/aarch64/mte_tag_read/TestAArch64LinuxMTEMemoryTagRead.py
@@ -0,0 +1,116 @@
+"""
+Test "memory tag read" command on AArch64 Linux with MTE.
+"""
+
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class AArch64LinuxMTEMemoryTagReadTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+NO_DEBUG_INFO_TESTCASE = True
+
+@skipUnlessArch("aarch64")
+@skipUnlessPlatform(["linux"])
+@skipUnlessAArch64MTELinuxCompiler
+def test_mte_tag_read(self):
+if not self.isAArch64MTE():
+self.skipTest('Target must support MTE.')
+
+self.build()
+self.runCmd("file " + self.getBuildArtifact("a.out"), CURRENT_EXECUTABLE_SET)
+
+lldbutil.run_break_set_by_file_and_line(self, "main.c",
+line_number('main.c', '// Breakpoint here'),
+num_expected_locations=1)
+
+self.runCmd("run", RUN_SUCCEEDED)
+
+if self.process().GetState() == lldb.eStateExited:
+self.fail("Test program failed to run.")
+
+self.expect("thread list", STOPPED_DUE_TO_BREAKPOINT,
+substrs=['stopped',
+ 'stop reason = breakpoint'])
+
+# Argument validation
+self.expect("memory tag read",
+substrs=["error: wrong number of arguments; expected at least , "
+ "at most "],
+error=True)
+self.expect("memory tag read buf buf+16 32",
+substrs=["error: wrong number of arguments; expected at least , "
+ "at most "],
+error=True)
+self.expect("memory tag read not_a_symbol",
+substrs=["error: Invalid address expression, address expression \"not_a_symbol\" "
+ "evaluation failed"],
+error=True)
+self.expect("memory tag read buf not_a_symbol",
+substrs=["error: Invalid end address expres
[Lldb-commits] [PATCH] D97287: [lldb][NFC] Rename the second ValueObjectManager to ValueObjectUpdater and remove the dead code
teemperor created this revision.
teemperor added a reviewer: LLDB.
teemperor added a project: LLDB.
Herald added subscribers: JDevlieghere, mgorny.
teemperor requested review of this revision.
`ValueObject.h` contains the `ValueObject::ValueObjectManager` type which is
just a typedef
for the ClusterManager that takes care of the whole ValueObject memory
management. However,
there is also `ValueObjectManager` defined in the same header which is only
used in the curses UI
implementation and consists mostly of dead and completely untested code.
This code been around since a while (it was added in 2016 as
8369b28da0750129ababae357bea98940800a0e0),
so I think we shouldn't just revert the whole patch.
Instead this patch just moves the class to its own header that it isn't just
hiding in the ValueObject
header and renames it to `ValueObjectUpdater` that it at least has a unique
name (which I hope also slightly
better reflects the purpose of this class). I also deleted all the dead code
branches and functions.
https://reviews.llvm.org/D97287
Files:
lldb/include/lldb/Core/ValueObject.h
lldb/include/lldb/Core/ValueObjectUpdater.h
lldb/source/Core/CMakeLists.txt
lldb/source/Core/IOHandlerCursesGUI.cpp
lldb/source/Core/ValueObject.cpp
lldb/source/Core/ValueObjectUpdater.cpp
Index: lldb/source/Core/ValueObjectUpdater.cpp
===
--- /dev/null
+++ lldb/source/Core/ValueObjectUpdater.cpp
@@ -0,0 +1,56 @@
+//===-- ValueObjectUpdater.cpp ===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/Core/ValueObjectUpdater.h"
+
+using namespace lldb_private;
+
+ValueObjectUpdater::ValueObjectUpdater(lldb::ValueObjectSP in_valobj_sp) {
+ if (!in_valobj_sp)
+return;
+ // If the user passes in a value object that is dynamic or synthetic, then
+ // water it down to the static type.
+ m_root_valobj_sp = in_valobj_sp->GetQualifiedRepresentationIfAvailable(
+ lldb::eNoDynamicValues, false);
+}
+
+lldb::ValueObjectSP ValueObjectUpdater::GetSP() {
+ lldb::ProcessSP process_sp = GetProcessSP();
+ if (!process_sp)
+return lldb::ValueObjectSP();
+
+ const uint32_t current_stop_id = process_sp->GetLastNaturalStopID();
+ if (current_stop_id == m_stop_id)
+return m_user_valobj_sp;
+
+ m_stop_id = current_stop_id;
+
+ if (!m_root_valobj_sp) {
+m_user_valobj_sp.reset();
+return m_root_valobj_sp;
+ }
+
+ m_user_valobj_sp = m_root_valobj_sp;
+
+ lldb::ValueObjectSP dynamic_sp =
+ m_user_valobj_sp->GetDynamicValue(lldb::eDynamicDontRunTarget);
+ if (dynamic_sp)
+m_user_valobj_sp = dynamic_sp;
+
+ lldb::ValueObjectSP synthetic_sp = m_user_valobj_sp->GetSyntheticValue();
+ if (synthetic_sp)
+m_user_valobj_sp = synthetic_sp;
+
+ return m_user_valobj_sp;
+}
+
+lldb::ProcessSP ValueObjectUpdater::GetProcessSP() const {
+ if (m_root_valobj_sp)
+return m_root_valobj_sp->GetProcessSP();
+ return lldb::ProcessSP();
+}
Index: lldb/source/Core/ValueObject.cpp
===
--- lldb/source/Core/ValueObject.cpp
+++ lldb/source/Core/ValueObject.cpp
@@ -3212,97 +3212,3 @@
uint64_t ValueObject::GetLanguageFlags() { return m_language_flags; }
void ValueObject::SetLanguageFlags(uint64_t flags) { m_language_flags = flags; }
-
-ValueObjectManager::ValueObjectManager(lldb::ValueObjectSP in_valobj_sp,
- lldb::DynamicValueType use_dynamic,
- bool use_synthetic) : m_root_valobj_sp(),
-m_user_valobj_sp(), m_use_dynamic(use_dynamic), m_stop_id(UINT32_MAX),
-m_use_synthetic(use_synthetic) {
- if (!in_valobj_sp)
-return;
- // If the user passes in a value object that is dynamic or synthetic, then
- // water it down to the static type.
- m_root_valobj_sp = in_valobj_sp->GetQualifiedRepresentationIfAvailable(lldb::eNoDynamicValues, false);
-}
-
-bool ValueObjectManager::IsValid() const {
- if (!m_root_valobj_sp)
-return false;
- lldb::TargetSP target_sp = GetTargetSP();
- if (target_sp)
-return target_sp->IsValid();
- return false;
-}
-
-lldb::ValueObjectSP ValueObjectManager::GetSP() {
- lldb::ProcessSP process_sp = GetProcessSP();
- if (!process_sp)
-return lldb::ValueObjectSP();
-
- const uint32_t current_stop_id = process_sp->GetLastNaturalStopID();
- if (current_stop_id == m_stop_id)
-return m_user_valobj_sp;
-
- m_stop_id = current_stop_id;
-
- if (!m_root_valobj_sp) {
-m_user_valobj_sp.reset();
-return m_root_valobj_sp;
- }
-
- m_user_valobj_sp = m_root_valobj_sp;
-
- if (m_use_dynamic != lldb::eNoDynamicValues) {
-lldb::ValueObjectSP dynam
[Lldb-commits] [PATCH] D97287: [lldb][NFC] Rename the second ValueObjectManager to ValueObjectUpdater and remove the dead code
mib accepted this revision. mib added a comment. This revision is now accepted and ready to land. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97287/new/ https://reviews.llvm.org/D97287 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D97298: [lldb][NFC] Move trivial ValueObject getters/setters to the header
teemperor created this revision.
teemperor added a reviewer: LLDB.
teemperor added a project: LLDB.
Herald added a subscriber: JDevlieghere.
teemperor requested review of this revision.
NFC refactoring that moves the definitions of all the trivial getters/setters
to the header file
which is what we usually do in LLVM.
https://reviews.llvm.org/D97298
Files:
lldb/include/lldb/Core/ValueObject.h
lldb/source/Core/ValueObject.cpp
Index: lldb/source/Core/ValueObject.cpp
===
--- lldb/source/Core/ValueObject.cpp
+++ lldb/source/Core/ValueObject.cpp
@@ -271,11 +271,7 @@
return compiler_type;
}
-CompilerType ValueObject::GetCompilerType() {
- return MaybeCalculateCompleteType();
-}
-TypeImpl ValueObject::GetTypeImpl() { return TypeImpl(GetCompilerType()); }
DataExtractor &ValueObject::GetDataExtractor() {
UpdateValueIfNeeded(false);
@@ -287,12 +283,6 @@
return m_error;
}
-ConstString ValueObject::GetName() const { return m_name; }
-
-const char *ValueObject::GetLocationAsCString() {
- return GetLocationAsCStringImpl(m_value, m_data);
-}
-
const char *ValueObject::GetLocationAsCStringImpl(const Value &value,
const DataExtractor &data) {
if (UpdateValueIfNeeded(false)) {
@@ -337,10 +327,6 @@
return m_location_str.c_str();
}
-Value &ValueObject::GetValue() { return m_value; }
-
-const Value &ValueObject::GetValue() const { return m_value; }
-
bool ValueObject::ResolveValue(Scalar &scalar) {
if (UpdateValueIfNeeded(
false)) // make sure that you are up to date before returning anything
@@ -384,16 +370,6 @@
return ret;
}
-bool ValueObject::GetValueIsValid() const { return m_flags.m_value_is_valid; }
-
-void ValueObject::SetValueIsValid(bool b) { m_flags.m_value_is_valid = b; }
-
-bool ValueObject::GetValueDidChange() { return m_flags.m_value_did_change; }
-
-void ValueObject::SetValueDidChange(bool value_changed) {
- m_flags.m_value_did_change = value_changed;
-}
-
ValueObjectSP ValueObject::GetChildAtIndex(size_t idx, bool can_create) {
ValueObjectSP child_sp;
// We may need to update our value if we are dynamic
@@ -550,8 +526,6 @@
m_children.SetChildrenCount(num_children);
}
-void ValueObject::SetName(ConstString name) { m_name = name; }
-
ValueObject *ValueObject::CreateChildAtIndex(size_t idx,
bool synthetic_array_member,
int32_t synthetic_index) {
@@ -1573,20 +1547,6 @@
return false;
}
-ConstString ValueObject::GetTypeName() {
- return GetCompilerType().GetTypeName();
-}
-
-ConstString ValueObject::GetDisplayTypeName() { return GetTypeName(); }
-
-ConstString ValueObject::GetQualifiedTypeName() {
- return GetCompilerType().GetTypeName();
-}
-
-LanguageType ValueObject::GetObjectRuntimeLanguage() {
- return GetCompilerType().GetMinimumLanguage();
-}
-
void ValueObject::AddSyntheticChild(ConstString key,
ValueObject *valobj) {
m_synthetic_children[key] = valobj;
@@ -1601,25 +1561,6 @@
return synthetic_child_sp;
}
-uint32_t
-ValueObject::GetTypeInfo(CompilerType *pointee_or_element_compiler_type) {
- return GetCompilerType().GetTypeInfo(pointee_or_element_compiler_type);
-}
-
-bool ValueObject::IsPointerType() { return GetCompilerType().IsPointerType(); }
-
-bool ValueObject::IsArrayType() { return GetCompilerType().IsArrayType(); }
-
-bool ValueObject::IsScalarType() { return GetCompilerType().IsScalarType(); }
-
-bool ValueObject::IsIntegerType(bool &is_signed) {
- return GetCompilerType().IsIntegerType(is_signed);
-}
-
-bool ValueObject::IsPointerOrReferenceType() {
- return GetCompilerType().IsPointerOrReferenceType();
-}
-
bool ValueObject::IsPossibleDynamicType() {
ExecutionContext exe_ctx(GetExecutionContextRef());
Process *process = exe_ctx.GetProcessPtr();
@@ -1899,10 +1840,6 @@
return ValueObjectSP();
}
-ValueObjectSP ValueObject::GetStaticValue() { return GetSP(); }
-
-lldb::ValueObjectSP ValueObject::GetNonSyntheticValue() { return GetSP(); }
-
ValueObjectSP ValueObject::GetSyntheticValue() {
CalculateSyntheticValue();
@@ -3154,10 +3091,6 @@
return (m_preferred_display_language = type); // only compute it once
}
-void ValueObject::SetPreferredDisplayLanguage(lldb::LanguageType lt) {
- m_preferred_display_language = lt;
-}
-
void ValueObject::SetPreferredDisplayLanguageIfNeeded(lldb::LanguageType lt) {
if (m_preferred_display_language == lldb::eLanguageTypeUnknown)
SetPreferredDisplayLanguage(lt);
@@ -3201,18 +3134,6 @@
return persistent_var_sp->GetValueObject();
}
-bool ValueObject::IsSyntheticChildrenGenerated() {
- return m_flags.m_is_synthetic_children_generated;
-}
-
-void ValueObject::SetSyntheticChildrenGenerated(bool b) {
- m_flags.m_is_synthetic_children_generated = b;
-}
-
-uint64_t ValueObject::GetLanguageFlags() {
[Lldb-commits] [PATCH] D97165: [lldb] Add deref support and tests to shared_ptr synthetic
kastiglione updated this revision to Diff 325808.
kastiglione added a comment.
check ptr_sp before use
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D97165/new/
https://reviews.llvm.org/D97165
Files:
lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
lldb/source/Plugins/Language/CPlusPlus/LibCxx.h
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/shared_ptr/Makefile
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/shared_ptr/TestDataFormatterLibcxxSharedPtr.py
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/shared_ptr/main.cpp
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/shared_ptr/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/shared_ptr/main.cpp
@@ -0,0 +1,18 @@
+#include
+#include
+
+struct User {
+ int id = 30;
+ std::string name = "steph";
+};
+
+int main() {
+ std::shared_ptr sp_empty;
+ std::shared_ptr sp_int = std::make_shared(10);
+ std::shared_ptr sp_str = std::make_shared("hello");
+ std::shared_ptr &sp_int_ref = sp_int;
+ std::shared_ptr &&sp_int_ref_ref = std::make_shared(10);
+ std::shared_ptr sp_user = std::make_shared();
+
+ return 0; // break here
+}
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/shared_ptr/TestDataFormatterLibcxxSharedPtr.py
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/shared_ptr/TestDataFormatterLibcxxSharedPtr.py
@@ -0,0 +1,88 @@
+"""
+Test lldb data formatter for libc++ std::shared_ptr.
+"""
+
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+@add_test_categories(["libc++"])
+def test_shared_ptr_variables(self):
+"""Test `frame variable` output for `std::shared_ptr` types."""
+self.build()
+
+lldbutil.run_to_source_breakpoint(
+self, "// break here", lldb.SBFileSpec("main.cpp")
+)
+
+valobj = self.expect_var_path(
+"sp_empty",
+type="std::shared_ptr",
+summary="nullptr",
+children=[ValueCheck(name="__ptr_")],
+)
+self.assertEqual(
+valobj.child[0].GetValueAsUnsigned(lldb.LLDB_INVALID_ADDRESS), 0
+)
+
+self.expect(
+"frame variable *sp_empty", substrs=["(int) *sp_empty = "]
+)
+
+valobj = self.expect_var_path(
+"sp_int",
+type="std::shared_ptr",
+children=[ValueCheck(name="__ptr_")],
+)
+self.assertRegex(valobj.summary, r"^10( strong=1)? weak=1$")
+self.assertNotEqual(valobj.child[0].unsigned, 0)
+
+valobj = self.expect_var_path(
+"sp_int_ref",
+type="std::shared_ptr &",
+children=[ValueCheck(name="__ptr_")],
+)
+self.assertRegex(valobj.summary, r"^10( strong=1)? weak=1$")
+self.assertNotEqual(valobj.child[0].unsigned, 0)
+
+valobj = self.expect_var_path(
+"sp_int_ref_ref",
+type="std::shared_ptr &&",
+children=[ValueCheck(name="__ptr_")],
+)
+self.assertRegex(valobj.summary, r"^10( strong=1)? weak=1$")
+self.assertNotEqual(valobj.child[0].unsigned, 0)
+
+valobj = self.expect_var_path(
+"sp_str",
+type="std::shared_ptr, std::allocator > >",
+children=[ValueCheck(name="__ptr_", summary='"hello"')],
+)
+self.assertRegex(valobj.summary, r'^"hello"( strong=1)? weak=1$')
+
+valobj = self.expect_var_path("sp_user", type="std::shared_ptr")
+self.assertRegex(
+valobj.summary,
+"^std(::__1)?::shared_ptr::element_type @ 0x0*[1-9a-f][0-9a-f]+( strong=1)? weak=1",
+)
+self.assertNotEqual(valobj.child[0].unsigned, 0)
+
+valobj = self.expect_var_path(
+"*sp_user",
+type="User",
+children=[
+ValueCheck(name="id", value="30"),
+ValueCheck(name="name", summary='"steph"'),
+],
+)
+self.assertEqual(str(valobj), '(User) *__ptr_ = (id = 30, name = "steph")')
+
+self.expect_var_path("sp_user->id", type="int", value="30")
+self.expect_var_path("sp_user->name", type="std::string", summary='"steph"')
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/shared_ptr/Makefile
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/shared_ptr/Makefile
@@ -0,0 +1,6 @@
+CXX_SOURCES := main
[Lldb-commits] [lldb] 0ac42fd - [lldb] Add deref support and tests to shared_ptr synthetic
Author: Dave Lee
Date: 2021-02-23T09:03:46-08:00
New Revision: 0ac42fd26d738b2d7b2811fc995bd7cacf994144
URL:
https://github.com/llvm/llvm-project/commit/0ac42fd26d738b2d7b2811fc995bd7cacf994144
DIFF:
https://github.com/llvm/llvm-project/commit/0ac42fd26d738b2d7b2811fc995bd7cacf994144.diff
LOG: [lldb] Add deref support and tests to shared_ptr synthetic
Add `frame variable` dereference suppport to libc++ `std::shared_ptr`.
This change allows for commands like `v *thing_sp` and `v thing_sp->m_id`. These
commands now work the same way they do with raw pointers. This is done by
adding an
unaccounted for child member named `$$dereference$$`.
Also, add API tests for `std::shared_ptr`, previously there were none.
Differential Revision: https://reviews.llvm.org/D97165
Added:
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/shared_ptr/Makefile
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/shared_ptr/TestDataFormatterLibcxxSharedPtr.py
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/shared_ptr/main.cpp
Modified:
lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
lldb/source/Plugins/Language/CPlusPlus/LibCxx.h
Removed:
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
index 2b16ebe79daf..925076f10d72 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
@@ -379,8 +379,7 @@
lldb_private::formatters::LibCxxVectorIteratorSyntheticFrontEndCreator(
lldb_private::formatters::LibcxxSharedPtrSyntheticFrontEnd::
LibcxxSharedPtrSyntheticFrontEnd(lldb::ValueObjectSP valobj_sp)
-: SyntheticChildrenFrontEnd(*valobj_sp), m_cntrl(nullptr), m_count_sp(),
- m_weak_count_sp(), m_ptr_size(0), m_byte_order(lldb::eByteOrderInvalid) {
+: SyntheticChildrenFrontEnd(*valobj_sp), m_cntrl(nullptr) {
if (valobj_sp)
Update();
}
@@ -403,42 +402,23 @@
lldb_private::formatters::LibcxxSharedPtrSyntheticFrontEnd::GetChildAtIndex(
if (idx == 0)
return valobj_sp->GetChildMemberWithName(ConstString("__ptr_"), true);
- if (idx > 2)
-return lldb::ValueObjectSP();
-
if (idx == 1) {
-if (!m_count_sp) {
- ValueObjectSP shared_owners_sp(m_cntrl->GetChildMemberWithName(
- ConstString("__shared_owners_"), true));
- if (!shared_owners_sp)
-return lldb::ValueObjectSP();
- uint64_t count = 1 + shared_owners_sp->GetValueAsUnsigned(0);
- DataExtractor data(&count, 8, m_byte_order, m_ptr_size);
- m_count_sp = CreateValueObjectFromData(
- "count", data, valobj_sp->GetExecutionContextRef(),
- shared_owners_sp->GetCompilerType());
-}
-return m_count_sp;
- } else /* if (idx == 2) */
- {
-if (!m_weak_count_sp) {
- ValueObjectSP shared_weak_owners_sp(m_cntrl->GetChildMemberWithName(
- ConstString("__shared_weak_owners_"), true));
- if (!shared_weak_owners_sp)
-return lldb::ValueObjectSP();
- uint64_t count = 1 + shared_weak_owners_sp->GetValueAsUnsigned(0);
- DataExtractor data(&count, 8, m_byte_order, m_ptr_size);
- m_weak_count_sp = CreateValueObjectFromData(
- "count", data, valobj_sp->GetExecutionContextRef(),
- shared_weak_owners_sp->GetCompilerType());
+if (auto ptr_sp =
+valobj_sp->GetChildMemberWithName(ConstString("__ptr_"), true)) {
+ Status status;
+ auto value_sp = ptr_sp->Dereference(status);
+ if (status.Success()) {
+auto value_type_sp =
+valobj_sp->GetCompilerType().GetTypeTemplateArgument(0);
+return value_sp->Cast(value_type_sp);
+ }
}
-return m_weak_count_sp;
}
+
+ return lldb::ValueObjectSP();
}
bool lldb_private::formatters::LibcxxSharedPtrSyntheticFrontEnd::Update() {
- m_count_sp.reset();
- m_weak_count_sp.reset();
m_cntrl = nullptr;
ValueObjectSP valobj_sp = m_backend.GetSP();
@@ -449,9 +429,6 @@ bool
lldb_private::formatters::LibcxxSharedPtrSyntheticFrontEnd::Update() {
if (!target_sp)
return false;
- m_byte_order = target_sp->GetArchitecture().GetByteOrder();
- m_ptr_size = target_sp->GetArchitecture().GetAddressByteSize();
-
lldb::ValueObjectSP cntrl_sp(
valobj_sp->GetChildMemberWithName(ConstString("__cntrl_"), true));
@@ -469,10 +446,8 @@ size_t
lldb_private::formatters::LibcxxSharedPtrSyntheticFrontEnd::
GetIndexOfChildWithName(ConstString name) {
if (name == "__ptr_")
return 0;
- if (name == "count")
+ if (name == "$$dereference$$")
return 1;
- if (name == "weak_count")
-return 2;
return UINT32_MAX;
}
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.h
b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.h
index ea5a7c178178..907025fe8ac8 100644
--- a/lldb/source/Plugins/Language/CPlus
[Lldb-commits] [PATCH] D97165: [lldb] Add deref support and tests to shared_ptr synthetic
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG0ac42fd26d73: [lldb] Add deref support and tests to
shared_ptr synthetic (authored by kastiglione).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D97165/new/
https://reviews.llvm.org/D97165
Files:
lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
lldb/source/Plugins/Language/CPlusPlus/LibCxx.h
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/shared_ptr/Makefile
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/shared_ptr/TestDataFormatterLibcxxSharedPtr.py
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/shared_ptr/main.cpp
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/shared_ptr/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/shared_ptr/main.cpp
@@ -0,0 +1,18 @@
+#include
+#include
+
+struct User {
+ int id = 30;
+ std::string name = "steph";
+};
+
+int main() {
+ std::shared_ptr sp_empty;
+ std::shared_ptr sp_int = std::make_shared(10);
+ std::shared_ptr sp_str = std::make_shared("hello");
+ std::shared_ptr &sp_int_ref = sp_int;
+ std::shared_ptr &&sp_int_ref_ref = std::make_shared(10);
+ std::shared_ptr sp_user = std::make_shared();
+
+ return 0; // break here
+}
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/shared_ptr/TestDataFormatterLibcxxSharedPtr.py
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/shared_ptr/TestDataFormatterLibcxxSharedPtr.py
@@ -0,0 +1,88 @@
+"""
+Test lldb data formatter for libc++ std::shared_ptr.
+"""
+
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+@add_test_categories(["libc++"])
+def test_shared_ptr_variables(self):
+"""Test `frame variable` output for `std::shared_ptr` types."""
+self.build()
+
+lldbutil.run_to_source_breakpoint(
+self, "// break here", lldb.SBFileSpec("main.cpp")
+)
+
+valobj = self.expect_var_path(
+"sp_empty",
+type="std::shared_ptr",
+summary="nullptr",
+children=[ValueCheck(name="__ptr_")],
+)
+self.assertEqual(
+valobj.child[0].GetValueAsUnsigned(lldb.LLDB_INVALID_ADDRESS), 0
+)
+
+self.expect(
+"frame variable *sp_empty", substrs=["(int) *sp_empty = "]
+)
+
+valobj = self.expect_var_path(
+"sp_int",
+type="std::shared_ptr",
+children=[ValueCheck(name="__ptr_")],
+)
+self.assertRegex(valobj.summary, r"^10( strong=1)? weak=1$")
+self.assertNotEqual(valobj.child[0].unsigned, 0)
+
+valobj = self.expect_var_path(
+"sp_int_ref",
+type="std::shared_ptr &",
+children=[ValueCheck(name="__ptr_")],
+)
+self.assertRegex(valobj.summary, r"^10( strong=1)? weak=1$")
+self.assertNotEqual(valobj.child[0].unsigned, 0)
+
+valobj = self.expect_var_path(
+"sp_int_ref_ref",
+type="std::shared_ptr &&",
+children=[ValueCheck(name="__ptr_")],
+)
+self.assertRegex(valobj.summary, r"^10( strong=1)? weak=1$")
+self.assertNotEqual(valobj.child[0].unsigned, 0)
+
+valobj = self.expect_var_path(
+"sp_str",
+type="std::shared_ptr, std::allocator > >",
+children=[ValueCheck(name="__ptr_", summary='"hello"')],
+)
+self.assertRegex(valobj.summary, r'^"hello"( strong=1)? weak=1$')
+
+valobj = self.expect_var_path("sp_user", type="std::shared_ptr")
+self.assertRegex(
+valobj.summary,
+"^std(::__1)?::shared_ptr::element_type @ 0x0*[1-9a-f][0-9a-f]+( strong=1)? weak=1",
+)
+self.assertNotEqual(valobj.child[0].unsigned, 0)
+
+valobj = self.expect_var_path(
+"*sp_user",
+type="User",
+children=[
+ValueCheck(name="id", value="30"),
+ValueCheck(name="name", summary='"steph"'),
+],
+)
+self.assertEqual(str(valobj), '(User) *__ptr_ = (id = 30, name = "steph")')
+
+self.expect_var_path("sp_user->id", type="int", value="30")
+self.expect_var_path("sp_user->name", type="std::string", summary='"steph"')
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/shared_ptr/Makefile
===
[Lldb-commits] [PATCH] D97249: [lldb] Support debugging utility functions
jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. This will be really handy! The code for UserExpressions ends up doing this same thing (over in ClangExpressionParser::ParseInternal. But given how light-weight creating the file is I'm pretty sure it isn't worth trying to activate that code starting from the UtilityExpression. LGTM. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97249/new/ https://reviews.llvm.org/D97249 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D97307: [lldb][NFC] Extract ValueObject's expression path parsing into own class
teemperor created this revision.
teemperor added a reviewer: LLDB.
teemperor added a project: LLDB.
Herald added subscribers: JDevlieghere, mgorny.
teemperor requested review of this revision.
A large chunk of the `ValueObject` implementation is the hand-written
expression path parser
and nearly all of the enums in the class are just concerned with expression
paths. The `ValueObject`
code could be far more concise if we had all the parsing logic in its own
file/class. I also want to
merge the tab-completion parser for expression paths (which is currently its
own separate implementation
in `Variable.cpp`) into the normal parser which is less awkward if the parser
wasn't just an implementation
detail of ValueObject.
As I anyway had to touch all the enums I changed them to scoped enums.
https://reviews.llvm.org/D97307
Files:
lldb/include/lldb/Core/ExpressionPath.h
lldb/include/lldb/Core/ValueObject.h
lldb/include/lldb/Core/ValueObjectRegister.h
lldb/source/Commands/CommandObjectFrame.cpp
lldb/source/Core/CMakeLists.txt
lldb/source/Core/ExpressionPath.cpp
lldb/source/Core/FormatEntity.cpp
lldb/source/Core/ValueObject.cpp
lldb/source/Core/ValueObjectRegister.cpp
lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
lldb/source/Target/Process.cpp
Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -859,9 +859,8 @@
ValueObjectSP valobj_sp = StopInfo::GetCrashingDereference(
curr_thread_stop_info_sp, &crashing_address);
if (valobj_sp) {
- const ValueObject::GetExpressionPathFormat format =
- ValueObject::GetExpressionPathFormat::
- eGetExpressionPathFormatHonorPointers;
+ const ExpressionPath::PathFormat format =
+ ExpressionPath::PathFormat::HonorPointers;
stream->PutCString("Likely cause: ");
valobj_sp->GetExpressionPath(*stream, format);
stream->Printf(" accessed 0x%" PRIx64 "\n", crashing_address);
Index: lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
===
--- lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
@@ -236,10 +236,10 @@
m_pair_ptr = valobj_sp
->GetValueForExpressionPath(
".__i_.__ptr_->__value_", nullptr, nullptr,
- ValueObject::GetValueForExpressionPathOptions()
+ ExpressionPath::GetValueOptions()
.DontCheckDotVsArrowSyntax()
.SetSyntheticChildrenTraversal(
- ValueObject::GetValueForExpressionPathOptions::
+ ExpressionPath::GetValueOptions::
SyntheticChildrenTraversal::None),
nullptr)
.get();
@@ -248,10 +248,10 @@
m_pair_ptr = valobj_sp
->GetValueForExpressionPath(
".__i_.__ptr_", nullptr, nullptr,
- ValueObject::GetValueForExpressionPathOptions()
+ ExpressionPath::GetValueOptions()
.DontCheckDotVsArrowSyntax()
.SetSyntheticChildrenTraversal(
- ValueObject::GetValueForExpressionPathOptions::
+ ExpressionPath::GetValueOptions::
SyntheticChildrenTraversal::None),
nullptr)
.get();
Index: lldb/source/Core/ValueObjectRegister.cpp
===
--- lldb/source/Core/ValueObjectRegister.cpp
+++ lldb/source/Core/ValueObjectRegister.cpp
@@ -299,7 +299,7 @@
return false;
}
-void ValueObjectRegister::GetExpressionPath(Stream &s,
-GetExpressionPathFormat epformat) {
+void ValueObjectRegister::GetExpressionPath(
+Stream &s, ExpressionPath::PathFormat epformat) {
s.Printf("$%s", m_reg_info.name);
}
Index: lldb/source/Core/ValueObject.cpp
===
--- lldb/source/Core/ValueObject.cpp
+++ lldb/source/Core/ValueObject.cpp
@@ -1835,9 +1835,8 @@
// make one and cache it for any future reference.
synthetic_child_sp = GetValueForExpressionPath(
expression, nullptr, nullptr,
-GetValueForExpressionPathOptions().SetSyntheticChildrenTraversal(
-GetValueForExpressionPathOptions::SyntheticChildrenTraversal::
-None));
+ExpressionPath::GetValueOptions().SetSyntheticChildrenTraversal(
+ExpressionPath::GetValueOptions::SyntheticChildrenTraversal::None));
// C
[Lldb-commits] [PATCH] D97249: [lldb] Support debugging utility functions
shafik added inline comments.
Comment at: lldb/source/Expression/FunctionCaller.cpp:324
EvaluateExpressionOptions real_options = options;
real_options.SetDebug(false);
+ real_options.SetGenerateDebugInfo(debug);
It feels a little weird you are using the name `debug` but using it in the call
to `SetDebug(...)` but are using it in the call to `SetGenerateDebugInfo(...)`
Comment at:
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp:38
const char *ClangExpressionSourceCode::g_expression_prefix =
-"#line 1 \"" PREFIX_NAME R"("
+"#line 1 \"" PREFIX_NAME R"("#line 1
#ifndef offsetof
Why additional `#line 1`.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D97249/new/
https://reviews.llvm.org/D97249
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D97249: [lldb] Support debugging utility functions
JDevlieghere marked 2 inline comments as done.
JDevlieghere added inline comments.
Comment at: lldb/source/Expression/FunctionCaller.cpp:324
EvaluateExpressionOptions real_options = options;
real_options.SetDebug(false);
+ real_options.SetGenerateDebugInfo(debug);
shafik wrote:
> It feels a little weird you are using the name `debug` but using it in the
> call to `SetDebug(...)` but are using it in the call to
> `SetGenerateDebugInfo(...)`
Do you have a suggestion? Change the variable to `debug_utility_function`?
Comment at:
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp:38
const char *ClangExpressionSourceCode::g_expression_prefix =
-"#line 1 \"" PREFIX_NAME R"("
+"#line 1 \"" PREFIX_NAME R"("#line 1
#ifndef offsetof
shafik wrote:
> Why additional `#line 1`.
Good catch, this is a remnant of me playing around with the line directive, it
shouldn't have made it into the patch.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D97249/new/
https://reviews.llvm.org/D97249
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D97249: [lldb] Support debugging utility functions
teemperor added inline comments.
Comment at:
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp:38
const char *ClangExpressionSourceCode::g_expression_prefix =
-"#line 1 \"" PREFIX_NAME R"("
+"#line 1 \"" PREFIX_NAME R"("#line 1
#ifndef offsetof
I assume this is to get the proper file name in the final source code. This
would in theory also be required for top-level expressions, but as they are not
curiously not using ClangExpressionSourceCode they never get this prefix (and
are missing all our fancy type definitions here).
Anyway, this change will prompt the ClangModulesDeclVendor (which usually loads
Obj-C modules) to also start loading the imported C++ modules (which I believe
won't break anything but just cause a bunch of warnings about failing to load
modules). See the only other use of `g_prefix_file_name` where we filter out
imported modules from the wrapper file.
I think the idea is that the prefix starts the wrapper and then
ClangExpressionSourceCode adds some generated prefixes and then we do a `#line`
to terminate the wrapper. Not sure that's the right place for this use case,
but just putting it into the `ClangUtilityFunction` where we concat the source
code should work (even though then this logic is stuck in the initalizer, but
oh well...)
Comment at:
lldb/source/Plugins/ExpressionParser/Clang/ClangUtilityFunction.cpp:46
+ std::string text, std::string name,
+ bool debug)
: UtilityFunction(
Can you document `debug` (and maybe we could call it `allow_debugging` or
`generate_debuginfo` which is how we call that in normal expressions)?
Comment at:
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp:1595
+ const Symbol *symbol = mod_sp->FindFirstSymbolWithNameAndType(
+ g_class_getNameRaw_symbol_name, lldb::eSymbolTypeCode);
if (symbol &&
Unrelated?
Comment at: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp:9715
ScratchTypeSystemClang::CreateUtilityFunction(std::string text,
- std::string name) {
+ std::string name, bool debug) {
TargetSP target_sp = m_target_wp.lock();
Not sure if that's needed. `ScratchTypeSystem` is bound to a specific target,
so this seems like a good central place to read/use the value of the setting. I
wouldn't mind the current way, but this is extending the generic TypeSystem
interface and that's just always a bit of a pain to change in the future (when
we hopefully have more TypeSystems for different langauges).
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D97249/new/
https://reviews.llvm.org/D97249
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D97249: [lldb] Support debugging utility functions
JDevlieghere updated this revision to Diff 325837.
JDevlieghere marked 6 inline comments as done.
JDevlieghere added a comment.
Address code review feedback
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D97249/new/
https://reviews.llvm.org/D97249
Files:
lldb/include/lldb/Expression/UtilityFunction.h
lldb/include/lldb/Target/Target.h
lldb/source/Expression/FunctionCaller.cpp
lldb/source/Expression/UtilityFunction.cpp
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.h
lldb/source/Plugins/ExpressionParser/Clang/ClangUtilityFunction.cpp
lldb/source/Plugins/ExpressionParser/Clang/ClangUtilityFunction.h
lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
lldb/source/Target/Target.cpp
lldb/source/Target/TargetProperties.td
Index: lldb/source/Target/TargetProperties.td
===
--- lldb/source/Target/TargetProperties.td
+++ lldb/source/Target/TargetProperties.td
@@ -172,6 +172,9 @@
def AutoInstallMainExecutable: Property<"auto-install-main-executable", "Boolean">,
DefaultTrue,
Desc<"Always install the main executable when connected to a remote platform.">;
+ def DebugUtilityExpression: Property<"debug-utility-expression", "Boolean">,
+DefaultFalse,
+Desc<"Enable debugging of LLDB-internal utility expressions.">;
}
let Definition = "process_experimental" in {
Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -4307,6 +4307,17 @@
m_launch_info.GetFlags().Clear(lldb::eLaunchFlagDisableSTDIO);
}
+bool TargetProperties::GetDebugUtilityExpression() const {
+ const uint32_t idx = ePropertyDebugUtilityExpression;
+ return m_collection_sp->GetPropertyAtIndexAsBoolean(
+ nullptr, idx, g_target_properties[idx].default_uint_value != 0);
+}
+
+void TargetProperties::SetDebugUtilityExpression(bool debug) {
+ const uint32_t idx = ePropertyDebugUtilityExpression;
+ m_collection_sp->SetPropertyAtIndexAsBoolean(nullptr, idx, debug);
+}
+
// Target::TargetEventData
Target::TargetEventData::TargetEventData(const lldb::TargetSP &target_sp)
Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
===
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -9718,7 +9718,8 @@
return {};
return std::make_unique(
- *target_sp.get(), std::move(text), std::move(name));
+ *target_sp.get(), std::move(text), std::move(name),
+ target_sp->GetDebugUtilityExpression());
}
PersistentExpressionState *
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangUtilityFunction.h
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangUtilityFunction.h
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangUtilityFunction.h
@@ -49,7 +49,7 @@
/// \param[in] name
/// The name of the function, as used in the text.
ClangUtilityFunction(ExecutionContextScope &exe_scope, std::string text,
- std::string name);
+ std::string name, bool debug = false);
~ClangUtilityFunction() override;
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangUtilityFunction.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangUtilityFunction.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangUtilityFunction.cpp
@@ -34,19 +34,34 @@
char ClangUtilityFunction::ID;
-/// Constructor
-///
-/// \param[in] text
-/// The text of the function. Must be a full translation unit.
-///
-/// \param[in] name
-/// The name of the function, as used in the text.
ClangUtilityFunction::ClangUtilityFunction(ExecutionContextScope &exe_scope,
- std::string text, std::string name)
+ std::string text, std::string name,
+ bool enable_debugging)
: UtilityFunction(
exe_scope,
- std::string(ClangExpressionSourceCode::g_expression_prefix) + text,
- std::move(name)) {}
+ std::string(ClangExpressionSourceCode::g_expression_prefix) + text +
+ std::string(ClangExpressionSourceCode::g_expression_suffix),
+ std::move(name), enable_debugging) {
+ if (enable_debugging) {
+int temp_fd = -1;
+llvm::SmallString<128> result_path;
+llvm::sys::fs::createTemporaryFile("lldb", "expr", temp_fd, result_path);
+if (temp_fd != -1) {
+ lldb_private::NativeFile file(temp_fd, File::eOpenOptionWrite, true);
+ text = "#line 1 \"" + std::string(result_path) + "\"\n" + text;
+ size_t byte
[Lldb-commits] [PATCH] D97298: [lldb][NFC] Move trivial ValueObject getters/setters to the header
JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97298/new/ https://reviews.llvm.org/D97298 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D97287: [lldb][NFC] Rename the second ValueObjectManager to ValueObjectUpdater and remove the dead code
JDevlieghere accepted this revision. JDevlieghere added a comment. Nice CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97287/new/ https://reviews.llvm.org/D97287 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D97230: [lldb] [test] Workaround symlink-related test failures
mgorny updated this revision to Diff 325870.
mgorny retitled this revision from "[lldb] [test] Workaround symlink-related
create_after_attach test failure" to "[lldb] [test] Workaround symlink-related
test failures".
mgorny edited the summary of this revision.
mgorny added a comment.
Added a similar hack for
`tools/lldb-vscode/breakpoint/TestVSCode_setBreakpoints.py` test.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D97230/new/
https://reviews.llvm.org/D97230
Files:
lldb/test/API/functionalities/thread/create_after_attach/TestCreateAfterAttach.py
lldb/test/API/tools/lldb-vscode/breakpoint/TestVSCode_setBreakpoints.py
Index: lldb/test/API/tools/lldb-vscode/breakpoint/TestVSCode_setBreakpoints.py
===
--- lldb/test/API/tools/lldb-vscode/breakpoint/TestVSCode_setBreakpoints.py
+++ lldb/test/API/tools/lldb-vscode/breakpoint/TestVSCode_setBreakpoints.py
@@ -21,11 +21,10 @@
lldbvscode_testcase.VSCodeTestCaseBase.setUp(self)
self.main_basename = 'main-copy.cpp'
-self.main_path = self.getBuildArtifact(self.main_basename)
+self.main_path =
os.path.realpath(self.getBuildArtifact(self.main_basename))
@skipIfWindows
@skipIfRemote
-@expectedFailureAll(oslist=["freebsd"], bugnumber="llvm.org/pr48421")
def test_source_map(self):
self.build_and_create_debug_adaptor()
@@ -90,7 +89,6 @@
@skipIfWindows
@skipIfRemote
-@expectedFailureAll(oslist=["freebsd"], bugnumber="llvm.org/pr48421")
def test_set_and_clear(self):
'''Tests setting and clearing source file and line breakpoints.
This packet is a bit tricky on the debug adaptor side since there
@@ -223,7 +221,6 @@
@skipIfWindows
@skipIfRemote
-@expectedFailureAll(oslist=["freebsd"], bugnumber="llvm.org/pr48421")
def test_clear_breakpoints_unset_breakpoints(self):
'''Test clearing breakpoints like test_set_and_clear, but clear
breakpoints by omitting the breakpoints array instead of sending an
@@ -266,7 +263,6 @@
@skipIfWindows
@skipIfRemote
-@expectedFailureAll(oslist=["freebsd"], bugnumber="llvm.org/pr48421")
def test_functionality(self):
'''Tests hitting breakpoints and the functionality of a single
breakpoint, like 'conditions' and 'hitCondition' settings.'''
Index:
lldb/test/API/functionalities/thread/create_after_attach/TestCreateAfterAttach.py
===
---
lldb/test/API/functionalities/thread/create_after_attach/TestCreateAfterAttach.py
+++
lldb/test/API/functionalities/thread/create_after_attach/TestCreateAfterAttach.py
@@ -25,7 +25,6 @@
# Occasionally hangs on Windows, may be same as other issues.
@skipIfWindows
@skipIfiOSSimulator
-@expectedFailureAll(oslist=["freebsd"], bugnumber="llvm.org/pr48376")
@expectedFailureNetBSD
def test_create_after_attach(self):
"""Test thread creation after process attach."""
@@ -33,7 +32,8 @@
exe = self.getBuildArtifact("a.out")
# Spawn a new process
-popen = self.spawnSubprocess(exe)
+# use realpath to workaround llvm.org/pr48376
+popen = self.spawnSubprocess(os.path.realpath(exe))
pid = popen.pid
# Attach to the spawned process
Index: lldb/test/API/tools/lldb-vscode/breakpoint/TestVSCode_setBreakpoints.py
===
--- lldb/test/API/tools/lldb-vscode/breakpoint/TestVSCode_setBreakpoints.py
+++ lldb/test/API/tools/lldb-vscode/breakpoint/TestVSCode_setBreakpoints.py
@@ -21,11 +21,10 @@
lldbvscode_testcase.VSCodeTestCaseBase.setUp(self)
self.main_basename = 'main-copy.cpp'
-self.main_path = self.getBuildArtifact(self.main_basename)
+self.main_path = os.path.realpath(self.getBuildArtifact(self.main_basename))
@skipIfWindows
@skipIfRemote
-@expectedFailureAll(oslist=["freebsd"], bugnumber="llvm.org/pr48421")
def test_source_map(self):
self.build_and_create_debug_adaptor()
@@ -90,7 +89,6 @@
@skipIfWindows
@skipIfRemote
-@expectedFailureAll(oslist=["freebsd"], bugnumber="llvm.org/pr48421")
def test_set_and_clear(self):
'''Tests setting and clearing source file and line breakpoints.
This packet is a bit tricky on the debug adaptor side since there
@@ -223,7 +221,6 @@
@skipIfWindows
@skipIfRemote
-@expectedFailureAll(oslist=["freebsd"], bugnumber="llvm.org/pr48421")
def test_clear_breakpoints_unset_breakpoints(self):
'''Test clearing breakpoints like test_set_and_clear, but clear
breakpoints by omitting the breakpoints array instead of sending an
@@ -266,7 +263,6 @@
@skipIfWindows
@skipIfRemote
-@expectedFailureAll(oslist=["freebsd"], bugnumber="llvm.org/pr48421")
def test_func
[Lldb-commits] [PATCH] D96194: Defer the decision whether to use the CU or TU index until after reading the unit header.
jgorbe updated this revision to Diff 325884. jgorbe added a comment. Changed logic that checked the type of unit to use `header.IsTypeUnit()` as suggested by reviewer. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96194/new/ https://reviews.llvm.org/D96194 Files: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h lldb/test/Shell/SymbolFile/DWARF/dwarf5_tu_index_abbrev_offset.s Index: lldb/test/Shell/SymbolFile/DWARF/dwarf5_tu_index_abbrev_offset.s === --- /dev/null +++ lldb/test/Shell/SymbolFile/DWARF/dwarf5_tu_index_abbrev_offset.s @@ -0,0 +1,323 @@ +## This test checks that lldb uses the abbrev_offset from .debug_tu_index when +## reading TUs in the .debug_info section of a DWARF v5 DWP. +## +## The assembly here is, essentially, slightly hand-reduced output from +## `clang -gsplit-dwarf -gdwarf-5 -fdebug-types-section`, with a manually-added +## .debug_cu_index and a .debug_tu_index to create a DWP, and a twist: abbrevs +## from the TU are listed *AFTER* abbrevs from the CU so that they don't begin +## at offset 0. + +# RUN: llvm-mc --filetype=obj --triple x86_64 %s -o %t --defsym MAIN=1 +# RUN: llvm-mc --filetype=obj --triple x86_64 %s -o %t.dwp +# RUN: %lldb %t -o "image lookup -t t1" -b | FileCheck %s +# CHECK: struct t1 + +.ifdef MAIN +## Main file + .text + .globl main# -- Begin function main +main: # @main +.Lfunc_begin0: + pushq %rbp + movq %rsp, %rbp + xorl %eax, %eax + popq %rbp + retq +.Lfunc_end0: +# -- End function + .section .debug_abbrev,"",@progbits + .byte 1 # Abbreviation Code + .byte 74 # DW_TAG_skeleton_unit + .byte 0 # DW_CHILDREN_no + .byte 118 # DW_AT_dwo_name + .byte 8 # DW_FORM_string + .byte 17 # DW_AT_low_pc + .byte 27 # DW_FORM_addrx + .byte 18 # DW_AT_high_pc + .byte 6 # DW_FORM_data4 + .byte 115 # DW_AT_addr_base + .byte 23 # DW_FORM_sec_offset + .byte 0 # EOM(1) + .byte 0 # EOM(2) + .byte 0 # EOM(3) + + + .section .debug_info,"",@progbits +.Lcu_begin0: + .long .Ldebug_info_end0-.Ldebug_info_start0 # Length of Unit +.Ldebug_info_start0: + .short 5 # DWARF version number + .byte 4 # DWARF Unit Type + .byte 8 # Address Size (in bytes) + .long .debug_abbrev # Offset Into Abbrev. Section + .quad -8218585293556409984# dwo_id + .byte 1 # Abbrev [1] 0x14:0x14 DW_TAG_skeleton_unit + .asciz "hello.dwo"# DW_AT_dwo_name + .byte 0 # DW_AT_low_pc + .long .Lfunc_end0-.Lfunc_begin0 # DW_AT_high_pc + .long .Laddr_table_base0 # DW_AT_addr_base +.Ldebug_info_end0: + + + .section .debug_addr,"",@progbits + .long .Ldebug_addr_end0-.Ldebug_addr_start0 # Length of contribution +.Ldebug_addr_start0: + .short 5 # DWARF version number + .byte 8 # Address size + .byte 0 # Segment selector size +.Laddr_table_base0: + .quad .Lfunc_begin0 +.Ldebug_addr_end0: + + +.else +## DWP file + .section .debug_info.dwo,"e",@progbits + .long .Ldebug_info_dwo_end0-.Ldebug_info_dwo_start0 # Length of Unit +.Ldebug_info_dwo_start0: + .short 5 # DWARF version number + .byte 6 # DWARF Unit Type + .byte 8 # Address Size (in bytes) + .long 0 # Offset Into Abbrev. Section + .quad -4149699470930386446# Type Signature + .long 31 # Type DIE Offset + .byte 1 # Abbrev [1] 0x18:0xe DW_TAG_type_unit + .short 33 # DW_AT_language + .long 0 # DW_AT_stmt_list + .byte 2 # Abbrev [2] 0x1f:0x6 DW_TAG_structure_type + .byte 5 # DW_AT_calling_convention + .byte 3 # DW_AT_name + .byte 1 # DW_AT_byte_size + .byte 0 # DW_AT_decl_file + .byte 1 # DW_AT_decl_line + .byte 0 # End Of Children Mark +.Ldebug_info_dwo_end0: + .long .Ldebug_info_dwo_end1-.Ldebug_info_dwo_start1 # Length of Unit +.Ldebug_in
[Lldb-commits] [PATCH] D96194: Defer the decision whether to use the CU or TU index until after reading the unit header.
jgorbe added a comment.
I'm going to go ahead and commit this given that I have just made the last
suggested modification and the patch has already been up for review without
further comments for a long while (sorry for the late replies, work keeps
getting in the way of work).
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:808-809
+ if (cu_index && (header.m_unit_type == llvm::dwarf::DW_UT_compile ||
+ header.m_unit_type == llvm::dwarf::DW_UT_split_compile)) {
+header.m_index_entry = cu_index->getFromOffset(header.m_offset);
labath wrote:
> jgorbe wrote:
> > labath wrote:
> > > I guess this could be `header.IsTypeUnit()` (and
> > > `!header.IsTypeUnit())`)...
> > But `!header.IsTypeUnit` would also treat DW_UT_partial and DW_UT_skeleton
> > as compile units, right?
> That's true, but can either of those units legitimately appear in a dwp file?
> Even if they do appear for whatever reason, it wouldn't make any sense to use
> them without an index entry, and it would be (somewhat) more reasonable to
> put them in the cu index.
>
> I took this idea from the equivalent llvm code:
>
Good point. Changed.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D96194/new/
https://reviews.llvm.org/D96194
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 979ca1c - Defer the decision whether to use the CU or TU index until after reading the unit header.
Author: Jorge Gorbe Moya
Date: 2021-02-23T13:26:11-08:00
New Revision: 979ca1c05f83114483caec3e6d1b75daae86da79
URL:
https://github.com/llvm/llvm-project/commit/979ca1c05f83114483caec3e6d1b75daae86da79
DIFF:
https://github.com/llvm/llvm-project/commit/979ca1c05f83114483caec3e6d1b75daae86da79.diff
LOG: Defer the decision whether to use the CU or TU index until after reading
the unit header.
In DWARF v4 compile units go in .debug_info and type units go in
.debug_types. However, in v5 both kinds of units are in .debug_info.
Therefore we can't decide whether to use the CU or TU index just by
looking at which section we're reading from. We have to wait until we
have read the unit type from the header.
Differential Revision: https://reviews.llvm.org/D96194
Added:
lldb/test/Shell/SymbolFile/DWARF/dwarf5_tu_index_abbrev_offset.s
Modified:
lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
Removed:
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
index 8d393b295443..5f0af57da9e4 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
@@ -72,16 +72,10 @@ void DWARFDebugInfo::ParseUnitsFor(DIERef::Section section)
{
DWARFDataExtractor data = section == DIERef::Section::DebugTypes
? m_context.getOrLoadDebugTypesData()
: m_context.getOrLoadDebugInfoData();
- const llvm::DWARFUnitIndex *index = nullptr;
- if (m_context.isDwo())
-index = &llvm::getDWARFUnitIndex(m_context.GetAsLLVM(),
- section == DIERef::Section::DebugTypes
- ? llvm::DW_SECT_EXT_TYPES
- : llvm::DW_SECT_INFO);
lldb::offset_t offset = 0;
while (data.ValidOffset(offset)) {
llvm::Expected unit_sp = DWARFUnit::extract(
-m_dwarf, m_units.size(), data, section, &offset, index);
+m_dwarf, m_units.size(), data, section, &offset);
if (!unit_sp) {
// FIXME: Propagate this error up.
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
index a761dd3daac4..bb60ab9878c8 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -784,12 +784,11 @@ const DWARFDebugAranges &DWARFUnit::GetFunctionAranges() {
llvm::Expected
DWARFUnitHeader::extract(const DWARFDataExtractor &data,
- DIERef::Section section, lldb::offset_t *offset_ptr,
- const llvm::DWARFUnitIndex *index) {
+ DIERef::Section section,
+ lldb_private::DWARFContext &context,
+ lldb::offset_t *offset_ptr) {
DWARFUnitHeader header;
header.m_offset = *offset_ptr;
- if (index)
-header.m_index_entry = index->getFromOffset(*offset_ptr);
header.m_length = data.GetDWARFInitialLength(offset_ptr);
header.m_version = data.GetU16(offset_ptr);
if (header.m_version == 5) {
@@ -806,6 +805,16 @@ DWARFUnitHeader::extract(const DWARFDataExtractor &data,
section == DIERef::Section::DebugTypes ? DW_UT_type : DW_UT_compile;
}
+ if (context.isDwo()) {
+if (header.IsTypeUnit()) {
+ header.m_index_entry =
+ context.GetAsLLVM().getTUIndex().getFromOffset(header.m_offset);
+} else {
+ header.m_index_entry =
+ context.GetAsLLVM().getCUIndex().getFromOffset(header.m_offset);
+}
+ }
+
if (header.m_index_entry) {
if (header.m_abbr_offset) {
return llvm::createStringError(
@@ -856,12 +865,11 @@ DWARFUnitHeader::extract(const DWARFDataExtractor &data,
llvm::Expected
DWARFUnit::extract(SymbolFileDWARF &dwarf, user_id_t uid,
const DWARFDataExtractor &debug_info,
- DIERef::Section section, lldb::offset_t *offset_ptr,
- const llvm::DWARFUnitIndex *index) {
+ DIERef::Section section, lldb::offset_t *offset_ptr) {
assert(debug_info.ValidOffset(*offset_ptr));
- auto expected_header =
- DWARFUnitHeader::extract(debug_info, section, offset_ptr, index);
+ auto expected_header = DWARFUnitHeader::extract(
+ debug_info, section, dwarf.GetDWARFContext(), offset_ptr);
if (!expected_header)
return expected_header.takeError();
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
index 5739c36bbacb..0fc8ceae81d2 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
@@ -73,7 +73,8 @@ class DWARFUn
[Lldb-commits] [PATCH] D96194: Defer the decision whether to use the CU or TU index until after reading the unit header.
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG979ca1c05f83: Defer the decision whether to use the CU or TU index until after reading the… (authored by jgorbe). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96194/new/ https://reviews.llvm.org/D96194 Files: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h lldb/test/Shell/SymbolFile/DWARF/dwarf5_tu_index_abbrev_offset.s Index: lldb/test/Shell/SymbolFile/DWARF/dwarf5_tu_index_abbrev_offset.s === --- /dev/null +++ lldb/test/Shell/SymbolFile/DWARF/dwarf5_tu_index_abbrev_offset.s @@ -0,0 +1,323 @@ +## This test checks that lldb uses the abbrev_offset from .debug_tu_index when +## reading TUs in the .debug_info section of a DWARF v5 DWP. +## +## The assembly here is, essentially, slightly hand-reduced output from +## `clang -gsplit-dwarf -gdwarf-5 -fdebug-types-section`, with a manually-added +## .debug_cu_index and a .debug_tu_index to create a DWP, and a twist: abbrevs +## from the TU are listed *AFTER* abbrevs from the CU so that they don't begin +## at offset 0. + +# RUN: llvm-mc --filetype=obj --triple x86_64 %s -o %t --defsym MAIN=1 +# RUN: llvm-mc --filetype=obj --triple x86_64 %s -o %t.dwp +# RUN: %lldb %t -o "image lookup -t t1" -b | FileCheck %s +# CHECK: struct t1 + +.ifdef MAIN +## Main file + .text + .globl main# -- Begin function main +main: # @main +.Lfunc_begin0: + pushq %rbp + movq %rsp, %rbp + xorl %eax, %eax + popq %rbp + retq +.Lfunc_end0: +# -- End function + .section .debug_abbrev,"",@progbits + .byte 1 # Abbreviation Code + .byte 74 # DW_TAG_skeleton_unit + .byte 0 # DW_CHILDREN_no + .byte 118 # DW_AT_dwo_name + .byte 8 # DW_FORM_string + .byte 17 # DW_AT_low_pc + .byte 27 # DW_FORM_addrx + .byte 18 # DW_AT_high_pc + .byte 6 # DW_FORM_data4 + .byte 115 # DW_AT_addr_base + .byte 23 # DW_FORM_sec_offset + .byte 0 # EOM(1) + .byte 0 # EOM(2) + .byte 0 # EOM(3) + + + .section .debug_info,"",@progbits +.Lcu_begin0: + .long .Ldebug_info_end0-.Ldebug_info_start0 # Length of Unit +.Ldebug_info_start0: + .short 5 # DWARF version number + .byte 4 # DWARF Unit Type + .byte 8 # Address Size (in bytes) + .long .debug_abbrev # Offset Into Abbrev. Section + .quad -8218585293556409984# dwo_id + .byte 1 # Abbrev [1] 0x14:0x14 DW_TAG_skeleton_unit + .asciz "hello.dwo"# DW_AT_dwo_name + .byte 0 # DW_AT_low_pc + .long .Lfunc_end0-.Lfunc_begin0 # DW_AT_high_pc + .long .Laddr_table_base0 # DW_AT_addr_base +.Ldebug_info_end0: + + + .section .debug_addr,"",@progbits + .long .Ldebug_addr_end0-.Ldebug_addr_start0 # Length of contribution +.Ldebug_addr_start0: + .short 5 # DWARF version number + .byte 8 # Address size + .byte 0 # Segment selector size +.Laddr_table_base0: + .quad .Lfunc_begin0 +.Ldebug_addr_end0: + + +.else +## DWP file + .section .debug_info.dwo,"e",@progbits + .long .Ldebug_info_dwo_end0-.Ldebug_info_dwo_start0 # Length of Unit +.Ldebug_info_dwo_start0: + .short 5 # DWARF version number + .byte 6 # DWARF Unit Type + .byte 8 # Address Size (in bytes) + .long 0 # Offset Into Abbrev. Section + .quad -4149699470930386446# Type Signature + .long 31 # Type DIE Offset + .byte 1 # Abbrev [1] 0x18:0xe DW_TAG_type_unit + .short 33 # DW_AT_language + .long 0 # DW_AT_stmt_list + .byte 2 # Abbrev [2] 0x1f:0x6 DW_TAG_structure_type + .byte 5 # DW_AT_calling_convention + .byte 3 # DW_AT_name + .byte 1 # DW_AT_byte_size + .byte 0 # DW_AT_decl_file + .byte 1 # DW_AT_decl_line + .byte 0
