-
Notifications
You must be signed in to change notification settings - Fork 14.9k
[lldb] Mark scripted frames as synthetic instead of artificial #153117
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
e20e532
to
b8fc0de
Compare
@llvm/pr-subscribers-lldb Author: Med Ismail Bennani (medismailben) ChangesThis patch changes the way frames created from scripted affordances like Scripted Threads are displayed. Currently, they're marked artificial which is used usually for compiler generated frames. This patch changes that behaviour by introducing a new synthetic StackFrame kind and moves 'artificial' to be a distinct StackFrame attribut. On top of making these frames less confusing, this allows us to know when a frame was created from a scripted affordance. rdar://155949703 Patch is 33.65 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/153117.diff 17 Files Affected:
diff --git a/lldb/include/lldb/API/SBFrame.h b/lldb/include/lldb/API/SBFrame.h
index 08de0605b3240..e4bbcd5ddcd9c 100644
--- a/lldb/include/lldb/API/SBFrame.h
+++ b/lldb/include/lldb/API/SBFrame.h
@@ -104,6 +104,8 @@ class LLDB_API SBFrame {
bool IsArtificial() const;
+ bool IsSynthetic() const;
+
/// Return whether a frame recognizer decided this frame should not
/// be displayes in backtraces etc.
bool IsHidden() const;
diff --git a/lldb/include/lldb/Core/FormatEntity.h b/lldb/include/lldb/Core/FormatEntity.h
index d602edffb4c88..4da3cc5a596b2 100644
--- a/lldb/include/lldb/Core/FormatEntity.h
+++ b/lldb/include/lldb/Core/FormatEntity.h
@@ -80,6 +80,7 @@ struct Entry {
FrameRegisterFlags,
FrameRegisterByName,
FrameIsArtificial,
+ FrameIsSynthetic,
ScriptFrame,
FunctionID,
FunctionDidChange,
diff --git a/lldb/include/lldb/Target/StackFrame.h b/lldb/include/lldb/Target/StackFrame.h
index 3f51c9a7f22f0..be0940f1f8d18 100644
--- a/lldb/include/lldb/Target/StackFrame.h
+++ b/lldb/include/lldb/Target/StackFrame.h
@@ -63,7 +63,7 @@ class StackFrame : public ExecutionContextScope,
/// An artificial stack frame (e.g. a synthesized result of inferring
/// missing tail call frames from a backtrace) with limited support for
/// local variables.
- Artificial
+ Synthetic
};
/// Construct a StackFrame object without supplying a RegisterContextSP.
@@ -109,7 +109,8 @@ class StackFrame : public ExecutionContextScope,
StackFrame(const lldb::ThreadSP &thread_sp, lldb::user_id_t frame_idx,
lldb::user_id_t concrete_frame_idx, lldb::addr_t cfa,
bool cfa_is_valid, lldb::addr_t pc, Kind frame_kind,
- bool behaves_like_zeroth_frame, const SymbolContext *sc_ptr);
+ bool artificial, bool behaves_like_zeroth_frame,
+ const SymbolContext *sc_ptr);
StackFrame(const lldb::ThreadSP &thread_sp, lldb::user_id_t frame_idx,
lldb::user_id_t concrete_frame_idx,
@@ -396,6 +397,9 @@ class StackFrame : public ExecutionContextScope,
/// true if this is an inlined frame.
bool IsInlined();
+ /// Query whether this frame is synthetic.
+ bool IsSynthetic() const;
+
/// Query whether this frame is part of a historical backtrace.
bool IsHistorical() const;
@@ -567,6 +571,10 @@ class StackFrame : public ExecutionContextScope,
/// Does this frame have a CFA? Different from CFA == LLDB_INVALID_ADDRESS.
bool m_cfa_is_valid;
Kind m_stack_frame_kind;
+ /// Is this an artificial stack frame (e.g. a synthesized result of inferring
+ /// missing tail call frames from a backtrace) with limited support for
+ /// local variables.
+ bool m_artificial;
/// Whether this frame behaves like the zeroth frame, in the sense
/// that its pc value might not immediately follow a call (and thus might
diff --git a/lldb/source/API/SBFrame.cpp b/lldb/source/API/SBFrame.cpp
index b12cfceacd75d..b6724bb0c4119 100644
--- a/lldb/source/API/SBFrame.cpp
+++ b/lldb/source/API/SBFrame.cpp
@@ -1118,6 +1118,22 @@ bool SBFrame::IsArtificial() const {
return false;
}
+bool SBFrame::IsSynthetic() const {
+ LLDB_INSTRUMENT_VA(this);
+
+ llvm::Expected<StoppedExecutionContext> exe_ctx =
+ GetStoppedExecutionContext(m_opaque_sp);
+ if (!exe_ctx) {
+ LLDB_LOG_ERROR(GetLog(LLDBLog::API), exe_ctx.takeError(), "{0}");
+ return false;
+ }
+
+ if (StackFrame *frame = exe_ctx->GetFramePtr())
+ return frame->IsSynthetic();
+
+ return false;
+}
+
bool SBFrame::IsHidden() const {
LLDB_INSTRUMENT_VA(this);
diff --git a/lldb/source/Core/CoreProperties.td b/lldb/source/Core/CoreProperties.td
index 53dd333f045c9..4ac49b70c9877 100644
--- a/lldb/source/Core/CoreProperties.td
+++ b/lldb/source/Core/CoreProperties.td
@@ -59,7 +59,7 @@ let Definition = "debugger" in {
Desc<"The default disassembly format string to use when disassembling instruction sequences.">;
def FrameFormat: Property<"frame-format", "FormatEntity">,
Global,
- DefaultStringValue<"frame #${frame.index}: ${ansi.fg.cyan}${frame.pc}${ansi.normal}{ ${module.file.basename}{`${function.name-with-args}{${frame.no-debug}${function.pc-offset}}}}{ at ${ansi.fg.cyan}${line.file.basename}${ansi.normal}:${ansi.fg.yellow}${line.number}${ansi.normal}{:${ansi.fg.yellow}${line.column}${ansi.normal}}}{${function.is-optimized} [opt]}{${function.is-inlined} [inlined]}{${frame.is-artificial} [artificial]}\\\\n">,
+ DefaultStringValue<"frame #${frame.index}: ${ansi.fg.cyan}${frame.pc}${ansi.normal}{ ${module.file.basename}{`${function.name-with-args}{${frame.no-debug}${function.pc-offset}}}}{ at ${ansi.fg.cyan}${line.file.basename}${ansi.normal}:${ansi.fg.yellow}${line.number}${ansi.normal}{:${ansi.fg.yellow}${line.column}${ansi.normal}}}{${function.is-optimized} [opt]}{${function.is-inlined} [inlined]}{${frame.is-artificial} [artificial]}{${frame.is-synthetic} [synthetic]}\\\\n">,
Desc<"The default frame format string to use when displaying stack frame information for threads.">;
def NotiftVoid: Property<"notify-void", "Boolean">,
Global,
@@ -233,7 +233,7 @@ let Definition = "debugger" in {
Desc<"If true, LLDB will automatically escape non-printable and escape characters when formatting strings.">;
def FrameFormatUnique: Property<"frame-format-unique", "FormatEntity">,
Global,
- DefaultStringValue<"frame #${frame.index}: ${ansi.fg.cyan}${frame.pc}${ansi.normal}{ ${module.file.basename}{`${function.name-without-args}{${frame.no-debug}${function.pc-offset}}}}{ at ${ansi.fg.cyan}${line.file.basename}${ansi.normal}:${ansi.fg.yellow}${line.number}${ansi.normal}{:${ansi.fg.yellow}${line.column}${ansi.normal}}}{${function.is-optimized} [opt]}{${function.is-inlined} [inlined]}{${frame.is-artificial} [artificial]}\\\\n">,
+ DefaultStringValue<"frame #${frame.index}: ${ansi.fg.cyan}${frame.pc}${ansi.normal}{ ${module.file.basename}{`${function.name-without-args}{${frame.no-debug}${function.pc-offset}}}}{ at ${ansi.fg.cyan}${line.file.basename}${ansi.normal}:${ansi.fg.yellow}${line.number}${ansi.normal}{:${ansi.fg.yellow}${line.column}${ansi.normal}}}{${function.is-optimized} [opt]}{${function.is-inlined} [inlined]}{${frame.is-artificial} [artificial]}{${frame.is-synthetic} [synthetic]}\\\\n">,
Desc<"The default frame format string to use when displaying stack frame information for threads from thread backtrace unique.">;
def ShowAutosuggestion: Property<"show-autosuggestion", "Boolean">,
Global,
diff --git a/lldb/source/Core/FormatEntity.cpp b/lldb/source/Core/FormatEntity.cpp
index 5d3c8b421d5d1..095250a90ae2c 100644
--- a/lldb/source/Core/FormatEntity.cpp
+++ b/lldb/source/Core/FormatEntity.cpp
@@ -108,6 +108,7 @@ constexpr Definition g_frame_child_entries[] = {
Entry::DefinitionWithChildren("reg", EntryType::FrameRegisterByName,
g_string_entry),
Definition("is-artificial", EntryType::FrameIsArtificial),
+ Definition("is-synthetic", EntryType::FrameIsSynthetic),
};
constexpr Definition g_function_child_entries[] = {
@@ -380,6 +381,7 @@ const char *FormatEntity::Entry::TypeToCString(Type t) {
ENUM_TO_CSTR(FrameRegisterFlags);
ENUM_TO_CSTR(FrameRegisterByName);
ENUM_TO_CSTR(FrameIsArtificial);
+ ENUM_TO_CSTR(FrameIsSynthetic);
ENUM_TO_CSTR(ScriptFrame);
ENUM_TO_CSTR(FunctionID);
ENUM_TO_CSTR(FunctionDidChange);
@@ -1747,6 +1749,13 @@ bool FormatEntity::Format(const Entry &entry, Stream &s,
return false;
}
+ case Entry::Type::FrameIsSynthetic: {
+ if (exe_ctx)
+ if (StackFrame *frame = exe_ctx->GetFramePtr())
+ return frame->IsSynthetic();
+ return false;
+ }
+
case Entry::Type::ScriptFrame:
if (exe_ctx) {
StackFrame *frame = exe_ctx->GetFramePtr();
diff --git a/lldb/source/Plugins/Process/scripted/ScriptedThread.cpp b/lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
index d0d1508e85172..45ad83e4ed671 100644
--- a/lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
+++ b/lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
@@ -200,13 +200,15 @@ bool ScriptedThread::LoadArtificialStackFrames() {
lldb::addr_t cfa = LLDB_INVALID_ADDRESS;
bool cfa_is_valid = false;
+ const bool artificial = false;
const bool behaves_like_zeroth_frame = false;
SymbolContext sc;
symbol_addr.CalculateSymbolContext(&sc);
StackFrameSP synth_frame_sp = std::make_shared<StackFrame>(
this->shared_from_this(), idx, idx, cfa, cfa_is_valid, pc,
- StackFrame::Kind::Artificial, behaves_like_zeroth_frame, &sc);
+ StackFrame::Kind::Synthetic, artificial, behaves_like_zeroth_frame,
+ &sc);
if (!frames->SetFrameAtIndex(static_cast<uint32_t>(idx), synth_frame_sp))
return ScriptedInterface::ErrorWithMessage<bool>(
diff --git a/lldb/source/Target/StackFrame.cpp b/lldb/source/Target/StackFrame.cpp
index 40049d40aa413..8e950b748f85e 100644
--- a/lldb/source/Target/StackFrame.cpp
+++ b/lldb/source/Target/StackFrame.cpp
@@ -57,14 +57,14 @@ using namespace lldb_private;
StackFrame::StackFrame(const ThreadSP &thread_sp, user_id_t frame_idx,
user_id_t unwind_frame_index, addr_t cfa,
bool cfa_is_valid, addr_t pc, StackFrame::Kind kind,
- bool behaves_like_zeroth_frame,
+ bool artificial, bool behaves_like_zeroth_frame,
const SymbolContext *sc_ptr)
: m_thread_wp(thread_sp), m_frame_index(frame_idx),
m_concrete_frame_index(unwind_frame_index), m_reg_context_sp(),
m_id(pc, cfa, nullptr, thread_sp->GetProcess().get()),
m_frame_code_addr(pc), m_sc(), m_flags(), m_frame_base(),
m_frame_base_error(), m_cfa_is_valid(cfa_is_valid),
- m_stack_frame_kind(kind),
+ m_stack_frame_kind(kind), m_artificial(artificial),
m_behaves_like_zeroth_frame(behaves_like_zeroth_frame),
m_variable_list_sp(), m_variable_list_value_objects(),
m_recognized_frame_sp(), m_disassembly(), m_mutex() {
@@ -92,7 +92,7 @@ StackFrame::StackFrame(const ThreadSP &thread_sp, user_id_t frame_idx,
m_id(pc, cfa, nullptr, thread_sp->GetProcess().get()),
m_frame_code_addr(pc), m_sc(), m_flags(), m_frame_base(),
m_frame_base_error(), m_cfa_is_valid(true),
- m_stack_frame_kind(StackFrame::Kind::Regular),
+ m_stack_frame_kind(StackFrame::Kind::Regular), m_artificial(false),
m_behaves_like_zeroth_frame(behaves_like_zeroth_frame),
m_variable_list_sp(), m_variable_list_value_objects(),
m_recognized_frame_sp(), m_disassembly(), m_mutex() {
@@ -120,7 +120,7 @@ StackFrame::StackFrame(const ThreadSP &thread_sp, user_id_t frame_idx,
nullptr, thread_sp->GetProcess().get()),
m_frame_code_addr(pc_addr), m_sc(), m_flags(), m_frame_base(),
m_frame_base_error(), m_cfa_is_valid(true),
- m_stack_frame_kind(StackFrame::Kind::Regular),
+ m_stack_frame_kind(StackFrame::Kind::Regular), m_artificial(false),
m_behaves_like_zeroth_frame(behaves_like_zeroth_frame),
m_variable_list_sp(), m_variable_list_value_objects(),
m_recognized_frame_sp(), m_disassembly(), m_mutex() {
@@ -1261,10 +1261,12 @@ bool StackFrame::IsHistorical() const {
return m_stack_frame_kind == StackFrame::Kind::History;
}
-bool StackFrame::IsArtificial() const {
- return m_stack_frame_kind == StackFrame::Kind::Artificial;
+bool StackFrame::IsSynthetic() const {
+ return m_stack_frame_kind == StackFrame::Kind::Synthetic;
}
+bool StackFrame::IsArtificial() const { return m_artificial; }
+
bool StackFrame::IsHidden() {
if (auto recognized_frame_sp = GetRecognizedFrame())
return recognized_frame_sp->ShouldHide();
diff --git a/lldb/source/Target/StackFrameList.cpp b/lldb/source/Target/StackFrameList.cpp
index aedfc52cfb4cb..d83c90582ba9c 100644
--- a/lldb/source/Target/StackFrameList.cpp
+++ b/lldb/source/Target/StackFrameList.cpp
@@ -321,13 +321,14 @@ void StackFrameList::SynthesizeTailCallFrames(StackFrame &next_frame) {
addr_t pc = calleeInfo.address;
// If the callee address refers to the call instruction, we do not want to
// subtract 1 from this value.
+ const bool artificial = true;
const bool behaves_like_zeroth_frame =
calleeInfo.address_type == CallEdge::AddrType::Call;
SymbolContext sc;
callee->CalculateSymbolContext(&sc);
auto synth_frame = std::make_shared<StackFrame>(
m_thread.shared_from_this(), frame_idx, concrete_frame_idx, cfa,
- cfa_is_valid, pc, StackFrame::Kind::Artificial,
+ cfa_is_valid, pc, StackFrame::Kind::Regular, artificial,
behaves_like_zeroth_frame, &sc);
m_frames.push_back(synth_frame);
LLDB_LOG(log, "Pushed frame {0} at {1:x}", callee->GetDisplayName(), pc);
@@ -470,7 +471,8 @@ bool StackFrameList::FetchFramesUpTo(uint32_t end_idx,
const bool cfa_is_valid = true;
unwind_frame_sp = std::make_shared<StackFrame>(
m_thread.shared_from_this(), m_frames.size(), idx, cfa, cfa_is_valid,
- pc, StackFrame::Kind::Regular, behaves_like_zeroth_frame, nullptr);
+ pc, StackFrame::Kind::Regular, false, behaves_like_zeroth_frame,
+ nullptr);
// Create synthetic tail call frames between the previous frame and the
// newly-found frame. The new frame's index may change after this call,
diff --git a/lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py b/lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
index 9519c576689d0..5916e62c44f2e 100644
--- a/lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
+++ b/lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
@@ -284,6 +284,6 @@ def cleanup():
break
self.assertEqual(idx, int(reg.value, 16))
- self.assertTrue(frame.IsArtificial(), "Frame is not artificial")
+ self.assertTrue(frame.IsSynthetic(), "Frame is not synthetic")
pc = frame.GetPCAddress().GetLoadAddress(target_0)
self.assertEqual(pc, 0x0100001B00)
diff --git a/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/app_specific_backtrace_crashlog.test b/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/app_specific_backtrace_crashlog.test
index 9c0510c34ccae..f680158fdf735 100644
--- a/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/app_specific_backtrace_crashlog.test
+++ b/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/app_specific_backtrace_crashlog.test
@@ -11,7 +11,7 @@
# CHECK: (lldb) process status --verbose
# CHECK-NEXT: Process 96535 stopped
# CHECK-NEXT: * thread #1, queue = 'com.apple.main-thread', stop reason = EXC_CRASH (code=0, subcode=0x0)
-# CHECK-NEXT: frame #0: 0x00000001a08c7224{{.*}}[artificial]
+# CHECK-NEXT: frame #0: 0x00000001a08c7224{{.*}}[synthetic]
# CHECK: Extended Crash Information:
# CHECK: Application Specific Information:
# CHECK-NEXT: CoreFoundation: *** Terminating app due to uncaught exception 'NSRangeException', reason: '*** __boundsFail: index 10 beyond bounds [0 .. 3]'
@@ -21,21 +21,21 @@
# CHECK: (lldb) thread backtrace --extended true
# CHECK: * thread #1, queue = 'com.apple.main-thread', stop reason = EXC_CRASH (code=0, subcode=0x0)
-# CHECK-NEXT: * frame #0: 0x00000001a08c7224{{.*}}[artificial]
-# CHECK-NEXT: frame #1: 0x00000001a08fdceb{{.*}}[artificial]
-# CHECK-NEXT: frame #2: 0x00000001a08372c7{{.*}}[artificial]
-# CHECK-NEXT: frame #3: 0x00000001a08b7b17{{.*}}[artificial]
-# CHECK-NEXT: frame #4: 0x00000001a08a7a0b{{.*}}[artificial]
-# CHECK-NEXT: frame #5: 0x00000001a05ab763{{.*}}[artificial]
-# CHECK-NEXT: frame #6: 0x00000001a08b6eb3{{.*}}[artificial]
-# CHECK-NEXT: frame #7: 0x00000001a08b9c2b{{.*}}[artificial]
-# CHECK-NEXT: frame #8: 0x00000001a08b9bd7{{.*}}[artificial]
-# CHECK-NEXT: frame #9: 0x00000001a05a3007{{.*}}[artificial]
-# CHECK-NEXT: frame #10: 0x00000001a0b3dcc3{{.*}}[artificial]
-# CHECK-NEXT: frame #11: 0x00000001a0b46af3{{.*}}[artificial]
-# CHECK-NEXT: frame #12: 0x00000001a09a12a3{{.*}}[artificial]
-# CHECK-NEXT: frame #13: 0x00000001047e3ecf asi`main{{.*}}[artificial]
-# CHECK-NEXT: frame #14: 0x00000001a05d3e4f{{.*}}[artificial]
+# CHECK-NEXT: * frame #0: 0x00000001a08c7224{{.*}}[synthetic]
+# CHECK-NEXT: frame #1: 0x00000001a08fdceb{{.*}}[synthetic]
+# CHECK-NEXT: frame #2: 0x00000001a08372c7{{.*}}[synthetic]
+# CHECK-NEXT: frame #3: 0x00000001a08b7b17{{.*}}[synthetic]
+# CHECK-NEXT: frame #4: 0x00000001a08a7a0b{{.*}}[synthetic]
+# CHECK-NEXT: frame #5: 0x00000001a05ab763{{.*}}[synthetic]
+# CHECK-NEXT: frame #6: 0x00000001a08b6eb3{{.*}}[synthetic]
+# CHECK-NEXT: frame #7: 0x00000001a08b9c2b{{.*}}[synthetic]
+# CHECK-NEXT: frame #8: 0x00000001a08b9bd7{{.*}}[synthetic]
+# CHECK-NEXT: frame #9: 0x00000001a05a3007{{.*}}[synthetic]
+# CHECK-NEXT: frame #10: 0x00000001a0b3dcc3{{.*}}[synthetic]
+# CHECK-NEXT: frame #11: 0x00000001a0b46af3{{.*}}[synthetic]
+# CHECK-NEXT: frame #12: 0x00000001a09a12a3{{.*}}[synthetic]
+# CHECK-NEXT: frame #13: 0x00000001047e3ecf asi`main{{.*}}[synthetic]
+# CHECK-NEXT: frame #14: 0x00000001a05d3e4f{{.*}}[synthetic]
# CHECK: thread #4294967295: tid = 0x0001, 0x00000001a0a58418{{.*}}, queue = 'Application Specific Backtrace'
# CHECK-NEXT: frame #0: 0x00000001a0a58418{{.*}}
diff --git a/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/interactive_crashlog_arm64_register.test b/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/interactive_crashlog_arm64_register.test
index 3f572c3300c0f..9da21d9aab78d 100644
--- a/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/interactive_crashlog_arm64_register.test
+++ b/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/interactive_crashlog_arm64_register.test
@@ -15,9 +15,9 @@
# CHECK: (lldb) thread backtrace
# CHECK-NEXT: * thread #3, stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
-# CHECK-NEXT: * frame #0: 0x0000000100ec58f4 multithread-test`bar{{.*}} [artificial]
-# CHECK-NEXT: frame #1: 0x0000000100ec591b multithread-test`foo{{.*}} [artificial]
-# CHECK-NEXT: frame #2: 0x0000000100ec5a87 multithread-test`compute_pow{{.*}} [artificial]
+# CHECK-NEXT: * frame #0: 0x0000000100ec58f4 multithread-test`bar{{.*}} [synthetic]
+# CHECK-NEXT: frame #1: 0x0000000100ec591b multithread-test`foo{{.*}} [synthetic]
+# CHECK-NEXT: frame #2: 0x0000000100ec5a87 multithread-test`compute_pow{{.*}} [synthetic]
# CHECK: (lldb) thread list
# CHECK-NEXT: Process 22511 stopped
@@ -27,20 +27,20 @@
# CHECK: (lldb) bt all
# CHECK: thread #1, queue = 'com.apple.main-thread'
-# CHECK: frame #{{[0-9]+}}: 0x000000019cc40b84{{.*}} [artificial]
-# CHECK: frame #{{[0-9]+}}: 0x0000000100ec5b3b multithread-test`main{{.*}} [artificial]
-# CHECK: frame #{{[0-9]+}}: 0x00000002230f8da7{{.*}} [artificial]
+# CHECK: frame #{{[0-9]+}}: 0x000000019cc40b84{{.*}} [synthetic]
+# CHECK: frame #{{[0-9]+}}: 0x0000000100ec5b3b multithread-test`main{{.*}} [synthetic]
+# CHECK: frame #{{[0-9]+}}: 0x00000002230f8da7{{.*}} [synthetic]
# CHECK-NEXT: thread #2
-# CHECK-NEXT: frame #0: 0x000000019cc42c9c{{.*}} [artificial]
-# CHECK: frame #{{[0-9]+}}: 0x0000000100ec5957 multithread-test`call_and_wait{{.*}} [artificial]
-# CHECK: frame #{{[0-9]+}}: 0x000000019cc7e06b{{.*}} [artificial]
-# CHECK: frame #{{[0-9]+}}: 0x000000019cc78e2b{{.*}} [artificial]
+# CHECK-NEXT: frame #0: 0x000000019cc42c9c{{.*}} [synthetic]
+# CHECK: frame #{{[0-9]+}}: 0x0000000100ec5957 multithread-test`call_and_wait{{.*}} [synthetic]
+# CHECK: frame #{{[0-9]+}}: 0x000000019cc7e06b{{.*}} [synthetic]
+# CHECK: frame #{{[0-9]+}}: 0x000000019cc78e2b{{.*}} [synthetic]
# CHECK-NEXT:* thread #3, stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
-# CHECK-NEXT: * frame #0: 0x0000000100ec58f4 multithread-test`bar{{.*}} [artificial]
-# CHECK-NEXT: frame #1: 0x0000000100ec591b multithread-test`foo{{.*}} [artificial]
-# CHECK-NEXT: frame #2: 0x0000000100ec5a87 multithread-test`compute_pow{{.*}} [artificial]
-# CHECK: frame #{{[0-9]+}}: 0x000000019cc7e06b{{.*}} [artificial]
-# CHECK: frame #{{[0-9]+}}: 0x000000019cc78e2b{{.*}} [artificial]
+# CHECK-NEXT: * frame #0: 0x0000000100ec58f4 multithread-test`bar...
[truncated]
|
/// Is this an artificial stack frame (e.g. a synthesized result of inferring | ||
/// missing tail call frames from a backtrace) with limited support for | ||
/// local variables. | ||
bool m_artificial; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess the alternative to this would be to make m_stack_frame_kind
an enum mask? So we could have frames that are Synthetic | Artificial
. Don't have a strong opinion on that. I'll let the others chime in
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've discussed this option with @jimingham offline but according to him, artificial
like inlined
is orthogonal to the stack frame kind. For instance, a frame with StackFrame::Kind::History
or StackFrame::Kind::Synthetic
kind can be artificial or inlined.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note, we also use "artificial" to mean the frame is a language-generated frame that most users probably don't want to see. There's been a suggestion that lldb should auto-hide and auto-step through this sort of "artificial" frame. But this is very much NOT how we want to treat the "missing tail call frames". So they need a different flag to indicate what they are.
They really should be "historical" frames - like other historical frames they are just a PC & CFA, but their stack doesn't currently exist - it's been overwritten by the function it tail-called to.
Anyway, you should change the comment here to refer to the more common usage...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So may be we should make the distinction between tail call frames and artificial (compiler generated / runtime glue) frames, as a follow-up.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure.
Right. Seems to me there are two orthogonal notions here: the provider of the frame info: Historical, Unwind Based, Synthetic (we should also include "Inferred" for instance the insertion of tail call frames is inferred); and the language based characterization of the frames (Inline, Artificial). |
So it's a little messier that I had thought. When we see a Symbol in DWARF that is marked with DW_AT_artificial, we set the Having to ask if the frame represents a function that is marked as Artificial in the DWARF not by asking IsArtificial but getting its Symbol and asking if that is artificial is super-confusing. Especially since StackFrame::IsArtificial is right there tempting you to misinterpret it... I think this would be much cleaner if we used Artificial for a frame to mean the same thing as it does for the function occupying that frame. And then we should come up with something else to call these |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reading the discussion, my main feedback is:
- Add a comment to
StackFrame::Kind
to explain that it's about where a stack frame comes from. - Add a comment to
m_artificial
to explain that it's orthogonal to the kind. Also right now it has the same comment asKind::Synthetic
which can't be right. - Rather than a format entity for whether a frame is synthetic, it seems that we should have a format entity that prints the kind (which could be empty for a regular frame and print
[historical]
and[synthetic]
respectively
b8fc0de
to
706b8d3
Compare
This patch changes the way frames created from scripted affordances like Scripted Threads are displayed. Currently, they're marked artificial which is used usually for compiler generated frames. This patch changes that behaviour by introducing a new synthetic StackFrame kind and moves 'artificial' to be a distinct StackFrame attribut. On top of making these frames less confusing, this allows us to know when a frame was created from a scripted affordance. rdar://155949703 Signed-off-by: Med Ismail Bennani <[email protected]>
706b8d3
to
583a83e
Compare
@JDevlieghere @jimingham I think I've address all the comments here, do you mind taking another look ? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
LGTM2 |
* main: (1483 commits) [clang] fix error recovery for invalid nested name specifiers (llvm#156772) Revert "[lldb] Add count for errors of DWO files in statistics and combine DWO file count functions" (llvm#156777) AMDGPU: Add agpr variants of multi-data DS instructions (llvm#156420) [libc][NFC] disable localtime on aarch64/baremetal (llvm#156776) [win/asan] Improve SharedReAlloc with HEAP_REALLOC_IN_PLACE_ONLY. (llvm#132558) [LLDB] Make internal shell the default for running LLDB lit tests. (llvm#156729) [lldb][debugserver] Max response size for qSpeedTest (llvm#156099) [AMDGPU] Define 1024 VGPRs on gfx1250 (llvm#156765) [flang] Check for BIND(C) name conflicts with alternate entries (llvm#156563) [RISCV] Add exhausted_gprs_fprs test to calling-conv-half.ll. NFC (llvm#156586) [NFC] Remove trailing whitespaces from `clang/include/clang/Basic/AttrDocs.td` [lldb] Mark scripted frames as synthetic instead of artificial (llvm#153117) [docs] Refine some of the wording in the quality developer policy (llvm#156555) [MLIR] Apply clang-tidy fixes for readability-identifier-naming in TransformOps.cpp (NFC) [MLIR] Add LDBG() tracing to VectorTransferOpTransforms.cpp (NFC) [NFC] Apply clang-format to PPCInstrFutureMMA.td (llvm#156749) [libc] implement template functions for localtime (llvm#110363) [llvm-objcopy][COFF] Update .symidx values after stripping (llvm#153322) Add documentation on debugging LLVM. [lldb] Add count for errors of DWO files in statistics and combine DWO file count functions (llvm#155023) ...
…153117) This patch changes the way frames created from scripted affordances like Scripted Threads are displayed. Currently, they're marked artificial which is used usually for compiler generated frames. This patch changes that behaviour by introducing a new synthetic StackFrame kind and moves 'artificial' to be a distinct StackFrame attribut. On top of making these frames less confusing, this allows us to know when a frame was created from a scripted affordance. rdar://155949703 Signed-off-by: Med Ismail Bennani <[email protected]> (cherry picked from commit 6c10ab8)
…153117) This patch changes the way frames created from scripted affordances like Scripted Threads are displayed. Currently, they're marked artificial which is used usually for compiler generated frames. This patch changes that behaviour by introducing a new synthetic StackFrame kind and moves 'artificial' to be a distinct StackFrame attribut. On top of making these frames less confusing, this allows us to know when a frame was created from a scripted affordance. rdar://155949703 Signed-off-by: Med Ismail Bennani <[email protected]> (cherry picked from commit 6c10ab8)
…153117) This patch changes the way frames created from scripted affordances like Scripted Threads are displayed. Currently, they're marked artificial which is used usually for compiler generated frames. This patch changes that behaviour by introducing a new synthetic StackFrame kind and moves 'artificial' to be a distinct StackFrame attribut. On top of making these frames less confusing, this allows us to know when a frame was created from a scripted affordance. rdar://155949703 Signed-off-by: Med Ismail Bennani <[email protected]> (cherry picked from commit 6c10ab8)
…153117) This patch changes the way frames created from scripted affordances like Scripted Threads are displayed. Currently, they're marked artificial which is used usually for compiler generated frames. This patch changes that behaviour by introducing a new synthetic StackFrame kind and moves 'artificial' to be a distinct StackFrame attribut. On top of making these frames less confusing, this allows us to know when a frame was created from a scripted affordance. rdar://155949703 Signed-off-by: Med Ismail Bennani <[email protected]> (cherry picked from commit 6c10ab8)
This patch changes the way frames created from scripted affordances like Scripted Threads are displayed. Currently, they're marked artificial which is used usually for compiler generated frames.
This patch changes that behaviour by introducing a new synthetic StackFrame kind and moves 'artificial' to be a distinct StackFrame attribut.
On top of making these frames less confusing, this allows us to know when a frame was created from a scripted affordance.
rdar://155949703