Re: [Lldb-commits] [PATCH] D19216: test infra cleanup: make test_runner/lib into the test_runner package

[Todd Fiala via lldb-commits]

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

[Lldb-commits] [lldb] r266710 - test infra cleanup: convert test_runner lib into package

[Todd Fiala via lldb-commits]

Author: tfiala
Date: Mon Apr 18 23:20:35 2016
New Revision: 266710

URL: http://llvm.org/viewvc/llvm-project?rev=266710=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=lldb/trunk/packages/Python/lldbsuite/test/test_runner/lib/lldb_utils.py=266702=266710=266710=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=266709=266710=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 dotest_channels
 from . import 

Re: [Lldb-commits] [PATCH] D19230: Properly unload modules from target image list when using svr4 packets

[Todd Fiala via lldb-commits]

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 ([](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] [PATCH] D19252: Handle invalid values of PLT entry size generated by ld + gcc on arm linux targets.

[Muhammad Omair Javaid via lldb-commits]

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

[Lldb-commits] [lldb] r266702 - Fix Windows build.

[Chaoren Lin via lldb-commits]

Author: chaoren
Date: Mon Apr 18 20:09:37 2016
New Revision: 266702

URL: http://llvm.org/viewvc/llvm-project?rev=266702=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=266701=266702=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] LLVM buildmaster will be updated and restarted tonight

[Galina Kistanova via lldb-commits]

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

Re: [Lldb-commits] [PATCH] D19230: Properly unload modules from target image list when using svr4 packets

[Francis Ricci via lldb-commits]

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 ([](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

Re: [Lldb-commits] [PATCH] D19124: [LLDB] Added support for PHI nodes to IR interpreter

[Cameron via lldb-commits]

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 _target_data;
 lldb_private::IRExecutionUnit  _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', 

Re: [Lldb-commits] [PATCH] D19216: test infra cleanup: make test_runner/lib into the test_runner package

[Zachary Turner via lldb-commits]

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] D19216: test infra cleanup: make test_runner/lib into the test_runner package

[Todd Fiala via lldb-commits]

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 

Re: [Lldb-commits] [PATCH] D19230: Properly unload modules from target image list when using svr4 packets

[Todd Fiala via lldb-commits]

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 ([](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] D19215: normalize test file extension for test filenames

[Todd Fiala via lldb-commits]

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

[Lldb-commits] [lldb] r266664 - ensure lldbinline remembers .py extension

[Todd Fiala via lldb-commits]

Author: tfiala
Date: Mon Apr 18 15:26:56 2016
New Revision: 24

URL: http://llvm.org/viewvc/llvm-project?rev=24=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=23=24=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=23=24=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-commits mailing list

Re: [Lldb-commits] [PATCH] D19215: normalize test file extension for test filenames

[Todd Fiala via lldb-commits]

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 

Re: [Lldb-commits] [PATCH] D19215: normalize test file extension for test filenames

[Todd Fiala via lldb-commits]

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] D19122: LLDB: Fixed race condition on timeout when stopping private state thread

[Greg Clayton via lldb-commits]

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] D19122: LLDB: Fixed race condition on timeout when stopping private state thread

[Cameron via lldb-commits]

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();
+m_private_state_thread.Join();
 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();
+m_private_state_thread.Join();
 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

[Cameron via lldb-commits]

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();
+m_private_state_thread.Join();
 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();
+m_private_state_thread.Join();
 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] D19215: normalize test file extension for test filenames

[Todd Fiala via lldb-commits]

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] D19215: normalize test file extension for test filenames

[Todd Fiala via lldb-commits]

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

[Zachary Turner via lldb-commits]

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

[Zachary Turner via lldb-commits]

(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

[Jim Ingham via lldb-commits]

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

[Zachary Turner via lldb-commits]

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

[Jim Ingham via lldb-commits]

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] D19216: test infra cleanup: make test_runner/lib into the test_runner package

[Todd Fiala via lldb-commits]

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] D19216: test infra cleanup: make test_runner/lib into the test_runner package

[Todd Fiala via lldb-commits]

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] D19215: normalize test file extension for test filenames

[Zachary Turner via lldb-commits]

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

[Zachary Turner via lldb-commits]

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

[Lldb-commits] [PATCH] D19230: Properly unload modules from target image list when using svr4 packets

[Francis Ricci via lldb-commits]

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  = GetTarget();
+ModuleList _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 ([](const lldb::ModuleSP module_sp) -> bool
 {
@@ -4895,13 +4919,11 @@
 return false;
 });
 
-ModuleList _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  = GetTarget();
+ModuleList _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 ([](const lldb::ModuleSP module_sp) -> bool
 {
@@ -4895,13 +4919,11 @@
 return false;
 });
 
-ModuleList _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] D19215: normalize test file extension for test filenames

[Pavel Labath via lldb-commits]

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

Re: [Lldb-commits] [PATCH] D19215: normalize test file extension for test filenames

[Todd Fiala via lldb-commits]

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 

Re: [Lldb-commits] [PATCH] D19215: normalize test file extension for test filenames

[Todd Fiala via lldb-commits]

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 

Re: [Lldb-commits] [PATCH] D19215: normalize test file extension for test filenames

[Todd Fiala via lldb-commits]

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

[Lldb-commits] [lldb] r266624 - fix a race is the LLDB test suite results collection

[Todd Fiala via lldb-commits]

Author: tfiala
Date: Mon Apr 18 11:09:21 2016
New Revision: 266624

URL: http://llvm.org/viewvc/llvm-project?rev=266624=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=266623=266624=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=266623=266624=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=266623=266624=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 test_timeout_rerun_succeeds(self):
-   

Re: [Lldb-commits] [PATCH] D19215: normalize test file extension for test filenames

[Todd Fiala via lldb-commits]

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

Re: [Lldb-commits] [PATCH] D19214: fix a race is the LLDB test suite results collection

[Todd Fiala via lldb-commits]

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

[Lldb-commits] [lldb] r266605 - Attempt to fix darwin build after header refactor in llvm (r266595)

[Pavel Labath via lldb-commits]

Author: labath
Date: Mon Apr 18 07:18:35 2016
New Revision: 266605

URL: http://llvm.org/viewvc/llvm-project?rev=266605=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=266604=266605=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

[Lldb-commits] [lldb] r266598 - Fixup r266327

[Pavel Labath via lldb-commits]

Author: labath
Date: Mon Apr 18 06:01:41 2016
New Revision: 266598

URL: http://llvm.org/viewvc/llvm-project?rev=266598=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=266597=266598=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

Re: [Lldb-commits] [PATCH] D19216: test infra cleanup: make test_runner/lib into the test_runner package

[Pavel Labath via lldb-commits]

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

Re: [Lldb-commits] [PATCH] D19215: normalize test file extension for test filenames

[Pavel Labath via lldb-commits]

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] D19214: fix a race is the LLDB test suite results collection

[Pavel Labath via lldb-commits]

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] D19067: Make sure to use lib instead of lib64 for LLDB_LIB_DIR

[Pavel Labath via lldb-commits]

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