[Lldb-commits] [PATCH] D127605: [lldb] Support non-pointer implicit this/self in GetValueForVariableExpressionPath
kastiglione created this revision.
kastiglione added reviewers: aprantl, jingham.
Herald added a project: All.
kastiglione requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
The `frame variable` command supports an implicit `this`/`self`, allowing a
user to run `v some_field` instead of `v this->some_field`. However, some
languages have non-pointer `this`/`self` types (for example, Swift).
With this change, support for non-pointer implicit `this`/`self` is supported.
This is done by consulting the type of the instance variable. If the type is
known to be non-pointer, then the dot operator `.` is used instead of the arrow
operator `->`.
The C language of families each have a pointer instance type, which makes
testing of this difficult. Tests for this feature will be done in the Swift
downstream fork, as Swift's `self` is a non-pointer (reference) type.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D127605
Files:
lldb/source/Target/StackFrame.cpp
Index: lldb/source/Target/StackFrame.cpp
===
--- lldb/source/Target/StackFrame.cpp
+++ lldb/source/Target/StackFrame.cpp
@@ -563,7 +563,12 @@
var_sp = variable_list->FindVariable(method_object_name);
if (var_sp) {
separator_idx = 0;
- var_expr_storage = "->";
+ if (auto var_type = var_sp->GetType())
+if (!var_type->GetForwardCompilerType().IsPointerType())
+ var_expr_storage = ".";
+
+ if (var_expr_storage.empty())
+var_expr_storage = "->";
var_expr_storage += var_expr;
var_expr = var_expr_storage;
synthetically_added_instance_object = true;
Index: lldb/source/Target/StackFrame.cpp
===
--- lldb/source/Target/StackFrame.cpp
+++ lldb/source/Target/StackFrame.cpp
@@ -563,7 +563,12 @@
var_sp = variable_list->FindVariable(method_object_name);
if (var_sp) {
separator_idx = 0;
- var_expr_storage = "->";
+ if (auto var_type = var_sp->GetType())
+if (!var_type->GetForwardCompilerType().IsPointerType())
+ var_expr_storage = ".";
+
+ if (var_expr_storage.empty())
+var_expr_storage = "->";
var_expr_storage += var_expr;
var_expr = var_expr_storage;
synthetically_added_instance_object = true;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D127605: [lldb] Support non-pointer implicit this/self in GetValueForVariableExpressionPath
kastiglione updated this revision to Diff 436243.
kastiglione added a comment.
Herald added a subscriber: JDevlieghere.
use explicit type instead of auto
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D127605/new/
https://reviews.llvm.org/D127605
Files:
lldb/source/Target/StackFrame.cpp
Index: lldb/source/Target/StackFrame.cpp
===
--- lldb/source/Target/StackFrame.cpp
+++ lldb/source/Target/StackFrame.cpp
@@ -563,7 +563,12 @@
var_sp = variable_list->FindVariable(method_object_name);
if (var_sp) {
separator_idx = 0;
- var_expr_storage = "->";
+ if (Type *var_type = var_sp->GetType())
+if (!var_type->GetForwardCompilerType().IsPointerType())
+ var_expr_storage = ".";
+
+ if (var_expr_storage.empty())
+var_expr_storage = "->";
var_expr_storage += var_expr;
var_expr = var_expr_storage;
synthetically_added_instance_object = true;
Index: lldb/source/Target/StackFrame.cpp
===
--- lldb/source/Target/StackFrame.cpp
+++ lldb/source/Target/StackFrame.cpp
@@ -563,7 +563,12 @@
var_sp = variable_list->FindVariable(method_object_name);
if (var_sp) {
separator_idx = 0;
- var_expr_storage = "->";
+ if (Type *var_type = var_sp->GetType())
+if (!var_type->GetForwardCompilerType().IsPointerType())
+ var_expr_storage = ".";
+
+ if (var_expr_storage.empty())
+var_expr_storage = "->";
var_expr_storage += var_expr;
var_expr = var_expr_storage;
synthetically_added_instance_object = true;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D126394: [trace][intelpt] Support system-wide tracing [14] - Decode per cpu
jj10306 requested changes to this revision.
jj10306 added inline comments.
This revision now requires changes to proceed.
Comment at: lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h:83-85
+ uint64_t ToNanos(uint64_t tsc) const;
+ uint64_t ToTSC(uint64_t nanos) const;
why get rid of chronos here?
Comment at: lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp:22-38
struct TscInfo {
uint64_t tsc = 0;
LazyBool has_tsc = eLazyBoolCalculate;
explicit operator bool() const { return has_tsc == eLazyBoolYes; }
};
nit: Is there a reason these classes/structs are not declared in the header
file?
Comment at: lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp:100-102
+ /// If \b true, decoding
+ /// An optional offset to a given PSB. Decoding stops if a different PSB is
+ /// reached.
is this what you intended to say here?
Comment at: lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp:353
+auto variant = execution.thread_execution.variant;
+// If we haven't seen a PSB yet, then it's fine not to show errors
+if (has_seen_psbs) {
Why is this the case?
Comment at: lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.h:21-24
+struct IntelPTThreadSubtrace {
+ uint64_t tsc;
+ uint64_t psb_offset;
+};
docs
Comment at: lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.h:47-53
+void DecodeTrace(
+DecodedThread &decoded_thread, TraceIntelPT &trace_intel_pt,
+const llvm::DenseMap> &buffers,
+const std::vector &executions);
+
+llvm::Expected>
+SplitTraceInContinuousExecutions(TraceIntelPT &trace_intel_pt,
docs
Comment at:
lldb/source/Plugins/Trace/intel-pt/PerfContextSwitchDecoder.cpp:15-81
+/// Copied from to avoid depending on perf_event.h on
+/// non-linux platforms.
+/// \{
+struct perf_event_header {
+ uint32_t type;
+ uint16_t misc;
+ uint16_t size;
why not put these declarations in the header file?
Comment at: lldb/source/Plugins/Trace/intel-pt/PerfContextSwitchDecoder.cpp:227
+
+#include
+
what's this doing here?
Comment at: lldb/source/Plugins/Trace/intel-pt/PerfContextSwitchDecoder.cpp:239
+
+ auto do_decode = [&]() -> Error {
+Optional prev_record;
does this need to be a lambda? iiuc this is only called once (at the end of
this function), so it seems like this could just be placed inline instead of
being in a lambda
Comment at: lldb/source/Plugins/Trace/intel-pt/PerfContextSwitchDecoder.h:1
+//===-- PerfContextSwitchDecoder.h --==--*- C++
-*-===//
+//
do the structures/logic contained in this file (and its .cpp) belong with the
other Perf logic of LLDB or should does this belong with the intelpt specific
code?
Comment at: lldb/source/Plugins/Trace/intel-pt/PerfContextSwitchDecoder.h:124
+
+/// Decodes a context switch trace gotten with perf_event_open.
+///
Comment at:
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTMultiCoreDecoder.cpp:43
+ m_cores, IntelPTDataKinds::kTraceBuffer,
+ [&](const DenseMap> buffers) -> Error {
+auto it = m_continuous_executions_per_thread->find(thread.GetID());
Comment at:
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTMultiCoreDecoder.cpp:64
+ for (core_id_t core_id : m_cores) {
+std::vector intel_pt_executions;
+
I think naming this subtraces would help as it makes it makes the distinction
between the subtraces and thread executions more clear while reading the code
below.
Comment at:
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTMultiCoreDecoder.cpp:84
+auto on_new_thread_execution =
+[&](ThreadContinuousExecution thread_execution) {
+ IntelPTThreadContinousExecution execution(thread_execution);
Comment at:
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTMultiCoreDecoder.cpp:93
+} else {
+ m_unattributed_intelpt_subtraces++;
+}
perhaps I'm misunderstanding the intention of this counter, but won't this be
incremented for all subtraces that don't belong to **the specific
ThreadContinousExecution currently being operated** on instead of being
incremented for the subtraces that don't belong to **any
ThreadContinousExecution**?
Comment at:
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTMultiCoreDecoder.cpp:121
Error TraceIntelPTMultiCoreDecoder::DecodeContextSwitchTraces() {
if (m_setup_error)
Should this be named differently since this is doing much more than decoding
the context switches (it splits the intel pt traces and c
[Lldb-commits] [PATCH] D127605: [lldb] Support non-pointer implicit this/self in GetValueForVariableExpressionPath
kastiglione updated this revision to Diff 436260.
kastiglione added a comment.
check valid compiler type
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D127605/new/
https://reviews.llvm.org/D127605
Files:
lldb/source/Target/StackFrame.cpp
Index: lldb/source/Target/StackFrame.cpp
===
--- lldb/source/Target/StackFrame.cpp
+++ lldb/source/Target/StackFrame.cpp
@@ -552,7 +552,7 @@
if (!var_sp && (options & eExpressionPathOptionsAllowDirectIVarAccess)) {
// Check for direct ivars access which helps us with implicit access to
-// ivars with the "this->" or "self->"
+// ivars using "this" or "self".
GetSymbolContext(eSymbolContextFunction | eSymbolContextBlock);
lldb::LanguageType method_language = eLanguageTypeUnknown;
bool is_instance_method = false;
@@ -563,7 +563,13 @@
var_sp = variable_list->FindVariable(method_object_name);
if (var_sp) {
separator_idx = 0;
- var_expr_storage = "->";
+ if (Type *var_type = var_sp->GetType())
+if (auto compiler_type = var_type->GetForwardCompilerType())
+ if (!compiler_type.IsPointerType())
+var_expr_storage = ".";
+
+ if (var_expr_storage.empty())
+var_expr_storage = "->";
var_expr_storage += var_expr;
var_expr = var_expr_storage;
synthetically_added_instance_object = true;
Index: lldb/source/Target/StackFrame.cpp
===
--- lldb/source/Target/StackFrame.cpp
+++ lldb/source/Target/StackFrame.cpp
@@ -552,7 +552,7 @@
if (!var_sp && (options & eExpressionPathOptionsAllowDirectIVarAccess)) {
// Check for direct ivars access which helps us with implicit access to
-// ivars with the "this->" or "self->"
+// ivars using "this" or "self".
GetSymbolContext(eSymbolContextFunction | eSymbolContextBlock);
lldb::LanguageType method_language = eLanguageTypeUnknown;
bool is_instance_method = false;
@@ -563,7 +563,13 @@
var_sp = variable_list->FindVariable(method_object_name);
if (var_sp) {
separator_idx = 0;
- var_expr_storage = "->";
+ if (Type *var_type = var_sp->GetType())
+if (auto compiler_type = var_type->GetForwardCompilerType())
+ if (!compiler_type.IsPointerType())
+var_expr_storage = ".";
+
+ if (var_expr_storage.empty())
+var_expr_storage = "->";
var_expr_storage += var_expr;
var_expr = var_expr_storage;
synthetically_added_instance_object = true;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
