[Lldb-commits] [PATCH] D12427: [LLDB][MIPS] Aligning code with rL245831
mohit.bhakkad created this revision.
mohit.bhakkad added a reviewer: jaydeep.
mohit.bhakkad added a subscriber: lldb-commits.
mohit.bhakkad set the repository for this revision to rL LLVM.
Resolving build failure due to some changes in rL245831
Repository:
rL LLVM
http://reviews.llvm.org/D12427
Files:
source/Plugins/Process/Linux/NativeProcessLinux.cpp
Index: source/Plugins/Process/Linux/NativeProcessLinux.cpp
===
--- source/Plugins/Process/Linux/NativeProcessLinux.cpp
+++ source/Plugins/Process/Linux/NativeProcessLinux.cpp
@@ -1302,7 +1302,7 @@
log->Printf("NativeProcessLinux::%s() "
"received error while checking for watchpoint hits, "
"pid = %" PRIu64 " error = %s",
-__FUNCTION__, pid, error.AsCString());
+__FUNCTION__, thread.GetID(), error.AsCString());
if (wp_index != LLDB_INVALID_INDEX32)
{
MonitorWatchpoint(thread, wp_index);
Index: source/Plugins/Process/Linux/NativeProcessLinux.cpp
===
--- source/Plugins/Process/Linux/NativeProcessLinux.cpp
+++ source/Plugins/Process/Linux/NativeProcessLinux.cpp
@@ -1302,7 +1302,7 @@
log->Printf("NativeProcessLinux::%s() "
"received error while checking for watchpoint hits, "
"pid = %" PRIu64 " error = %s",
-__FUNCTION__, pid, error.AsCString());
+__FUNCTION__, thread.GetID(), error.AsCString());
if (wp_index != LLDB_INVALID_INDEX32)
{
MonitorWatchpoint(thread, wp_index);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D10761: [LLDB][MIPS] Handle false positives for MIPS hardware watchpoints
mohit.bhakkad abandoned this revision. mohit.bhakkad added a comment. Resolved by http://reviews.llvm.org/rL244864 Repository: rL LLVM http://reviews.llvm.org/D10761 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D12427: [LLDB][MIPS] Aligning code with rL245831
jaydeep accepted this revision. jaydeep added a comment. This revision is now accepted and ready to land. Looks good to me. Repository: rL LLVM http://reviews.llvm.org/D12427 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D12420: Make ProcessGDBRemote get a //copy// of platform Unix signals.
tberghammer added a subscriber: tberghammer. Comment at: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:853 @@ -852,3 +852,3 @@ if (platform_sp && platform_sp->IsConnected()) -SetUnixSignals(platform_sp->GetUnixSignals()); + SetUnixSignals(std::make_shared(platform_sp->GetUnixSignals())); else I think it would be better to change SetUnixSignals to create the copy (or force the client side copy with some magic with rvalue references) because this error will happen almost every time somebody calls SetUnixSignals. http://reviews.llvm.org/D12420 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D12427: [LLDB][MIPS] Aligning code with rL245831
This revision was automatically updated to reflect the committed changes.
Closed by commit rL246293: [LLDB][MIPS] Aligning code with rL245831 (authored
by mohit.bhakkad).
Changed prior to commit:
http://reviews.llvm.org/D12427?vs=33399&id=33415#toc
Repository:
rL LLVM
http://reviews.llvm.org/D12427
Files:
lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp
Index: lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp
===
--- lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp
+++ lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp
@@ -1302,7 +1302,7 @@
log->Printf("NativeProcessLinux::%s() "
"received error while checking for watchpoint hits, "
"pid = %" PRIu64 " error = %s",
-__FUNCTION__, pid, error.AsCString());
+__FUNCTION__, thread.GetID(), error.AsCString());
if (wp_index != LLDB_INVALID_INDEX32)
{
MonitorWatchpoint(thread, wp_index);
Index: lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp
===
--- lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp
+++ lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp
@@ -1302,7 +1302,7 @@
log->Printf("NativeProcessLinux::%s() "
"received error while checking for watchpoint hits, "
"pid = %" PRIu64 " error = %s",
-__FUNCTION__, pid, error.AsCString());
+__FUNCTION__, thread.GetID(), error.AsCString());
if (wp_index != LLDB_INVALID_INDEX32)
{
MonitorWatchpoint(thread, wp_index);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r246293 - [LLDB][MIPS] Aligning code with rL245831
Author: mohit.bhakkad
Date: Fri Aug 28 07:08:26 2015
New Revision: 246293
URL: http://llvm.org/viewvc/llvm-project?rev=246293&view=rev
Log:
[LLDB][MIPS] Aligning code with rL245831
Reviewers: jaydeep
Subscribers: lldb-commits.
Differential Revision: http://reviews.llvm.org/D12427
Modified:
lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp
Modified: lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp?rev=246293&r1=246292&r2=246293&view=diff
==
--- lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp (original)
+++ lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp Fri Aug 28
07:08:26 2015
@@ -1302,7 +1302,7 @@ NativeProcessLinux::MonitorSIGTRAP(const
log->Printf("NativeProcessLinux::%s() "
"received error while checking for watchpoint hits, "
"pid = %" PRIu64 " error = %s",
-__FUNCTION__, pid, error.AsCString());
+__FUNCTION__, thread.GetID(), error.AsCString());
if (wp_index != LLDB_INVALID_INDEX32)
{
MonitorWatchpoint(thread, wp_index);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r246294 - Avoid usage of F_DUPFD_CLOEXEC where not available (e.g. kfreebsd*)
Author: sylvestre
Date: Fri Aug 28 07:24:07 2015
New Revision: 246294
URL: http://llvm.org/viewvc/llvm-project?rev=246294&view=rev
Log:
Avoid usage of F_DUPFD_CLOEXEC where not available (e.g. kfreebsd*)
Summary:
kfreebsd doesn't have F_DUPFD_CLOEXEC, so use it conditionally.
Author: Emilio Pozuelo Monfort
Author: Petr Salinger
Author: Gianfranco Costamagna
Reviewers: emaste
Subscribers: emaste
Differential Revision: http://reviews.llvm.org/D12429
Modified:
lldb/trunk/source/Plugins/Process/FreeBSD/ProcessFreeBSD.cpp
Modified: lldb/trunk/source/Plugins/Process/FreeBSD/ProcessFreeBSD.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/FreeBSD/ProcessFreeBSD.cpp?rev=246294&r1=246293&r2=246294&view=diff
==
--- lldb/trunk/source/Plugins/Process/FreeBSD/ProcessFreeBSD.cpp (original)
+++ lldb/trunk/source/Plugins/Process/FreeBSD/ProcessFreeBSD.cpp Fri Aug 28
07:24:07 2015
@@ -462,11 +462,25 @@ ProcessFreeBSD::DoLaunch (Module *module
int terminal = m_monitor->GetTerminalFD();
if (terminal >= 0) {
// The reader thread will close the file descriptor when done, so we
pass it a copy.
+#ifdef F_DUPFD_CLOEXEC
int stdio = fcntl(terminal, F_DUPFD_CLOEXEC, 0);
if (stdio == -1) {
error.SetErrorToErrno();
return error;
}
+#else
+// Special case when F_DUPFD_CLOEXEC does not exist (Debian kFreeBSD)
+int stdio = fcntl(terminal, F_DUPFD, 0);
+if (stdio == -1) {
+error.SetErrorToErrno();
+return error;
+}
+stdio = fcntl(terminal, F_SETFD, FD_CLOEXEC);
+if (stdio == -1) {
+error.SetErrorToErrno();
+return error;
+}
+#endif
SetSTDIOFileDescriptor(stdio);
}
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D12416: Parse dotest.py/dosep.py output, providing general stats and skip reason breakdown by counts.
labath added a subscriber: labath. labath added a comment. Same question as Zachary. This sounds like a very useful feature and it would be nice to have it integrated into the current test system, instead of making another layer on top of that (it's bad enough we have dosep and dotest already). Parsing the output like this is likely to break the script due to random changes in the other parts. http://reviews.llvm.org/D12416 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D12420: Make ProcessGDBRemote get a //copy// of platform Unix signals.
labath added a subscriber: labath. Comment at: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:853 @@ -852,3 +852,3 @@ if (platform_sp && platform_sp->IsConnected()) -SetUnixSignals(platform_sp->GetUnixSignals()); + SetUnixSignals(std::make_shared(platform_sp->GetUnixSignals())); else tberghammer wrote: > I think it would be better to change SetUnixSignals to create the copy (or > force the client side copy with some magic with rvalue references) because > this error will happen almost every time somebody calls SetUnixSignals. If the platform is supposed to hold signals, which are not to be changed by its users, I would have it hand out `std::shared_ptr` (or even `const UnixSignals &` ?). Then the users will be forced to make a copy if they want to do anything to them. http://reviews.llvm.org/D12420 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r246302 - Differential Review: http://reviews.llvm.org/D12363
Author: amccarth
Date: Fri Aug 28 09:42:03 2015
New Revision: 246302
URL: http://llvm.org/viewvc/llvm-project?rev=246302&view=rev
Log:
Differential Review: http://reviews.llvm.org/D12363
Modified:
lldb/trunk/source/Plugins/Process/Windows/MiniDump/ProcessWinMiniDump.cpp
lldb/trunk/source/Plugins/Process/Windows/MiniDump/ProcessWinMiniDump.h
Modified:
lldb/trunk/source/Plugins/Process/Windows/MiniDump/ProcessWinMiniDump.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Windows/MiniDump/ProcessWinMiniDump.cpp?rev=246302&r1=246301&r2=246302&view=diff
==
--- lldb/trunk/source/Plugins/Process/Windows/MiniDump/ProcessWinMiniDump.cpp
(original)
+++ lldb/trunk/source/Plugins/Process/Windows/MiniDump/ProcessWinMiniDump.cpp
Fri Aug 28 09:42:03 2015
@@ -30,6 +30,7 @@
#include "lldb/Target/UnixSignals.h"
#include "llvm/Support/Format.h"
#include "llvm/Support/raw_ostream.h"
+#include "llvm/Support/ConvertUTF.h"
#include "Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.h"
#include "ExceptionRecord.h"
@@ -37,6 +38,36 @@
using namespace lldb_private;
+namespace
+{
+
+// Getting a string out of a mini dump is a chore. You're usually given a
+// relative virtual address (RVA), which points to a counted string that's in
+// Windows Unicode (UTF-16). This wrapper handles all the redirection and
+// returns a UTF-8 copy of the string.
+std::string
+GetMiniDumpString(const void *base_addr, const RVA rva)
+{
+std::string result;
+if (!base_addr)
+{
+return result;
+}
+auto md_string = reinterpret_cast(static_cast(base_addr) + rva);
+auto source_start = reinterpret_cast(md_string->Buffer);
+const auto source_length = ::wcslen(md_string->Buffer);
+const auto source_end = source_start + source_length;
+result.resize(4*source_length); // worst case length
+auto result_start = reinterpret_cast(&result[0]);
+const auto result_end = result_start + result.size();
+ConvertUTF16toUTF8(&source_start, source_end, &result_start, result_end,
strictConversion);
+const auto result_size = std::distance(reinterpret_cast(&result[0]), result_start);
+result.resize(result_size); // shrink to actual length
+return result;
+}
+
+} // anonymous namespace
+
// Encapsulates the private data for ProcessWinMiniDump.
// TODO(amccarth): Determine if we need a mutex for access.
class ProcessWinMiniDump::Data
@@ -133,8 +164,7 @@ ProcessWinMiniDump::DoLoadCore()
}
m_target.SetArchitecture(DetermineArchitecture());
-// TODO(amccarth): Build the module list.
-
+ReadModuleList();
ReadExceptionRecord();
return error;
@@ -341,6 +371,31 @@ ProcessWinMiniDump::ReadExceptionRecord(
}
}
+void
+ProcessWinMiniDump::ReadModuleList() {
+size_t size = 0;
+auto module_list_ptr =
static_cast(FindDumpStream(ModuleListStream, &size));
+if (!module_list_ptr || module_list_ptr->NumberOfModules == 0)
+{
+return;
+}
+
+for (ULONG32 i = 0; i < module_list_ptr->NumberOfModules; ++i)
+{
+const auto &module = module_list_ptr->Modules[i];
+const auto file_name = GetMiniDumpString(m_data_up->m_base_addr,
module.ModuleNameRva);
+ModuleSpec module_spec = FileSpec(file_name, true);
+
+lldb::ModuleSP module_sp = GetTarget().GetSharedModule(module_spec);
+if (!module_sp)
+{
+continue;
+}
+bool load_addr_changed = false;
+module_sp->SetLoadAddress(GetTarget(), module.BaseOfImage, false,
load_addr_changed);
+}
+}
+
void *
ProcessWinMiniDump::FindDumpStream(unsigned stream_number, size_t *size_out) {
void *stream = nullptr;
Modified:
lldb/trunk/source/Plugins/Process/Windows/MiniDump/ProcessWinMiniDump.h
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Windows/MiniDump/ProcessWinMiniDump.h?rev=246302&r1=246301&r2=246302&view=diff
==
--- lldb/trunk/source/Plugins/Process/Windows/MiniDump/ProcessWinMiniDump.h
(original)
+++ lldb/trunk/source/Plugins/Process/Windows/MiniDump/ProcessWinMiniDump.h Fri
Aug 28 09:42:03 2015
@@ -100,6 +100,9 @@ private:
void
ReadExceptionRecord();
+void
+ReadModuleList();
+
// A thin wrapper around WinAPI's MiniDumpReadDumpStream to avoid redundant
// checks. If there's a failure (e.g., if the requested stream doesn't
exist),
// the function returns nullptr and sets *size_out to 0.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D12360: RenderScript pending kernel breakpoints.
EwanCrawford updated this revision to Diff 33425.
EwanCrawford added a comment.
Thanks for taking a look.
Previously we just tried to find a way to leverage the existing
BreakpointResovlerName, and SearchFilter.
However your suggestion to create a new BreakpointResolver works a lot better,
and indeed removes the need for the extra commands.
This updated patch impements the RS breakpoint resolver, allowing us to set
pending breakpoints on future kernels.
Repository:
rL LLVM
http://reviews.llvm.org/D12360
Files:
source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.h
Index: source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.h
===
--- source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.h
+++ source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.h
@@ -22,6 +22,9 @@
namespace lldb_private
{
+namespace lldb_renderscript
+{
+
typedef uint32_t RSSlot;
class RSModuleDescriptor;
struct RSGlobalDescriptor;
@@ -31,8 +34,54 @@
typedef std::shared_ptr RSGlobalDescriptorSP;
typedef std::shared_ptr RSKernelDescriptorSP;
+// Breakpoint Resolvers decide where a breakpoint is placed,
+// so having our own allows us to limit the search scope to RS kernel modules.
+// As well as check for .expand kernels as a fallback.
+class RSBreakpointResolver : public BreakpointResolver
+{
+ public:
+RSBreakpointResolver(Breakpoint *bkpt, ConstString name):
+ BreakpointResolver (bkpt, BreakpointResolver::NameResolver),
+ m_kernel_name(name)
+{
+}
+void
+GetDescription(Stream *strm) override
+{
+if (strm)
+strm->Printf("RenderScript kernel breakpoint for '%s'", m_kernel_name.AsCString());
+}
+
+void
+Dump(Stream *s) const override
+{
+}
+
+Searcher::CallbackReturn
+SearchCallback(SearchFilter &filter,
+ SymbolContext &context,
+ Address *addr,
+ bool containing) override;
+
+Searcher::Depth
+GetDepth() override
+{
+return Searcher::eDepthModule;
+}
+
+lldb::BreakpointResolverSP
+CopyForBreakpoint(Breakpoint &breakpoint) override
+{
+lldb::BreakpointResolverSP ret_sp(new RSBreakpointResolver(&breakpoint, m_kernel_name));
+return ret_sp;
+}
+
+ protected:
+ConstString m_kernel_name;
+};
+
struct RSKernelDescriptor
{
public:
@@ -86,6 +135,8 @@
std::string m_resname;
};
+} // end lldb_renderscript namespace
+
class RenderScriptRuntime : public lldb_private::CPPLanguageRuntime
{
public:
@@ -147,7 +198,7 @@
void DumpKernels(Stream &strm) const;
-void AttemptBreakpointAtKernelName(Stream &strm, const char *name, Error &error);
+void AttemptBreakpointAtKernelName(Stream &strm, const char *name, Error &error, lldb::TargetSP target);
void Status(Stream &strm) const;
@@ -163,7 +214,7 @@
protected:
-void FixupScriptDetails(RSModuleDescriptorSP rsmodule_sp);
+void FixupScriptDetails(lldb_renderscript::RSModuleDescriptorSP rsmodule_sp);
void LoadRuntimeHooks(lldb::ModuleSP module, ModuleKind kind);
@@ -200,10 +251,10 @@
lldb::ModuleSP m_libRS;
lldb::ModuleSP m_libRSDriver;
lldb::ModuleSP m_libRSCpuRef;
-std::vector m_rsmodules;
+std::vector m_rsmodules;
std::vector m_scripts;
-std::map m_scriptMappings;
+std::map m_scriptMappings;
std::map m_runtimeHooks;
bool m_initiated;
Index: source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
===
--- source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
+++ source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
@@ -30,6 +30,7 @@
using namespace lldb;
using namespace lldb_private;
+using namespace lldb_renderscript;
//--
// Static Functions
@@ -44,6 +45,47 @@
return NULL;
}
+// Callback with a module to search for matching symbols.
+// We first check that the module contains RS kernels.
+// Then look for a symbol which matches our kernel name.
+// The breakpoint address is finally set using the address of this symbol.
+Searcher::CallbackReturn
+RSBreakpointResolver::SearchCallback(SearchFilter &filter,
+ SymbolContext &context,
+ Address*,
+ bool)
+{
+ModuleSP module = context.module_sp;
+
+if (!module)
+return Searcher::eCallbackReturnContinue;
+
+// Is this a module containing r
Re: [Lldb-commits] [PATCH] D12416: Parse dotest.py/dosep.py output, providing general stats and skip reason breakdown by counts.
tfiala added a comment. In http://reviews.llvm.org/D12416#235112, @labath wrote: > Same question as Zachary. This sounds like a very useful feature and it would > be nice to have it integrated into the current test system, Well, I'd be happy to do that in dosep.py. There are a couple of challenges: 1. It requires information that is only provided when parsable output mode is invoked in dotest.py. So it mandates a particular output style that is not necessarily what everyone wants to see. 2. Does everyone always want this output from dosep.py? I agree with Zachary's earlier comment that having a bunch of options is more painful than not having them, particularly for things that everyone wants. But this can be verbose. > instead of making another layer on top of that (it's bad enough we have dosep > and dotest already). The existence of those two is at least partly due to the organic nature in which they developed. Some energy could be put into that to wrap them together unless dosep.py has grown wildly since I last touched it. > Parsing the output like this is likely to break the script due to random > changes in the other parts. That probably cuts both ways. Changing output of a test script (the output I'm adding) seems quite plausible to break other scripts that parse the output of the test infrastructure. All that aside, if we did want to move this more firmly into dotest/dosep, it currently requires the data to be generated from dotest and collated in dosep. So it could change like so: 1. Dosep.py could be modified to collect the skip reason data if it detects its presence, and can collate and display if the skip reason info is there. 2. If the data isn't present, dosep.py can just skip presenting the skip reason report piece. 3. We can add a skip reason report suppression flag that skips printing out the skip reason in dosep.py if for some reason somebody doesn't want to see it. By default it would print skip reason tabulations when available unless this flag was specified. I see that as an optional task pending anybody really wanting/needing it. Then the user requirement becomes "add so and such an option to your dotest.py invocation options and you'll get skip counts in your dosep.py output." Seem reasonable? http://reviews.llvm.org/D12416 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D12416: Parse dotest.py/dosep.py output, providing general stats and skip reason breakdown by counts.
tfiala added a comment. > Then the user requirement becomes "add so and such an option to your > dotest.py invocation options and you'll get skip counts in your dosep.py > output." And the "add so and such an option" may already be there for many. Not a new dotest.py option, just to be clear. It's whatever turns on parseable output. http://reviews.llvm.org/D12416 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D12416: Parse dotest.py/dosep.py output, providing general stats and skip reason breakdown by counts.
labath added a comment. In http://reviews.llvm.org/D12416#235166, @tfiala wrote: > > instead of making another layer on top of that (it's bad enough we have > > dosep and dotest already). > > > The existence of those two is at least partly due to the organic nature in > which they developed. I understand that. I tried to merge them when I initially started on the project, but failed miserably. One day we should still go ahead and do that, but there are more pressing issues now. I am just trying to make sure we don't end up with three things to merge :). > All that aside, if we did want to move this more firmly into dotest/dosep, it > currently requires the data to be generated from dotest and collated in > dosep. So it could change like so: > > 1. Dosep.py could be modified to collect the skip reason data if it detects > its presence, and can collate and display if the skip reason info is there. > 2. If the data isn't present, dosep.py can just skip presenting the skip > reason report piece. > 3. We can add a skip reason report suppression flag that skips printing out > the skip reason in dosep.py if for some reason somebody doesn't want to see > it. By default it would print skip reason tabulations when available unless > this flag was specified. I see that as an optional task pending anybody > really wanting/needing it. > > Then the user requirement becomes "add so and such an option to your > dotest.py invocation options and you'll get skip counts in your dosep.py > output." > > Seem reasonable? Sounds great to me. :) http://reviews.llvm.org/D12416 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D12416: Parse dotest.py/dosep.py output, providing general stats and skip reason breakdown by counts.
clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. I would like to see a few things as long as we are chaning things: - get rid of dosep.py and just put the functionality into dotest.py if possible, it can just spawn itself in different modes - if we can't get rid of dosep.py lets make all options that work for dotest.py work in dosep.py - we should never have to launch dosep.py with any arguments in order for it to run correctly - add options for formatted (JSON) output and have dosep.py (or dotest.py if we move dosep.py functionality over there) enable that option when spawning subprocesses. We can then parse the output easily in the tools that spawns the sub processes and we can expand the format as needed. This would also allow for buildbots to translate the results of the testing into their preferred format for correct display for the buildbot web interfaces. - all functionality should exist in dotest.py (no external reporting python scripts...) http://reviews.llvm.org/D12416 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D12416: Parse dotest.py/dosep.py output, providing general stats and skip reason breakdown by counts.
tfiala added a comment. > > Seem reasonable? > > > Sounds great to me. :) Okay, I'll give that a shot and we can see what that looks like. http://reviews.llvm.org/D12416 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D12416: Parse dotest.py/dosep.py output, providing general stats and skip reason breakdown by counts.
tfiala added a comment. In http://reviews.llvm.org/D12416#235191, @clayborg wrote: > I would like to see a few things as long as we are chaning things: > > - get rid of dosep.py and just put the functionality into dotest.py if > possible, it can just spawn itself in different modes I'm all for doing that. It's a bit of a bigger job, though. I'd like to tackle that separate from getting the skip reason tabulation in (and I'm happy to get skip tabulation into dosep.py now). > - if we can't get rid of dosep.py lets make all options that work for > dotest.py work in dosep.py That makes sense. We should be able to build the options for dotest.py without requiring them to be passed through, and would aid in a transition to a dotest.py-only world. > - we should never have to launch dosep.py with any arguments in order for it > to run correctly Hmm, would you say that you can currently run dotest.py without any arguments and it does an admirable job? I think at the very least we need to specify the architecture(s) we're testing, and the compiler specification on Linux is pretty critical per some other bugs we've discussed (not resolving symlinks before making decisions based on compiler name, for example). I'd shoot for saying dosep.py should match dotest.py in terms of what it needs as arguments, and independently tackling the items in dotest.py that currently require options to make them smarter. (i.e. the goal is command-line parity on dosep.py/dotest.py, only adding args to dosep.py for additional functionality that is related to the parallel test running). > - add options for formatted (JSON) output and have dosep.py (or dotest.py if > we move dosep.py functionality over there) enable that option when spawning > subprocesses. We can then parse the output easily in the tools that spawns > the sub processes and we can expand the format as needed. This would also > allow for buildbots to translate the results of the testing into their > preferred format for correct display for the buildbot web interfaces. I like that. This could be done as a separate task. > - all functionality should exist in dotest.py (no external reporting python > scripts...) I'm on board with that. For now I'd like to just tackle the part of getting the reporting in dosep.py. I'm happy to take a crack at both (1) merging the command line options so dosep.py no longer requires the pass-through dotest.py options, and instead takes the options directly on the dosep.py command line, and (2) trying to eliminate dosep.py altogether. (And I'd like to see our test harness get some more tests for itself!() These are great longer-term roadmap items for the test infrastructure IMHO. -Todd http://reviews.llvm.org/D12416 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D12416: Parse dotest.py/dosep.py output, providing general stats and skip reason breakdown by counts.
On Fri, Aug 28, 2015 at 8:41 AM Todd Fiala wrote: > tfiala added a comment. > > In http://reviews.llvm.org/D12416#235112, @labath wrote: > > > Same question as Zachary. This sounds like a very useful feature and it > would be nice to have it integrated into the current test system, > > > Well, I'd be happy to do that in dosep.py. There are a couple of > challenges: > > 1. It requires information that is only provided when parsable output mode > is invoked in dotest.py. So it mandates a particular output style that is > not necessarily what everyone wants to see. > One solution to this that I've thought of is to have a way to launch dotest in IPC mode. Piping stdin and stdout is slow because it results in having to parse text which was not printed with fast consumption in mind. If dosep could could launch dotest with an option like --pipe=dotest_pipe_instance_1 or something, then they could communicate over that pipe in a structured format like json that is easy to parse. > > Parsing the output like this is likely to break the script due to random > changes in the other parts. > > > That probably cuts both ways. Changing output of a test script (the > output I'm adding) seems quite plausible to break other scripts that parse > the output of the test infrastructure. > The pipe issue would solve that, but another way to at least alleviate the pain associated with that is to agree that we print all summary information at the end, and we try to keep the summary information as stable as possible. We still might have to change stuff sometimes, but the benefit of doing things this way is that we can then agree that everything EXCEPT the summary information can change for any reason at all, or no reason at all, and it won't break anyone. The summary informatino could be printed in a nice structured format with 1 result on each line, so that new information could simply be appended, and the only time you'd break something is if you deleted a statistic. Thoughts? It might be more work to do things this way, but I kind of feel like the complexity of dosep and dotest is getting to the point where it might be worth considering putting some extra time in to think about things like this. ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D12420: Make ProcessGDBRemote get a //copy// of platform Unix signals.
chaoren updated this revision to Diff 33460.
chaoren added a comment.
- Force client side copy.
http://reviews.llvm.org/D12420
Files:
include/lldb/Target/Process.h
source/Plugins/Process/elf-core/ProcessElfCore.cpp
source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
source/Target/Process.cpp
Index: source/Target/Process.cpp
===
--- source/Target/Process.cpp
+++ source/Target/Process.cpp
@@ -4120,7 +4120,7 @@
}
void
-Process::SetUnixSignals (const UnixSignalsSP &signals_sp)
+Process::SetUnixSignals(UnixSignalsSP &&signals_sp)
{
assert (signals_sp && "null signals_sp");
m_unix_signals_sp = signals_sp;
Index: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -850,7 +850,7 @@
{
PlatformSP platform_sp = GetTarget().GetPlatform();
if (platform_sp && platform_sp->IsConnected())
-SetUnixSignals(platform_sp->GetUnixSignals());
+
SetUnixSignals(std::make_shared(platform_sp->GetUnixSignals()));
else
SetUnixSignals(UnixSignals::Create(GetTarget().GetArchitecture()));
}
Index: source/Plugins/Process/elf-core/ProcessElfCore.cpp
===
--- source/Plugins/Process/elf-core/ProcessElfCore.cpp
+++ source/Plugins/Process/elf-core/ProcessElfCore.cpp
@@ -352,8 +352,7 @@
m_thread_list.Clear();
m_os = llvm::Triple::UnknownOS;
-static const auto s_default_unix_signals_sp =
std::make_shared();
-SetUnixSignals(s_default_unix_signals_sp);
+SetUnixSignals(std::make_shared());
}
void
Index: include/lldb/Target/Process.h
===
--- include/lldb/Target/Process.h
+++ include/lldb/Target/Process.h
@@ -1432,7 +1432,7 @@
Signal (int signal);
void
-SetUnixSignals(const lldb::UnixSignalsSP &signals_sp);
+SetUnixSignals(lldb::UnixSignalsSP &&signals_sp);
const lldb::UnixSignalsSP &
GetUnixSignals();
Index: source/Target/Process.cpp
===
--- source/Target/Process.cpp
+++ source/Target/Process.cpp
@@ -4120,7 +4120,7 @@
}
void
-Process::SetUnixSignals (const UnixSignalsSP &signals_sp)
+Process::SetUnixSignals(UnixSignalsSP &&signals_sp)
{
assert (signals_sp && "null signals_sp");
m_unix_signals_sp = signals_sp;
Index: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -850,7 +850,7 @@
{
PlatformSP platform_sp = GetTarget().GetPlatform();
if (platform_sp && platform_sp->IsConnected())
-SetUnixSignals(platform_sp->GetUnixSignals());
+SetUnixSignals(std::make_shared(platform_sp->GetUnixSignals()));
else
SetUnixSignals(UnixSignals::Create(GetTarget().GetArchitecture()));
}
Index: source/Plugins/Process/elf-core/ProcessElfCore.cpp
===
--- source/Plugins/Process/elf-core/ProcessElfCore.cpp
+++ source/Plugins/Process/elf-core/ProcessElfCore.cpp
@@ -352,8 +352,7 @@
m_thread_list.Clear();
m_os = llvm::Triple::UnknownOS;
-static const auto s_default_unix_signals_sp = std::make_shared();
-SetUnixSignals(s_default_unix_signals_sp);
+SetUnixSignals(std::make_shared());
}
void
Index: include/lldb/Target/Process.h
===
--- include/lldb/Target/Process.h
+++ include/lldb/Target/Process.h
@@ -1432,7 +1432,7 @@
Signal (int signal);
void
-SetUnixSignals(const lldb::UnixSignalsSP &signals_sp);
+SetUnixSignals(lldb::UnixSignalsSP &&signals_sp);
const lldb::UnixSignalsSP &
GetUnixSignals();
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D12420: Make ProcessGDBRemote get a //copy// of platform Unix signals.
chaoren marked an inline comment as done. Comment at: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:853 @@ -852,3 +852,3 @@ if (platform_sp && platform_sp->IsConnected()) -SetUnixSignals(platform_sp->GetUnixSignals()); + SetUnixSignals(std::make_shared(platform_sp->GetUnixSignals())); else labath wrote: > tberghammer wrote: > > I think it would be better to change SetUnixSignals to create the copy (or > > force the client side copy with some magic with rvalue references) because > > this error will happen almost every time somebody calls SetUnixSignals. > If the platform is supposed to hold signals, which are not to be changed by > its users, I would have it hand out `std::shared_ptr` (or > even `const UnixSignals &` ?). Then the users will be forced to make a copy > if they want to do anything to them. > If the platform is supposed to hold signals, which are not to be changed by > its users I think in my original discussion with Greg, we decided that it would be a good idea if we allow setting suppress/stop/notify for the platform signal set, and will be inherited by any process signal set created from it. http://reviews.llvm.org/D12420 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] LLVM buildmaster will be restarted tonight
Hello everyone, LLVM buildmaster will be restarted after 6 PM Pacific time today. Thanks Galina ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D12420: Make ProcessGDBRemote get a //copy// of platform Unix signals.
chaoren updated this revision to Diff 33485.
chaoren added a comment.
- ProcessGDBRemote should automatically get a copy of GDBRemoteSignals.
http://reviews.llvm.org/D12420
Files:
include/lldb/Target/Process.h
source/Plugins/Process/elf-core/ProcessElfCore.cpp
source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
source/Target/Process.cpp
Index: source/Target/Process.cpp
===
--- source/Target/Process.cpp
+++ source/Target/Process.cpp
@@ -4120,7 +4120,7 @@
}
void
-Process::SetUnixSignals (const UnixSignalsSP &signals_sp)
+Process::SetUnixSignals(UnixSignalsSP &&signals_sp)
{
assert (signals_sp && "null signals_sp");
m_unix_signals_sp = signals_sp;
Index: source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
===
--- source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
+++ source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
@@ -152,6 +152,9 @@
void
RefreshStateAfterStop() override;
+void
+SetUnixSignals(const lldb::UnixSignalsSP &signals_sp);
+
//--
// Process Queries
//--
Index: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -2904,6 +2904,12 @@
}
}
+void
+ProcessGDBRemote::SetUnixSignals(const UnixSignalsSP &signals_sp)
+{
+Process::SetUnixSignals(std::make_shared(signals_sp));
+}
+
//--
// Process Queries
//--
Index: source/Plugins/Process/elf-core/ProcessElfCore.cpp
===
--- source/Plugins/Process/elf-core/ProcessElfCore.cpp
+++ source/Plugins/Process/elf-core/ProcessElfCore.cpp
@@ -352,8 +352,7 @@
m_thread_list.Clear();
m_os = llvm::Triple::UnknownOS;
-static const auto s_default_unix_signals_sp =
std::make_shared();
-SetUnixSignals(s_default_unix_signals_sp);
+SetUnixSignals(std::make_shared());
}
void
Index: include/lldb/Target/Process.h
===
--- include/lldb/Target/Process.h
+++ include/lldb/Target/Process.h
@@ -1432,7 +1432,7 @@
Signal (int signal);
void
-SetUnixSignals(const lldb::UnixSignalsSP &signals_sp);
+SetUnixSignals(lldb::UnixSignalsSP &&signals_sp);
const lldb::UnixSignalsSP &
GetUnixSignals();
Index: source/Target/Process.cpp
===
--- source/Target/Process.cpp
+++ source/Target/Process.cpp
@@ -4120,7 +4120,7 @@
}
void
-Process::SetUnixSignals (const UnixSignalsSP &signals_sp)
+Process::SetUnixSignals(UnixSignalsSP &&signals_sp)
{
assert (signals_sp && "null signals_sp");
m_unix_signals_sp = signals_sp;
Index: source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
===
--- source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
+++ source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
@@ -152,6 +152,9 @@
void
RefreshStateAfterStop() override;
+void
+SetUnixSignals(const lldb::UnixSignalsSP &signals_sp);
+
//--
// Process Queries
//--
Index: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -2904,6 +2904,12 @@
}
}
+void
+ProcessGDBRemote::SetUnixSignals(const UnixSignalsSP &signals_sp)
+{
+Process::SetUnixSignals(std::make_shared(signals_sp));
+}
+
//--
// Process Queries
//--
Index: source/Plugins/Process/elf-core/ProcessElfCore.cpp
===
--- source/Plugins/Process/elf-core/ProcessElfCore.cpp
+++ source/Plugins/Process/elf-core/ProcessElfCore.cpp
@@ -352,8 +352,7 @@
m_thread_list.Clear();
m_os = llvm::Triple::UnknownOS;
-static const auto s_default_unix_signals_sp = std::make_shared();
-SetUnixSignals(s_default_unix_signals_sp);
+SetUnixSignals(std::make_shared());
}
void
Index: include/lldb/Target/Process.h
===
--- include/lldb/Ta
