[Lldb-commits] [PATCH] D33426: Introduce new command: thread backtrace unique
bgianfo updated this revision to Diff 100830.
bgianfo added a comment.
Fix bug in "unique" backtrace output that Greg pointed out.
Introduced a new format for unique frames and plumbed that
through the stacks to be able to toggle between them both
depending on the calling arguments.
https://reviews.llvm.org/D33426
Files:
include/lldb/Core/Debugger.h
include/lldb/Target/StackFrame.h
include/lldb/Target/StackFrameList.h
include/lldb/Target/Thread.h
packages/Python/lldbsuite/test/functionalities/thread/num_threads/TestNumThreads.py
packages/Python/lldbsuite/test/functionalities/thread/num_threads/main.cpp
source/Commands/CommandObjectThread.cpp
source/Core/Debugger.cpp
source/Target/StackFrame.cpp
source/Target/StackFrameList.cpp
source/Target/Thread.cpp
Index: source/Target/Thread.cpp
===
--- source/Target/Thread.cpp
+++ source/Target/Thread.cpp
@@ -1913,47 +1913,50 @@
size_t Thread::GetStatus(Stream &strm, uint32_t start_frame,
uint32_t num_frames, uint32_t num_frames_with_source,
- bool stop_format) {
- ExecutionContext exe_ctx(shared_from_this());
- Target *target = exe_ctx.GetTargetPtr();
- Process *process = exe_ctx.GetProcessPtr();
- size_t num_frames_shown = 0;
- strm.Indent();
- bool is_selected = false;
- if (process) {
-if (process->GetThreadList().GetSelectedThread().get() == this)
- is_selected = true;
- }
- strm.Printf("%c ", is_selected ? '*' : ' ');
- if (target && target->GetDebugger().GetUseExternalEditor()) {
-StackFrameSP frame_sp = GetStackFrameAtIndex(start_frame);
-if (frame_sp) {
- SymbolContext frame_sc(
- frame_sp->GetSymbolContext(eSymbolContextLineEntry));
- if (frame_sc.line_entry.line != 0 && frame_sc.line_entry.file) {
-Host::OpenFileInExternalEditor(frame_sc.line_entry.file,
- frame_sc.line_entry.line);
+ bool stop_format, bool only_stacks) {
+
+ if (!only_stacks) {
+ExecutionContext exe_ctx(shared_from_this());
+Target *target = exe_ctx.GetTargetPtr();
+Process *process = exe_ctx.GetProcessPtr();
+strm.Indent();
+bool is_selected = false;
+if (process) {
+ if (process->GetThreadList().GetSelectedThread().get() == this)
+is_selected = true;
+}
+strm.Printf("%c ", is_selected ? '*' : ' ');
+if (target && target->GetDebugger().GetUseExternalEditor()) {
+ StackFrameSP frame_sp = GetStackFrameAtIndex(start_frame);
+ if (frame_sp) {
+SymbolContext frame_sc(
+frame_sp->GetSymbolContext(eSymbolContextLineEntry));
+if (frame_sc.line_entry.line != 0 && frame_sc.line_entry.file) {
+ Host::OpenFileInExternalEditor(frame_sc.line_entry.file,
+ frame_sc.line_entry.line);
+}
}
}
- }
- DumpUsingSettingsFormat(strm, start_frame, stop_format);
+DumpUsingSettingsFormat(strm, start_frame, stop_format);
+ }
+ size_t num_frames_shown = 0;
if (num_frames > 0) {
strm.IndentMore();
const bool show_frame_info = true;
-
+const bool show_frame_unique = only_stacks;
const char *selected_frame_marker = nullptr;
-if (num_frames == 1 ||
+if (num_frames == 1 || only_stacks ||
(GetID() != GetProcess()->GetThreadList().GetSelectedThread()->GetID()))
strm.IndentMore();
else
selected_frame_marker = "* ";
num_frames_shown = GetStackFrameList()->GetStatus(
strm, start_frame, num_frames, show_frame_info, num_frames_with_source,
-selected_frame_marker);
+show_frame_unique, selected_frame_marker);
if (num_frames == 1)
strm.IndentLess();
strm.IndentLess();
Index: source/Target/StackFrameList.cpp
===
--- source/Target/StackFrameList.cpp
+++ source/Target/StackFrameList.cpp
@@ -801,7 +801,7 @@
size_t StackFrameList::GetStatus(Stream &strm, uint32_t first_frame,
uint32_t num_frames, bool show_frame_info,
- uint32_t num_frames_with_source,
+ uint32_t num_frames_with_source, bool show_unique,
const char *selected_frame_marker) {
size_t num_frames_displayed = 0;
@@ -842,7 +842,7 @@
if (!frame_sp->GetStatus(strm, show_frame_info,
num_frames_with_source > (first_frame - frame_idx),
- marker))
+ show_unique, marker))
break;
++num_frames_displayed;
}
Index: source/Target/StackFrame.cpp
===
--- source/Target/StackFrame.cpp
+++ source/Target/StackFrame.cpp
@@ -1744,6 +1744,7 @@
}
void StackFrame::DumpUsingSettingsFormat(Stream *s
[Lldb-commits] [PATCH] D33674: Implementation of Intel(R) Processor Trace support for Linux
labath added a comment.
First batch of comments from me, I think I will have some more once I gain more
insight into this. The main one is about the constructor/initialize,
destructor/destroy duality, which we should abolish. The rest is mostly
stylistic.
Comment at: source/Plugins/Process/Linux/NativeProcessLinux.cpp:618
+traceMonitor);
+ if (traceMonitor.get() != nullptr) {
+traceMonitor->setUserID(m_pt_process_uid);
`if(traceMonitor)`
Comment at: source/Plugins/Process/Linux/NativeProcessLinux.cpp:619
+ if (traceMonitor.get() != nullptr) {
+traceMonitor->setUserID(m_pt_process_uid);
+m_pt_traced_thread_group.insert(thread_sp->GetID());
As far as, I can tell, every call to `StartProcessorTracing` is followed by
setUserId. Can we move the logic of setting the id inside the
`StartProcessorTracing` function
Comment at: source/Plugins/Process/Linux/NativeProcessLinux.cpp:2483
+template
+static Status ReadFromFile(const char *filename, T &read,
+ std::ios_base::fmtflags mode = (std::ios_base::hex |
Please use llvm file API for this (see how `getProcFile` works -- it's just a
very thin wrapper around the llvm functions)
Comment at: source/Plugins/Process/Linux/NativeProcessLinux.cpp:2510
+ if (thread == LLDB_INVALID_THREAD_ID && uid == m_pt_process_uid) {
+if (log)
+ log->Printf("NativeProcessLinux %s_thread not specified: %" PRIu64,
Please use LLDB_LOG (here and everywhere else in this patch)
Comment at: source/Plugins/Process/Linux/NativeProcessLinux.cpp:2847
+
+Status NativeProcessLinux::StopProcessorTracingOnProcess(lldb::user_id_t uid) {
+ Status ret_error;
You are calling this function with uid == m_pt_process_uid, which means this
parameter is redundant, and leads to redundant sanity checks. Please remove it.
Comment at: source/Plugins/Process/Linux/NativeProcessLinux.cpp:2965
+
+ config.setTraceBufferSize(bufsize);
+ config.setMetaDataBufferSize(metabufsize);
You are modifying the config, but the caller is ignoring these modifications.
If you don't need these, we could remove the modifications, make the config
argument `const` and end up with a much cleaner interface.
Comment at: source/Plugins/Process/Linux/NativeProcessLinux.cpp:3056
+
+ auto BufferOrError = getProcFile("cpuinfo");
+ if (!BufferOrError) {
I don't see a getProcFile overload with this signature (although I am not
opposed to adding one). Are you sure this compiles?
Comment at: source/Plugins/Process/Linux/NativeProcessLinux.cpp:3064
+std::tie(Line, Rest) = Rest.split('\n');
+{
+ SmallVector columns;
Is this scope necessary? I find it just confuses the reader...
Comment at: source/Plugins/Process/Linux/NativeProcessLinux.cpp:3070
+continue; // continue searching
+
+ if (log)
add: `columns[1] = columns[1].trim();`
Then you can skip calling trim everywhere else.
Comment at: source/Plugins/Process/Linux/NativeProcessLinux.cpp:3075
+
+ if ((columns[0].find("cpu family") != std::string::npos) &&
+ columns[1].trim(" ").getAsInteger(10, cpu_family))
columns[0].contains(foo)
Comment at: source/Plugins/Process/Linux/NativeProcessLinux.cpp:3100
+ (stepping != static_cast(-1)) && (!vendor_id.empty()))
+break; // we are done
+}
return here. Then you can avoid duplicating the check outside the loop.
Comment at: source/Plugins/Process/Linux/NativeProcessLinux.h:156
+ // -
+ static size_t ReadCyclicBuffer(void *buf, size_t buf_size, void *cyc_buf,
+ size_t cyc_buf_size, size_t cyc_start,
How about ` void ReadCyclicBuffer(ArrayRef buffer, size_t position,
MutableArrayRef &dest)`
Comment at: source/Plugins/Process/Linux/NativeProcessLinux.h:160
+ enum PTErrorCode {
+FileNotFound = 0x23,
+ThreadNotTraced,
This seems suspicious. How did you come to choose this number?
Comment at: source/Plugins/Process/Linux/NativeProcessLinux.h:317
+ // -
+ class ProcessorTraceMonitor {
+int m_fd;
Please move this into a separate file. NativeProcessLinux.cpp is big enough as
it is.
Comment at: source/Plugins/Process/Linux/NativeProcessLinux.h:331
+ public:
+ProcessorTraceMonitor()
+: m_fd(-1), m_mmap_data(nullptr), m_mmap_aux(nullptr),
This would be much cleaner, if we co
[Lldb-commits] [PATCH] D33035: Tool for using Intel(R) Processor Trace hardware feature
labath added a comment. In https://reviews.llvm.org/D33035#767029, @abhishek.aggarwal wrote: > In https://reviews.llvm.org/D33035#754640, @labath wrote: > > > I don't really like that we are adding a public shared library for every > > tiny intel feature. Could we at least merge this "plugin" with the existing > > "intel-mpx plugin" to create one "intel support" library? > > > > Also, adding an external dependency probably deserves a discussion on > > lldb-dev. > > > Hi Paval ... Before starting the development of this whole feature, we had > submitted the full proposal to lldb dev list 1.5 years ago. During the > discussions, it was proposed to keep the external dependency outside LLDB > (i.e. not to be bundled in liblldb shared library). The External dependency > required for this tool is not and will never be a part of lldb repository. > Users who are interested to use this tool, will download this external > dependency separately. Yes I remember that. But, as you say, that was 1.5 years ago, and we haven't heard anything since. Honestly, I had assumed you abandoned that work until you reappeared last month. :) So I think it's worth updating that thread, as new things may have come up since then (for example, the decision whether to merge the intel stuff into a single library). https://reviews.llvm.org/D33035 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D33426: Introduce new command: thread backtrace unique
labath added a comment. In https://reviews.llvm.org/D33426#766525, @bgianfo wrote: > Address Pavel and Greg's feedback on Diff 100365. > > Pavel: I took your suggestions to make the test case more readable, > I really appreciate the guidance. I did have to tweak some of the > functionality to make the test case pass reliably, as there were > still some races possible. I also saw that SBThread.Resume() seems > to occasionally result in a StopReason of eStopReasonNon. So I worked > around that by only including threads int expected output that the Resume > resulted in making it to our breakpoint. I have verified the test is > consistently passes by executing it on repeat 100 times, Thanks. The fact that we are not able to rely on the operation of Resume in this case sounds like a bug. Obviously we can't condition the acceptance of this patch by fixing that issue, but we should at least track it. Can you create a bug on bugs.llvm.org, and reference it in your workaround. BTW, what's the platform you are testing this on? https://reviews.llvm.org/D33426 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D32930: New framework for lldb client-server communication tests.
labath added a comment. Thanks for the support, @beanz. Comment at: unittests/tools/CMakeLists.txt:1 +if(UNIX AND NOT APPLE) + add_subdirectory(lldb-server) beanz wrote: > labath wrote: > > This is not what I meant. The only targets (at least until we have > > debugserver support) that can realistically pass these tests are linux, > > android, and netbsd. The other targets (right now, I guess that would mean > > freebsd) don't even pretend to support debugging via lldb-server, so we > > should not fail their build because of that. Check for usages of > > CMAKE_SYSTEM_NAME to see how to discriminate those. > Darwin pretends to support lldb-server in several places, it would be nice to > be able to run these tests on Darwin if they work. One of my big goals for > the future of testing on LLDB is to get to the point where the only > differences in test coverage when running tests on different hosts is truly > platform-specific code. Today we are nowhere near that. > > Also, as Pavel pointed out in email, the lldb-server tests are also run > against debugserver, so we need to make sure that still works too. Which lldb-server support do you refer to here? There is some llgs (debugging) support in lldb-server, but I have no idea what's the state of it -- it was added by Todd during his week of code as an "NFC" commit, and it hasn't been touched since. I'd like to avoid this keeping the build red if there is no intention of working on it. The "platform" mode of lldb-server should work on darwin afaik, and we definitely want to be able to run it there. It's not what we are focusing on now though. We'd like to migrate the "debug" tests first (there are no "platform" tests), so the old ones can be removed. In any case, I think of the apple exclusion part as a temporary thing, so we can check this in without breaking the build, we will pretty soon want to include it as well, so that we can run debugserver tests, at least. (At which point we will need a different way of disabling unsupported tests). https://reviews.llvm.org/D32930 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D33426: Introduce new command: thread backtrace unique
clayborg accepted this revision. clayborg added a comment. Looks like a good starting point. Thanks for the changes. https://reviews.llvm.org/D33426 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D33674: Implementation of Intel(R) Processor Trace support for Linux
clayborg resigned from this revision. clayborg added a comment. I trust Pavel to review this since it is in the Linux native plugins mostly. https://reviews.llvm.org/D33674 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r304314 - Added a testcase for local/namespaced name conflicts.
Author: spyffe
Date: Wed May 31 12:18:10 2017
New Revision: 304314
URL: http://llvm.org/viewvc/llvm-project?rev=304314&view=rev
Log:
Added a testcase for local/namespaced name conflicts.
This works on SVN but is a bit fragile on the Swift branch.
I'm adding the test to both, so we have this path covered.
Added:
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/namespace_conflicts/
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/namespace_conflicts/Makefile
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/namespace_conflicts/TestNamespaceConflicts.py
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/namespace_conflicts/main.cpp
Added:
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/namespace_conflicts/Makefile
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/namespace_conflicts/Makefile?rev=304314&view=auto
==
---
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/namespace_conflicts/Makefile
(added)
+++
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/namespace_conflicts/Makefile
Wed May 31 12:18:10 2017
@@ -0,0 +1,3 @@
+LEVEL = ../../../make
+CXX_SOURCES := main.cpp
+include $(LEVEL)/Makefile.rules
Added:
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/namespace_conflicts/TestNamespaceConflicts.py
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/namespace_conflicts/TestNamespaceConflicts.py?rev=304314&view=auto
==
---
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/namespace_conflicts/TestNamespaceConflicts.py
(added)
+++
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/namespace_conflicts/TestNamespaceConflicts.py
Wed May 31 12:18:10 2017
@@ -0,0 +1,7 @@
+from lldbsuite.test import lldbinline
+from lldbsuite.test import decorators
+
+lldbinline.MakeInlineTest(
+__file__, globals(), [
+decorators.expectedFailureAll(
+oslist=["windows"], bugnumber="llvm.org/pr24764")])
Added:
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/namespace_conflicts/main.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/namespace_conflicts/main.cpp?rev=304314&view=auto
==
---
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/namespace_conflicts/main.cpp
(added)
+++
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/namespace_conflicts/main.cpp
Wed May 31 12:18:10 2017
@@ -0,0 +1,29 @@
+//===-- main.cpp *- C++
-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+namespace n {
+struct D {
+int i;
+static int anInt() { return 2; }
+int dump() { return i; }
+};
+}
+
+using namespace n;
+
+int foo(D* D) {
+return D->dump(); //% self.expect("expression -- D->dump()",
DATA_TYPES_DISPLAYED_CORRECTLY, substrs = ["int", "2"])
+}
+
+int main (int argc, char const *argv[])
+{
+D myD { D::anInt() };
+foo(&myD);
+return 0;
+}
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D32930: New framework for lldb client-server communication tests.
beanz added inline comments. Comment at: unittests/tools/CMakeLists.txt:1 +if(UNIX AND NOT APPLE) + add_subdirectory(lldb-server) labath wrote: > beanz wrote: > > labath wrote: > > > This is not what I meant. The only targets (at least until we have > > > debugserver support) that can realistically pass these tests are linux, > > > android, and netbsd. The other targets (right now, I guess that would > > > mean freebsd) don't even pretend to support debugging via lldb-server, so > > > we should not fail their build because of that. Check for usages of > > > CMAKE_SYSTEM_NAME to see how to discriminate those. > > Darwin pretends to support lldb-server in several places, it would be nice > > to be able to run these tests on Darwin if they work. One of my big goals > > for the future of testing on LLDB is to get to the point where the only > > differences in test coverage when running tests on different hosts is truly > > platform-specific code. Today we are nowhere near that. > > > > Also, as Pavel pointed out in email, the lldb-server tests are also run > > against debugserver, so we need to make sure that still works too. > Which lldb-server support do you refer to here? > > There is some llgs (debugging) support in lldb-server, but I have no idea > what's the state of it -- it was added by Todd during his week of code as an > "NFC" commit, and it hasn't been touched since. I'd like to avoid this > keeping the build red if there is no intention of working on it. > > The "platform" mode of lldb-server should work on darwin afaik, and we > definitely want to be able to run it there. It's not what we are focusing on > now though. We'd like to migrate the "debug" tests first (there are no > "platform" tests), so the old ones can be removed. > > In any case, I think of the apple exclusion part as a temporary thing, so we > can check this in without breaking the build, we will pretty soon want to > include it as well, so that we can run debugserver tests, at least. (At which > point we will need a different way of disabling unsupported tests). I don't know the state of this stuff on Darwin either. I had spent a little time a few weeks ago trying to get lldb to use lldb-server on Darwin to see if I could answer that question, but I didn't get very far before I had to stop. As long as the Apple exclusion is temporary I'm fine with this as-is. I'll see if I can free up some time this summer to look more closely at lldb-server on Darwin and figure out what state it is in. https://reviews.llvm.org/D32930 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D32930: New framework for lldb client-server communication tests.
zturner added a comment. In https://reviews.llvm.org/D32930#767820, @beanz wrote: > One small comment below. In general I agree with the thoughts here, and I > think that this is a huge step forward for testing the debug server > components. > > I also agree with Zachary in principal that it would be nice to come up with > lit-based test harnesses for more parts of LLDB, although I'm skeptical about > whether or not that is actually the best way to test the debug server pieces. > Either way, this is a huge step forward from what we have today, so we should > go with it. It would be nice if, at some point, we could move past "It's hard" and start getting into the details of what's hard about it. (Note this goes for LLDB client as well as lldb server). I see a lot of general hand-wavy comments about how conditionals are needed, or variables, etc, but that doesn't really do anything to convince me that it's hard. After all, we wrote a C++ compiler! And I'm pretty sure that the compiler-rt and sanitizer test suite is just as complicated as, if not more complicated than any hypothetical lldb test suite. And those have been solved. What *would* help would be to ignore how difficult it may or may not be, and just take a couple of tests and re-write them in some DSL that you invent specifically for this purpose that is as concise as possible yet as expressive as you need, and we go from there. I did this with a couple of fairly hairy tests a few months ago and it didn't seem that bad to me. The thing is, the set of people who are experts on the client side of LLDB and the set of people who are experts on the client side of LLVM/lit/etc are mostly disjoint, so nothing is ever going to happen without some sort of collaboration. For example, I'm more than willing to help out writing the lit bits of it, but I would need a specification of what the test language needs to look like to support all of the use cases. And someone else has to provide that since we want to get the design right. Even if implementing the language is hard, deciding what it needs to look like is supposed to be the easy part! https://reviews.llvm.org/D32930 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D32732: "target variable" not showing all global variable if we print any global variable before execution starts
jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. Okay, that seems reasonable. Repository: rL LLVM https://reviews.llvm.org/D32732 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D33674: Implementation of Intel(R) Processor Trace support for Linux
zturner added inline comments.
Comment at: source/Plugins/Process/Linux/NativeProcessLinux.cpp:738-742
+StartProcessorTracing(tid, m_pt_process_config, traceMonitor);
+if (traceMonitor.get() != nullptr) {
+ traceMonitor->setUserID(m_pt_process_uid);
+ m_pt_traced_thread_group.insert(tid);
+}
This function returns a `Status`. Can't we assume that `traceMonitor` will be
valid if and only if the returned `Status` is a success condition? And if it's
not a success condition, shouldn't you log the error?
Comment at: source/Plugins/Process/Linux/NativeProcessLinux.cpp:2517-2522
+ auto iter = m_processor_trace_monitor.begin();
+ for (; iter != m_processor_trace_monitor.end(); iter++) {
+if (uid == iter->second->getUID() &&
+(thread == iter->first || thread == LLDB_INVALID_THREAD_ID))
+ return iter->second;
+ }
Please use a range based for here.
Comment at: source/Plugins/Process/Linux/NativeProcessLinux.cpp:2626
+ lldb::user_id_t uid = LLDB_INVALID_UID;
+ if (config.getType() == lldb::TraceType::eTraceTypeProcessorTrace) {
+if (m_pt_process_uid == LLDB_INVALID_UID) {
Can you do an early return here if the condition is not true?
Comment at: source/Plugins/Process/Linux/NativeProcessLinux.cpp:2627
+ if (config.getType() == lldb::TraceType::eTraceTypeProcessorTrace) {
+if (m_pt_process_uid == LLDB_INVALID_UID) {
+ m_pt_process_config = config;
And here
Comment at: source/Plugins/Process/Linux/NativeProcessLinux.cpp:2638-2642
+llvm::StringRef intel_custom_params_key("intel-pt");
+llvm::StringRef cpu_family_key("cpu_family");
+llvm::StringRef cpu_model_key("cpu_model");
+llvm::StringRef cpu_stepping_key("cpu_stepping");
+llvm::StringRef cpu_vendor_intel_key("cpu_vendor_intel");
Nothing specifically wrong with this, but it's implicitly convertible, so if
you like it you can simply just pass these strings into the `AddIntegerItem`
function as string literals.
Comment at: source/Plugins/Process/Linux/NativeProcessLinux.cpp:2717
+ m_processor_trace_monitor.insert(
+ std::pair(thread, traceMonitor));
+
`std::make_pair(thread, traceMonitor)` might allow this to fit on one line.
`m_processor_trace_monitor.emplace(thread, traceMonitor);` almost definitely
would.
Comment at: source/Plugins/Process/Linux/NativeProcessLinux.cpp:2744
+ error = getCPUType(cpu_family, model, stepping, vendor_id);
+ if (error.Success()) {
+StructuredData::Dictionary *intel_params = new
StructuredData::Dictionary();
Early return here.
Comment at: source/Plugins/Process/Linux/NativeProcessLinux.cpp:2745
+ if (error.Success()) {
+StructuredData::Dictionary *intel_params = new
StructuredData::Dictionary();
+
Who is responsible for freeing this memory?
Comment at: source/Plugins/Process/Linux/NativeProcessLinux.cpp:2794-2795
+
+ switch (trace_type) {
+ case lldb::TraceType::eTraceTypeProcessorTrace:
+error =
Seems like this would be more straightforward to just say:
```
if (trace_type != eTraceTypeProcessorTrace)
return NativeProcessProtocol::StartTrace(config, error);
```
Comment at: source/Plugins/Process/Linux/NativeProcessLinux.cpp:2870-2871
+ lldb::tid_t thread) {
+ Status error;
+ Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_PTRACE));
+
Bunch of opportunities in this function for early returning on error conditions.
Comment at: source/Plugins/Process/Linux/NativeProcessLinux.cpp:2876-2877
+bool uid_found = false;
+for (; iter != m_processor_trace_monitor.end(); iter++) {
+ if (iter->second->getUID() == uid) {
+// Stopping a trace instance for an individual thread
Range based for with an inverted conditional and early continue inside the loop.
Comment at: source/Plugins/Process/Linux/NativeProcessLinux.cpp:2931-2938
+static uint64_t ComputeFloorLog2(uint64_t input) {
+ uint64_t prev = input;
+ while ((input & (input - 1)) != 0) {
+input &= (input - 1);
+prev = input;
+ }
+ return prev;
Delete and replace callsites with `llvm::Log2_64`
Comment at: source/Plugins/Process/Linux/NativeProcessLinux.cpp:2940-2943
+Status NativeProcessLinux::ProcessorTraceMonitor::StartTrace(
+lldb::pid_t pid, lldb::tid_t tid, TraceOptions &config) {
+ Status error;
+ Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_PTRACE));
What happens if you call this function twice in a row? Or from different
threads? Is that something you care about?
Re: [Lldb-commits] [PATCH] D33426: Introduce new command: thread backtrace unique
SBThread::Resume instructs lldb to set the resume state on a thread to "eStateRunning", meaning that means the next time you continue the process, that thread will get a chance to run. It has no effect on the StopReason for the thread (it doesn't even start it running except maybe in the not well supported "thread-centric" debugging mode.) In the normal case, nothing is going to happen till the process is resumed (with SBProcess::Continue). At that point, the thread you just allowed to run might or might not actually get scheduled to run. And if it does actually run, it might or might not have stopped for an interesting reason by the time the process stopped. So getting eStopReasonNone in this case is not at all unexpected. If NONE of the threads in the program had an interesting stop reason, that would be weird, and worth looking at. But that you might have to wait around for a while for any particular thread to stop for some reason is not unexpected. Jim > On May 31, 2017, at 3:44 AM, Pavel Labath via Phabricator > wrote: > > labath added a comment. > > In https://reviews.llvm.org/D33426#766525, @bgianfo wrote: > >> Address Pavel and Greg's feedback on Diff 100365. >> >> Pavel: I took your suggestions to make the test case more readable, >> I really appreciate the guidance. I did have to tweak some of the >> functionality to make the test case pass reliably, as there were >> still some races possible. I also saw that SBThread.Resume() seems >> to occasionally result in a StopReason of eStopReasonNon. So I worked >> around that by only including threads int expected output that the Resume >> resulted in making it to our breakpoint. I have verified the test is >> consistently passes by executing it on repeat 100 times, > > > Thanks. The fact that we are not able to rely on the operation of Resume in > this case sounds like a bug. Obviously we can't condition the acceptance of > this patch by fixing that issue, but we should at least track it. Can you > create a bug on bugs.llvm.org, and reference it in your workaround. BTW, > what's the platform you are testing this on? > > > https://reviews.llvm.org/D33426 > > > ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D32930: New framework for lldb client-server communication tests.
Before we get past "it's hard" to "just to do it", it would help me to be clear first on what you are actually trying to achieve with this proposal. It's not clear to me what problem are people trying to solve here? If it is writing tests for the decomposable parts of lldb - like the tests Jason wrote for the unwinder recently - why was the gtest path not a good way to do this? If it is rewriting the parts of the testsuite that exercise the debugger on live targets what would a lit-based suite do that we can't do with the current testsuite? Or maybe you are thinking of some other good I'm missing? Jim > On May 31, 2017, at 10:37 AM, Zachary Turner via Phabricator via lldb-commits > wrote: > > zturner added a comment. > > In https://reviews.llvm.org/D32930#767820, @beanz wrote: > >> One small comment below. In general I agree with the thoughts here, and I >> think that this is a huge step forward for testing the debug server >> components. >> >> I also agree with Zachary in principal that it would be nice to come up with >> lit-based test harnesses for more parts of LLDB, although I'm skeptical >> about whether or not that is actually the best way to test the debug server >> pieces. Either way, this is a huge step forward from what we have today, so >> we should go with it. > > > It would be nice if, at some point, we could move past "It's hard" and start > getting into the details of what's hard about it. (Note this goes for LLDB > client as well as lldb server). I see a lot of general hand-wavy comments > about how conditionals are needed, or variables, etc, but that doesn't really > do anything to convince me that it's hard. After all, we wrote a C++ > compiler! And I'm pretty sure that the compiler-rt and sanitizer test suite > is just as complicated as, if not more complicated than any hypothetical lldb > test suite. And those have been solved. > > What *would* help would be to ignore how difficult it may or may not be, and > just take a couple of tests and re-write them in some DSL that you invent > specifically for this purpose that is as concise as possible yet as > expressive as you need, and we go from there. I did this with a couple of > fairly hairy tests a few months ago and it didn't seem that bad to me. > > The thing is, the set of people who are experts on the client side of LLDB > and the set of people who are experts on the client side of LLVM/lit/etc are > mostly disjoint, so nothing is ever going to happen without some sort of > collaboration. For example, I'm more than willing to help out writing the > lit bits of it, but I would need a specification of what the test language > needs to look like to support all of the use cases. And someone else has to > provide that since we want to get the design right. Even if implementing the > language is hard, deciding what it needs to look like is supposed to be the > easy part! > > > https://reviews.llvm.org/D32930 > > > > ___ > lldb-commits mailing list > lldb-commits@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D32930: New framework for lldb client-server communication tests.
Writing tests that are A) as easy as possible to write and understand B) built on a multiprocessing infrastructure that is battle tested and known to not be flaky C) Familiar to other llvm developers, so as to not discourage subject matter experts from other areas to make relevant improvements to LLDB For example, I assume you are on board at least to some extent with lldbinline tests. After all, they're simpler than the other style of test. Now, suppose there were some hypothetical DSL that allowed every test to be an inline test but still test equally complicated scenarios in half the code. Then suppose it also ran on a more robust multiprocessing infrastructure than dotest.py. That's all we're really talking about On Wed, May 31, 2017 at 12:06 PM Jim Ingham via lldb-commits < lldb-commits@lists.llvm.org> wrote: > Before we get past "it's hard" to "just to do it", it would help me to be > clear first on what you are actually trying to achieve with this proposal. > It's not clear to me what problem are people trying to solve here? If it > is writing tests for the decomposable parts of lldb - like the tests Jason > wrote for the unwinder recently - why was the gtest path not a good way to > do this? If it is rewriting the parts of the testsuite that exercise the > debugger on live targets what would a lit-based suite do that we can't do > with the current testsuite? Or maybe you are thinking of some other good > I'm missing? > > Jim > > > > On May 31, 2017, at 10:37 AM, Zachary Turner via Phabricator via > lldb-commits wrote: > > > > zturner added a comment. > > > > In https://reviews.llvm.org/D32930#767820, @beanz wrote: > > > >> One small comment below. In general I agree with the thoughts here, and > I think that this is a huge step forward for testing the debug server > components. > >> > >> I also agree with Zachary in principal that it would be nice to come up > with lit-based test harnesses for more parts of LLDB, although I'm > skeptical about whether or not that is actually the best way to test the > debug server pieces. Either way, this is a huge step forward from what we > have today, so we should go with it. > > > > > > It would be nice if, at some point, we could move past "It's hard" and > start getting into the details of what's hard about it. (Note this goes > for LLDB client as well as lldb server). I see a lot of general hand-wavy > comments about how conditionals are needed, or variables, etc, but that > doesn't really do anything to convince me that it's hard. After all, we > wrote a C++ compiler! And I'm pretty sure that the compiler-rt and > sanitizer test suite is just as complicated as, if not more complicated > than any hypothetical lldb test suite. And those have been solved. > > > > What *would* help would be to ignore how difficult it may or may not be, > and just take a couple of tests and re-write them in some DSL that you > invent specifically for this purpose that is as concise as possible yet as > expressive as you need, and we go from there. I did this with a couple of > fairly hairy tests a few months ago and it didn't seem that bad to me. > > > > The thing is, the set of people who are experts on the client side of > LLDB and the set of people who are experts on the client side of > LLVM/lit/etc are mostly disjoint, so nothing is ever going to happen > without some sort of collaboration. For example, I'm more than willing to > help out writing the lit bits of it, but I would need a specification of > what the test language needs to look like to support all of the use cases. > And someone else has to provide that since we want to get the design > right. Even if implementing the language is hard, deciding what it needs > to look like is supposed to be the easy part! > > > > > > https://reviews.llvm.org/D32930 > > > > > > > > ___ > > lldb-commits mailing list > > lldb-commits@lists.llvm.org > > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits > > ___ > lldb-commits mailing list > lldb-commits@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits > ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D32930: New framework for lldb client-server communication tests.
> On May 31, 2017, at 12:22 PM, Zachary Turner wrote: > > Writing tests that are > > A) as easy as possible to write and understand I've never understood why you consider the tests in our test case hard to write or understand. Till I added the example directory, you had to go search around for an exemplar to work from, which was a bit of a pain. But now you copy whichever of the examples you want into place, and you have a process stopped somewhere and ready to poke at. You do need to learn some SB API's but on that see below... > B) built on a multiprocessing infrastructure that is battle tested and known > to not be flaky We haven't come across failures in the tests caused by the test infrastructure in a while now. The multiprocessing flakiness I've seen is because you are trying to run many tests in parallel, and some of those tests require complex setup and that times out when machines are heavily loaded. And for the most part we solve that by running the tests that require timeouts serially. That seems a solved problem as well. > C) Familiar to other llvm developers, so as to not discourage subject matter > experts from other areas to make relevant improvements to LLDB > If I were a developer coming new to lldb now, and had to write a test case, I would have to learn something about how the SB API's work (that and a little Python.) The test case part is pretty trivial, especially when copying from the sample tests. Learning the SB API's is a bit of a pain, but having done that the next time I wanted to write some little breakpoint command to do something clever when doing my own debugging, I now have some skills at my fingertips that are going to come in really handy. If we change over the tests so what I did instead was learn some DSL to describe lldb tests (which would probably not be entirely trivial, and likely not much easier than the SB API's which are very straight-forward) I have learned nothing useful outside of writing tests. Not sure how that's a benefit. For the tests that can be decoupled from running processes, that have simple data in and out and don't require much other state - again the frame unwinder tests are a perfect example - writing separable tests for that seems great. But in those cases you are generally poking API's and I don't see how coming up with some DSL that is general enough to represent this is any advantage over how those tests are currently written with gtest. > > For example, I assume you are on board at least to some extent with > lldbinline tests. The part of the inline tests that allow you to express the tests you want to run next to the code you stop in is fine till it doesn't work - at which point debugging them becomes a real PITN. But they still use the common currencies of the command line or SB API's to actually perform the tests. I'd be less interested if this was some special purpose language of our own devising. I can't at all see wanting to learn that if the only use it would be to me is for writing tests. > After all, they're simpler than the other style of test. Now, suppose there > were some hypothetical DSL that allowed every test to be an inline test but > still test equally complicated scenarios in half the code. Then suppose it > also ran on a more robust multiprocessing infrastructure than dotest.py. > That's all we're really talking about Thanks for the clarification. Jim > On Wed, May 31, 2017 at 12:06 PM Jim Ingham via lldb-commits > wrote: > Before we get past "it's hard" to "just to do it", it would help me to be > clear first on what you are actually trying to achieve with this proposal. > It's not clear to me what problem are people trying to solve here? If it is > writing tests for the decomposable parts of lldb - like the tests Jason wrote > for the unwinder recently - why was the gtest path not a good way to do this? > If it is rewriting the parts of the testsuite that exercise the debugger on > live targets what would a lit-based suite do that we can't do with the > current testsuite? Or maybe you are thinking of some other good I'm missing? > > Jim > > > > On May 31, 2017, at 10:37 AM, Zachary Turner via Phabricator via > > lldb-commits wrote: > > > > zturner added a comment. > > > > In https://reviews.llvm.org/D32930#767820, @beanz wrote: > > > >> One small comment below. In general I agree with the thoughts here, and I > >> think that this is a huge step forward for testing the debug server > >> components. > >> > >> I also agree with Zachary in principal that it would be nice to come up > >> with lit-based test harnesses for more parts of LLDB, although I'm > >> skeptical about whether or not that is actually the best way to test the > >> debug server pieces. Either way, this is a huge step forward from what we > >> have today, so we should go with it. > > > > > > It would be nice if, at some point, we could move past "It's hard" and
Re: [Lldb-commits] [PATCH] D32930: New framework for lldb client-server communication tests.
This hypothetical DSL could still run SB API statements. I don't
necessarily think it *should*, since what the SB API does and what the
commands do are very different (for example SBTarget.Create() does
something entirely different than "target create", so you're not actually
testing what the debugger does when you create a target, you're only
testing what this API does that is not really used outside of IDE
integration and scripting scenarios. But it could.
I mean in theory you could have a test that looks like:
# input_file: foo.cpp
# int main(int argc, char **argv) {
#return 0;
# }
bp1 = set_breakpoint(foo.cpp:2)
bp2 = set_breakpoint(foo.cpp@_main)
run
expect(bp1.hits == 1)
expect(bp2.hits == 1)
A test consisting of 1 file and 9 lines is a lot easier to understand to me
than a test consisting of a python file, a Makefile, and a source file,
where the python file is a minimum of 20 lines with complicated setup and
initialization steps that are mostly boilerplate.
Note that I'm not necessarily even advocating for something that looks like
the above, it just is the first thing that came to mind.
As a first pass, you could literally just have lit be the thing that walks
the directory tree scanning for test files, then invoking Make and running
the existing .py files. That would already be a big improvement. 1000
lines of python code that many people understand and actively maintain /
improve is better than 2000 lines of python code that nobody understands
and nobody really maintains / improves.
On Wed, May 31, 2017 at 1:45 PM Jim Ingham wrote:
>
> > On May 31, 2017, at 12:22 PM, Zachary Turner wrote:
> >
> > Writing tests that are
> >
> > A) as easy as possible to write and understand
>
> I've never understood why you consider the tests in our test case hard to
> write or understand. Till I added the example directory, you had to go
> search around for an exemplar to work from, which was a bit of a pain. But
> now you copy whichever of the examples you want into place, and you have a
> process stopped somewhere and ready to poke at. You do need to learn some
> SB API's but on that see below...
>
> > B) built on a multiprocessing infrastructure that is battle tested and
> known to not be flaky
>
> We haven't come across failures in the tests caused by the test
> infrastructure in a while now. The multiprocessing flakiness I've seen is
> because you are trying to run many tests in parallel, and some of those
> tests require complex setup and that times out when machines are heavily
> loaded. And for the most part we solve that by running the tests that
> require timeouts serially. That seems a solved problem as well.
>
> > C) Familiar to other llvm developers, so as to not discourage subject
> matter experts from other areas to make relevant improvements to LLDB
> >
>
> If I were a developer coming new to lldb now, and had to write a test
> case, I would have to learn something about how the SB API's work (that and
> a little Python.) The test case part is pretty trivial, especially when
> copying from the sample tests. Learning the SB API's is a bit of a pain,
> but having done that the next time I wanted to write some little breakpoint
> command to do something clever when doing my own debugging, I now have some
> skills at my fingertips that are going to come in really handy.
>
> If we change over the tests so what I did instead was learn some DSL to
> describe lldb tests (which would probably not be entirely trivial, and
> likely not much easier than the SB API's which are very straight-forward) I
> have learned nothing useful outside of writing tests. Not sure how that's
> a benefit.
>
> For the tests that can be decoupled from running processes, that have
> simple data in and out and don't require much other state - again the frame
> unwinder tests are a perfect example - writing separable tests for that
> seems great. But in those cases you are generally poking API's and I don't
> see how coming up with some DSL that is general enough to represent this is
> any advantage over how those tests are currently written with gtest.
>
> >
> > For example, I assume you are on board at least to some extent with
> lldbinline tests.
>
> The part of the inline tests that allow you to express the tests you want
> to run next to the code you stop in is fine till it doesn't work - at which
> point debugging them becomes a real PITN. But they still use the common
> currencies of the command line or SB API's to actually perform the tests.
> I'd be less interested if this was some special purpose language of our own
> devising. I can't at all see wanting to learn that if the only use it
> would be to me is for writing tests.
>
> > After all, they're simpler than the other style of test. Now, suppose
> there were some hypothetical DSL that allowed every test to be an inline
> test but still test equally complicated scenarios in half the code. Then
> suppose it also ran on a more robust multipr
Re: [Lldb-commits] [PATCH] D32930: New framework for lldb client-server communication tests.
And once you start annotating source code with statements, things become
even more concise. For example, you could write the above test:
# input_file: foo.cpp
# int main(int argc, char **argv) { // bp1 = BREAK_HERE
# return 0;// bp2 = BREAK_HERE
# }
# // VERIFY(bp1.hits == 1)
# // VERIFY(bp2.hits == 1)
You can have cross references, conditionals, variables, etc. The only real
limit is your imagination. Anyone can mentally digest this entire test in
about 5 seconds without having to read through multiple files and groking
Python code and an a separate API.
On Wed, May 31, 2017 at 2:02 PM Zachary Turner wrote:
> This hypothetical DSL could still run SB API statements. I don't
> necessarily think it *should*, since what the SB API does and what the
> commands do are very different (for example SBTarget.Create() does
> something entirely different than "target create", so you're not actually
> testing what the debugger does when you create a target, you're only
> testing what this API does that is not really used outside of IDE
> integration and scripting scenarios. But it could.
>
> I mean in theory you could have a test that looks like:
>
> # input_file: foo.cpp
> # int main(int argc, char **argv) {
> #return 0;
> # }
> bp1 = set_breakpoint(foo.cpp:2)
> bp2 = set_breakpoint(foo.cpp@_main)
> run
> expect(bp1.hits == 1)
> expect(bp2.hits == 1)
>
> A test consisting of 1 file and 9 lines is a lot easier to understand to
> me than a test consisting of a python file, a Makefile, and a source file,
> where the python file is a minimum of 20 lines with complicated setup and
> initialization steps that are mostly boilerplate.
>
> Note that I'm not necessarily even advocating for something that looks
> like the above, it just is the first thing that came to mind.
>
> As a first pass, you could literally just have lit be the thing that walks
> the directory tree scanning for test files, then invoking Make and running
> the existing .py files. That would already be a big improvement. 1000
> lines of python code that many people understand and actively maintain /
> improve is better than 2000 lines of python code that nobody understands
> and nobody really maintains / improves.
>
> On Wed, May 31, 2017 at 1:45 PM Jim Ingham wrote:
>
>>
>> > On May 31, 2017, at 12:22 PM, Zachary Turner
>> wrote:
>> >
>> > Writing tests that are
>> >
>> > A) as easy as possible to write and understand
>>
>> I've never understood why you consider the tests in our test case hard to
>> write or understand. Till I added the example directory, you had to go
>> search around for an exemplar to work from, which was a bit of a pain. But
>> now you copy whichever of the examples you want into place, and you have a
>> process stopped somewhere and ready to poke at. You do need to learn some
>> SB API's but on that see below...
>>
>> > B) built on a multiprocessing infrastructure that is battle tested and
>> known to not be flaky
>>
>> We haven't come across failures in the tests caused by the test
>> infrastructure in a while now. The multiprocessing flakiness I've seen is
>> because you are trying to run many tests in parallel, and some of those
>> tests require complex setup and that times out when machines are heavily
>> loaded. And for the most part we solve that by running the tests that
>> require timeouts serially. That seems a solved problem as well.
>>
>> > C) Familiar to other llvm developers, so as to not discourage subject
>> matter experts from other areas to make relevant improvements to LLDB
>> >
>>
>> If I were a developer coming new to lldb now, and had to write a test
>> case, I would have to learn something about how the SB API's work (that and
>> a little Python.) The test case part is pretty trivial, especially when
>> copying from the sample tests. Learning the SB API's is a bit of a pain,
>> but having done that the next time I wanted to write some little breakpoint
>> command to do something clever when doing my own debugging, I now have some
>> skills at my fingertips that are going to come in really handy.
>>
>> If we change over the tests so what I did instead was learn some DSL to
>> describe lldb tests (which would probably not be entirely trivial, and
>> likely not much easier than the SB API's which are very straight-forward) I
>> have learned nothing useful outside of writing tests. Not sure how that's
>> a benefit.
>>
>> For the tests that can be decoupled from running processes, that have
>> simple data in and out and don't require much other state - again the frame
>> unwinder tests are a perfect example - writing separable tests for that
>> seems great. But in those cases you are generally poking API's and I don't
>> see how coming up with some DSL that is general enough to represent this is
>> any advantage over how those tests are currently written with gtest.
>>
>> >
>> > For example, I assume you are on board at least to some extent with
>> lldbinline tests.
>>
>> The part
Re: [Lldb-commits] [PATCH] D32930: New framework for lldb client-server communication tests.
> On May 31, 2017, at 2:02 PM, Zachary Turner wrote:
>
> This hypothetical DSL could still run SB API statements. I don't necessarily
> think it *should*, since what the SB API does and what the commands do are
> very different (for example SBTarget.Create() does something entirely
> different than "target create", so you're not actually testing what the
> debugger does when you create a target, you're only testing what this API
> does that is not really used outside of IDE integration and scripting
> scenarios. But it could.
I don't have hard data, but I am pretty sure the majority of users of lldb use
it through Xcode. We command line users have moral superiority, but I don't
think we have numerical superiority. So in my mind, testing the SB API's
throughly is at least as, if not more, important than testing lldb command line
behavior.
If you are worried about splitting the testing between CLI lldb & the SB API's,
the obvious solution is to rewrite all the lldb commands to use the SB API's.
That is independently a useful project in lldb. It would clean up its surface
area and make the higher layers coherent. It isn't that way now purely for
historical reasons - we needed something we could start to poke at long before
we were ready to design the outward-facing API's. But it leads to code
duplication and introduces an unnecessary bifurcation between "how the debugger
works" from a command line user's perspective and "how the debugger works" as
it presents itself to most of the users of lldb which is not good long term.
>
> I mean in theory you could have a test that looks like:
>
> # input_file: foo.cpp
> # int main(int argc, char **argv) {
> #return 0;
> # }
> bp1 = set_breakpoint(foo.cpp:2)
> bp2 = set_breakpoint(foo.cpp@_main)
> run
> expect(bp1.hits == 1)
> expect(bp2.hits == 1)
>
> A test consisting of 1 file and 9 lines is a lot easier to understand to me
> than a test consisting of a python file, a Makefile, and a source file, where
> the python file is a minimum of 20 lines with complicated setup and
> initialization steps that are mostly boilerplate.
>
That's pretty much the inline tests except for the inline tests you have to do
obvious modifications on a 10 line Python file and for your example I have to
learn an API set "e.g. set_breakpoint" that will do me no good in any other
area of my work.
> Note that I'm not necessarily even advocating for something that looks like
> the above, it just is the first thing that came to mind.
>
> As a first pass, you could literally just have lit be the thing that walks
> the directory tree scanning for test files, then invoking Make and running
> the existing .py files. That would already be a big improvement. 1000 lines
> of python code that many people understand and actively maintain / improve is
> better than 2000 lines of python code that nobody understands and nobody
> really maintains / improves.
When we've talked about using lit before the argument always went "Lit can do
anything, it can certainly do this." Then after a bit "well it doesn't do
exactly this but we can make it do this..." So the "this" would be a bunch of
new lines of new python code that do things most of its current users haven't
needed to do - on top of the 1000 that people understand. I'm unconvinced that
that new code would be trivial or trouble-free, or that the effort to implement
and make it trouble-free it would not be more productively spent fixing up the
extant suite.
But that's just my guess. The free effort of other developers is indeed
free...
As a practical concern, however - since we're relying on the lldb testsuite for
PR's on github, and of course internally since we have product to ship - I
would really prefer that folks doing intrusive work on the testsuite either do
it on a branch, or as a parallel mechanism, rather than trying to splice it
into the extant suite. I'd really rather not have to field test a new
testsuite while needing to get stable & useful results out of it.
Jim
>
> On Wed, May 31, 2017 at 1:45 PM Jim Ingham wrote:
>
> > On May 31, 2017, at 12:22 PM, Zachary Turner wrote:
> >
> > Writing tests that are
> >
> > A) as easy as possible to write and understand
>
> I've never understood why you consider the tests in our test case hard to
> write or understand. Till I added the example directory, you had to go
> search around for an exemplar to work from, which was a bit of a pain. But
> now you copy whichever of the examples you want into place, and you have a
> process stopped somewhere and ready to poke at. You do need to learn some SB
> API's but on that see below...
>
> > B) built on a multiprocessing infrastructure that is battle tested and
> > known to not be flaky
>
> We haven't come across failures in the tests caused by the test
> infrastructure in a while now. The multiprocessing flakiness I've seen is
> because you are trying to run man
[Lldb-commits] [lldb] r304379 - Forgot to mention rewriting CommandObject::DoExecute
Author: jingham Date: Wed May 31 20:05:30 2017 New Revision: 304379 URL: http://llvm.org/viewvc/llvm-project?rev=304379&view=rev Log: Forgot to mention rewriting CommandObject::DoExecute using the SB API's not the lldb_private API's. Modified: lldb/trunk/www/projects.html Modified: lldb/trunk/www/projects.html URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/www/projects.html?rev=304379&r1=304378&r2=304379&view=diff == --- lldb/trunk/www/projects.html (original) +++ lldb/trunk/www/projects.html Wed May 31 20:05:30 2017 @@ -244,6 +244,25 @@ + Reimplement the command interpreter commands using the SB API + +Currently, all the CommandObject::DoExecute methods are implemented +using the lldb_private API's. That generally means that there's code +that gets duplicated between the CommandObject and the SB API that does +roughly the same thing. We would reduce this code duplication, present a +single coherent face to the users of lldb, and keep +ourselves more honest about what we need in the SB API's if we implemented +the CommandObjects::DoExecute methods using the SB API's. + + +BTW, it is only the way it was much easier to develop lldb if it had a functioning +command-line early on. So we did that first, and developed the SB API's when lldb +was more mature. There's no good technical reason to have the commands use the +lldb_private API's. + + + + Documentation and better examples ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
