Re: [Lldb-commits] [PATCH] D19067: Make sure to use lib instead of lib64 for LLDB_LIB_DIR
labath added a comment. Yes, please use a command-line argument to pass that information into python. http://reviews.llvm.org/D19067 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D19214: fix a race is the LLDB test suite results collection
labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. This sounds a lot like the problem that has been plaguing our darwin buildbot (interestingly, I have never seen it happen on linux). Thanks for tracking that down. I am not sure I understand the architecture of this system completely, but it sounds like a reasonable thing to do. http://reviews.llvm.org/D19214 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D19215: normalize test file extension for test filenames
labath added a comment. So, I think I have already fixed the problem with r266192. This seems a bit like using a sledgehammer to kill a fly. If anything, I'd add an assert somewhere that makes sure the file names are in the right form to begin with. http://reviews.llvm.org/D19215 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D19216: test infra cleanup: make test_runner/lib into the test_runner package
labath resigned from this revision. labath removed a reviewer: labath. labath added a comment. Looks good, but I'll defer to Zachary. http://reviews.llvm.org/D19216 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r266598 - Fixup r266327
Author: labath Date: Mon Apr 18 06:01:41 2016 New Revision: 266598 URL: http://llvm.org/viewvc/llvm-project?rev=266598&view=rev Log: Fixup r266327 Fix XFAILed tests in TestThreadStates for the new signature of wait_for_running_event. Modified: lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/state/TestThreadStates.py Modified: lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/state/TestThreadStates.py URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/state/TestThreadStates.py?rev=266598&r1=266597&r2=266598&view=diff == --- lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/state/TestThreadStates.py (original) +++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/state/TestThreadStates.py Mon Apr 18 06:01:41 2016 @@ -180,7 +180,7 @@ class ThreadStateTestCase(TestBase): # Continue, the inferior will go into an infinite loop waiting for 'g_test' to change. self.dbg.SetAsync(True) self.runCmd("continue") -self.wait_for_running_event() +self.wait_for_running_event(process) # Go back to synchronous interactions self.dbg.SetAsync(False) @@ -221,7 +221,7 @@ class ThreadStateTestCase(TestBase): # Continue, the inferior will go into an infinite loop waiting for 'g_test' to change. self.dbg.SetAsync(True) self.runCmd("continue") -self.wait_for_running_event() +self.wait_for_running_event(process) # Check the thread state. It should be running. self.assertFalse(thread.IsStopped(), "Thread state is \'stopped\' when it should be running.") ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r266605 - Attempt to fix darwin build after header refactor in llvm (r266595)
Author: labath Date: Mon Apr 18 07:18:35 2016 New Revision: 266605 URL: http://llvm.org/viewvc/llvm-project?rev=266605&view=rev Log: Attempt to fix darwin build after header refactor in llvm (r266595) Modified: lldb/trunk/source/API/SBHostOS.cpp Modified: lldb/trunk/source/API/SBHostOS.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/API/SBHostOS.cpp?rev=266605&r1=266604&r2=266605&view=diff == --- lldb/trunk/source/API/SBHostOS.cpp (original) +++ lldb/trunk/source/API/SBHostOS.cpp Mon Apr 18 07:18:35 2016 @@ -18,7 +18,7 @@ #include "lldb/Host/ThreadLauncher.h" #include "llvm/Support/Path.h" -#include "llvm/ADT/SmallVector.h" +#include "llvm/ADT/SmallString.h" using namespace lldb; using namespace lldb_private; ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D19214: fix a race is the LLDB test suite results collection
tfiala closed this revision. tfiala added a comment. Closing with commit: r266624 http://reviews.llvm.org/D19214 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D19215: normalize test file extension for test filenames
tfiala added a comment. In http://reviews.llvm.org/D19215#403844, @labath wrote: > So, I think I have already fixed the problem with r266192. I was able to replicate just prior to applying the change, but I'll double check. Regarding r266192 (which came through when I wasn't looking, sorry), that started showing up because somebody changed something where the test_filename attribute was no longer showing up on test case instances every place where it used to show up. I suspect there was some test refactoring that did that. (So it was a latent bug, on a code path that didn't used to get hit). > This seems a bit like using a sledgehammer to kill a fly. If anything, I'd > add an assert somewhere that makes sure the file names are in the right form > to begin with. http://reviews.llvm.org/D19215 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r266624 - fix a race is the LLDB test suite results collection
Author: tfiala Date: Mon Apr 18 11:09:21 2016 New Revision: 266624 URL: http://llvm.org/viewvc/llvm-project?rev=266624&view=rev Log: fix a race is the LLDB test suite results collection The race boiled down to this: If a test worker queue is able to run the test inferior and clean up before the dosep.py listener socket is spun up, and the worker queue is the last one (as would be the case when there's only one test rerunning in the rerun queue), then the test suite will exit the main loop before having a chance to process any test events coming from the test inferior or the worker queue job control. I found this race to be far more likely on fast hardware. Our Linux CI is one such example. While it will show up primarily during meta test events generated by a worker thread when a test inferior times out or exits with an exceptional exit (e.g. seg fault), it only requires that the OS takes longer to hook up the listener socket than it takes for the final test inferior and worker thread to shut down. See: http://reviews.llvm.org/D19214 reviewed by: Pavel Labath Modified: lldb/trunk/packages/Python/lldbsuite/test/dosep.py lldb/trunk/packages/Python/lldbsuite/test/dotest_channels.py lldb/trunk/packages/Python/lldbsuite/test/issue_verification/TestRerunTimeout.py.park lldb/trunk/packages/Python/lldbsuite/test/result_formatter.py Modified: lldb/trunk/packages/Python/lldbsuite/test/dosep.py URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/dosep.py?rev=266624&r1=266623&r2=266624&view=diff == --- lldb/trunk/packages/Python/lldbsuite/test/dosep.py (original) +++ lldb/trunk/packages/Python/lldbsuite/test/dosep.py Mon Apr 18 11:09:21 2016 @@ -109,13 +109,17 @@ def setup_global_variables( global GET_WORKER_INDEX GET_WORKER_INDEX = get_worker_index_use_pid -def report_test_failure(name, command, output): +def report_test_failure(name, command, output, timeout): global output_lock with output_lock: if not (RESULTS_FORMATTER and RESULTS_FORMATTER.is_using_terminal()): print(file=sys.stderr) print(output, file=sys.stderr) -print("[%s FAILED]" % name, file=sys.stderr) +if timeout: +timeout_str = " (TIMEOUT)" +else: +timeout_str = "" +print("[%s FAILED]%s" % (name, timeout_str), file=sys.stderr) print("Command invoked: %s" % ' '.join(command), file=sys.stderr) update_progress(name) @@ -211,7 +215,7 @@ class DoTestProcessDriver(process_contro # only stderr does. report_test_pass(self.file_name, output[1]) else: -report_test_failure(self.file_name, command, output[1]) +report_test_failure(self.file_name, command, output[1], was_timeout) # Save off the results for the caller. self.results = ( Modified: lldb/trunk/packages/Python/lldbsuite/test/dotest_channels.py URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/dotest_channels.py?rev=266624&r1=266623&r2=266624&view=diff == --- lldb/trunk/packages/Python/lldbsuite/test/dotest_channels.py (original) +++ lldb/trunk/packages/Python/lldbsuite/test/dotest_channels.py Mon Apr 18 11:09:21 2016 @@ -55,6 +55,14 @@ class UnpicklingForwardingReaderChannel( # unpickled results. raise Exception("forwarding function must be set") +# Initiate all connections by sending an ack. This allows +# the initiators of the socket to await this to ensure +# that this end is up and running (and therefore already +# into the async map). +ack_bytes = bytearray() +ack_bytes.append(chr(42)) +file_object.send(ack_bytes) + def deserialize_payload(self): """Unpickles the collected input buffer bytes and forwards.""" if len(self.ibuffer) > 0: Modified: lldb/trunk/packages/Python/lldbsuite/test/issue_verification/TestRerunTimeout.py.park URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/issue_verification/TestRerunTimeout.py.park?rev=266624&r1=266623&r2=266624&view=diff == --- lldb/trunk/packages/Python/lldbsuite/test/issue_verification/TestRerunTimeout.py.park (original) +++ lldb/trunk/packages/Python/lldbsuite/test/issue_verification/TestRerunTimeout.py.park Mon Apr 18 11:09:21 2016 @@ -3,19 +3,21 @@ from __future__ import print_function import time -import lldbsuite.test.lldbtest as lldbtest +import lldbsuite.test.decorators as decorators import rerun_base class RerunTimeoutTestCase(rerun_base.RerunBaseTestCase): -@lldbtest.no_debug_info_test +@decorators.no_debug_info_test def t
Re: [Lldb-commits] [PATCH] D19067: Make sure to use lib instead of lib64 for LLDB_LIB_DIR
fjricci updated this revision to Diff 54077. fjricci added a comment. Pass the lldb lib directory to python swig scripts http://reviews.llvm.org/D19067 Files: CMakeLists.txt scripts/Python/finishSwigPythonLLDB.py scripts/finishSwigWrapperClasses.py Index: scripts/finishSwigWrapperClasses.py === --- scripts/finishSwigWrapperClasses.py +++ scripts/finishSwigWrapperClasses.py @@ -72,15 +72,17 @@ be installed. Where non-Darwin systems want to put\n\ the .py and .so files so that Python can find them\n\ automatically. Python install directory.\n\ +--lldbLibDir(optional) The name of the directory containing liblldb.so.\n\ +\"lib\" by default.\n\ --cmakeBuildConfiguration= (optional) Is the build configuration(Debug, Release, RelWithDebugInfo)\n\ used to determine where the bin and lib directories are \n\ created for a Windows build.\n\ --argsFile= The args are read from a file instead of the\n\ command line. Other command line args are ignored.\n\ \n\ Usage:\n\ finishSwigWrapperClasses.py --srcRoot=ADirPath --targetDir=ADirPath\n\ ---cfgBldDir=ADirPath --prefix=ADirPath -m -d\n\ +--cfgBldDir=ADirPath --prefix=ADirPath --lldbLibDir=ADirPath -m -d\n\ \n\ " #TAG_PROGRAM_HELP_INFO @@ -158,15 +160,16 @@ nResult = 0 strListArgs = "hdm" # Format "hiox:" = -h -i -o -x listLongArgs = ["srcRoot=", "targetDir=", "cfgBldDir=", "prefix=", "cmakeBuildConfiguration=", -"argsFile"] +"lldbLibDir=", "argsFile"] dictArgReq = { "-h": "o", # o = optional, m = mandatory "-d": "o", "-m": "o", "--srcRoot": "m", "--targetDir": "m", "--cfgBldDir": "o", "--prefix": "o", "--cmakeBuildConfiguration": "o", +"--lldbLibDir": "o", "--argsFile": "o" } # Check for mandatory parameters @@ -337,11 +340,13 @@ --cmakeBuildConfiguration= (optional) Is the build configuration(Debug, Release, RelWithDebugInfo)\n\ used to determine where the bin and lib directories are \n\ created for a Windows build.\n\ +--lldbLibDir= The name of the directory containing liblldb.so. +(optional) "lib" by default. --argsFile= The args are read from a file instead of the command line. Other command line args are ignored. Usage: finishSwigWrapperClasses.py --srcRoot=ADirPath --targetDir=ADirPath ---cfgBldDir=ADirPath --prefix=ADirPath -m -d +--cfgBldDir=ADirPath --prefix=ADirPath --lldbLibDir=ADirPath -m -d Results:0 Success -1 Error - invalid parameters passed. Index: scripts/Python/finishSwigPythonLLDB.py === --- scripts/Python/finishSwigPythonLLDB.py +++ scripts/Python/finishSwigPythonLLDB.py @@ -358,11 +358,12 @@ # Args: vDictArgs - (R) Program input parameters. # vstrFrameworkPythonDir - (R) Python framework directory. # vstrLiblldbName - (R) File name for _lldb library. +# vstrLiblldbDir - (R) liblldb directory. # Returns: Bool - True = function success, False = failure. # Str - Error description on task failure. # Throws: None. #-- -def make_symlink_liblldb(vDictArgs, vstrFrameworkPythonDir, vstrLiblldbFileName): +def make_symlink_liblldb(vDictArgs, vstrFrameworkPythonDir, vstrLiblldbFileName, vstrLldbLibDir): dbg = utilsDebug.CDebugFnVerbose("Python script make_symlink_liblldb()") bOk = True strErrMsg = "" @@ -382,7 +383,7 @@ bMakeFileCalled = "-m" in vDictArgs if not bMakeFileCalled: -strSrc = os.path.join("lib", "LLDB") +strSrc = os.path.join(vstrLldbLibDir, "LLDB") else: strLibFileExtn = "" if eOSType == utilsOsType.EnumOsType.Windows: @@ -392,7 +393,7 @@ strLibFileExtn = ".dylib" else: strLibFileExtn = ".so" -strSrc = os.path.join("lib", "liblldb" + strLibFileExtn) +strSrc = os.path.join(vstrLldbLibDir, "liblldb" + strLibFileExtn) bOk, strErrMsg = make_symlink(vDictArgs, vstrFrameworkPythonDir, strSrc, strTarget) @@ -462,11 +463,12 @@ # the Python framework directory. # Args: vDictArgs - (R) Program input parameters. # vstrFrameworkPythonDir - (R) Python framework directory. +# vstrLldbLibDir - (R) liblldb directory. # Returns: Bool - True = function success, False = failu
Re: [Lldb-commits] [PATCH] D19215: normalize test file extension for test filenames
tfiala added a comment. > I was able to replicate just prior to applying the change, but I'll double > check. Yeah that change didn't cover all the cases. The test_filename that comes from this call stack (in startTest), originating within the unittest2 framework, has the .pyc (assuming this patch here, modifying the call to inspect.getfile() to inspect.getsource()): Traceback (most recent call last): File "test/dotest.py", line 7, in lldbsuite.test.run_suite() File "/Volumes/Data/src/lldb-llvm.org/lldb/packages/Python/lldbsuite/test/dotest.py", line 1083, in run_suite resultclass=test_result.LLDBTestResult).run(configuration.suite) File "/Volumes/Data/src/lldb-llvm.org/lldb/third_party/Python/module/unittest2/unittest2/runner.py", line 162, in run test(result) File "/Volumes/Data/src/lldb-llvm.org/lldb/third_party/Python/module/unittest2/unittest2/suite.py", line 65, in __call__ return self.run(*args, **kwds) File "/Volumes/Data/src/lldb-llvm.org/lldb/third_party/Python/module/unittest2/unittest2/suite.py", line 85, in run self._wrapped_run(result) File "/Volumes/Data/src/lldb-llvm.org/lldb/third_party/Python/module/unittest2/unittest2/suite.py", line 115, in _wrapped_run test._wrapped_run(result, debug) File "/Volumes/Data/src/lldb-llvm.org/lldb/third_party/Python/module/unittest2/unittest2/suite.py", line 117, in _wrapped_run test(result) File "/Volumes/Data/src/lldb-llvm.org/lldb/third_party/Python/module/unittest2/unittest2/case.py", line 433, in __call__ return self.run(*args, **kwds) File "/Volumes/Data/src/lldb-llvm.org/lldb/third_party/Python/module/unittest2/unittest2/case.py", line 338, in run result.startTest(self) File "/Volumes/Data/src/lldb-llvm.org/lldb/packages/Python/lldbsuite/test/test_result.py", line 133, in startTest EventBuilder.event_for_start(test)) File "/Volumes/Data/src/lldb-llvm.org/lldb/packages/Python/lldbsuite/test/result_formatter.py", line 366, in event_for_start test, EventBuilder.TYPE_TEST_START) File "/Volumes/Data/src/lldb-llvm.org/lldb/packages/Python/lldbsuite/test/result_formatter.py", line 269, in _event_dictionary_common test_filename = EventBuilder._normalize_test_filename(test.test_filename) File "/Volumes/Data/src/lldb-llvm.org/lldb/packages/Python/lldbsuite/test/result_formatter.py", line 249, in _normalize_test_filename raise Exception("filename ends in .pyc: {}".format(test_filename)) Exception: filename ends in .pyc: /Volumes/Data/src/lldb-llvm.org/lldb/packages/Python/lldbsuite/test/lang/cpp/lambdas/TestLambdas.pyc So we still want this I think. I also disagree about it being a sledgehammer - it is ensuring that input parameters that come from several different sources all follow the protocol of being .py files. That's just verifying input parameters. If they all came from the same place, I'd consider it an error to correct, but when they come from places like unittest2 that we don't need/want to change, this seems like fair game. http://reviews.llvm.org/D19215 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D19215: normalize test file extension for test filenames
tfiala updated this revision to Diff 54078. tfiala added a comment. Change inspect.getfile() => inspect.getsourcefile(). http://reviews.llvm.org/D19215 Files: packages/Python/lldbsuite/test/result_formatter.py Index: packages/Python/lldbsuite/test/result_formatter.py === --- packages/Python/lldbsuite/test/result_formatter.py +++ packages/Python/lldbsuite/test/result_formatter.py @@ -64,7 +64,7 @@ def create_socket(port): """Creates a socket to the localhost on the given port. -@param port the port number of the listenering port on +@param port the port number of the listening port on the localhost. @return (socket object, socket closing function) @@ -243,6 +243,15 @@ return event @staticmethod +def _normalize_test_filename(test_filename): +# Convert .pyc ending to .py. +if test_filename is not None and test_filename.endswith(".pyc"): +raise Exception("filename ends in .pyc: {}".format(test_filename)) +return test_filename[0:-1] +else: +return test_filename + +@staticmethod def _event_dictionary_common(test, event_type): """Returns an event dictionary setup with values for the given event type. @@ -257,9 +266,9 @@ # Determine the filename for the test case. If there is an attribute # for it, use it. Otherwise, determine from the TestCase class path. if hasattr(test, "test_filename"): -test_filename = test.test_filename +test_filename = EventBuilder._normalize_test_filename(test.test_filename) else: -test_filename = inspect.getsourcefile(test.__class__) +test_filename = EventBuilder._normalize_test_filename(inspect.getsourcefile(test.__class__)) event = EventBuilder.bare_event(event_type) event.update({ @@ -498,7 +507,7 @@ if exception_description is not None: event["exception_description"] = exception_description if test_filename is not None: -event["test_filename"] = test_filename +event["test_filename"] = EventBuilder._normalize_test_filename(test_filename) if command_line is not None: event["command_line"] = command_line return event @@ -522,7 +531,7 @@ if worker_index is not None: event["worker_index"] = int(worker_index) if test_filename is not None: -event["test_filename"] = test_filename +event["test_filename"] = EventBuilder._normalize_test_filename(test_filename) if command_line is not None: event["command_line"] = command_line return event Index: packages/Python/lldbsuite/test/result_formatter.py === --- packages/Python/lldbsuite/test/result_formatter.py +++ packages/Python/lldbsuite/test/result_formatter.py @@ -64,7 +64,7 @@ def create_socket(port): """Creates a socket to the localhost on the given port. -@param port the port number of the listenering port on +@param port the port number of the listening port on the localhost. @return (socket object, socket closing function) @@ -243,6 +243,15 @@ return event @staticmethod +def _normalize_test_filename(test_filename): +# Convert .pyc ending to .py. +if test_filename is not None and test_filename.endswith(".pyc"): +raise Exception("filename ends in .pyc: {}".format(test_filename)) +return test_filename[0:-1] +else: +return test_filename + +@staticmethod def _event_dictionary_common(test, event_type): """Returns an event dictionary setup with values for the given event type. @@ -257,9 +266,9 @@ # Determine the filename for the test case. If there is an attribute # for it, use it. Otherwise, determine from the TestCase class path. if hasattr(test, "test_filename"): -test_filename = test.test_filename +test_filename = EventBuilder._normalize_test_filename(test.test_filename) else: -test_filename = inspect.getsourcefile(test.__class__) +test_filename = EventBuilder._normalize_test_filename(inspect.getsourcefile(test.__class__)) event = EventBuilder.bare_event(event_type) event.update({ @@ -498,7 +507,7 @@ if exception_description is not None: event["exception_description"] = exception_description if test_filename is not None: -event["test_filename"] = test_filename +event["test_filename"] = EventBuilder._normalize_test_filename(test_filename) if command_line is not None: event["command_line"] = command_line return event @@ -522,7 +531,7 @@ if wo
Re: [Lldb-commits] [PATCH] D19215: normalize test file extension for test filenames
tfiala updated this revision to Diff 54079. tfiala added a comment. Whoops - don't include the 'raise' call. http://reviews.llvm.org/D19215 Files: packages/Python/lldbsuite/test/result_formatter.py Index: packages/Python/lldbsuite/test/result_formatter.py === --- packages/Python/lldbsuite/test/result_formatter.py +++ packages/Python/lldbsuite/test/result_formatter.py @@ -64,7 +64,7 @@ def create_socket(port): """Creates a socket to the localhost on the given port. -@param port the port number of the listenering port on +@param port the port number of the listening port on the localhost. @return (socket object, socket closing function) @@ -243,6 +243,15 @@ return event @staticmethod +def _normalize_test_filename(test_filename): +# Convert .pyc ending to .py. +if test_filename is not None and test_filename.endswith(".pyc"): +# raise Exception("filename ends in .pyc: {}".format(test_filename)) +return test_filename[0:-1] +else: +return test_filename + +@staticmethod def _event_dictionary_common(test, event_type): """Returns an event dictionary setup with values for the given event type. @@ -257,9 +266,9 @@ # Determine the filename for the test case. If there is an attribute # for it, use it. Otherwise, determine from the TestCase class path. if hasattr(test, "test_filename"): -test_filename = test.test_filename +test_filename = EventBuilder._normalize_test_filename(test.test_filename) else: -test_filename = inspect.getsourcefile(test.__class__) +test_filename = EventBuilder._normalize_test_filename(inspect.getsourcefile(test.__class__)) event = EventBuilder.bare_event(event_type) event.update({ @@ -498,7 +507,7 @@ if exception_description is not None: event["exception_description"] = exception_description if test_filename is not None: -event["test_filename"] = test_filename +event["test_filename"] = EventBuilder._normalize_test_filename(test_filename) if command_line is not None: event["command_line"] = command_line return event @@ -522,7 +531,7 @@ if worker_index is not None: event["worker_index"] = int(worker_index) if test_filename is not None: -event["test_filename"] = test_filename +event["test_filename"] = EventBuilder._normalize_test_filename(test_filename) if command_line is not None: event["command_line"] = command_line return event Index: packages/Python/lldbsuite/test/result_formatter.py === --- packages/Python/lldbsuite/test/result_formatter.py +++ packages/Python/lldbsuite/test/result_formatter.py @@ -64,7 +64,7 @@ def create_socket(port): """Creates a socket to the localhost on the given port. -@param port the port number of the listenering port on +@param port the port number of the listening port on the localhost. @return (socket object, socket closing function) @@ -243,6 +243,15 @@ return event @staticmethod +def _normalize_test_filename(test_filename): +# Convert .pyc ending to .py. +if test_filename is not None and test_filename.endswith(".pyc"): +# raise Exception("filename ends in .pyc: {}".format(test_filename)) +return test_filename[0:-1] +else: +return test_filename + +@staticmethod def _event_dictionary_common(test, event_type): """Returns an event dictionary setup with values for the given event type. @@ -257,9 +266,9 @@ # Determine the filename for the test case. If there is an attribute # for it, use it. Otherwise, determine from the TestCase class path. if hasattr(test, "test_filename"): -test_filename = test.test_filename +test_filename = EventBuilder._normalize_test_filename(test.test_filename) else: -test_filename = inspect.getsourcefile(test.__class__) +test_filename = EventBuilder._normalize_test_filename(inspect.getsourcefile(test.__class__)) event = EventBuilder.bare_event(event_type) event.update({ @@ -498,7 +507,7 @@ if exception_description is not None: event["exception_description"] = exception_description if test_filename is not None: -event["test_filename"] = test_filename +event["test_filename"] = EventBuilder._normalize_test_filename(test_filename) if command_line is not None: event["command_line"] = command_line return event @@ -522,7 +531,7 @@ if worker_in
Re: [Lldb-commits] [PATCH] D19215: normalize test file extension for test filenames
labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. Ok, makes sense then, thanks for making sure it's necessary. I don't quite understand how the string makes it's way there though, as the only place setting this field in the entire repository seems to be lldbinline.py http://reviews.llvm.org/D19215 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D19230: Properly unload modules from target image list when using svr4 packets
fjricci created this revision. fjricci added reviewers: ADodds, zturner, tfiala. fjricci added subscribers: sas, lldb-commits. When we receive an svr4 packet from the remote, we check for new modules and add them to the list of images in the target. However, we did not do the same for modules which have been removed. This was causing TestLoadUnload to fail when using ds2, which uses svr4 packets to communicate all library info on Linux. This patch fixes the failing test. http://reviews.llvm.org/D19230 Files: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp Index: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp === --- source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp +++ source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp @@ -4879,7 +4879,31 @@ if (new_modules.GetSize() > 0) { +ModuleList removed_modules; Target &target = GetTarget(); +ModuleList &loaded_modules = m_process->GetTarget().GetImages(); + +for (size_t i = 0; i < loaded_modules.GetSize(); ++i) +{ +const lldb::ModuleSP loaded_module = loaded_modules.GetModuleAtIndex(i); + +bool found = false; +for (size_t j = 0; j < new_modules.GetSize(); ++j) +{ +if (new_modules.GetModuleAtIndex(j).get() == loaded_module.get()) +found = true; +} + +if (!found) +{ +lldb_private::ObjectFile * obj = loaded_module->GetObjectFile (); +if (obj && obj->GetType () != ObjectFile::Type::eTypeExecutable) +removed_modules.Append (loaded_module); +} +} + +loaded_modules.Remove (removed_modules); +m_process->GetTarget().ModulesDidUnload (removed_modules, false); new_modules.ForEach ([&target](const lldb::ModuleSP module_sp) -> bool { @@ -4895,13 +4919,11 @@ return false; }); -ModuleList &loaded_modules = m_process->GetTarget().GetImages(); loaded_modules.AppendIfNeeded (new_modules); m_process->GetTarget().ModulesDidLoad (new_modules); } return new_modules.GetSize(); - } size_t Index: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp === --- source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp +++ source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp @@ -4879,7 +4879,31 @@ if (new_modules.GetSize() > 0) { +ModuleList removed_modules; Target &target = GetTarget(); +ModuleList &loaded_modules = m_process->GetTarget().GetImages(); + +for (size_t i = 0; i < loaded_modules.GetSize(); ++i) +{ +const lldb::ModuleSP loaded_module = loaded_modules.GetModuleAtIndex(i); + +bool found = false; +for (size_t j = 0; j < new_modules.GetSize(); ++j) +{ +if (new_modules.GetModuleAtIndex(j).get() == loaded_module.get()) +found = true; +} + +if (!found) +{ +lldb_private::ObjectFile * obj = loaded_module->GetObjectFile (); +if (obj && obj->GetType () != ObjectFile::Type::eTypeExecutable) +removed_modules.Append (loaded_module); +} +} + +loaded_modules.Remove (removed_modules); +m_process->GetTarget().ModulesDidUnload (removed_modules, false); new_modules.ForEach ([&target](const lldb::ModuleSP module_sp) -> bool { @@ -4895,13 +4919,11 @@ return false; }); -ModuleList &loaded_modules = m_process->GetTarget().GetImages(); loaded_modules.AppendIfNeeded (new_modules); m_process->GetTarget().ModulesDidLoad (new_modules); } return new_modules.GetSize(); - } size_t ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D19216: test infra cleanup: make test_runner/lib into the test_runner package
zturner added inline comments. Comment at: packages/Python/lldbsuite/test/test_runner/lldb_utils.py:1 @@ +1,2 @@ +""" +The LLVM Compiler Infrastructure I feel like this entire file should go under `lldbsuite/support`. If it's under `test/test_runner` we can't use it from anything in `lldb/scripts`, but this is generic enough that there's nothing actually test-specific about it. http://reviews.llvm.org/D19216 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D19215: normalize test file extension for test filenames
It seems very strange to me to be changing a .pyc filename to a .py filename. I think we should try to understand where the .pyc filename is coming from to begin with, and this is just masking the real error. On Mon, Apr 18, 2016 at 10:10 AM Pavel Labath via lldb-commits < lldb-commits@lists.llvm.org> wrote: > labath accepted this revision. > labath added a comment. > This revision is now accepted and ready to land. > > Ok, makes sense then, thanks for making sure it's necessary. > > I don't quite understand how the string makes it's way there though, as > the only place setting this field in the entire repository seems to be > lldbinline.py > > > http://reviews.llvm.org/D19215 > > > > ___ > 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] D19216: test infra cleanup: make test_runner/lib into the test_runner package
Yep, I agree with that now. Wasn't the case when I originally wrote it, but yeah that can move. I'll adjust that. On Mon, Apr 18, 2016 at 10:57 AM, Zachary Turner wrote: > zturner added inline comments. > > > Comment at: packages/Python/lldbsuite/test/test_runner/lldb_utils.py:1 > @@ +1,2 @@ > +""" > +The LLVM Compiler Infrastructure > > I feel like this entire file should go under `lldbsuite/support`. If it's > under `test/test_runner` we can't use it from anything in `lldb/scripts`, > but this is generic enough that there's nothing actually test-specific > about it. > > > http://reviews.llvm.org/D19216 > > > > -- -Todd ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D19216: test infra cleanup: make test_runner/lib into the test_runner package
tfiala added a subscriber: tfiala. tfiala added a comment. Yep, I agree with that now. Wasn't the case when I originally wrote it, but yeah that can move. I'll adjust that. http://reviews.llvm.org/D19216 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D19215: normalize test file extension for test filenames
IIRC, python will generate byte-compiled ".pyc" versions of the ".py" files automatically (and helpfully leave them in the CWD...) We turned that off for the testsuite through some Python magic I don't think I ever knew, but if I did I've certainly forgotten it... Anyway, sounds like that got turned back on again, and we should turn it off again... Jim > On Apr 18, 2016, at 11:01 AM, Zachary Turner via lldb-commits > wrote: > > It seems very strange to me to be changing a .pyc filename to a .py filename. > I think we should try to understand where the .pyc filename is coming from > to begin with, and this is just masking the real error. > > On Mon, Apr 18, 2016 at 10:10 AM Pavel Labath via lldb-commits > wrote: > labath accepted this revision. > labath added a comment. > This revision is now accepted and ready to land. > > Ok, makes sense then, thanks for making sure it's necessary. > > I don't quite understand how the string makes it's way there though, as the > only place setting this field in the entire repository seems to be > lldbinline.py > > > http://reviews.llvm.org/D19215 > > > > ___ > 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] D19215: normalize test file extension for test filenames
That's normal, but why would that cause an issue for the test suite? What is leading to it thinking that it should execute a .pyc file in the first place? On Mon, Apr 18, 2016 at 11:25 AM Jim Ingham wrote: > IIRC, python will generate byte-compiled ".pyc" versions of the ".py" > files automatically (and helpfully leave them in the CWD...) We turned > that off for the testsuite through some Python magic I don't think I ever > knew, but if I did I've certainly forgotten it... Anyway, sounds like that > got turned back on again, and we should turn it off again... > > Jim > > > On Apr 18, 2016, at 11:01 AM, Zachary Turner via lldb-commits < > lldb-commits@lists.llvm.org> wrote: > > > > It seems very strange to me to be changing a .pyc filename to a .py > filename. I think we should try to understand where the .pyc filename is > coming from to begin with, and this is just masking the real error. > > > > On Mon, Apr 18, 2016 at 10:10 AM Pavel Labath via lldb-commits < > lldb-commits@lists.llvm.org> wrote: > > labath accepted this revision. > > labath added a comment. > > This revision is now accepted and ready to land. > > > > Ok, makes sense then, thanks for making sure it's necessary. > > > > I don't quite understand how the string makes it's way there though, as > the only place setting this field in the entire repository seems to be > lldbinline.py > > > > > > http://reviews.llvm.org/D19215 > > > > > > > > ___ > > 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] D19215: normalize test file extension for test filenames
That I don't know. But we really shouldn't be generating the .pyc files, they just litter the test directories and I don't think they are much help. Jim > On Apr 18, 2016, at 11:29 AM, Zachary Turner wrote: > > That's normal, but why would that cause an issue for the test suite? What is > leading to it thinking that it should execute a .pyc file in the first place? > > On Mon, Apr 18, 2016 at 11:25 AM Jim Ingham wrote: > IIRC, python will generate byte-compiled ".pyc" versions of the ".py" files > automatically (and helpfully leave them in the CWD...) We turned that off > for the testsuite through some Python magic I don't think I ever knew, but if > I did I've certainly forgotten it... Anyway, sounds like that got turned > back on again, and we should turn it off again... > > Jim > > > On Apr 18, 2016, at 11:01 AM, Zachary Turner via lldb-commits > > wrote: > > > > It seems very strange to me to be changing a .pyc filename to a .py > > filename. I think we should try to understand where the .pyc filename is > > coming from to begin with, and this is just masking the real error. > > > > On Mon, Apr 18, 2016 at 10:10 AM Pavel Labath via lldb-commits > > wrote: > > labath accepted this revision. > > labath added a comment. > > This revision is now accepted and ready to land. > > > > Ok, makes sense then, thanks for making sure it's necessary. > > > > I don't quite understand how the string makes it's way there though, as the > > only place setting this field in the entire repository seems to be > > lldbinline.py > > > > > > http://reviews.llvm.org/D19215 > > > > > > > > ___ > > 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] D19215: normalize test file extension for test filenames
They make the test suite run faster. when a file wasn't changed. On Mon, Apr 18, 2016 at 11:34 AM Jim Ingham wrote: > That I don't know. But we really shouldn't be generating the .pyc files, > they just litter the test directories and I don't think they are much help. > > Jim > > > On Apr 18, 2016, at 11:29 AM, Zachary Turner wrote: > > > > That's normal, but why would that cause an issue for the test suite? > What is leading to it thinking that it should execute a .pyc file in the > first place? > > > > On Mon, Apr 18, 2016 at 11:25 AM Jim Ingham wrote: > > IIRC, python will generate byte-compiled ".pyc" versions of the ".py" > files automatically (and helpfully leave them in the CWD...) We turned > that off for the testsuite through some Python magic I don't think I ever > knew, but if I did I've certainly forgotten it... Anyway, sounds like that > got turned back on again, and we should turn it off again... > > > > Jim > > > > > On Apr 18, 2016, at 11:01 AM, Zachary Turner via lldb-commits < > lldb-commits@lists.llvm.org> wrote: > > > > > > It seems very strange to me to be changing a .pyc filename to a .py > filename. I think we should try to understand where the .pyc filename is > coming from to begin with, and this is just masking the real error. > > > > > > On Mon, Apr 18, 2016 at 10:10 AM Pavel Labath via lldb-commits < > lldb-commits@lists.llvm.org> wrote: > > > labath accepted this revision. > > > labath added a comment. > > > This revision is now accepted and ready to land. > > > > > > Ok, makes sense then, thanks for making sure it's necessary. > > > > > > I don't quite understand how the string makes it's way there though, > as the only place setting this field in the entire repository seems to be > lldbinline.py > > > > > > > > > http://reviews.llvm.org/D19215 > > > > > > > > > > > > ___ > > > 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] D19215: normalize test file extension for test filenames
(not sure how much though, as I haven't actually measured it) On Mon, Apr 18, 2016 at 11:40 AM Zachary Turner wrote: > They make the test suite run faster. when a file wasn't changed. > > On Mon, Apr 18, 2016 at 11:34 AM Jim Ingham wrote: > >> That I don't know. But we really shouldn't be generating the .pyc files, >> they just litter the test directories and I don't think they are much help. >> >> Jim >> >> > On Apr 18, 2016, at 11:29 AM, Zachary Turner >> wrote: >> > >> > That's normal, but why would that cause an issue for the test suite? >> What is leading to it thinking that it should execute a .pyc file in the >> first place? >> > >> > On Mon, Apr 18, 2016 at 11:25 AM Jim Ingham wrote: >> > IIRC, python will generate byte-compiled ".pyc" versions of the ".py" >> files automatically (and helpfully leave them in the CWD...) We turned >> that off for the testsuite through some Python magic I don't think I ever >> knew, but if I did I've certainly forgotten it... Anyway, sounds like that >> got turned back on again, and we should turn it off again... >> > >> > Jim >> > >> > > On Apr 18, 2016, at 11:01 AM, Zachary Turner via lldb-commits < >> lldb-commits@lists.llvm.org> wrote: >> > > >> > > It seems very strange to me to be changing a .pyc filename to a .py >> filename. I think we should try to understand where the .pyc filename is >> coming from to begin with, and this is just masking the real error. >> > > >> > > On Mon, Apr 18, 2016 at 10:10 AM Pavel Labath via lldb-commits < >> lldb-commits@lists.llvm.org> wrote: >> > > labath accepted this revision. >> > > labath added a comment. >> > > This revision is now accepted and ready to land. >> > > >> > > Ok, makes sense then, thanks for making sure it's necessary. >> > > >> > > I don't quite understand how the string makes it's way there though, >> as the only place setting this field in the entire repository seems to be >> lldbinline.py >> > > >> > > >> > > http://reviews.llvm.org/D19215 >> > > >> > > >> > > >> > > ___ >> > > 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] D19215: normalize test file extension for test filenames
tfiala added a comment. In http://reviews.llvm.org/D19215#404143, @zturner wrote: > It seems very strange to me to be changing a .pyc filename to a .py > filename. I think we should try to understand where the .pyc filename is > coming from to begin with, and this is just masking the real error. Testing: 38 test suites, 12 threads 7 out of 38 test suites processed - TestChar1632T.py Traceback (most recent call last): File "test/dotest.py", line 7, in lldbsuite.test.run_suite() File "/Volumes/Data/src/lldb-llvm.org/lldb/packages/Python/lldbsuite/test/dotest.py", line 952, in run_suite visit('Test', dirpath, filenames) File "/Volumes/Data/src/lldb-llvm.org/lldb/packages/Python/lldbsuite/test/dotest.py", line 747, in visit configuration.suite.addTests(unittest2.defaultTestLoader.loadTestsFromName(base)) File "/Volumes/Data/src/lldb-llvm.org/lldb/third_party/Python/module/unittest2/unittest2/loader.py", line 103, in loadTestsFromName module = __import__('.'.join(parts_copy)) File "/Volumes/Data/src/lldb-llvm.org/lldb/packages/Python/lldbsuite/test/lang/cpp/lambdas/TestLambdas.py", line 4, in lldbinline.MakeInlineTest(__file__, globals(), []) File "/Volumes/Data/src/lldb-llvm.org/lldb/packages/Python/lldbsuite/test/lldbinline.py", line 210, in MakeInlineTest raise Exception("lldbinline file ends with .pyc: {}".format(__file)) Exception: lldbinline file ends with .pyc: /Volumes/Data/src/lldb-llvm.org/lldb/packages/Python/lldbsuite/test/lang/cpp/lambdas/TestLambdas.pyc The __file__ for lldbinline.py-based tests is getting set to .pyc via the unittest2 loader. This callstack comes from inserting this code into lldbinline.py: diff --git a/packages/Python/lldbsuite/test/lldbinline.py b/packages/Python/lldbsuite/test/lldbinline.py index 4eaa2a7..3eef4ee 100644 --- a/packages/Python/lldbsuite/test/lldbinline.py +++ b/packages/Python/lldbsuite/test/lldbinline.py @@ -206,6 +206,10 @@ def MakeInlineTest(__file, __globals, decorators=None): __globals.update({test_name : test}) # Store the name of the originating file.o -test.test_filename = __file +if __file is not None and __file.endswith(".pyc"): +raise Exception("lldbinline file ends with .pyc: {}".format(__file)) +test.test_filename = __file[0:-1] +else: +test.test_filename = __file return test So - I can either intercept it right there and convert to .py, or I can protect it at the API ingestion level. If we go with the former, then I want to add an assert to the latter to make sure any cases of this sneaking in are caught. Since this case was only happening for lldbinline tests, I suspect it was always happening and just wasn't caught. I'll adjust the patch shortly to have the API ingestion of the test_filename assert on .pyc (and I'll ensure it ends in .py, not just that it *isn't* .pyc). I'll also fix up the lldbinline case at the point where we grab it from the unittest2 loader code. http://reviews.llvm.org/D19215 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D19215: normalize test file extension for test filenames
tfiala added a comment. > The __file__ for lldbinline.py-based tests is getting set to .pyc via the > unittest2 loader. This callstack comes from inserting this code into > lldbinline.py: > > diff --git a/packages/Python/lldbsuite/test/lldbinline.py > b/packages/Python/lldbsuite/test/lldbinline.py > index 4eaa2a7..3eef4ee 100644 > --- a/packages/Python/lldbsuite/test/lldbinline.py > +++ b/packages/Python/lldbsuite/test/lldbinline.py > @@ -206,6 +206,10 @@ def MakeInlineTest(__file, __globals, decorators=None): >__globals.update({test_name : test}) > ># Store the name of the originating file.o > -test.test_filename = __file > +if __file is not None and __file.endswith(".pyc"): > +raise Exception("lldbinline file ends with .pyc: > {}".format(__file)) > +test.test_filename = __file[0:-1] > +else: > +test.test_filename = __file >return test > > > So - I can either intercept it right there and convert to .py, or I can > protect it at the API ingestion level. If we go with the former, then I want > to add an assert to the latter to make sure any cases of this sneaking in are > caught. I should mention that I verified we do *not* instruct unittest2 to load a .pyc (i.e. this isn't happening because we somehow asked it to parse a .pyc): diff --git a/packages/Python/lldbsuite/test/dotest.py b/packages/Python/lldbsuite/test/dotest.py index 198e87c..71d8c58 100644 --- a/packages/Python/lldbsuite/test/dotest.py +++ b/packages/Python/lldbsuite/test/dotest.py @@ -744,6 +744,8 @@ def visit(prefix, dir, names): # A simple case of just the module name. Also the failover case # from the filterspec branch when the (base, filterspec) combo # doesn't make sense. +if base is not None and base.endswith(".pyc"): + raise Exception("base ends with .pyc: {}".format(base)) configuration.suite.addTests(unittest2.defaultTestLoader.loadTestsFromName(base)) I'm not hitting that when I hit the callstack listed above. http://reviews.llvm.org/D19215 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D19122: LLDB: Fixed race condition on timeout when stopping private state thread
cameron314 updated the summary for this revision. cameron314 removed rL LLVM as the repository for this revision. cameron314 updated this revision to Diff 54095. cameron314 added a comment. I've updated the diff to include a fix for the race between Detach and Stop. This has been working nicely for us without any problems for the last few days. I looked again at putting back the wrapper for the m_private_state_thread, but it really doesn't make sense to me. Anything that can go wrong without the wrapper can still happen with the wrapper. http://reviews.llvm.org/D19122 Files: include/lldb/Target/Process.h source/Target/Process.cpp Index: source/Target/Process.cpp === --- source/Target/Process.cpp +++ source/Target/Process.cpp @@ -4112,11 +4112,8 @@ if (log) log->Printf ("Process::%s (signal = %d)", __FUNCTION__, signal); -// Signal the private state thread. First we should copy this is case the -// thread starts exiting since the private state thread will NULL this out -// when it exits -HostThread private_state_thread(m_private_state_thread); -if (private_state_thread.IsJoinable()) +// Signal the private state thread +if (m_private_state_thread.IsJoinable()) { TimeValue timeout_time; bool timed_out; @@ -4134,7 +4131,7 @@ { if (timed_out) { -Error error = private_state_thread.Cancel(); +Error error = m_private_state_thread.Cancel(); if (log) log->Printf ("Timed out responding to the control event, cancel got error: \"%s\".", error.AsCString()); } @@ -4145,7 +4142,7 @@ } thread_result_t result = NULL; -private_state_thread.Join(&result); +m_private_state_thread.Join(&result); m_private_state_thread.Reset(); } } @@ -4449,7 +4446,6 @@ if (!is_secondary_thread) m_public_run_lock.SetStopped(); m_private_state_control_wait.SetValue (true, eBroadcastAlways); -m_private_state_thread.Reset(); return NULL; } Index: include/lldb/Target/Process.h === --- include/lldb/Target/Process.h +++ include/lldb/Target/Process.h @@ -3310,7 +3310,10 @@ bool PrivateStateThreadIsValid () const { -return m_private_state_thread.IsJoinable(); +lldb::StateType state = m_private_state.GetValue(); +return state != lldb::eStateDetached && + state != lldb::eStateExited && + m_private_state_thread.IsJoinable(); } void Index: source/Target/Process.cpp === --- source/Target/Process.cpp +++ source/Target/Process.cpp @@ -4112,11 +4112,8 @@ if (log) log->Printf ("Process::%s (signal = %d)", __FUNCTION__, signal); -// Signal the private state thread. First we should copy this is case the -// thread starts exiting since the private state thread will NULL this out -// when it exits -HostThread private_state_thread(m_private_state_thread); -if (private_state_thread.IsJoinable()) +// Signal the private state thread +if (m_private_state_thread.IsJoinable()) { TimeValue timeout_time; bool timed_out; @@ -4134,7 +4131,7 @@ { if (timed_out) { -Error error = private_state_thread.Cancel(); +Error error = m_private_state_thread.Cancel(); if (log) log->Printf ("Timed out responding to the control event, cancel got error: \"%s\".", error.AsCString()); } @@ -4145,7 +4142,7 @@ } thread_result_t result = NULL; -private_state_thread.Join(&result); +m_private_state_thread.Join(&result); m_private_state_thread.Reset(); } } @@ -4449,7 +4446,6 @@ if (!is_secondary_thread) m_public_run_lock.SetStopped(); m_private_state_control_wait.SetValue (true, eBroadcastAlways); -m_private_state_thread.Reset(); return NULL; } Index: include/lldb/Target/Process.h === --- include/lldb/Target/Process.h +++ include/lldb/Target/Process.h @@ -3310,7 +3310,10 @@ bool PrivateStateThreadIsValid () const { -return m_private_state_thread.IsJoinable(); +lldb::StateType state = m_private_state.GetValue(); +return state != lldb::eStateDetached && + state != lldb::eStateExited && + m_private_state_thread.IsJoinable(); } void ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D19122: LLDB: Fixed race condition on timeout when stopping private state thread
cameron314 updated this revision to Diff 54098. cameron314 added a comment. Oops, uploaded wrong diff a minute ago. Here's the one with the full fix. http://reviews.llvm.org/D19122 Files: include/lldb/Target/Process.h source/Target/Process.cpp Index: source/Target/Process.cpp === --- source/Target/Process.cpp +++ source/Target/Process.cpp @@ -4112,11 +4112,8 @@ if (log) log->Printf ("Process::%s (signal = %d)", __FUNCTION__, signal); -// Signal the private state thread. First we should copy this is case the -// thread starts exiting since the private state thread will NULL this out -// when it exits -HostThread private_state_thread(m_private_state_thread); -if (private_state_thread.IsJoinable()) +// Signal the private state thread +if (PrivateStateThreadIsValid()) { TimeValue timeout_time; bool timed_out; @@ -4134,7 +4131,7 @@ { if (timed_out) { -Error error = private_state_thread.Cancel(); +Error error = m_private_state_thread.Cancel(); if (log) log->Printf ("Timed out responding to the control event, cancel got error: \"%s\".", error.AsCString()); } @@ -4145,7 +4142,7 @@ } thread_result_t result = NULL; -private_state_thread.Join(&result); +m_private_state_thread.Join(&result); m_private_state_thread.Reset(); } } @@ -4449,7 +4446,6 @@ if (!is_secondary_thread) m_public_run_lock.SetStopped(); m_private_state_control_wait.SetValue (true, eBroadcastAlways); -m_private_state_thread.Reset(); return NULL; } Index: include/lldb/Target/Process.h === --- include/lldb/Target/Process.h +++ include/lldb/Target/Process.h @@ -3310,7 +3310,10 @@ bool PrivateStateThreadIsValid () const { -return m_private_state_thread.IsJoinable(); +lldb::StateType state = m_private_state.GetValue(); +return state != lldb::eStateDetached && + state != lldb::eStateExited && + m_private_state_thread.IsJoinable(); } void Index: source/Target/Process.cpp === --- source/Target/Process.cpp +++ source/Target/Process.cpp @@ -4112,11 +4112,8 @@ if (log) log->Printf ("Process::%s (signal = %d)", __FUNCTION__, signal); -// Signal the private state thread. First we should copy this is case the -// thread starts exiting since the private state thread will NULL this out -// when it exits -HostThread private_state_thread(m_private_state_thread); -if (private_state_thread.IsJoinable()) +// Signal the private state thread +if (PrivateStateThreadIsValid()) { TimeValue timeout_time; bool timed_out; @@ -4134,7 +4131,7 @@ { if (timed_out) { -Error error = private_state_thread.Cancel(); +Error error = m_private_state_thread.Cancel(); if (log) log->Printf ("Timed out responding to the control event, cancel got error: \"%s\".", error.AsCString()); } @@ -4145,7 +4142,7 @@ } thread_result_t result = NULL; -private_state_thread.Join(&result); +m_private_state_thread.Join(&result); m_private_state_thread.Reset(); } } @@ -4449,7 +4446,6 @@ if (!is_secondary_thread) m_public_run_lock.SetStopped(); m_private_state_control_wait.SetValue (true, eBroadcastAlways); -m_private_state_thread.Reset(); return NULL; } Index: include/lldb/Target/Process.h === --- include/lldb/Target/Process.h +++ include/lldb/Target/Process.h @@ -3310,7 +3310,10 @@ bool PrivateStateThreadIsValid () const { -return m_private_state_thread.IsJoinable(); +lldb::StateType state = m_private_state.GetValue(); +return state != lldb::eStateDetached && + state != lldb::eStateExited && + m_private_state_thread.IsJoinable(); } void ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D19122: LLDB: Fixed race condition on timeout when stopping private state thread
clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. Looks good. http://reviews.llvm.org/D19122 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D19215: normalize test file extension for test filenames
tfiala added a comment. Side note: Okay I think the mystery of this was caught by Jim. When we started accepting usage of .pycs again, we're allowing the python runtime to select using the pyc if available and not older than the .py file. This makes it possible to get a test file that is sometimes the .py and sometimes the .pyc. I'm going to be looking into getting a verification mechanism for the test infrastructure that covers items like this. This caused a whole lot of trouble on CI and, had we had tests for the test infrastructure itself, we could have avoided this. I'll take an action item on that. http://reviews.llvm.org/D19215 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D19215: normalize test file extension for test filenames
tfiala updated this revision to Diff 54102. tfiala added a comment. Final change: this change: 1. has the lldbinline.py test intercept its __file parameter and convert from .pyc to .py before storing to the test.test_filename attribute. 2. has the test event creation mechanism and test_event usage mechanism always validate that the filename ends in .py. It will generate an exception if not. If we find that #2 is triggering sometimes, I want to go back to being permissive in input now that it is clear this started when we removed the disabling feature that prohibited .pyc files from being generated. http://reviews.llvm.org/D19215 Files: packages/Python/lldbsuite/test/lldbinline.py packages/Python/lldbsuite/test/result_formatter.py Index: packages/Python/lldbsuite/test/result_formatter.py === --- packages/Python/lldbsuite/test/result_formatter.py +++ packages/Python/lldbsuite/test/result_formatter.py @@ -64,7 +64,7 @@ def create_socket(port): """Creates a socket to the localhost on the given port. -@param port the port number of the listenering port on +@param port the port number of the listening port on the localhost. @return (socket object, socket closing function) @@ -243,6 +243,13 @@ return event @staticmethod +def _assert_is_python_sourcefile(test_filename): +if test_filename is not None: +if not test_filename.endswith(".py"): +raise Exception("source python filename has unexpected extension: {}".format(test_filename)) +return test_filename + +@staticmethod def _event_dictionary_common(test, event_type): """Returns an event dictionary setup with values for the given event type. @@ -257,9 +264,9 @@ # Determine the filename for the test case. If there is an attribute # for it, use it. Otherwise, determine from the TestCase class path. if hasattr(test, "test_filename"): -test_filename = test.test_filename +test_filename = EventBuilder._assert_is_python_sourcefile(test.test_filename) else: -test_filename = inspect.getsourcefile(test.__class__) +test_filename = EventBuilder._assert_is_python_sourcefile(inspect.getsourcefile(test.__class__)) event = EventBuilder.bare_event(event_type) event.update({ @@ -498,7 +505,7 @@ if exception_description is not None: event["exception_description"] = exception_description if test_filename is not None: -event["test_filename"] = test_filename +event["test_filename"] = EventBuilder._assert_is_python_sourcefile(test_filename) if command_line is not None: event["command_line"] = command_line return event @@ -522,7 +529,7 @@ if worker_index is not None: event["worker_index"] = int(worker_index) if test_filename is not None: -event["test_filename"] = test_filename +event["test_filename"] = EventBuilder._assert_is_python_sourcefile(test_filename) if command_line is not None: event["command_line"] = command_line return event Index: packages/Python/lldbsuite/test/lldbinline.py === --- packages/Python/lldbsuite/test/lldbinline.py +++ packages/Python/lldbsuite/test/lldbinline.py @@ -189,6 +189,12 @@ def MakeInlineTest(__file, __globals, decorators=None): +# Adjust the filename if it ends in .pyc. We want filenames to +# reflect the source python file, not the compiled variant. +if __file is not None and __file.endswith(".pyc"): +# Strip the trailing "c" +__file = __file[0:-1] + # Derive the test name from the current file name file_basename = os.path.basename(__file) InlineTest.mydir = TestBase.compute_mydir(__file) @@ -205,7 +211,8 @@ # Add the test case to the globals, and hide InlineTest __globals.update({test_name : test}) -# Store the name of the originating file.o +# Keep track of the original test filename so we report it +# correctly in test results. test.test_filename = __file return test Index: packages/Python/lldbsuite/test/result_formatter.py === --- packages/Python/lldbsuite/test/result_formatter.py +++ packages/Python/lldbsuite/test/result_formatter.py @@ -64,7 +64,7 @@ def create_socket(port): """Creates a socket to the localhost on the given port. -@param port the port number of the listenering port on +@param port the port number of the listening port on the localhost. @return (socket object, socket closing function) @@ -243,6 +243,13 @@ return event @staticmethod +def _assert_is_python_so
[Lldb-commits] [lldb] r266664 - ensure lldbinline remembers .py extension
Author: tfiala Date: Mon Apr 18 15:26:56 2016 New Revision: 24 URL: http://llvm.org/viewvc/llvm-project?rev=24&view=rev Log: ensure lldbinline remembers .py extension This ensure lldbinline.test_file paths are tracked as .py files rather than .pyc files. Also, this change adds an assert to the test infrastructure if a filename that is not ending in .py is attempted to be added to the test events infrastructure where we track test results. See: http://reviews.llvm.org/D19215 Earlier revision reviewed by: Pavel Labath Modified: lldb/trunk/packages/Python/lldbsuite/test/lldbinline.py lldb/trunk/packages/Python/lldbsuite/test/result_formatter.py Modified: lldb/trunk/packages/Python/lldbsuite/test/lldbinline.py URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lldbinline.py?rev=24&r1=23&r2=24&view=diff == --- lldb/trunk/packages/Python/lldbsuite/test/lldbinline.py (original) +++ lldb/trunk/packages/Python/lldbsuite/test/lldbinline.py Mon Apr 18 15:26:56 2016 @@ -189,6 +189,12 @@ def ApplyDecoratorsToFunction(func, deco def MakeInlineTest(__file, __globals, decorators=None): +# Adjust the filename if it ends in .pyc. We want filenames to +# reflect the source python file, not the compiled variant. +if __file is not None and __file.endswith(".pyc"): +# Strip the trailing "c" +__file = __file[0:-1] + # Derive the test name from the current file name file_basename = os.path.basename(__file) InlineTest.mydir = TestBase.compute_mydir(__file) @@ -205,7 +211,8 @@ def MakeInlineTest(__file, __globals, de # Add the test case to the globals, and hide InlineTest __globals.update({test_name : test}) -# Store the name of the originating file.o +# Keep track of the original test filename so we report it +# correctly in test results. test.test_filename = __file return test Modified: lldb/trunk/packages/Python/lldbsuite/test/result_formatter.py URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/result_formatter.py?rev=24&r1=23&r2=24&view=diff == --- lldb/trunk/packages/Python/lldbsuite/test/result_formatter.py (original) +++ lldb/trunk/packages/Python/lldbsuite/test/result_formatter.py Mon Apr 18 15:26:56 2016 @@ -64,7 +64,7 @@ def create_results_formatter(config): def create_socket(port): """Creates a socket to the localhost on the given port. -@param port the port number of the listenering port on +@param port the port number of the listening port on the localhost. @return (socket object, socket closing function) @@ -243,6 +243,13 @@ class EventBuilder(object): return event @staticmethod +def _assert_is_python_sourcefile(test_filename): +if test_filename is not None: +if not test_filename.endswith(".py"): +raise Exception("source python filename has unexpected extension: {}".format(test_filename)) +return test_filename + +@staticmethod def _event_dictionary_common(test, event_type): """Returns an event dictionary setup with values for the given event type. @@ -257,9 +264,9 @@ class EventBuilder(object): # Determine the filename for the test case. If there is an attribute # for it, use it. Otherwise, determine from the TestCase class path. if hasattr(test, "test_filename"): -test_filename = test.test_filename +test_filename = EventBuilder._assert_is_python_sourcefile(test.test_filename) else: -test_filename = inspect.getsourcefile(test.__class__) +test_filename = EventBuilder._assert_is_python_sourcefile(inspect.getsourcefile(test.__class__)) event = EventBuilder.bare_event(event_type) event.update({ @@ -498,7 +505,7 @@ class EventBuilder(object): if exception_description is not None: event["exception_description"] = exception_description if test_filename is not None: -event["test_filename"] = test_filename +event["test_filename"] = EventBuilder._assert_is_python_sourcefile(test_filename) if command_line is not None: event["command_line"] = command_line return event @@ -522,7 +529,7 @@ class EventBuilder(object): if worker_index is not None: event["worker_index"] = int(worker_index) if test_filename is not None: -event["test_filename"] = test_filename +event["test_filename"] = EventBuilder._assert_is_python_sourcefile(test_filename) if command_line is not None: event["command_line"] = command_line return event ___ lldb
Re: [Lldb-commits] [PATCH] D19215: normalize test file extension for test filenames
tfiala closed this revision. tfiala added a comment. Closed via commit: r24 http://reviews.llvm.org/D19215 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D19230: Properly unload modules from target image list when using svr4 packets
tfiala added inline comments. Comment at: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:4907 @@ -4883,2 +4906,3 @@ +m_process->GetTarget().ModulesDidUnload (removed_modules, false); new_modules.ForEach ([&target](const lldb::ModuleSP module_sp) -> bool It looks like this code will unload any modules currently listed as loaded via m_process->GetTarget().GetImages(), if they do not appear in the module_list argument to this function. Is that correct behavior? (It might be, but that's not what I would have guessed without digging deeper). I might be not following the flow here correctly, though. Can you clarify? Did the full test suite run with this change? Thanks! http://reviews.llvm.org/D19230 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D19216: test infra cleanup: make test_runner/lib into the test_runner package
tfiala updated this revision to Diff 54115. tfiala added a comment. Moves optional_with into its own lldbsuite/support module. http://reviews.llvm.org/D19216 Files: packages/Python/lldbsuite/support/optional_with.py packages/Python/lldbsuite/test/dosep.py packages/Python/lldbsuite/test/test_runner/lib/lldb_utils.py packages/Python/lldbsuite/test/test_runner/lib/process_control.py Index: packages/Python/lldbsuite/test/test_runner/lib/process_control.py === --- packages/Python/lldbsuite/test/test_runner/lib/process_control.py +++ /dev/null @@ -1,705 +0,0 @@ -""" -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. - - -This module provides process-management support for the LLDB test -running infrasructure. -""" - -# System imports -import os -import re -import signal -import subprocess -import sys -import threading - - -class CommunicatorThread(threading.Thread): -"""Provides a thread class that communicates with a subprocess.""" -def __init__(self, process, event, output_file): -super(CommunicatorThread, self).__init__() -# Don't let this thread prevent shutdown. -self.daemon = True -self.process = process -self.pid = process.pid -self.event = event -self.output_file = output_file -self.output = None - -def run(self): -try: -# Communicate with the child process. -# This will not complete until the child process terminates. -self.output = self.process.communicate() -except Exception as exception: # pylint: disable=broad-except -if self.output_file: -self.output_file.write( -"exception while using communicate() for pid: {}\n".format( -exception)) -finally: -# Signal that the thread's run is complete. -self.event.set() - - -# Provides a regular expression for matching gtimeout-based durations. -TIMEOUT_REGEX = re.compile(r"(^\d+)([smhd])?$") - - -def timeout_to_seconds(timeout): -"""Converts timeout/gtimeout timeout values into seconds. - -@param timeout a timeout in the form of xm representing x minutes. - -@return None if timeout is None, or the number of seconds as a float -if a valid timeout format was specified. -""" -if timeout is None: -return None -else: -match = TIMEOUT_REGEX.match(timeout) -if match: -value = float(match.group(1)) -units = match.group(2) -if units is None: -# default is seconds. No conversion necessary. -return value -elif units == 's': -# Seconds. No conversion necessary. -return value -elif units == 'm': -# Value is in minutes. -return 60.0 * value -elif units == 'h': -# Value is in hours. -return (60.0 * 60.0) * value -elif units == 'd': -# Value is in days. -return 24 * (60.0 * 60.0) * value -else: -raise Exception("unexpected units value '{}'".format(units)) -else: -raise Exception("could not parse TIMEOUT spec '{}'".format( -timeout)) - - -class ProcessHelper(object): -"""Provides an interface for accessing process-related functionality. - -This class provides a factory method that gives the caller a -platform-specific implementation instance of the class. - -Clients of the class should stick to the methods provided in this -base class. - -@see ProcessHelper.process_helper() -""" -def __init__(self): -super(ProcessHelper, self).__init__() - -@classmethod -def process_helper(cls): -"""Returns a platform-specific ProcessHelper instance. -@return a ProcessHelper instance that does the right thing for -the current platform. -""" - -# If you add a new platform, create an instance here and -# return it. -if os.name == "nt": -return WindowsProcessHelper() -else: -# For all POSIX-like systems. -return UnixProcessHelper() - -def create_piped_process(self, command, new_process_group=True): -# pylint: disable=no-self-use,unused-argument -# As expected. We want derived classes to implement this. -"""Creates a subprocess.Popen-based class with I/O piped to the parent. - -@param command the command line list as would be passed to -subprocess.Popen(). Use the list form rather than the string form. - -@param new_process_group indicates if the call
Re: [Lldb-commits] [PATCH] D19216: test infra cleanup: make test_runner/lib into the test_runner package
zturner accepted this revision. zturner added a comment. This revision is now accepted and ready to land. thanks http://reviews.llvm.org/D19216 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D19124: [LLDB] Added support for PHI nodes to IR interpreter
cameron314 removed rL LLVM as the repository for this revision. cameron314 updated this revision to Diff 54134. cameron314 added a comment. I've added a test for this patch. http://reviews.llvm.org/D19124 Files: packages/Python/lldbsuite/test/expression_command/ir-interpreter/Makefile packages/Python/lldbsuite/test/expression_command/ir-interpreter/TestIRInterpreter.py packages/Python/lldbsuite/test/expression_command/ir-interpreter/main.cpp source/Commands/CommandObjectExpression.cpp source/Commands/CommandObjectExpression.h source/Expression/IRInterpreter.cpp Index: source/Expression/IRInterpreter.cpp === --- source/Expression/IRInterpreter.cpp +++ source/Expression/IRInterpreter.cpp @@ -106,6 +106,7 @@ DataLayout &m_target_data; lldb_private::IRExecutionUnit &m_execution_unit; const BasicBlock *m_bb; +const BasicBlock *m_prev_bb; BasicBlock::const_iterator m_ii; BasicBlock::const_iterator m_ie; @@ -121,7 +122,9 @@ lldb::addr_t stack_frame_bottom, lldb::addr_t stack_frame_top) : m_target_data (target_data), -m_execution_unit (execution_unit) +m_execution_unit (execution_unit), +m_bb (nullptr), +m_prev_bb (nullptr) { m_byte_order = (target_data.isLittleEndian() ? lldb::eByteOrderLittle : lldb::eByteOrderBig); m_addr_byte_size = (target_data.getPointerSize(0)); @@ -137,6 +140,7 @@ void Jump (const BasicBlock *bb) { +m_prev_bb = m_bb; m_bb = bb; m_ii = m_bb->begin(); m_ie = m_bb->end(); @@ -562,6 +566,7 @@ case Instruction::Alloca: case Instruction::BitCast: case Instruction::Br: +case Instruction::PHI: break; case Instruction::Call: { @@ -1055,6 +1060,46 @@ } } continue; +case Instruction::PHI: +{ +const PHINode *phi_inst = dyn_cast(inst); + +if (!phi_inst) +{ +if (log) +log->Printf("getOpcode() returns PHI, but instruction is not a PHINode"); +error.SetErrorToGenericError(); +error.SetErrorString(interpreter_internal_error); +return false; +} +if (!frame.m_prev_bb) +{ +if (log) +log->Printf("Encountered PHI node without having jumped from another basic block"); +error.SetErrorToGenericError(); +error.SetErrorString(interpreter_internal_error); +return false; +} + +Value* value = phi_inst->getIncomingValueForBlock(frame.m_prev_bb); +lldb_private::Scalar result; +if (!frame.EvaluateValue(result, value, module)) +{ +if (log) +log->Printf("Couldn't evaluate %s", PrintValue(value).c_str()); +error.SetErrorToGenericError(); +error.SetErrorString(bad_value_error); +return false; +} +frame.AssignValue(inst, result, module); + +if (log) +{ +log->Printf("Interpreted a %s", inst->getOpcodeName()); +log->Printf(" Incoming value : %s", frame.SummarizeValue(value).c_str()); +} +} +break; case Instruction::GetElementPtr: { const GetElementPtrInst *gep_inst = dyn_cast(inst); Index: source/Commands/CommandObjectExpression.h === --- source/Commands/CommandObjectExpression.h +++ source/Commands/CommandObjectExpression.h @@ -57,6 +57,7 @@ booltop_level; boolunwind_on_error; boolignore_breakpoints; +boolallow_jit; boolshow_types; boolshow_summary; booldebug; Index: source/Commands/CommandObjectExpression.cpp === --- source/Commands/CommandObjectExpression.cpp +++ source/Commands/CommandObjectExpression.cpp @@ -63,7 +63,8 @@ { LLDB_OPT_SET_1 | LLDB_OPT_SET_2, false, "language", 'l', OptionParser::eRequiredArgument, nullptr, nullptr, 0, eArgTypeLanguage, "Specifies the Language to use when parsing the expression. If not set the target.language setting is used." }, { LLDB_OPT_SET_1 | LLDB_OPT_SET_2, false, "apply-fixits", 'X', OptionParser::eRequiredA
Re: [Lldb-commits] [PATCH] D19230: Properly unload modules from target image list when using svr4 packets
fjricci added inline comments. Comment at: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:4907 @@ -4883,2 +4906,3 @@ +m_process->GetTarget().ModulesDidUnload (removed_modules, false); new_modules.ForEach ([&target](const lldb::ModuleSP module_sp) -> bool tfiala wrote: > It looks like this code will unload any modules currently listed as loaded > via m_process->GetTarget().GetImages(), if they do not appear in the > module_list argument to this function. Is that correct behavior? (It might > be, but that's not what I would have guessed without digging deeper). > > I might be not following the flow here correctly, though. Can you clarify? > Did the full test suite run with this change? Thanks! So yes, this will remove any existing modules that are not in the svr4 packet, provided that there were modules listed in the svr4 packet (indicating that the remote is using the packet) - if `new_modules.GetSize() == 0`, we won't remove anything. As far as I can tell from the gdb protocol specs, the svr4 packet should contain a list of all loaded libraries, so as long as the svr4 packet contains libraries, I believe that removing modules which are no longer listed is the correct behavior. I did run the full suite on linux (with lldb-server), and it passes with this change. As a side note, the module_list argument is actually an output parameter, filled by GetLoadedModuleList(). http://reviews.llvm.org/D19230 ___ 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 updated and restarted tonight
Hello everyone, LLVM buildmaster will be updated and 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
[Lldb-commits] [lldb] r266702 - Fix Windows build.
Author: chaoren Date: Mon Apr 18 20:09:37 2016 New Revision: 266702 URL: http://llvm.org/viewvc/llvm-project?rev=266702&view=rev Log: Fix Windows build. Modified: lldb/trunk/source/Host/windows/Windows.cpp Modified: lldb/trunk/source/Host/windows/Windows.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/windows/Windows.cpp?rev=266702&r1=266701&r2=266702&view=diff == --- lldb/trunk/source/Host/windows/Windows.cpp (original) +++ lldb/trunk/source/Host/windows/Windows.cpp Mon Apr 18 20:09:37 2016 @@ -14,6 +14,7 @@ #include "llvm/Support/ConvertUTF.h" +#include #include #include #include ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D19252: Handle invalid values of PLT entry size generated by ld + gcc on arm linux targets.
omjavaid created this revision. omjavaid added reviewers: tberghammer, rengolin, clayborg. omjavaid added a subscriber: lldb-commits. Herald added subscribers: danalbert, tberghammer, rengolin, aemerson. This patch provides a fix for wrong plt entry size generated for binaries built with gcc and linked with ld for arm linux targets. Many tests fail on arm-linux targets for this very issues. Luckily on Android arm32 targets we get a zero size for which there is already a fix available in the code. Effect of this patch appears when code jumps into plt code and tries to calculate frame for current PC. A wrong calculation of plt entry addresses ranges results in failure to calculate frame hence stepping failures when dealing with any library functions using procedure linkage table. LD produces 12 byte plt entries for arm and can also produce 16 byte entries but by no means plt entry can be 4 bytes which appears while we decode plt header. No other architecture in my knowledge uses a PLT slot of less than or equal to 4bytes. I could be wrong but in my knowledge a PLT slot is at least 2 instructions on a 32bit machine s which is 8 bytes and a lot higher for 64 bit machines so I have made the code change to handle all casses below or equal 4 bytes with manual calculation. This fixes issues on arm targets. LGTM? or comments? http://reviews.llvm.org/D19252 Files: source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp Index: source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp === --- source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp +++ source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp @@ -2510,7 +2510,7 @@ elf_xword plt_entsize = plt_hdr->sh_addralign ? llvm::alignTo (plt_hdr->sh_entsize, plt_hdr->sh_addralign) : plt_hdr->sh_entsize; -if (plt_entsize == 0) +if (plt_entsize <= 4) { // The linker haven't set the plt_hdr->sh_entsize field. Try to guess the size of the plt // entries based on the number of entries and the size of the plt section with the Index: source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp === --- source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp +++ source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp @@ -2510,7 +2510,7 @@ elf_xword plt_entsize = plt_hdr->sh_addralign ? llvm::alignTo (plt_hdr->sh_entsize, plt_hdr->sh_addralign) : plt_hdr->sh_entsize; -if (plt_entsize == 0) +if (plt_entsize <= 4) { // The linker haven't set the plt_hdr->sh_entsize field. Try to guess the size of the plt // entries based on the number of entries and the size of the plt section with the ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D19230: Properly unload modules from target image list when using svr4 packets
tfiala accepted this revision. tfiala added a comment. This revision is now accepted and ready to land. LGTM. Comment at: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:4907 @@ -4883,2 +4906,3 @@ +m_process->GetTarget().ModulesDidUnload (removed_modules, false); new_modules.ForEach ([&target](const lldb::ModuleSP module_sp) -> bool fjricci wrote: > tfiala wrote: > > It looks like this code will unload any modules currently listed as loaded > > via m_process->GetTarget().GetImages(), if they do not appear in the > > module_list argument to this function. Is that correct behavior? (It > > might be, but that's not what I would have guessed without digging deeper). > > > > I might be not following the flow here correctly, though. Can you clarify? > > Did the full test suite run with this change? Thanks! > So yes, this will remove any existing modules that are not in the svr4 > packet, provided that there were modules listed in the svr4 packet > (indicating that the remote is using the packet) - if `new_modules.GetSize() > == 0`, we won't remove anything. > > As far as I can tell from the gdb protocol specs, the svr4 packet should > contain a list of all loaded libraries, so as long as the svr4 packet > contains libraries, I believe that removing modules which are no longer > listed is the correct behavior. > > I did run the full suite on linux (with lldb-server), and it passes with this > change. > > As a side note, the module_list argument is actually an output parameter, > filled by GetLoadedModuleList(). Oh of course I see the flow. http://reviews.llvm.org/D19230 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r266710 - test infra cleanup: convert test_runner lib into package
Author: tfiala Date: Mon Apr 18 23:20:35 2016 New Revision: 266710 URL: http://llvm.org/viewvc/llvm-project?rev=266710&view=rev Log: test infra cleanup: convert test_runner lib into package Also does the following: * adopts PEP8 naming convention for OptionalWith class (now optional_with). * moves test_runner/lldb_utils.py to lldbsuite/support/optional_with.py. * packages tests in a subpackage of test_runner per recommendations in http://the-hitchhikers-guide-to-packaging.readthedocs.org/en/latest/creation.html Tests can be run from within pacakges/Python/lldbsuite/test via this command: python -m unittest discover test_runner The primary cleanup this allows is avoiding the need to muck with the PYTHONPATH variable from within the source files. This also aids some of the static code checkers as they don't need to run code to determine the proper python path. Added: lldb/trunk/packages/Python/lldbsuite/support/optional_with.py - copied, changed from r266702, lldb/trunk/packages/Python/lldbsuite/test/test_runner/lib/lldb_utils.py lldb/trunk/packages/Python/lldbsuite/test/test_runner/__init__.py lldb/trunk/packages/Python/lldbsuite/test/test_runner/process_control.py - copied, changed from r266702, lldb/trunk/packages/Python/lldbsuite/test/test_runner/lib/process_control.py lldb/trunk/packages/Python/lldbsuite/test/test_runner/test/__init__.py lldb/trunk/packages/Python/lldbsuite/test/test_runner/test/test_process_control.py - copied, changed from r266702, lldb/trunk/packages/Python/lldbsuite/test/test_runner/test/process_control_tests.py Removed: lldb/trunk/packages/Python/lldbsuite/test/test_runner/lib/lldb_utils.py lldb/trunk/packages/Python/lldbsuite/test/test_runner/lib/process_control.py lldb/trunk/packages/Python/lldbsuite/test/test_runner/test/process_control_tests.py Modified: lldb/trunk/packages/Python/lldbsuite/test/dosep.py Copied: lldb/trunk/packages/Python/lldbsuite/support/optional_with.py (from r266702, lldb/trunk/packages/Python/lldbsuite/test/test_runner/lib/lldb_utils.py) URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/support/optional_with.py?p2=lldb/trunk/packages/Python/lldbsuite/support/optional_with.py&p1=lldb/trunk/packages/Python/lldbsuite/test/test_runner/lib/lldb_utils.py&r1=266702&r2=266710&rev=266710&view=diff == --- lldb/trunk/packages/Python/lldbsuite/test/test_runner/lib/lldb_utils.py (original) +++ lldb/trunk/packages/Python/lldbsuite/support/optional_with.py Mon Apr 18 23:20:35 2016 @@ -1,18 +1,8 @@ -""" -The LLVM Compiler Infrastructure +# +# Provides a with-style resource handler for optionally-None resources +# -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. - - -This module contains utilities used by the lldb test framwork. -""" - - -class OptionalWith(object): +class optional_with(object): # pylint: disable=too-few-public-methods # This is a wrapper - it is not meant to provide any extra methods. """Provides a wrapper for objects supporting "with", allowing None. @@ -22,13 +12,13 @@ class OptionalWith(object): e.g. -wrapped_lock = OptionalWith(thread.Lock()) +wrapped_lock = optional_with(thread.Lock()) with wrapped_lock: # Do something while the lock is obtained. pass might_be_none = None -wrapped_none = OptionalWith(might_be_none) +wrapped_none = optional_with(might_be_none) with wrapped_none: # This code here still works. pass @@ -40,13 +30,13 @@ class OptionalWith(object): lock.acquire() try: -code_fragament_always_run() +code_fragment_always_run() finally: if lock: lock.release() And I'd posit it is safer, as it becomes impossible to -forget the try/finally using OptionalWith(), since +forget the try/finally using optional_with(), since the with syntax can be used. """ def __init__(self, wrapped_object): Modified: lldb/trunk/packages/Python/lldbsuite/test/dosep.py URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/dosep.py?rev=266710&r1=266709&r2=266710&view=diff == --- lldb/trunk/packages/Python/lldbsuite/test/dosep.py (original) +++ lldb/trunk/packages/Python/lldbsuite/test/dosep.py Mon Apr 18 23:20:35 2016 @@ -52,6 +52,7 @@ from six.moves import queue import lldbsuite import lldbsuite.support.seven as seven +from lldbsuite.support import optional_with from . import configuration from . import d
Re: [Lldb-commits] [PATCH] D19216: test infra cleanup: make test_runner/lib into the test_runner package
tfiala closed this revision. tfiala added a comment. Closed via commit: r266710 commit 4ef9c1c5dcb05a159929cd3b407481ed86a73ef5 (HEAD -> master, origin/master, origin/HEAD) Author: Todd Fiala Date: Mon Apr 18 21:20:35 2016 test infra cleanup: convert test_runner lib into package Also does the following: * adopts PEP8 naming convention for OptionalWith class (now optional_with). * moves test_runner/lldb_utils.py to lldbsuite/support/optional_with.py. * packages tests in a subpackage of test_runner per recommendations in http://the-hitchhikers-guide-to-packaging.readthedocs.org/en/latest/creation.html Tests can be run from within pacakges/Python/lldbsuite/test via this command: python -m unittest discover test_runner The primary cleanup this allows is avoiding the need to muck with the PYTHONPATH variable from within the source files. This also aids some of the static code checkers as they don't need to run code to determine the proper python path. git-svn-id: https://llvm.org/svn/llvm-project/lldb/trunk@266710 91177308-0d34-0410-b5e6-96231b3b80d8 http://reviews.llvm.org/D19216 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits