[Lldb-commits] [PATCH] D77452: [intel-pt] Improve the way the test determines whether to run

[Phabricator via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGda0e91fee614: [intel-pt] Improve the way the test determines 
whether to run (authored by Walter Erquinigo wall...@fb.com).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77452/new/

https://reviews.llvm.org/D77452

Files:
  lldb/packages/Python/lldbsuite/test/configuration.py
  lldb/packages/Python/lldbsuite/test/dotest.py
  lldb/packages/Python/lldbsuite/test/dotest_args.py
  lldb/test/API/lit.cfg.py
  lldb/test/API/lit.site.cfg.py.in
  lldb/test/API/tools/intel-features/intel-pt/test/TestIntelPTSimpleBinary.py
  lldb/test/CMakeLists.txt
  lldb/utils/lldb-dotest/CMakeLists.txt
  lldb/utils/lldb-dotest/lldb-dotest.in

Index: lldb/utils/lldb-dotest/lldb-dotest.in
===
--- lldb/utils/lldb-dotest/lldb-dotest.in
+++ lldb/utils/lldb-dotest/lldb-dotest.in
@@ -11,6 +11,7 @@
 dsymutil = '@LLDB_TEST_DSYMUTIL_CONFIGURED@'
 filecheck = '@LLDB_TEST_FILECHECK_CONFIGURED@'
 lldb_libs_dir = "@LLDB_LIBS_DIR_CONFIGURED@"
+lldb_build_intel_pt = "@LLDB_BUILD_INTEL_PT@"
 
 if __name__ == '__main__':
 wrapper_args = sys.argv[1:]
@@ -25,6 +26,8 @@
 cmd.extend(['--dsymutil', dsymutil])
 cmd.extend(['--filecheck', filecheck])
 cmd.extend(['--lldb-libs-dir', lldb_libs_dir])
+if lldb_build_intel_pt == "1":
+cmd.extend(['--enable-plugin', 'intel-pt'])
 cmd.extend(wrapper_args)
 # Invoke dotest.py and return exit code.
 print(' '.join(cmd))
Index: lldb/utils/lldb-dotest/CMakeLists.txt
===
--- lldb/utils/lldb-dotest/CMakeLists.txt
+++ lldb/utils/lldb-dotest/CMakeLists.txt
@@ -6,6 +6,10 @@
 get_property(LLDB_DOTEST_ARGS GLOBAL PROPERTY LLDB_DOTEST_ARGS_PROPERTY)
 set(LLDB_LIBS_DIR "${LLVM_LIBRARY_OUTPUT_INTDIR}")
 
+llvm_canonicalize_cmake_booleans(
+  LLDB_BUILD_INTEL_PT
+)
+
 # Generate lldb-dotest Python driver script for each build mode.
 if(LLDB_BUILT_STANDALONE)
   set(config_types ".")
Index: lldb/test/CMakeLists.txt
===
--- lldb/test/CMakeLists.txt
+++ lldb/test/CMakeLists.txt
@@ -158,6 +158,7 @@
 
 # These values are not canonicalized within LLVM.
 llvm_canonicalize_cmake_booleans(
+  LLDB_BUILD_INTEL_PT
   LLDB_ENABLE_PYTHON
   LLDB_ENABLE_LUA
   LLDB_ENABLE_LZMA
Index: lldb/test/API/tools/intel-features/intel-pt/test/TestIntelPTSimpleBinary.py
===
--- lldb/test/API/tools/intel-features/intel-pt/test/TestIntelPTSimpleBinary.py
+++ lldb/test/API/tools/intel-features/intel-pt/test/TestIntelPTSimpleBinary.py
@@ -7,6 +7,7 @@
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
+from lldbsuite.test import configuration
 
 
 class TestIntelPTSimpleBinary(TestBase):
@@ -14,22 +15,22 @@
 mydir = TestBase.compute_mydir(__file__)
 NO_DEBUG_INFO_TESTCASE = True
 
+def setUp(self):
+TestBase.setUp(self)
+
+if 'intel-pt' not in configuration.enabled_plugins:
+self.skipTest("The '" + self.plugin + "' test plugin is not enabled")
+
+plugin_path = os.path.join(os.environ["LLDB_IMPLIB_DIR"], "liblldbIntelFeatures.so")
+self.runCmd("plugin load " + plugin_path)
+
 @skipIf(oslist=no_match(['linux']))
 @skipIf(archs=no_match(['i386', 'x86_64']))
 @skipIfRemote
 def test_basic_flow(self):
 """Test collection, decoding, and dumping instructions"""
-if os.environ.get('TEST_INTEL_PT') != '1':
-self.skipTest("The environment variable TEST_INTEL_PT=1 is needed to run this test.")
-
-lldb_exec_dir = os.environ["LLDB_IMPLIB_DIR"]
-lldb_lib_dir = os.path.join(lldb_exec_dir, os.pardir, "lib")
-plugin_file = os.path.join(lldb_lib_dir, "liblldbIntelFeatures.so")
 
 self.build()
-
-self.runCmd("plugin load " + plugin_file)
-
 exe = self.getBuildArtifact("a.out")
 lldbutil.run_to_name_breakpoint(self, "main", exe_name=exe)
 # We start tracing from main
@@ -52,9 +53,9 @@
 self.expect("processor-trace show-instr-log -c 100",
 patterns=[
 # We expect to have seen the first instruction of 'fun'
-hex(fun_start_adddress),  
+hex(fun_start_adddress),
 # We expect to see the exit condition of the for loop
-"at main.cpp:" + str(line_number('main.cpp', '// Break for loop')) 
+"at main.cpp:" + str(line_number('main.cpp', '// Break for loop'))
 ])
 
 self.runCmd("processor-trace stop")
Index: lldb/test/API/lit.site.cfg.py.in
===
--- lldb/test/API/lit.site.cfg.py.in
+++ lldb/test/API/lit.site.cfg.py.in
@@ -32,6 +32,11 @@
 

[Lldb-commits] [PATCH] D77452: [intel-pt] Improve the way the test determines whether to run

[walter erquinigo via Phabricator via lldb-commits]

wallace updated this revision to Diff 257450.
wallace added a comment.

move the skipping logic to the actual test


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77452/new/

https://reviews.llvm.org/D77452

Files:
  lldb/packages/Python/lldbsuite/test/configuration.py
  lldb/packages/Python/lldbsuite/test/dotest.py
  lldb/packages/Python/lldbsuite/test/dotest_args.py
  lldb/test/API/lit.cfg.py
  lldb/test/API/lit.site.cfg.py.in
  lldb/test/API/tools/intel-features/intel-pt/test/TestIntelPTSimpleBinary.py
  lldb/test/CMakeLists.txt
  lldb/utils/lldb-dotest/CMakeLists.txt
  lldb/utils/lldb-dotest/lldb-dotest.in

Index: lldb/utils/lldb-dotest/lldb-dotest.in
===
--- lldb/utils/lldb-dotest/lldb-dotest.in
+++ lldb/utils/lldb-dotest/lldb-dotest.in
@@ -11,6 +11,7 @@
 dsymutil = '@LLDB_TEST_DSYMUTIL_CONFIGURED@'
 filecheck = '@LLDB_TEST_FILECHECK_CONFIGURED@'
 lldb_libs_dir = "@LLDB_LIBS_DIR_CONFIGURED@"
+lldb_build_intel_pt = "@LLDB_BUILD_INTEL_PT@"
 
 if __name__ == '__main__':
 wrapper_args = sys.argv[1:]
@@ -25,6 +26,8 @@
 cmd.extend(['--dsymutil', dsymutil])
 cmd.extend(['--filecheck', filecheck])
 cmd.extend(['--lldb-libs-dir', lldb_libs_dir])
+if lldb_build_intel_pt == "1":
+cmd.extend(['--enable-plugin', 'intel-pt'])
 cmd.extend(wrapper_args)
 # Invoke dotest.py and return exit code.
 print(' '.join(cmd))
Index: lldb/utils/lldb-dotest/CMakeLists.txt
===
--- lldb/utils/lldb-dotest/CMakeLists.txt
+++ lldb/utils/lldb-dotest/CMakeLists.txt
@@ -6,6 +6,10 @@
 get_property(LLDB_DOTEST_ARGS GLOBAL PROPERTY LLDB_DOTEST_ARGS_PROPERTY)
 set(LLDB_LIBS_DIR "${LLVM_LIBRARY_OUTPUT_INTDIR}")
 
+llvm_canonicalize_cmake_booleans(
+  LLDB_BUILD_INTEL_PT
+)
+
 # Generate lldb-dotest Python driver script for each build mode.
 if(LLDB_BUILT_STANDALONE)
   set(config_types ".")
Index: lldb/test/CMakeLists.txt
===
--- lldb/test/CMakeLists.txt
+++ lldb/test/CMakeLists.txt
@@ -158,6 +158,7 @@
 
 # These values are not canonicalized within LLVM.
 llvm_canonicalize_cmake_booleans(
+  LLDB_BUILD_INTEL_PT
   LLDB_ENABLE_PYTHON
   LLDB_ENABLE_LUA
   LLDB_ENABLE_LZMA
Index: lldb/test/API/tools/intel-features/intel-pt/test/TestIntelPTSimpleBinary.py
===
--- lldb/test/API/tools/intel-features/intel-pt/test/TestIntelPTSimpleBinary.py
+++ lldb/test/API/tools/intel-features/intel-pt/test/TestIntelPTSimpleBinary.py
@@ -7,6 +7,7 @@
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
+from lldbsuite.test import configuration
 
 
 class TestIntelPTSimpleBinary(TestBase):
@@ -14,22 +15,22 @@
 mydir = TestBase.compute_mydir(__file__)
 NO_DEBUG_INFO_TESTCASE = True
 
+def setUp(self):
+TestBase.setUp(self)
+
+if 'intel-pt' not in configuration.enabled_plugins:
+self.skipTest("The '" + self.plugin + "' test plugin is not enabled")
+
+plugin_path = os.path.join(os.environ["LLDB_IMPLIB_DIR"], "liblldbIntelFeatures.so")
+self.runCmd("plugin load " + plugin_path)
+
 @skipIf(oslist=no_match(['linux']))
 @skipIf(archs=no_match(['i386', 'x86_64']))
 @skipIfRemote
 def test_basic_flow(self):
 """Test collection, decoding, and dumping instructions"""
-if os.environ.get('TEST_INTEL_PT') != '1':
-self.skipTest("The environment variable TEST_INTEL_PT=1 is needed to run this test.")
-
-lldb_exec_dir = os.environ["LLDB_IMPLIB_DIR"]
-lldb_lib_dir = os.path.join(lldb_exec_dir, os.pardir, "lib")
-plugin_file = os.path.join(lldb_lib_dir, "liblldbIntelFeatures.so")
 
 self.build()
-
-self.runCmd("plugin load " + plugin_file)
-
 exe = self.getBuildArtifact("a.out")
 lldbutil.run_to_name_breakpoint(self, "main", exe_name=exe)
 # We start tracing from main
@@ -52,9 +53,9 @@
 self.expect("processor-trace show-instr-log -c 100",
 patterns=[
 # We expect to have seen the first instruction of 'fun'
-hex(fun_start_adddress),  
+hex(fun_start_adddress),
 # We expect to see the exit condition of the for loop
-"at main.cpp:" + str(line_number('main.cpp', '// Break for loop')) 
+"at main.cpp:" + str(line_number('main.cpp', '// Break for loop'))
 ])
 
 self.runCmd("processor-trace stop")
Index: lldb/test/API/lit.site.cfg.py.in
===
--- lldb/test/API/lit.site.cfg.py.in
+++ lldb/test/API/lit.site.cfg.py.in
@@ -32,6 +32,11 @@
 config.lldb_module_cache = os.path.join("@LLDB_TEST_MODULE_CACHE_LLDB@", "lldb-api")
 

[Lldb-commits] [PATCH] D77452: [intel-pt] Improve the way the test determines whether to run

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Heh.. I wasn't an attempt expecting a fully generic solution. Since we don't 
invoke dotest.py manually these days (we have lldb-dotest for that) making a 
separate argument specifically for this plugin would be just fine. However, I 
don't see anything inherently wrong with this generic approach, so I guess we 
can leave it. The main think I don't like is the introduction of the skipping 
code into the base test class. These classes are already more complicated than 
they ought to be, so I'd like to avoid adding things to them, if those things 
can be implemented elsewhere.




Comment at: lldb/packages/Python/lldbsuite/test/lldbtest.py:736-738
+if self.plugin and self.plugin not in configuration.enabled_plugins:
+self.skipTest("The '" + self.plugin + "' plugin is not enabled")
+

Let's put this stuff in `TestIntelPTSimpleBinary` for now -- it can be moved 
into a separate base class (along with other common code) once we have more of 
these tests.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77452/new/

https://reviews.llvm.org/D77452



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D77452: [intel-pt] Improve the way the test determines whether to run

[walter erquinigo via Phabricator via lldb-commits]

wallace updated this revision to Diff 256715.
wallace edited the summary of this revision.
wallace removed a reviewer: JDevlieghere.
wallace removed a subscriber: JDevlieghere.
wallace added a comment.
Herald added a subscriber: mgorny.

Addressed comments. The diff description now reflects the latest changes.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77452/new/

https://reviews.llvm.org/D77452

Files:
  lldb/packages/Python/lldbsuite/test/configuration.py
  lldb/packages/Python/lldbsuite/test/dotest.py
  lldb/packages/Python/lldbsuite/test/dotest_args.py
  lldb/packages/Python/lldbsuite/test/lldbtest.py
  lldb/test/API/lit.cfg.py
  lldb/test/API/lit.site.cfg.py.in
  lldb/test/API/tools/intel-features/intel-pt/test/TestIntelPTSimpleBinary.py
  lldb/test/CMakeLists.txt
  lldb/utils/lldb-dotest/CMakeLists.txt
  lldb/utils/lldb-dotest/lldb-dotest.in

Index: lldb/utils/lldb-dotest/lldb-dotest.in
===
--- lldb/utils/lldb-dotest/lldb-dotest.in
+++ lldb/utils/lldb-dotest/lldb-dotest.in
@@ -11,6 +11,7 @@
 dsymutil = '@LLDB_TEST_DSYMUTIL_CONFIGURED@'
 filecheck = '@LLDB_TEST_FILECHECK_CONFIGURED@'
 lldb_libs_dir = "@LLDB_LIBS_DIR_CONFIGURED@"
+lldb_build_intel_pt = "@LLDB_BUILD_INTEL_PT@"
 
 if __name__ == '__main__':
 wrapper_args = sys.argv[1:]
@@ -25,7 +26,11 @@
 cmd.extend(['--dsymutil', dsymutil])
 cmd.extend(['--filecheck', filecheck])
 cmd.extend(['--lldb-libs-dir', lldb_libs_dir])
+if lldb_build_intel_pt == "1":
+cmd.extend(['--enable-plugin', 'intel-pt'])
+
 cmd.extend(wrapper_args)
+
 # Invoke dotest.py and return exit code.
 print(' '.join(cmd))
 sys.exit(subprocess.call(cmd))
Index: lldb/utils/lldb-dotest/CMakeLists.txt
===
--- lldb/utils/lldb-dotest/CMakeLists.txt
+++ lldb/utils/lldb-dotest/CMakeLists.txt
@@ -6,6 +6,10 @@
 get_property(LLDB_DOTEST_ARGS GLOBAL PROPERTY LLDB_DOTEST_ARGS_PROPERTY)
 set(LLDB_LIBS_DIR "${LLVM_LIBRARY_OUTPUT_INTDIR}")
 
+llvm_canonicalize_cmake_booleans(
+  LLDB_BUILD_INTEL_PT
+)
+
 # Generate lldb-dotest Python driver script for each build mode.
 if(LLDB_BUILT_STANDALONE)
   set(config_types ".")
Index: lldb/test/CMakeLists.txt
===
--- lldb/test/CMakeLists.txt
+++ lldb/test/CMakeLists.txt
@@ -158,6 +158,7 @@
 
 # These values are not canonicalized within LLVM.
 llvm_canonicalize_cmake_booleans(
+  LLDB_BUILD_INTEL_PT
   LLDB_ENABLE_PYTHON
   LLDB_ENABLE_LUA
   LLDB_ENABLE_LZMA
Index: lldb/test/API/tools/intel-features/intel-pt/test/TestIntelPTSimpleBinary.py
===
--- lldb/test/API/tools/intel-features/intel-pt/test/TestIntelPTSimpleBinary.py
+++ lldb/test/API/tools/intel-features/intel-pt/test/TestIntelPTSimpleBinary.py
@@ -12,24 +12,22 @@
 class TestIntelPTSimpleBinary(TestBase):
 
 mydir = TestBase.compute_mydir(__file__)
+plugin = 'intel-pt'
 NO_DEBUG_INFO_TESTCASE = True
 
+def setUp(self):
+TestBase.setUp(self)
+
+plugin_path = os.path.join(os.environ["LLDB_IMPLIB_DIR"], "liblldbIntelFeatures.so")
+self.runCmd("plugin load " + plugin_path)
+
 @skipIf(oslist=no_match(['linux']))
 @skipIf(archs=no_match(['i386', 'x86_64']))
 @skipIfRemote
 def test_basic_flow(self):
 """Test collection, decoding, and dumping instructions"""
-if os.environ.get('TEST_INTEL_PT') != '1':
-self.skipTest("The environment variable TEST_INTEL_PT=1 is needed to run this test.")
-
-lldb_exec_dir = os.environ["LLDB_IMPLIB_DIR"]
-lldb_lib_dir = os.path.join(lldb_exec_dir, os.pardir, "lib")
-plugin_file = os.path.join(lldb_lib_dir, "liblldbIntelFeatures.so")
 
 self.build()
-
-self.runCmd("plugin load " + plugin_file)
-
 exe = self.getBuildArtifact("a.out")
 lldbutil.run_to_name_breakpoint(self, "main", exe_name=exe)
 # We start tracing from main
@@ -52,9 +50,9 @@
 self.expect("processor-trace show-instr-log -c 100",
 patterns=[
 # We expect to have seen the first instruction of 'fun'
-hex(fun_start_adddress),  
+hex(fun_start_adddress),
 # We expect to see the exit condition of the for loop
-"at main.cpp:" + str(line_number('main.cpp', '// Break for loop')) 
+"at main.cpp:" + str(line_number('main.cpp', '// Break for loop'))
 ])
 
 self.runCmd("processor-trace stop")
Index: lldb/test/API/lit.site.cfg.py.in
===
--- lldb/test/API/lit.site.cfg.py.in
+++ lldb/test/API/lit.site.cfg.py.in
@@ -32,6 +32,11 @@
 config.lldb_module_cache = os.path.join("@LLDB_TEST_MODULE_CACHE_LLDB@", "lldb-api")
 config.clang_module_cache 

[Lldb-commits] [PATCH] D77452: [intel-pt] Improve the way the test determines whether to run

[walter erquinigo via Phabricator via lldb-commits]

wallace updated this revision to Diff 256716.
wallace added a comment.

remove blank line


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77452/new/

https://reviews.llvm.org/D77452

Files:
  lldb/packages/Python/lldbsuite/test/configuration.py
  lldb/packages/Python/lldbsuite/test/dotest.py
  lldb/packages/Python/lldbsuite/test/dotest_args.py
  lldb/packages/Python/lldbsuite/test/lldbtest.py
  lldb/test/API/lit.cfg.py
  lldb/test/API/lit.site.cfg.py.in
  lldb/test/API/tools/intel-features/intel-pt/test/TestIntelPTSimpleBinary.py
  lldb/test/CMakeLists.txt
  lldb/utils/lldb-dotest/CMakeLists.txt
  lldb/utils/lldb-dotest/lldb-dotest.in

Index: lldb/utils/lldb-dotest/lldb-dotest.in
===
--- lldb/utils/lldb-dotest/lldb-dotest.in
+++ lldb/utils/lldb-dotest/lldb-dotest.in
@@ -11,6 +11,7 @@
 dsymutil = '@LLDB_TEST_DSYMUTIL_CONFIGURED@'
 filecheck = '@LLDB_TEST_FILECHECK_CONFIGURED@'
 lldb_libs_dir = "@LLDB_LIBS_DIR_CONFIGURED@"
+lldb_build_intel_pt = "@LLDB_BUILD_INTEL_PT@"
 
 if __name__ == '__main__':
 wrapper_args = sys.argv[1:]
@@ -25,6 +26,8 @@
 cmd.extend(['--dsymutil', dsymutil])
 cmd.extend(['--filecheck', filecheck])
 cmd.extend(['--lldb-libs-dir', lldb_libs_dir])
+if lldb_build_intel_pt == "1":
+cmd.extend(['--enable-plugin', 'intel-pt'])
 cmd.extend(wrapper_args)
 # Invoke dotest.py and return exit code.
 print(' '.join(cmd))
Index: lldb/utils/lldb-dotest/CMakeLists.txt
===
--- lldb/utils/lldb-dotest/CMakeLists.txt
+++ lldb/utils/lldb-dotest/CMakeLists.txt
@@ -6,6 +6,10 @@
 get_property(LLDB_DOTEST_ARGS GLOBAL PROPERTY LLDB_DOTEST_ARGS_PROPERTY)
 set(LLDB_LIBS_DIR "${LLVM_LIBRARY_OUTPUT_INTDIR}")
 
+llvm_canonicalize_cmake_booleans(
+  LLDB_BUILD_INTEL_PT
+)
+
 # Generate lldb-dotest Python driver script for each build mode.
 if(LLDB_BUILT_STANDALONE)
   set(config_types ".")
Index: lldb/test/CMakeLists.txt
===
--- lldb/test/CMakeLists.txt
+++ lldb/test/CMakeLists.txt
@@ -158,6 +158,7 @@
 
 # These values are not canonicalized within LLVM.
 llvm_canonicalize_cmake_booleans(
+  LLDB_BUILD_INTEL_PT
   LLDB_ENABLE_PYTHON
   LLDB_ENABLE_LUA
   LLDB_ENABLE_LZMA
Index: lldb/test/API/tools/intel-features/intel-pt/test/TestIntelPTSimpleBinary.py
===
--- lldb/test/API/tools/intel-features/intel-pt/test/TestIntelPTSimpleBinary.py
+++ lldb/test/API/tools/intel-features/intel-pt/test/TestIntelPTSimpleBinary.py
@@ -12,24 +12,22 @@
 class TestIntelPTSimpleBinary(TestBase):
 
 mydir = TestBase.compute_mydir(__file__)
+plugin = 'intel-pt'
 NO_DEBUG_INFO_TESTCASE = True
 
+def setUp(self):
+TestBase.setUp(self)
+
+plugin_path = os.path.join(os.environ["LLDB_IMPLIB_DIR"], "liblldbIntelFeatures.so")
+self.runCmd("plugin load " + plugin_path)
+
 @skipIf(oslist=no_match(['linux']))
 @skipIf(archs=no_match(['i386', 'x86_64']))
 @skipIfRemote
 def test_basic_flow(self):
 """Test collection, decoding, and dumping instructions"""
-if os.environ.get('TEST_INTEL_PT') != '1':
-self.skipTest("The environment variable TEST_INTEL_PT=1 is needed to run this test.")
-
-lldb_exec_dir = os.environ["LLDB_IMPLIB_DIR"]
-lldb_lib_dir = os.path.join(lldb_exec_dir, os.pardir, "lib")
-plugin_file = os.path.join(lldb_lib_dir, "liblldbIntelFeatures.so")
 
 self.build()
-
-self.runCmd("plugin load " + plugin_file)
-
 exe = self.getBuildArtifact("a.out")
 lldbutil.run_to_name_breakpoint(self, "main", exe_name=exe)
 # We start tracing from main
@@ -52,9 +50,9 @@
 self.expect("processor-trace show-instr-log -c 100",
 patterns=[
 # We expect to have seen the first instruction of 'fun'
-hex(fun_start_adddress),  
+hex(fun_start_adddress),
 # We expect to see the exit condition of the for loop
-"at main.cpp:" + str(line_number('main.cpp', '// Break for loop')) 
+"at main.cpp:" + str(line_number('main.cpp', '// Break for loop'))
 ])
 
 self.runCmd("processor-trace stop")
Index: lldb/test/API/lit.site.cfg.py.in
===
--- lldb/test/API/lit.site.cfg.py.in
+++ lldb/test/API/lit.site.cfg.py.in
@@ -32,6 +32,11 @@
 config.lldb_module_cache = os.path.join("@LLDB_TEST_MODULE_CACHE_LLDB@", "lldb-api")
 config.clang_module_cache = os.path.join("@LLDB_TEST_MODULE_CACHE_CLANG@", "lldb-api")
 
+# Plugins
+lldb_build_intel_pt = '@LLDB_BUILD_INTEL_PT@'
+if lldb_build_intel_pt == '1':
+config.enabled_plugins = ['intel-pt']
+
 # Additional dotest arguments can be passed to lit by providing 

[Lldb-commits] [PATCH] D77452: [intel-pt] Improve the way the test determines whether to run

[Pavel Labath via Phabricator via lldb-commits]

labath added a reviewer: JDevlieghere.
labath added a subscriber: JDevlieghere.
labath added a comment.

Adding @JDevlieghere as he is in the same time zone and knows about all this 
stuff. This is on the right track, but changing lldb-dotest is not enough -- 
you'll also need to change `test/API/lit.cfg.py` as that's what's used for 
regular "check-lldb" runs. Also, instead of environment variables, please add a 
command line switch to dotest. Environment variables make it hard to reproduce 
steps when copy-pasting commands.




Comment at: lldb/utils/lldb-dotest/lldb-dotest.in:32
+
+if lldb_build_intel_pt not in ["", "false", "off", "n", "no", "ignore"] \
+and not lldb_build_intel_pt.endswith("-NOTFOUND"):

The usual solution to this is to run `llvm_canonicalize_cmake_booleans` in 
cmake.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77452/new/

https://reviews.llvm.org/D77452



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D77452: [intel-pt] Improve the way the test determines whether to run

[walter erquinigo via Phabricator via lldb-commits]

wallace updated this revision to Diff 255445.
wallace edited the summary of this revision.
wallace added a comment.

See the new description of the diff for the updates


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77452/new/

https://reviews.llvm.org/D77452

Files:
  lldb/test/API/tools/intel-features/intel-pt/test/TestIntelPTSimpleBinary.py
  lldb/utils/lldb-dotest/lldb-dotest.in


Index: lldb/utils/lldb-dotest/lldb-dotest.in
===
--- lldb/utils/lldb-dotest/lldb-dotest.in
+++ lldb/utils/lldb-dotest/lldb-dotest.in
@@ -1,6 +1,7 @@
 #!@PYTHON_EXECUTABLE@
 import subprocess
 import sys
+import os
 
 dotest_path = '@LLDB_SOURCE_DIR_CONFIGURED@/test/API/dotest.py'
 build_dir = '@LLDB_TEST_BUILD_DIRECTORY_CONFIGURED@'
@@ -11,6 +12,7 @@
 dsymutil = '@LLDB_TEST_DSYMUTIL_CONFIGURED@'
 filecheck = '@LLDB_TEST_FILECHECK_CONFIGURED@'
 lldb_libs_dir = "@LLDB_LIBS_DIR_CONFIGURED@"
+lldb_build_intel_pt = "@LLDB_BUILD_INTEL_PT@".lower()
 
 if __name__ == '__main__':
 wrapper_args = sys.argv[1:]
@@ -26,6 +28,11 @@
 cmd.extend(['--filecheck', filecheck])
 cmd.extend(['--lldb-libs-dir', lldb_libs_dir])
 cmd.extend(wrapper_args)
+
+if lldb_build_intel_pt not in ["", "false", "off", "n", "no", "ignore"] \
+and not lldb_build_intel_pt.endswith("-NOTFOUND"):
+os.environ['TEST_INTEL_PT'] = "true"
+
 # Invoke dotest.py and return exit code.
 print(' '.join(cmd))
 sys.exit(subprocess.call(cmd))
Index: 
lldb/test/API/tools/intel-features/intel-pt/test/TestIntelPTSimpleBinary.py
===
--- lldb/test/API/tools/intel-features/intel-pt/test/TestIntelPTSimpleBinary.py
+++ lldb/test/API/tools/intel-features/intel-pt/test/TestIntelPTSimpleBinary.py
@@ -14,22 +14,21 @@
 mydir = TestBase.compute_mydir(__file__)
 NO_DEBUG_INFO_TESTCASE = True
 
+def setUp(self):
+TestBase.setUp(self)
+if os.environ["TEST_INTEL_PT"] != "true":
+self.skipTest("Intel PT not supported")
+
+plugin_path = os.path.join(os.environ["LLDB_IMPLIB_DIR"], 
"liblldbIntelFeatures.so")
+self.runCmd("plugin load " + plugin_path)
+
 @skipIf(oslist=no_match(['linux']))
 @skipIf(archs=no_match(['i386', 'x86_64']))
 @skipIfRemote
 def test_basic_flow(self):
 """Test collection, decoding, and dumping instructions"""
-if os.environ.get('TEST_INTEL_PT') != '1':
-self.skipTest("The environment variable TEST_INTEL_PT=1 is needed 
to run this test.")
-
-lldb_exec_dir = os.environ["LLDB_IMPLIB_DIR"]
-lldb_lib_dir = os.path.join(lldb_exec_dir, os.pardir, "lib")
-plugin_file = os.path.join(lldb_lib_dir, "liblldbIntelFeatures.so")
 
 self.build()
-
-self.runCmd("plugin load " + plugin_file)
-
 exe = self.getBuildArtifact("a.out")
 lldbutil.run_to_name_breakpoint(self, "main", exe_name=exe)
 # We start tracing from main
@@ -52,9 +51,9 @@
 self.expect("processor-trace show-instr-log -c 100",
 patterns=[
 # We expect to have seen the first instruction of 'fun'
-hex(fun_start_adddress),  
+hex(fun_start_adddress),
 # We expect to see the exit condition of the for loop
-"at main.cpp:" + str(line_number('main.cpp', '// Break for 
loop')) 
+"at main.cpp:" + str(line_number('main.cpp', '// Break for 
loop'))
 ])
 
 self.runCmd("processor-trace stop")


Index: lldb/utils/lldb-dotest/lldb-dotest.in
===
--- lldb/utils/lldb-dotest/lldb-dotest.in
+++ lldb/utils/lldb-dotest/lldb-dotest.in
@@ -1,6 +1,7 @@
 #!@PYTHON_EXECUTABLE@
 import subprocess
 import sys
+import os
 
 dotest_path = '@LLDB_SOURCE_DIR_CONFIGURED@/test/API/dotest.py'
 build_dir = '@LLDB_TEST_BUILD_DIRECTORY_CONFIGURED@'
@@ -11,6 +12,7 @@
 dsymutil = '@LLDB_TEST_DSYMUTIL_CONFIGURED@'
 filecheck = '@LLDB_TEST_FILECHECK_CONFIGURED@'
 lldb_libs_dir = "@LLDB_LIBS_DIR_CONFIGURED@"
+lldb_build_intel_pt = "@LLDB_BUILD_INTEL_PT@".lower()
 
 if __name__ == '__main__':
 wrapper_args = sys.argv[1:]
@@ -26,6 +28,11 @@
 cmd.extend(['--filecheck', filecheck])
 cmd.extend(['--lldb-libs-dir', lldb_libs_dir])
 cmd.extend(wrapper_args)
+
+if lldb_build_intel_pt not in ["", "false", "off", "n", "no", "ignore"] \
+and not lldb_build_intel_pt.endswith("-NOTFOUND"):
+os.environ['TEST_INTEL_PT'] = "true"
+
 # Invoke dotest.py and return exit code.
 print(' '.join(cmd))
 sys.exit(subprocess.call(cmd))
Index: lldb/test/API/tools/intel-features/intel-pt/test/TestIntelPTSimpleBinary.py
===
--- lldb/test/API/tools/intel-features/intel-pt/test/TestIntelPTSimpleBinary.py
+++ 

[Lldb-commits] [PATCH] D77452: [intel-pt] Improve the way the test determines whether to run

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Checking for presence this way seems ok-ish (definitely better than the 
environment variable). A possible alternative would be to pass some argument to 
dotest from cmake which would identify the status of the pt support.

Anyway, if you're going to add a new public API for this purpose, please create 
a separate patch for that with a test.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77452/new/

https://reviews.llvm.org/D77452



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D77452: [intel-pt] Improve the way the test determines whether to run

[walter erquinigo via Phabricator via lldb-commits]

wallace updated this revision to Diff 254990.
wallace edited the summary of this revision.
wallace added a comment.

improve description


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77452/new/

https://reviews.llvm.org/D77452

Files:
  lldb/bindings/interface/SBCommandInterpreter.i
  lldb/include/lldb/API/SBCommandInterpreter.h
  lldb/source/API/SBCommandInterpreter.cpp
  lldb/test/API/tools/intel-features/intel-pt/test/TestIntelPTSimpleBinary.py

Index: lldb/test/API/tools/intel-features/intel-pt/test/TestIntelPTSimpleBinary.py
===
--- lldb/test/API/tools/intel-features/intel-pt/test/TestIntelPTSimpleBinary.py
+++ lldb/test/API/tools/intel-features/intel-pt/test/TestIntelPTSimpleBinary.py
@@ -14,22 +14,26 @@
 mydir = TestBase.compute_mydir(__file__)
 NO_DEBUG_INFO_TESTCASE = True
 
+def setUp(self):
+TestBase.setUp(self)
+
+plugin_so = "liblldbIntelFeatures.so"
+plugin_path = os.path.join(os.environ["LLDB_IMPLIB_DIR"], plugin_so)
+if not os.path.exists(plugin_path):
+self.skipTest("Intel PT plugin %s missing." % (plugin_so))
+
+self.runCmd("plugin load " + plugin_path)
+if not self.dbg.GetCommandInterpreter().UserCommandExists("processor-trace"):
+self.skipTest("%s was built without Intel PT support" % (plugin_so))
+# This could happen if the user has intel-mpx support but not intel-pt.
+
 @skipIf(oslist=no_match(['linux']))
 @skipIf(archs=no_match(['i386', 'x86_64']))
 @skipIfRemote
 def test_basic_flow(self):
 """Test collection, decoding, and dumping instructions"""
-if os.environ.get('TEST_INTEL_PT') != '1':
-self.skipTest("The environment variable TEST_INTEL_PT=1 is needed to run this test.")
-
-lldb_exec_dir = os.environ["LLDB_IMPLIB_DIR"]
-lldb_lib_dir = os.path.join(lldb_exec_dir, os.pardir, "lib")
-plugin_file = os.path.join(lldb_lib_dir, "liblldbIntelFeatures.so")
 
 self.build()
-
-self.runCmd("plugin load " + plugin_file)
-
 exe = self.getBuildArtifact("a.out")
 lldbutil.run_to_name_breakpoint(self, "main", exe_name=exe)
 # We start tracing from main
Index: lldb/source/API/SBCommandInterpreter.cpp
===
--- lldb/source/API/SBCommandInterpreter.cpp
+++ lldb/source/API/SBCommandInterpreter.cpp
@@ -216,6 +216,14 @@
   : false);
 }
 
+bool SBCommandInterpreter::UserCommandExists(const char *cmd) {
+  LLDB_RECORD_METHOD(bool, SBCommandInterpreter, UserCommandExists,
+ (const char *), cmd);
+
+  return (((cmd != nullptr) && IsValid()) ? m_opaque_ptr->UserCommandExists(cmd)
+  : false);
+}
+
 bool SBCommandInterpreter::AliasExists(const char *cmd) {
   LLDB_RECORD_METHOD(bool, SBCommandInterpreter, AliasExists, (const char *),
  cmd);
@@ -874,6 +882,8 @@
   LLDB_REGISTER_METHOD_CONST(bool, SBCommandInterpreter, operator bool, ());
   LLDB_REGISTER_METHOD(bool, SBCommandInterpreter, CommandExists,
(const char *));
+  LLDB_REGISTER_METHOD(bool, SBCommandInterpreter, UserCommandExists,
+   (const char *));
   LLDB_REGISTER_METHOD(bool, SBCommandInterpreter, AliasExists,
(const char *));
   LLDB_REGISTER_METHOD(bool, SBCommandInterpreter, IsActive, ());
Index: lldb/include/lldb/API/SBCommandInterpreter.h
===
--- lldb/include/lldb/API/SBCommandInterpreter.h
+++ lldb/include/lldb/API/SBCommandInterpreter.h
@@ -91,8 +91,28 @@
 
   bool IsValid() const;
 
+  /// Determine whether a command exists in this CommandInterpreter.
+  ///
+  /// For commands registered using the LLDB API, see UserCommandExists
+  ///
+  /// \param[in] cmd
+  /// The command to look up.
+  ///
+  /// \return
+  /// \b true if the provided command exists, \b false otherwise.
   bool CommandExists(const char *cmd);
 
+  /// Determine whether a user command exists in this CommandInterpreter.
+  /// Users commands are the ones registered using the LLDB API and are not
+  /// provided by default in LLDB.
+  ///
+  /// \param[in] cmd
+  /// The command to look up.
+  ///
+  /// \return
+  /// \b true if the provided command exists, \b false otherwise.
+  bool UserCommandExists(const char *cmd);
+
   bool AliasExists(const char *cmd);
 
   lldb::SBBroadcaster GetBroadcaster();
Index: lldb/bindings/interface/SBCommandInterpreter.i
===
--- lldb/bindings/interface/SBCommandInterpreter.i
+++ lldb/bindings/interface/SBCommandInterpreter.i
@@ -169,6 +169,9 @@
 bool
 CommandExists (const char *cmd);
 
+bool
+UserCommandExists (const 

[Lldb-commits] [PATCH] D77452: [intel-pt] Improve the way the test determines whether to run

[walter erquinigo via Phabricator via lldb-commits]

wallace created this revision.
wallace added reviewers: labath, clayborg.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
wallace updated this revision to Diff 254990.
wallace edited the summary of this revision.
wallace added a comment.

improve description


@labath raised a concern on the way I was skipping this test. I think that was 
fair and I found a better way.
Now I'm skipping if the Intel features library is not present and if the 
processor-trace command doesn't exist.
It's worth mentioning that the Intel library is a collection of two features: 
intel-mpx and intel-pt. Intel PT is optional and has to be enabled at 
configuration step.

I had to add ExistsUserCommand to the CommandInterpreter API.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77452

Files:
  lldb/bindings/interface/SBCommandInterpreter.i
  lldb/include/lldb/API/SBCommandInterpreter.h
  lldb/source/API/SBCommandInterpreter.cpp
  lldb/test/API/tools/intel-features/intel-pt/test/TestIntelPTSimpleBinary.py

Index: lldb/test/API/tools/intel-features/intel-pt/test/TestIntelPTSimpleBinary.py
===
--- lldb/test/API/tools/intel-features/intel-pt/test/TestIntelPTSimpleBinary.py
+++ lldb/test/API/tools/intel-features/intel-pt/test/TestIntelPTSimpleBinary.py
@@ -14,22 +14,26 @@
 mydir = TestBase.compute_mydir(__file__)
 NO_DEBUG_INFO_TESTCASE = True
 
+def setUp(self):
+TestBase.setUp(self)
+
+plugin_so = "liblldbIntelFeatures.so"
+plugin_path = os.path.join(os.environ["LLDB_IMPLIB_DIR"], plugin_so)
+if not os.path.exists(plugin_path):
+self.skipTest("Intel PT plugin %s missing." % (plugin_so))
+
+self.runCmd("plugin load " + plugin_path)
+if not self.dbg.GetCommandInterpreter().UserCommandExists("processor-trace"):
+self.skipTest("%s was built without Intel PT support" % (plugin_so))
+# This could happen if the user has intel-mpx support but not intel-pt.
+
 @skipIf(oslist=no_match(['linux']))
 @skipIf(archs=no_match(['i386', 'x86_64']))
 @skipIfRemote
 def test_basic_flow(self):
 """Test collection, decoding, and dumping instructions"""
-if os.environ.get('TEST_INTEL_PT') != '1':
-self.skipTest("The environment variable TEST_INTEL_PT=1 is needed to run this test.")
-
-lldb_exec_dir = os.environ["LLDB_IMPLIB_DIR"]
-lldb_lib_dir = os.path.join(lldb_exec_dir, os.pardir, "lib")
-plugin_file = os.path.join(lldb_lib_dir, "liblldbIntelFeatures.so")
 
 self.build()
-
-self.runCmd("plugin load " + plugin_file)
-
 exe = self.getBuildArtifact("a.out")
 lldbutil.run_to_name_breakpoint(self, "main", exe_name=exe)
 # We start tracing from main
Index: lldb/source/API/SBCommandInterpreter.cpp
===
--- lldb/source/API/SBCommandInterpreter.cpp
+++ lldb/source/API/SBCommandInterpreter.cpp
@@ -216,6 +216,14 @@
   : false);
 }
 
+bool SBCommandInterpreter::UserCommandExists(const char *cmd) {
+  LLDB_RECORD_METHOD(bool, SBCommandInterpreter, UserCommandExists,
+ (const char *), cmd);
+
+  return (((cmd != nullptr) && IsValid()) ? m_opaque_ptr->UserCommandExists(cmd)
+  : false);
+}
+
 bool SBCommandInterpreter::AliasExists(const char *cmd) {
   LLDB_RECORD_METHOD(bool, SBCommandInterpreter, AliasExists, (const char *),
  cmd);
@@ -874,6 +882,8 @@
   LLDB_REGISTER_METHOD_CONST(bool, SBCommandInterpreter, operator bool, ());
   LLDB_REGISTER_METHOD(bool, SBCommandInterpreter, CommandExists,
(const char *));
+  LLDB_REGISTER_METHOD(bool, SBCommandInterpreter, UserCommandExists,
+   (const char *));
   LLDB_REGISTER_METHOD(bool, SBCommandInterpreter, AliasExists,
(const char *));
   LLDB_REGISTER_METHOD(bool, SBCommandInterpreter, IsActive, ());
Index: lldb/include/lldb/API/SBCommandInterpreter.h
===
--- lldb/include/lldb/API/SBCommandInterpreter.h
+++ lldb/include/lldb/API/SBCommandInterpreter.h
@@ -91,8 +91,28 @@
 
   bool IsValid() const;
 
+  /// Determine whether a command exists in this CommandInterpreter.
+  ///
+  /// For commands registered using the LLDB API, see UserCommandExists
+  ///
+  /// \param[in] cmd
+  /// The command to look up.
+  ///
+  /// \return
+  /// \b true if the provided command exists, \b false otherwise.
   bool CommandExists(const char *cmd);
 
+  /// Determine whether a user command exists in this CommandInterpreter.
+  /// Users commands are the ones registered using the LLDB API and are not
+  /// provided by default in LLDB.
+  ///
+  /// \param[in] cmd
+  /// The command to look up.
+  ///
+  ///