Re: [Lldb-commits] [PATCH] D13124: test runner: switch to pure-Python timeout mechanism
tfiala updated this revision to Diff 35829. tfiala added a comment. Tests added. Ready for review. The change now has two levels of terminate: - soft terminate, which uses a signal or process control mechanism to tell the process to end. It optionally can use a mechanism that triggers a core dump / crashlog / minidump if supported by the underlying platform. RIght now, only Linux of the Posix-y platforms requests the core (via SIGQUIT). I will plumb through an option for that later (as mentioned in prior comments in this review). But I'm now at max time I can spend on this right now and this'll need to do. There is also a timeout on how long the process driver will allow the soft timeout to take to wrap up. This is relevant for core dumps that might have a huge amount of memory to dump to disk. It defaults to 10 seconds, but can be overridden by an environment variable, LLDB_TEST_SOFT_TERMINATE_TIMEOUT. That env var will be converted to a float from whatever text is in it. It represents the number of seconds within which any soft terminate option must wrap up. If a process is actively blocking/ignoring that termination signal, then this represents the total time that will be "wasted" waiting to figure this out. - hard terminate: after the soft terminate attempt is made, plus the soft terminate timeout, then if the process is still not eliminated, the hard terminate mechanism will be used. On Posix-y systems, this is a SIGKILL. @zturner, you will want to run the tests in lldb/test/test_runner/test. Do that with: cd lldb/test/test_runner/test python process_control_tests.py In some cases where docs were clear, I did the bit that seemed to make sense on the Windows side. In others, I left it for you to fill in. Feel free to skip some/any tests that don't make sense if, for example, you only end up wanting to support one level of killing on Windows. (We can probably work in a "has soft terminate mechanism" query into the process_control.ProcessHelper class, which has per-platform derived classes, if we want to go that way). Also note I will start migrating test infrastructure bits into the lldb/test/test_runner/lib directory over time. The two new files from this change are going in there. I'd like to get this in sooner than later as it resolves numerous hangs on OS X where we don't have a timeout guard. I've tried it on Linux (4, 8 and 24 core machines) as well as OS X (8 core MBPs), and have run the process_control_tests on them without issue. http://reviews.llvm.org/D13124 Files: test/dosep.py test/test_runner/README.txt test/test_runner/lib/lldb_utils.py test/test_runner/lib/process_control.py test/test_runner/test/inferior.py test/test_runner/test/process_control_tests.py Index: test/test_runner/test/process_control_tests.py === --- /dev/null +++ test/test_runner/test/process_control_tests.py @@ -0,0 +1,198 @@ +#!/usr/bin/env python +""" +The LLVM Compiler Infrastructure + +This file is distributed under the University of Illinois Open Source +License. See LICENSE.TXT for details. + +Provides classes used by the test results reporting infrastructure +within the LLDB test suite. + + +Tests the process_control module. +""" + +# System imports. +import os +import platform +import unittest +import sys +import threading + +# Add lib dir to pythonpath +sys.path.append(os.path.join(os.path.dirname(__file__), '..', 'lib')) + +# Our imports. +import process_control + + +class TestInferiorDriver(process_control.ProcessDriver): +def __init__(self, soft_terminate_timeout=None): +super(TestInferiorDriver, self).__init__( +soft_terminate_timeout=soft_terminate_timeout) +self.started_event = threading.Event() +self.started_event.clear() + +self.completed_event = threading.Event() +self.completed_event.clear() + +self.was_timeout = False +self.returncode = None +self.output = None + +def write(self, content): +# We'll swallow this to keep tests non-noisy. +# Uncomment the following line if you want to see it. +# sys.stdout.write(content) +pass + +def on_process_started(self): +self.started_event.set() + +def on_process_exited(self, command, output, was_timeout, exit_status): +self.returncode = exit_status +self.was_timeout = was_timeout +self.output = output +self.returncode = exit_status +self.completed_event.set() + + +class ProcessControlTests(unittest.TestCase): +@classmethod +def _suppress_soft_terminate(cls, command): +if platform.system() == 'nt': +# Add whatever is needed to the command line to +# instruct inferior.py to ignore soft terminate +# mechanisms. +pass +else: +helper = process_control.ProcessHelper.process_helper() +
Re: [Lldb-commits] [PATCH] D13201: Fix segmentation fault in lldb_private::Symbols::LocateExecutableSymbolFile()
zturner requested changes to this revision.
zturner added a comment.
This revision now requires changes to proceed.
I know you are just following the existing pattern, but this whole function
appears broken in the presence of file systems that use backslash instead of
forward slash. Could you try to fix that? I've provided some suggestions
inline.
I also don't see any tests for this. We've been pretty lax about this
historically, but I want to start pushing back against CLs with no tests. If
you need help writing a test, let me know and we can figure something out. For
starters, what is the scenario you encountered this in?
Comment at: source/Host/common/Symbols.cpp:238-250
@@ -237,15 +237,15 @@
// Add /usr/lib/debug directory.
debug_file_search_paths.AppendIfUnique (FileSpec("/usr/lib/debug",
true));
std::string uuid_str;
const UUID &module_uuid = module_spec.GetUUID();
if (module_uuid.IsValid())
{
// Some debug files are stored in the .build-id directory like
this:
//
/usr/lib/debug/.build-id/ff/e7fe727889ad82bb153de2ad065b2189693315.debug
uuid_str = module_uuid.GetAsString("");
uuid_str.insert (2, 1, '/');
uuid_str = uuid_str + ".debug";
}
Can you do all of this only if the target is not Windows?
Comment at: source/Host/common/Symbols.cpp:263-270
@@ -266,6 +262,10 @@
files.push_back (dirname + "/" + symbol_filename);
files.push_back (dirname + "/.debug/" + symbol_filename);
files.push_back (dirname + "/.build-id/" + uuid_str);
-files.push_back (dirname + module_directory + "/" +
symbol_filename);
+
+// Some debug files may stored in the module directory like this:
+// /usr/lib/debug/usr/lib/library.so.debug
+if (!file_dir.IsEmpty())
+files.push_back (dirname + file_dir.AsCString() + "/" +
symbol_filename);
Don't use literal slashes anywhere. I know you're just copying the existing
code, which was already broken, but it seems easy enough to fix all this by
calling llvm::sys::path::append, which will do the right thing dependign on the
Host platform.
Since this code is in Host, it's ok to do this, since we're ultimately going to
pass this path directly through to the filesystem anyway, we already need to be
able to assume that we're dealing with a path that has the appropriate
separator for the platform anyway.
Repository:
rL LLVM
http://reviews.llvm.org/D13201
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D13201: Fix segmentation fault in lldb_private::Symbols::LocateExecutableSymbolFile()
brucem added a comment. Talking to Vadim (enlight) offline, this happened when debugging an inferior on a Linux target from a Windows host. He had only copied the executable back to the Windows host, so the shared libraries involved weren't present on his system. I'm not sure if this is the right place for this fix or if it should be somewhere else prior to this point. Repository: rL LLVM http://reviews.llvm.org/D13201 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D13124: test runner: switch to pure-Python timeout mechanism
Will test this out tomorrow On Sun, Sep 27, 2015 at 10:28 PM Todd Fiala wrote: > tfiala updated this revision to Diff 35829. > tfiala added a comment. > > Tests added. Ready for review. > > The change now has two levels of terminate: > > - soft terminate, which uses a signal or process control mechanism to tell > the process to end. It optionally can use a mechanism that triggers a core > dump / crashlog / minidump if supported by the underlying platform. RIght > now, only Linux of the Posix-y platforms requests the core (via SIGQUIT). > I will plumb through an option for that later (as mentioned in prior > comments in this review). But I'm now at max time I can spend on this > right now and this'll need to do. There is also a timeout on how long the > process driver will allow the soft timeout to take to wrap up. This is > relevant for core dumps that might have a huge amount of memory to dump to > disk. It defaults to 10 seconds, but can be overridden by an environment > variable, LLDB_TEST_SOFT_TERMINATE_TIMEOUT. That env var will be converted > to a float from whatever text is in it. It represents the number of > seconds within which any soft terminate option must wrap up. If a process > is actively blocking/ignoring that termination signal, then this represents > the total time that will be "wasted" waiting to figure this out. > > - hard terminate: after the soft terminate attempt is made, plus the soft > terminate timeout, then if the process is still not eliminated, the hard > terminate mechanism will be used. On Posix-y systems, this is a SIGKILL. > > @zturner, you will want to run the tests in lldb/test/test_runner/test. > Do that with: > cd lldb/test/test_runner/test > python process_control_tests.py > > In some cases where docs were clear, I did the bit that seemed to make > sense on the Windows side. In others, I left it for you to fill in. Feel > free to skip some/any tests that don't make sense if, for example, you only > end up wanting to support one level of killing on Windows. (We can > probably work in a "has soft terminate mechanism" query into the > process_control.ProcessHelper class, which has per-platform derived > classes, if we want to go that way). > > Also note I will start migrating test infrastructure bits into the > lldb/test/test_runner/lib directory over time. The two new files from this > change are going in there. > > I'd like to get this in sooner than later as it resolves numerous hangs on > OS X where we don't have a timeout guard. I've tried it on Linux (4, 8 and > 24 core machines) as well as OS X (8 core MBPs), and have run the > process_control_tests on them without issue. > > > http://reviews.llvm.org/D13124 > > Files: > test/dosep.py > test/test_runner/README.txt > test/test_runner/lib/lldb_utils.py > test/test_runner/lib/process_control.py > test/test_runner/test/inferior.py > test/test_runner/test/process_control_tests.py > > ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D13202: [LLDB] Fix display of value of a vector variables in watchpoint operations
mohit.bhakkad created this revision.
mohit.bhakkad added reviewers: clayborg, granata.enrico.
mohit.bhakkad added subscribers: jaydeep, bhushan, sagar, nitesh.jain,
lldb-commits.
mohit.bhakkad set the repository for this revision to rL LLVM.
Consider a vector variable 'v8i16 s0'
Right now if we print value of s0, it gives us proper value:
(lldb) print s0
(v8i16) $0 = (member 1, member 2, ,member 8)
But if we try to set a watchpoint on it, it shows null for its value:
(lldb) watchpoint set variable s0
Watchpoint created: Watchpoint 1: addr = size = 16 state = enabled type
= w
declare @ 'file_name:line_no'
watchpoint spec = 's0'
new value: (null)
Approach used in patch is already used in in function
ValueObjectPrinter::GetValueSummaryError,
which is called for command 'print s0'.
Repository:
rL LLVM
http://reviews.llvm.org/D13202
Files:
source/Breakpoint/Watchpoint.cpp
Index: source/Breakpoint/Watchpoint.cpp
===
--- source/Breakpoint/Watchpoint.cpp
+++ source/Breakpoint/Watchpoint.cpp
@@ -218,14 +218,21 @@
s->Printf("\nWatchpoint %u hit:", GetID());
prefix = "";
}
-
+
if (m_old_value_sp)
{
-s->Printf("\n%sold value: %s", prefix,
m_old_value_sp->GetValueAsCString());
+if (m_old_value_sp->GetValueAsCString())
+s->Printf("\n%sold value: %s", prefix,
m_old_value_sp->GetValueAsCString());
+else
+s->Printf("\n%sold value: %s", prefix,
m_old_value_sp->GetSummaryAsCString());
}
+
if (m_new_value_sp)
{
-s->Printf("\n%snew value: %s", prefix,
m_new_value_sp->GetValueAsCString());
+if (m_new_value_sp->GetValueAsCString())
+s->Printf("\n%snew value: %s", prefix,
m_new_value_sp->GetValueAsCString());
+else
+s->Printf("\n%snew value: %s", prefix,
m_new_value_sp->GetSummaryAsCString());
}
}
Index: source/Breakpoint/Watchpoint.cpp
===
--- source/Breakpoint/Watchpoint.cpp
+++ source/Breakpoint/Watchpoint.cpp
@@ -218,14 +218,21 @@
s->Printf("\nWatchpoint %u hit:", GetID());
prefix = "";
}
-
+
if (m_old_value_sp)
{
-s->Printf("\n%sold value: %s", prefix, m_old_value_sp->GetValueAsCString());
+if (m_old_value_sp->GetValueAsCString())
+s->Printf("\n%sold value: %s", prefix, m_old_value_sp->GetValueAsCString());
+else
+s->Printf("\n%sold value: %s", prefix, m_old_value_sp->GetSummaryAsCString());
}
+
if (m_new_value_sp)
{
-s->Printf("\n%snew value: %s", prefix, m_new_value_sp->GetValueAsCString());
+if (m_new_value_sp->GetValueAsCString())
+s->Printf("\n%snew value: %s", prefix, m_new_value_sp->GetValueAsCString());
+else
+s->Printf("\n%snew value: %s", prefix, m_new_value_sp->GetSummaryAsCString());
}
}
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D13202: [LLDB] Fix display of value of a vector variables in watchpoint operations
zturner added a subscriber: zturner. zturner requested changes to this revision. zturner added a reviewer: zturner. zturner added a comment. This revision now requires changes to proceed. Can you find a way to add a test for this? Repository: rL LLVM http://reviews.llvm.org/D13202 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D12994: Improve support of the ncurses dependency on NetBSD
brucem added a subscriber: brucem. brucem added a comment. Some mostly superficial comments. I'd like to hear if anyone else has an opinion about using a Config.h generated by cmake & autotools. I'm also not sure offhand how this would impact the Xcode build that the Apple folks use. And I didn't see anything that sets autoconf stuff to generate that new config.h.in? Comment at: cmake/modules/LLDBConfig.cmake:284 @@ +283,3 @@ + +find_library(NCURSES_PANEL_LIBRARY NAMES panel DOC "The ncureses panel library") +if (CURSES_FOUND) Typo here: `ncureses` Comment at: include/lldb/Config/config.h.cmake:6 @@ +5,3 @@ +#else +#define CONFIG_H + This should be `LLDB_CONFIG_CONFIG_H` rather than just `CONFIG_H`. Also, why not use the same `#ifndef` technique that everything else uses in the header files? Comment at: include/lldb/Config/config.h.cmake:6 @@ +5,3 @@ +#else +#define CONFIG_H + brucem wrote: > This should be `LLDB_CONFIG_CONFIG_H` rather than just `CONFIG_H`. > > Also, why not use the same `#ifndef` technique that everything else uses in > the header files? Also, other headers in LLDB tend to have capitalized names, so maybe this should be `include/lldb/Config/Config.h.cmake`. Comment at: include/lldb/Config/config.h.in:6 @@ +5,3 @@ +#else +#define CONFIG_H + Same as for `config.h.cmake`. Repository: rL LLVM http://reviews.llvm.org/D12994 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D12748: Include platform agnostic in the place of
brucem added a subscriber: brucem. Comment at: tools/lldb-server/lldb-gdbserver.cpp:10 @@ -9,1 +9,3 @@ +#include + I suspect that this should `#include "lldb/Host/HostGetOpt.h"` instead, and that it should be down below with the other Host includes. Repository: rL LLVM http://reviews.llvm.org/D12748 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
