[Lldb-commits] [PATCH] D141907: [CMake] Ensure `CLANG_RESOURCE_DIR` is respected

[Petr Hosek via Phabricator via lldb-commits]

phosek added inline comments.



Comment at: cmake/Modules/GetClangResourceDir.cmake:17
+
+  set(oneValueArgs PREFIX SUBDIR)
+  cmake_parse_arguments(RESOURCE_DIR "" "${oneValueArgs}" "" ${ARGN})

This variable is only used once on the following line, I'd just inline it which 
is more idiomatic.



Comment at: cmake/Modules/GetClangResourceDir.cmake:18
+  set(oneValueArgs PREFIX SUBDIR)
+  cmake_parse_arguments(RESOURCE_DIR "" "${oneValueArgs}" "" ${ARGN})
+

It's more idiomatic to parse arguments at the start of the function.



Comment at: cmake/Modules/GetClangResourceDir.cmake:18
+  set(oneValueArgs PREFIX SUBDIR)
+  cmake_parse_arguments(RESOURCE_DIR "" "${oneValueArgs}" "" ${ARGN})
+

phosek wrote:
> It's more idiomatic to parse arguments at the start of the function.
This is just a nit, but we usually use a prefix like `ARGS` to make it clear 
that these are input arguments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141907

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

[Lldb-commits] [lldb] 88ac913 - [lldb] Try harder to optimize away a test variable

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2023-02-02T11:24:47+01:00
New Revision: 88ac9138f4eb7b8c06154dd6e3f801d9882b66d7

URL: 
https://github.com/llvm/llvm-project/commit/88ac9138f4eb7b8c06154dd6e3f801d9882b66d7
DIFF: 
https://github.com/llvm/llvm-project/commit/88ac9138f4eb7b8c06154dd6e3f801d9882b66d7.diff

LOG: [lldb] Try harder to optimize away a test variable

The test was checking that we can print an error message when a variable
is optimized away, but the optimizer got smarter (D140404) in tracking
the variable's value (so that we were not able to recover its value).

Using a value in an argument registers (argc) makes it more likely to be
overwritten by subsequent function calls (and permanently lost).

Added: 


Modified: 
lldb/test/API/tools/lldb-vscode/optimized/TestVSCode_optimized.py
lldb/test/API/tools/lldb-vscode/optimized/main.cpp

Removed: 




diff  --git a/lldb/test/API/tools/lldb-vscode/optimized/TestVSCode_optimized.py 
b/lldb/test/API/tools/lldb-vscode/optimized/TestVSCode_optimized.py
index 3f7ac7cdc77a..5dfc35a927f2 100644
--- a/lldb/test/API/tools/lldb-vscode/optimized/TestVSCode_optimized.py
+++ b/lldb/test/API/tools/lldb-vscode/optimized/TestVSCode_optimized.py
@@ -45,6 +45,6 @@ def test_optimized_variable(self):
 self.assertEqual(len(breakpoint_ids), len(lines),
 "expect correct number of breakpoints")
 self.continue_to_breakpoints(breakpoint_ids)
-optimized_variable = self.vscode.get_local_variable('optimized')
+optimized_variable = self.vscode.get_local_variable('argc')
 
 self.assertTrue(optimized_variable['value'].startswith(' 1 ? std::stoi(argv[1]) : 0;
-
-  printf("argc: %d, optimized: %d\n", argc, optimized);
-  int result = foo(argc, 20);
+  printf("argc: %d\n", argc);
+  int result = foo(20, argv[0][0]);
   printf("result: %d\n", result); // breakpoint 2
   return 0;
 }



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

[Lldb-commits] [PATCH] D141907: [CMake] Ensure `CLANG_RESOURCE_DIR` is respected

[Junchang Liu via Phabricator via lldb-commits]

paperchalice updated this revision to Diff 494223.
paperchalice added a comment.

- Use `clang::driver::Driver::GetResourcesPath`
- Use `ARG` as prefix


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141907

Files:
  clang/lib/Headers/CMakeLists.txt
  clang/lib/Tooling/CMakeLists.txt
  clang/runtime/CMakeLists.txt
  cmake/Modules/GetClangResourceDir.cmake
  compiler-rt/cmake/base-config-ix.cmake
  lldb/source/Plugins/ExpressionParser/Clang/ClangHost.cpp
  lldb/unittests/Expression/ClangParserTest.cpp
  llvm/cmake/modules/LLVMExternalProjectUtils.cmake
  openmp/CMakeLists.txt

Index: openmp/CMakeLists.txt
===
--- openmp/CMakeLists.txt
+++ openmp/CMakeLists.txt
@@ -86,8 +86,8 @@
 if(${OPENMP_STANDALONE_BUILD})
   set(LIBOMP_HEADERS_INSTALL_PATH "${CMAKE_INSTALL_INCLUDEDIR}")
 else()
-  string(REGEX MATCH "[0-9]+" CLANG_VERSION ${PACKAGE_VERSION})
-  set(LIBOMP_HEADERS_INSTALL_PATH "${OPENMP_INSTALL_LIBDIR}/clang/${CLANG_VERSION}/include")
+  include(GetClangResourceDir)
+  get_clang_resource_dir(LIBOMP_HEADERS_INSTALL_PATH SUBDIR include)
 endif()
 
 # Build host runtime library, after LIBOMPTARGET variables are set since they are needed
Index: llvm/cmake/modules/LLVMExternalProjectUtils.cmake
===
--- llvm/cmake/modules/LLVMExternalProjectUtils.cmake
+++ llvm/cmake/modules/LLVMExternalProjectUtils.cmake
@@ -257,7 +257,11 @@
 if(CMAKE_CXX_COMPILER_ID MATCHES "Clang")
   string(REGEX MATCH "^[0-9]+" CLANG_VERSION_MAJOR
  ${PACKAGE_VERSION})
-  set(resource_dir "${LLVM_LIBRARY_DIR}/clang/${CLANG_VERSION_MAJOR}")
+  if(DEFINED CLANG_RESOURCE_DIR AND NOT CLANG_RESOURCE_DIR STREQUAL "")
+set(resource_dir ${LLVM_TOOLS_BINARY_DIR}/${CLANG_RESOURCE_DIR})
+  else()
+set(resource_dir "${LLVM_LIBRARY_DIR}/clang/${CLANG_VERSION_MAJOR}")
+  endif()
   set(flag_types ASM C CXX MODULE_LINKER SHARED_LINKER EXE_LINKER)
   foreach(type ${flag_types})
 set(${type}_flag -DCMAKE_${type}_FLAGS=-resource-dir=${resource_dir})
Index: lldb/unittests/Expression/ClangParserTest.cpp
===
--- lldb/unittests/Expression/ClangParserTest.cpp
+++ lldb/unittests/Expression/ClangParserTest.cpp
@@ -7,6 +7,8 @@
 //===--===//
 
 #include "clang/Basic/Version.h"
+#include "clang/Config/config.h"
+#include "clang/Driver/Driver.h"
 
 #include "Plugins/ExpressionParser/Clang/ClangHost.h"
 #include "TestingSupport/SubsystemRAII.h"
@@ -37,13 +39,11 @@
 TEST_F(ClangHostTest, ComputeClangResourceDirectory) {
 #if !defined(_WIN32)
   std::string path_to_liblldb = "/foo/bar/lib/";
-  std::string path_to_clang_dir =
-  "/foo/bar/" LLDB_INSTALL_LIBDIR_BASENAME "/clang/" CLANG_VERSION_MAJOR_STRING;
 #else
-  std::string path_to_liblldb = "C:\\foo\\bar\\lib";
-  std::string path_to_clang_dir =
-  "C:\\foo\\bar\\lib\\clang\\" CLANG_VERSION_MAJOR_STRING;
+  std::string path_to_liblldb = "C:\\foo\\bar\\lib\\";
 #endif
+  std::string path_to_clang_dir = clang::driver::Driver::GetResourcesPath(
+  path_to_liblldb + "liblldb", CLANG_RESOURCE_DIR);
   EXPECT_EQ(ComputeClangResourceDir(path_to_liblldb), path_to_clang_dir);
 
   // The path doesn't really exist, so setting verify to true should make
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangHost.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangHost.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangHost.cpp
@@ -10,6 +10,7 @@
 
 #include "clang/Basic/Version.h"
 #include "clang/Config/config.h"
+#include "clang/Driver/Driver.h"
 
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Twine.h"
@@ -51,11 +52,14 @@
   Log *log = GetLog(LLDBLog::Host);
   std::string raw_path = lldb_shlib_spec.GetPath();
   llvm::StringRef parent_dir = llvm::sys::path::parent_path(raw_path);
+  const std::string clang_resource_path =
+  clang::driver::Driver::GetResourcesPath("bin/lldb", CLANG_RESOURCE_DIR);
 
   static const llvm::StringRef kResourceDirSuffixes[] = {
   // LLVM.org's build of LLDB uses the clang resource directory placed
-  // in $install_dir/lib{,64}/clang/$clang_version.
-  CLANG_INSTALL_LIBDIR_BASENAME "/clang/" CLANG_VERSION_MAJOR_STRING,
+  // in $install_dir/lib{,64}/clang/$clang_version or
+  // $install_dir/bin/$CLANG_RESOURCE_DIR
+  clang_resource_path,
   // swift-lldb uses the clang resource directory copied from swift, which
   // by default is placed in $install_dir/lib{,64}/lldb/clang. LLDB places
   // it there, so we use LLDB_INSTALL_LIBDIR_BASENAME.
@@ -82,7 +86,8 @@
 }
 
 bool lldb_private::ComputeClangResourceDirectory(FileSpec &lldb_shlib_spec,
-  

[Lldb-commits] [lldb] 3c66729 - [lldb][Test] Fix import-std-module and data-formatter tests on older compilers

[Michael Buch via lldb-commits]

Author: Michael Buch
Date: 2023-02-02T11:34:07Z
New Revision: 3c66729887d6113068025776994b4c49bd8e45e2

URL: 
https://github.com/llvm/llvm-project/commit/3c66729887d6113068025776994b4c49bd8e45e2
DIFF: 
https://github.com/llvm/llvm-project/commit/3c66729887d6113068025776994b4c49bd8e45e2.diff

LOG: [lldb][Test] Fix import-std-module and data-formatter tests on older 
compilers

Fixes API tests for older compilers.
Since https://reviews.llvm.org/D141828 defaulted
arguments will be omitted, but older Clang's won't.

Differential Revision: https://reviews.llvm.org/D143022

Added: 


Modified: 

lldb/test/API/commands/expression/import-std-module/deque-basic/TestDequeFromStdModule.py

lldb/test/API/commands/expression/import-std-module/deque-dbg-info-content/TestDbgInfoContentDequeFromStdModule.py

lldb/test/API/commands/expression/import-std-module/forward_list-dbg-info-content/TestDbgInfoContentForwardListFromStdModule.py

lldb/test/API/commands/expression/import-std-module/forward_list/TestForwardListFromStdModule.py

lldb/test/API/commands/expression/import-std-module/list-dbg-info-content/TestDbgInfoContentListFromStdModule.py

lldb/test/API/commands/expression/import-std-module/list/TestListFromStdModule.py

lldb/test/API/commands/expression/import-std-module/non-module-type-separation/TestNonModuleTypeSeparation.py

lldb/test/API/commands/expression/import-std-module/queue/TestQueueFromStdModule.py

lldb/test/API/commands/expression/import-std-module/retry-with-std-module/TestRetryWithStdModule.py

lldb/test/API/commands/expression/import-std-module/unique_ptr-dbg-info-content/TestUniquePtrDbgInfoContent.py

lldb/test/API/commands/expression/import-std-module/unique_ptr/TestUniquePtrFromStdModule.py

lldb/test/API/commands/expression/import-std-module/vector-dbg-info-content/TestDbgInfoContentVectorFromStdModule.py

lldb/test/API/commands/expression/import-std-module/vector-of-vectors/TestVectorOfVectorsFromStdModule.py

lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/shared_ptr/TestDataFormatterLibcxxSharedPtr.py

lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py

lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string_view/TestDataFormatterLibcxxStringView.py

lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/unique_ptr/TestDataFormatterLibcxxUniquePtr.py

Removed: 




diff  --git 
a/lldb/test/API/commands/expression/import-std-module/deque-basic/TestDequeFromStdModule.py
 
b/lldb/test/API/commands/expression/import-std-module/deque-basic/TestDequeFromStdModule.py
index f77ba0d2417a4..f12f5f96aa5d6 100644
--- 
a/lldb/test/API/commands/expression/import-std-module/deque-basic/TestDequeFromStdModule.py
+++ 
b/lldb/test/API/commands/expression/import-std-module/deque-basic/TestDequeFromStdModule.py
@@ -20,7 +20,11 @@ def test(self):
 
 self.runCmd("settings set target.import-std-module true")
 
-deque_type = "std::deque"
+if self.expectedCompilerVersion(['>', '16.0']):
+deque_type = "std::deque"
+else:
+deque_type = "std::deque >"
+
 size_type = "size_type"
 value_type = "value_type"
 iterator = "iterator"

diff  --git 
a/lldb/test/API/commands/expression/import-std-module/deque-dbg-info-content/TestDbgInfoContentDequeFromStdModule.py
 
b/lldb/test/API/commands/expression/import-std-module/deque-dbg-info-content/TestDbgInfoContentDequeFromStdModule.py
index cf2f63d6b669b..d62e49f11cbb2 100644
--- 
a/lldb/test/API/commands/expression/import-std-module/deque-dbg-info-content/TestDbgInfoContentDequeFromStdModule.py
+++ 
b/lldb/test/API/commands/expression/import-std-module/deque-dbg-info-content/TestDbgInfoContentDequeFromStdModule.py
@@ -21,7 +21,11 @@ def test(self):
 
 self.runCmd("settings set target.import-std-module true")
 
-deque_type = "std::deque"
+if self.expectedCompilerVersion(['>', '16.0']):
+deque_type = "std::deque"
+else:
+deque_type = "std::deque >"
+
 size_type = "size_type"
 value_type = "value_type"
 

diff  --git 
a/lldb/test/API/commands/expression/import-std-module/forward_list-dbg-info-content/TestDbgInfoContentForwardListFromStdModule.py
 
b/lldb/test/API/commands/expression/import-std-module/forward_list-dbg-info-content/TestDbgInfoContentForwardListFromStdModule.py
index d794f3b127b3a..ad6c5f487b9d4 100644
--- 
a/lldb/test/API/commands/expression/import-std-module/forward_list-dbg-info-content/TestDbgInfoContentForwardListFromStdModule.py
+++ 
b/lldb/test/API/commands/expression/import-std-module/forward_list-dbg-info-content/TestDbgInfoContentForwardListFromStdModule.py
@@ -20,7 +20,11 @@ def test(self):
 
 self.runCmd("settings set target.impo

[Lldb-commits] [PATCH] D143022: [lldb][Test] Fix import-std-module and data-formatter tests on older compilers

[Michael Buch via Phabricator via lldb-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rG3c66729887d6: [lldb][Test] Fix import-std-module and 
data-formatter tests on older compilers (authored by Michael137).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143022

Files:
  
lldb/test/API/commands/expression/import-std-module/deque-basic/TestDequeFromStdModule.py
  
lldb/test/API/commands/expression/import-std-module/deque-dbg-info-content/TestDbgInfoContentDequeFromStdModule.py
  
lldb/test/API/commands/expression/import-std-module/forward_list-dbg-info-content/TestDbgInfoContentForwardListFromStdModule.py
  
lldb/test/API/commands/expression/import-std-module/forward_list/TestForwardListFromStdModule.py
  
lldb/test/API/commands/expression/import-std-module/list-dbg-info-content/TestDbgInfoContentListFromStdModule.py
  
lldb/test/API/commands/expression/import-std-module/list/TestListFromStdModule.py
  
lldb/test/API/commands/expression/import-std-module/non-module-type-separation/TestNonModuleTypeSeparation.py
  
lldb/test/API/commands/expression/import-std-module/queue/TestQueueFromStdModule.py
  
lldb/test/API/commands/expression/import-std-module/retry-with-std-module/TestRetryWithStdModule.py
  
lldb/test/API/commands/expression/import-std-module/unique_ptr-dbg-info-content/TestUniquePtrDbgInfoContent.py
  
lldb/test/API/commands/expression/import-std-module/unique_ptr/TestUniquePtrFromStdModule.py
  
lldb/test/API/commands/expression/import-std-module/vector-dbg-info-content/TestDbgInfoContentVectorFromStdModule.py
  
lldb/test/API/commands/expression/import-std-module/vector-of-vectors/TestVectorOfVectorsFromStdModule.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/shared_ptr/TestDataFormatterLibcxxSharedPtr.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string_view/TestDataFormatterLibcxxStringView.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/unique_ptr/TestDataFormatterLibcxxUniquePtr.py

Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/unique_ptr/TestDataFormatterLibcxxUniquePtr.py
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/unique_ptr/TestDataFormatterLibcxxUniquePtr.py
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/unique_ptr/TestDataFormatterLibcxxUniquePtr.py
@@ -11,6 +11,22 @@
 
 class TestCase(TestBase):
 
+def make_expected_type(self, pointee_type: str, qualifiers: str = "") -> str:
+if qualifiers:
+qualifiers = ' ' + qualifiers
+
+if self.expectedCompilerVersion(['>', '16.0']):
+return f'std::unique_ptr<{pointee_type}>{qualifiers}'
+else:
+return f'std::unique_ptr<{pointee_type}, std::default_delete<{pointee_type}> >{qualifiers}'
+
+def make_expected_basic_string_ptr(self) -> str:
+if self.expectedCompilerVersion(['>', '16.0']):
+return f'std::unique_ptr >'
+else:
+return 'std::unique_ptr, std::allocator >, ' \
+   'std::default_delete, std::allocator > > >'
+
 @add_test_categories(["libc++"])
 def test_unique_ptr_variables(self):
 """Test `frame variable` output for `std::unique_ptr` types."""
@@ -22,7 +38,7 @@
 
 valobj = self.expect_var_path(
 "up_empty",
-type="std::unique_ptr",
+type=self.make_expected_type("int"),
 summary="nullptr",
 children=[ValueCheck(name="__value_")],
 )
@@ -36,7 +52,7 @@
 
 valobj = self.expect_var_path(
 "up_int",
-type="std::unique_ptr",
+type=self.make_expected_type("int"),
 summary="10",
 children=[ValueCheck(name="__value_")],
 )
@@ -44,7 +60,7 @@
 
 valobj = self.expect_var_path(
 "up_int_ref",
-type="std::unique_ptr &",
+type=self.make_expected_type("int", qualifiers="&"),
 summary="10",
 children=[ValueCheck(name="__value_")],
 )
@@ -52,7 +68,7 @@
 
 valobj = self.expect_var_path(
 "up_int_ref_ref",
-type="std::unique_ptr &&",
+type=self.make_expected_type("int", qualifiers="&&"),
 summary="10",
 children=[ValueCheck(name="__value_")],
 )
@@ -60,13 +76,13 @@
 
 valobj = self.expect_var_path(
 "up_str",
-type="std::unique_ptr >",
+type=self.make_expected_basic_string_ptr(),
 summary='"hello"',
 children=[ValueCheck(name="__val

[Lldb-commits] [lldb] f8c9b30 - [lldb][SymbolFileDWARF] Support by-name lookup of global variables in inline namespaces

[Michael Buch via lldb-commits]

Author: Michael Buch
Date: 2023-02-02T11:35:29Z
New Revision: f8c9b30eb3e8cffc6c7adaa3003c774422643cf7

URL: 
https://github.com/llvm/llvm-project/commit/f8c9b30eb3e8cffc6c7adaa3003c774422643cf7
DIFF: 
https://github.com/llvm/llvm-project/commit/f8c9b30eb3e8cffc6c7adaa3003c774422643cf7.diff

LOG: [lldb][SymbolFileDWARF] Support by-name lookup of global variables in 
inline namespaces

Currently evaluating an expression involving a global variable inside
an inline namespace will fail to lookup said variable. This is because
the `SymbolFileDWARF::FindGlobalVariables` discards from consideration
all DIEs whose decl_context doesn't exactly match that of the lookup.

This patch relaxes this restriction by checking whether C++ rules
would permit the lookup. This is permitted by the DWARFv5 spec in
chapter `3.2.2 Namespace Entries`:
```
A namespace may have a DW_AT_export_symbols attribute which is a flag
which indicates that all member names defined within the namespace may be
referenced as if they were defined within the containing namespace.
```

The motivation for this is evaluating `std::ranges` expressions, which
heavily rely on global variables inside inline namespaces. E.g.,
`std::views::all(...)` is just an invocation of the `operator()`
on `std::ranges::views::__cpo::all`.

**Testing**

* Added API tests

Differential Revision: https://reviews.llvm.org/D143068

Added: 


Modified: 
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
lldb/test/API/commands/expression/inline-namespace/TestInlineNamespace.py
lldb/test/API/commands/expression/inline-namespace/main.cpp

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index 544c9e08d504f..6f257c2a71ce1 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -,8 +,14 @@ void SymbolFileDWARF::FindGlobalVariables(
   if (DWARFASTParser *dwarf_ast = GetDWARFParser(*die.GetCU())) {
 CompilerDeclContext actual_parent_decl_ctx =
 dwarf_ast->GetDeclContextContainingUIDFromDWARF(die);
+
+/// If the actual namespace is inline (i.e., had a 
DW_AT_export_symbols)
+/// and a child (possibly through other layers of inline namespaces)
+/// of the namespace referred to by 'basename', allow the lookup to
+/// succeed.
 if (!actual_parent_decl_ctx ||
-actual_parent_decl_ctx != parent_decl_ctx)
+(actual_parent_decl_ctx != parent_decl_ctx &&
+ !parent_decl_ctx.IsContainedInLookup(actual_parent_decl_ctx)))
   return true;
   }
 }

diff  --git 
a/lldb/test/API/commands/expression/inline-namespace/TestInlineNamespace.py 
b/lldb/test/API/commands/expression/inline-namespace/TestInlineNamespace.py
index 1772d61d29618..aae9b9b7e0165 100644
--- a/lldb/test/API/commands/expression/inline-namespace/TestInlineNamespace.py
+++ b/lldb/test/API/commands/expression/inline-namespace/TestInlineNamespace.py
@@ -23,3 +23,35 @@ def test(self):
 # But we should still find the function when we pretend the inline
 # namespace is not inline.
 self.expect_expr("A::B::f()", result_type="int", result_value="3")
+
+self.expect_expr("A::B::global_var", result_type="int", 
result_value="0")
+# FIXME: should be ambiguous lookup but ClangExpressionDeclMap takes
+#first global variable that the lookup found, which in this 
case
+#is A::B::global_var
+self.expect_expr("A::global_var", result_type="int", result_value="0")
+
+self.expect_expr("A::B::C::global_var", result_type="int", 
result_value="1")
+self.expect_expr("A::C::global_var", result_type="int", 
result_value="1")
+
+self.expect_expr("A::B::D::nested_var", result_type="int", 
result_value="2")
+self.expect_expr("A::D::nested_var", result_type="int", 
result_value="2")
+self.expect_expr("A::B::nested_var", result_type="int", 
result_value="2")
+self.expect_expr("A::nested_var", result_type="int", result_value="2")
+
+self.expect_expr("A::E::F::other_var", result_type="int", 
result_value="3")
+self.expect_expr("A::E::other_var", result_type="int", 
result_value="3")
+
+self.expect("expr A::E::global_var", error=True, substrs=["no member 
named 'global_var' in namespace 'A::E'"])
+self.expect("expr A::E::F::global_var", error=True, substrs=["no 
member named 'global_var' in namespace 'A::E::F'"])
+
+self.expect("expr A::other_var", error=True, substrs=["no member named 
'other_var' in namespace 'A'"])
+self.expect("expr A::B::other_var", error=True, substrs=["no member 
named 'other_var' in namespace 'A::B'"])
+self.expect("expr B::other_var", error=True, substrs=["no memb

[Lldb-commits] [PATCH] D143068: [lldb][SymbolFileDWARF] Support by-name lookup of global variables in inline namespaces

[Michael Buch via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGf8c9b30eb3e8: [lldb][SymbolFileDWARF] Support by-name lookup 
of global variables in inline… (authored by Michael137).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143068

Files:
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/test/API/commands/expression/inline-namespace/TestInlineNamespace.py
  lldb/test/API/commands/expression/inline-namespace/main.cpp


Index: lldb/test/API/commands/expression/inline-namespace/main.cpp
===
--- lldb/test/API/commands/expression/inline-namespace/main.cpp
+++ lldb/test/API/commands/expression/inline-namespace/main.cpp
@@ -1,10 +1,28 @@
 namespace A {
   inline namespace B {
 int f() { return 3; }
+int global_var = 0;
+
+namespace C {
+int global_var = 1;
+}
+
+inline namespace D {
+int nested_var = 2;
+}
   };
+
+  namespace E {
+  inline namespace F {
+  int other_var = 3;
+  }
+  } // namespace E
+
+  int global_var = 4;
 }
 
 int main(int argc, char **argv) {
   // Set break point at this line.
-  return A::f();
+  return A::f() + A::B::global_var + A::C::global_var + A::E::F::other_var +
+ A::B::D::nested_var;
 }
Index: lldb/test/API/commands/expression/inline-namespace/TestInlineNamespace.py
===
--- lldb/test/API/commands/expression/inline-namespace/TestInlineNamespace.py
+++ lldb/test/API/commands/expression/inline-namespace/TestInlineNamespace.py
@@ -23,3 +23,35 @@
 # But we should still find the function when we pretend the inline
 # namespace is not inline.
 self.expect_expr("A::B::f()", result_type="int", result_value="3")
+
+self.expect_expr("A::B::global_var", result_type="int", 
result_value="0")
+# FIXME: should be ambiguous lookup but ClangExpressionDeclMap takes
+#first global variable that the lookup found, which in this 
case
+#is A::B::global_var
+self.expect_expr("A::global_var", result_type="int", result_value="0")
+
+self.expect_expr("A::B::C::global_var", result_type="int", 
result_value="1")
+self.expect_expr("A::C::global_var", result_type="int", 
result_value="1")
+
+self.expect_expr("A::B::D::nested_var", result_type="int", 
result_value="2")
+self.expect_expr("A::D::nested_var", result_type="int", 
result_value="2")
+self.expect_expr("A::B::nested_var", result_type="int", 
result_value="2")
+self.expect_expr("A::nested_var", result_type="int", result_value="2")
+
+self.expect_expr("A::E::F::other_var", result_type="int", 
result_value="3")
+self.expect_expr("A::E::other_var", result_type="int", 
result_value="3")
+
+self.expect("expr A::E::global_var", error=True, substrs=["no member 
named 'global_var' in namespace 'A::E'"])
+self.expect("expr A::E::F::global_var", error=True, substrs=["no 
member named 'global_var' in namespace 'A::E::F'"])
+
+self.expect("expr A::other_var", error=True, substrs=["no member named 
'other_var' in namespace 'A'"])
+self.expect("expr A::B::other_var", error=True, substrs=["no member 
named 'other_var' in namespace 'A::B'"])
+self.expect("expr B::other_var", error=True, substrs=["no member named 
'other_var' in namespace 'A::B'"])
+
+# 'frame variable' can correctly distinguish between A::B::global_var 
and A::global_var
+gvars = self.target().FindGlobalVariables("A::global_var", 10)
+self.assertEqual(len(gvars), 1)
+self.assertEqual(gvars[0].GetValueAsSigned(), 4)
+
+self.expect("frame variable A::global_var", substrs=["(int) 
A::global_var = 4"])
+self.expect("frame variable A::B::global_var", substrs=["(int) 
A::B::global_var = 0"])
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -,8 +,14 @@
   if (DWARFASTParser *dwarf_ast = GetDWARFParser(*die.GetCU())) {
 CompilerDeclContext actual_parent_decl_ctx =
 dwarf_ast->GetDeclContextContainingUIDFromDWARF(die);
+
+/// If the actual namespace is inline (i.e., had a 
DW_AT_export_symbols)
+/// and a child (possibly through other layers of inline namespaces)
+/// of the namespace referred to by 'basename', allow the lookup to
+/// succeed.
 if (!actual_parent_decl_ctx ||
-actual_parent_decl_ctx != parent_decl_ctx)
+(actual_parent_decl_ctx != parent_decl_ctx &&
+ !parent_decl_ctx.IsContainedInLookup(actual_parent_decl_ctx)))
   return true;
   }
 }


Inde

[Lldb-commits] [PATCH] D141425: [lldb] Add --gdb-format flag to dwim-print

[Dave Lee via Phabricator via lldb-commits]

kastiglione updated this revision to Diff 494401.
kastiglione edited the summary of this revision.
kastiglione added a comment.

Add display options to `dwim-print` command


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141425

Files:
  lldb/include/lldb/Interpreter/OptionGroupFormat.h
  lldb/source/Commands/CommandObjectDWIMPrint.cpp
  lldb/source/Commands/CommandObjectDWIMPrint.h
  lldb/source/Interpreter/OptionGroupFormat.cpp
  lldb/test/API/commands/dwim-print/TestDWIMPrint.py

Index: lldb/test/API/commands/dwim-print/TestDWIMPrint.py
===
--- lldb/test/API/commands/dwim-print/TestDWIMPrint.py
+++ lldb/test/API/commands/dwim-print/TestDWIMPrint.py
@@ -27,9 +27,25 @@
 before, after = self.PERSISTENT_VAR.split(string, maxsplit=1)
 return re.escape(before) + r"\$\d+" + re.escape(after)
 
-def _expect_cmd(self, expr: str, base_cmd: str) -> None:
+def _expect_cmd(
+self,
+dwim_cmd: str,
+base_cmd: str,
+) -> None:
 """Run dwim-print and verify the output against the expected command."""
-cmd = f"{base_cmd} {expr}"
+match = re.match("dwim-print(?:/(.))? (.+)", dwim_cmd)
+format = match.group(1)
+expr = match.group(2)
+
+cmd_args = []
+if format:
+cmd_args.append(f"-G {format}")
+if base_cmd == "expression":
+cmd_args.append("--")
+cmd_args.append(expr)
+
+cmd_argstr = " ".join(cmd_args)
+cmd = f"{base_cmd} {cmd_argstr}"
 cmd_output = self._run_cmd(cmd)
 
 # Verify dwim-print chose the expected command.
@@ -37,12 +53,12 @@
 substrs = [f"note: ran `{cmd}`"]
 patterns = []
 
-if base_cmd == "expression --" and self.PERSISTENT_VAR.search(cmd_output):
+if base_cmd == "expression" and self.PERSISTENT_VAR.search(cmd_output):
 patterns.append(self._mask_persistent_var(cmd_output))
 else:
 substrs.append(cmd_output)
 
-self.expect(f"dwim-print {expr}", substrs=substrs, patterns=patterns)
+self.expect(dwim_cmd, substrs=substrs, patterns=patterns)
 
 def test_variables(self):
 """Test dwim-print with variables."""
@@ -50,7 +66,7 @@
 lldbutil.run_to_name_breakpoint(self, "main")
 vars = ("argc", "argv")
 for var in vars:
-self._expect_cmd(var, "frame variable")
+self._expect_cmd(f"dwim-print {var}", "frame variable")
 
 def test_variable_paths(self):
 """Test dwim-print with variable path expressions."""
@@ -58,7 +74,7 @@
 lldbutil.run_to_name_breakpoint(self, "main")
 exprs = ("&argc", "*argv", "argv[0]")
 for expr in exprs:
-self._expect_cmd(expr, "expression --")
+self._expect_cmd(f"dwim-print {expr}", "expression")
 
 def test_expressions(self):
 """Test dwim-print with expressions."""
@@ -66,8 +82,14 @@
 lldbutil.run_to_name_breakpoint(self, "main")
 exprs = ("argc + 1", "(void)argc", "(int)abs(argc)")
 for expr in exprs:
-self._expect_cmd(expr, "expression --")
+self._expect_cmd(f"dwim-print {expr}", "expression")
 
 def test_dummy_target_expressions(self):
 """Test dwim-print's ability to evaluate expressions without a target."""
-self._expect_cmd("1 + 2", "expression --")
+self._expect_cmd("dwim-print 1 + 2", "expression")
+
+def test_gdb_format(self):
+self.build()
+lldbutil.run_to_name_breakpoint(self, "main")
+self._expect_cmd(f"dwim-print/x argc", "frame variable")
+self._expect_cmd(f"dwim-print/x argc + 1", "expression")
Index: lldb/source/Interpreter/OptionGroupFormat.cpp
===
--- lldb/source/Interpreter/OptionGroupFormat.cpp
+++ lldb/source/Interpreter/OptionGroupFormat.cpp
@@ -12,6 +12,7 @@
 #include "lldb/Interpreter/CommandInterpreter.h"
 #include "lldb/Target/ExecutionContext.h"
 #include "lldb/Target/Target.h"
+#include "lldb/lldb-enumerations.h"
 
 using namespace lldb;
 using namespace lldb_private;
@@ -274,6 +275,37 @@
   return false;
 }
 
+char OptionGroupFormat::GetGDBFormat() const {
+  switch (GetFormat()) {
+  case lldb::eFormatOctal:
+return 'o';
+  case lldb::eFormatHex:
+return 'x';
+  case lldb::eFormatDecimal:
+return 'd';
+  case lldb::eFormatUnsigned:
+return 'u';
+  case lldb::eFormatBinary:
+return 't';
+  case lldb::eFormatFloat:
+return 'f';
+  case lldb::eFormatAddressInfo:
+return 'a';
+  case lldb::eFormatInstruction:
+return 'i';
+  case lldb::eFormatChar:
+return 'c';
+  case lldb::eFormatCString:
+return 's';
+  case lldb::eFormatOSType:
+return 'T';
+  case lldb::eFormatHexFloat:
+return 'A';
+  default:
+llvm_unreachab

[Lldb-commits] [PATCH] D143104: [lldb/Plugins] Add Attach capabilities to ScriptedProcess

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere requested changes to this revision.
JDevlieghere added inline comments.
This revision now requires changes to proceed.



Comment at: lldb/include/lldb/Utility/ProcessInfo.h:90-110
+  bool IsScriptedProcess() const {
+return !m_scripted_process_class_name.empty();
+  }
+
+  std::string GetScriptedProcessClassName() const {
+return m_scripted_process_class_name;
+  }

What's the relationship between this an the `ScriptedMetadata`? It seems like 
this is storing the same stuff. Also `IsScriptedProcess` is a pretty blatant 
violation of the scripted process being a plugin... 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143104

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

[Lldb-commits] [PATCH] D143127: [LLDB] Fix assertion failure by removing `CopyType` in `std::coroutine_handle` pretty printer

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

This change makes me a little nervous, but mostly because I don't quite 
understand the design here.
So ASTImporter does not have a check whether src context and dst context are 
identical?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143127

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

[Lldb-commits] [PATCH] D143104: [lldb/Plugins] Add Attach capabilities to ScriptedProcess

[Med Ismail Bennani via Phabricator via lldb-commits]

mib requested review of this revision.
mib added inline comments.



Comment at: lldb/include/lldb/Utility/ProcessInfo.h:90-110
+  bool IsScriptedProcess() const {
+return !m_scripted_process_class_name.empty();
+  }
+
+  std::string GetScriptedProcessClassName() const {
+return m_scripted_process_class_name;
+  }

JDevlieghere wrote:
> What's the relationship between this an the `ScriptedMetadata`? It seems like 
> this is storing the same stuff. Also `IsScriptedProcess` is a pretty blatant 
> violation of the scripted process being a plugin... 
I think you have a misunderstanding here: This is addition is not made in a 
process plugin, but rather in the `ProcessInfo` class, the base class for 
`ProcessLaunchInfo` and `ProcessAttachInfo`, so I don't think this violates in 
any way the plugin architecture.

Regarding the relationship with `ScriptedMetadata`: I guess we could replace 
`m_scripted_process_class_name` & `m_scripted_process_dictionary_sp` by a 
`m_scripted_metadata` object, but I didn't want to process info hold scripted 
process specific stuff. I don't have a strong opinion against it, either.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143104

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

[Lldb-commits] [PATCH] D143213: [lldb/Plugins] Introduce Multiplexer & ScriptedMultiplexer

[Med Ismail Bennani via Phabricator via lldb-commits]

mib created this revision.
mib added reviewers: JDevlieghere, jingham, labath.
mib added a project: LLDB.
Herald added a project: All.
mib requested review of this revision.
Herald added a subscriber: lldb-commits.

This patch introduces a new core component to LLDB: Multiplexer.

The goal of a multiplexer is to stand between the debugger and a list of
processes to intercept events and re-dispatch them to the right processes.

This patch also introduces a special kind of multiplexer: ScriptedMultiplexer.
As the name suggests, it lets the user have a custom implementation of
the multiplexer in a script file that can be imported into lldb.

The ScriptedMultiplexer implements a `FilterEvent(SBEvent&)` method that
will allow the user to iterate over the list of multiplexed processes
and decide to which one the event should be sent to.

The multiplexer gets instanciated when a ScriptedProcess implements the
`get_scripted_multiplexer` method that returns a well-formed dictionary.
The multiplexer instance gets stored in the debugger object.

For the process to be handled by the multiplexer, they all need to
provide the same multiplexer name in the dictionary.

Signed-off-by: Med Ismail Bennani 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D143213

Files:
  lldb/bindings/python/CMakeLists.txt
  lldb/bindings/python/python-swigsafecast.swig
  lldb/bindings/python/python-wrapper.swig
  lldb/examples/python/scripted_process/scripted_multiplexer.py
  lldb/examples/python/scripted_process/scripted_process.py
  lldb/include/lldb/API/SBEvent.h
  lldb/include/lldb/Core/Debugger.h
  lldb/include/lldb/Interpreter/ScriptInterpreter.h
  lldb/include/lldb/Interpreter/ScriptedMultiplexer.h
  lldb/include/lldb/Interpreter/ScriptedMultiplexerInterface.h
  lldb/include/lldb/Interpreter/ScriptedProcessInterface.h
  lldb/include/lldb/Target/Multiplexer.h
  lldb/include/lldb/lldb-forward.h
  lldb/source/Core/Debugger.cpp
  lldb/source/Interpreter/CMakeLists.txt
  lldb/source/Interpreter/ScriptInterpreter.cpp
  lldb/source/Interpreter/ScriptedMultiplexer.cpp
  lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
  lldb/source/Plugins/Process/scripted/ScriptedProcess.h
  lldb/source/Plugins/ScriptInterpreter/Python/CMakeLists.txt
  lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
  
lldb/source/Plugins/ScriptInterpreter/Python/ScriptedMultiplexerPythonInterface.cpp
  
lldb/source/Plugins/ScriptInterpreter/Python/ScriptedMultiplexerPythonInterface.h
  
lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.h
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.h
  lldb/source/Target/CMakeLists.txt
  lldb/source/Target/Multiplexer.cpp
  lldb/test/API/functionalities/scripted_multiplexer/Makefile
  lldb/test/API/functionalities/scripted_multiplexer/TestScriptedMultiplexer.py
  lldb/test/API/functionalities/scripted_multiplexer/main.cpp
  
lldb/test/API/functionalities/scripted_multiplexer/multiplexed_scripted_process.py
  lldb/test/API/functionalities/scripted_platform/Makefile
  lldb/test/API/functionalities/scripted_platform/TestScriptedPlatform.py
  lldb/test/API/functionalities/scripted_platform/main.cpp
  lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp

Index: lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
===
--- lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
+++ lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
@@ -159,6 +159,10 @@
   return nullptr;
 }
 
+void *lldb_private::LLDBSWIGPython_CastPyObjectToSBEvent(PyObject *data) {
+  return nullptr;
+}
+
 void *lldb_private::LLDBSWIGPython_CastPyObjectToSBError(PyObject *data) {
   return nullptr;
 }
Index: lldb/test/API/functionalities/scripted_platform/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/scripted_platform/main.cpp
@@ -0,0 +1,30 @@
+#include 
+#include 
+#include 
+#include 
+
+int main() {
+  size_t num_threads = 10;
+  std::vector threads;
+
+  std::mutex mutex;
+  std::unique_lock lock(mutex);
+
+  for (size_t i = 0; i < num_threads; i++) {
+std::cout << "Spawning thread " << i << std::endl;
+threads.push_back(std::thread([]() {
+  while (true) {
+;
+  }
+}));
+  }
+
+  for (auto &t : threads) {
+if (t.joinable())
+  t.join();
+  }
+
+  lock.unlock();
+
+  return 0;
+}
Index: lldb/test/API/functionalities/scripted_platform/TestScriptedPlatform.py
===
--- /dev/null
+++ lldb/test/API/functionalities/scripted_platform/TestScriptedPlatform.py
@@ -0,0 +1,151 @@
+"""
+Test python scri

[Lldb-commits] [PATCH] D143127: [LLDB] Fix assertion failure by removing `CopyType` in `std::coroutine_handle` pretty printer

[Adrian Vogelsgesang via Phabricator via lldb-commits]

avogelsgesang added a comment.

> This change makes me a little nervous, but mostly because I don't quite 
> understand the design here.
> So ASTImporter does not have a check whether src context and dst context are 
> identical?

I am completely with you. I also don't understand the design of `ASTImporter`.
I had to learn the hard way that there are multiple ASTs around at the same 
time (one per object file + one for the "debugging session" in which all 
temporary types etc. are created).
But I don't quite understand their lifetimes, and which types of cross-AST 
linking is supported and in which cases types need to be copied over.

In that sense, I see this commit mostly as a basis for discussions, but I am 
not completely confident that this is the right solution. I would hope to find 
someone more experienced with lldb who can point me in the right direction :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143127

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

[Lldb-commits] [PATCH] D143104: [lldb/Plugins] Add Attach capabilities to ScriptedProcess

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.



Comment at: lldb/include/lldb/Utility/ProcessInfo.h:90-110
+  bool IsScriptedProcess() const {
+return !m_scripted_process_class_name.empty();
+  }
+
+  std::string GetScriptedProcessClassName() const {
+return m_scripted_process_class_name;
+  }

mib wrote:
> JDevlieghere wrote:
> > What's the relationship between this an the `ScriptedMetadata`? It seems 
> > like this is storing the same stuff. Also `IsScriptedProcess` is a pretty 
> > blatant violation of the scripted process being a plugin... 
> I think you have a misunderstanding here: This is addition is not made in a 
> process plugin, but rather in the `ProcessInfo` class, the base class for 
> `ProcessLaunchInfo` and `ProcessAttachInfo`, so I don't think this violates 
> in any way the plugin architecture.
> 
> Regarding the relationship with `ScriptedMetadata`: I guess we could replace 
> `m_scripted_process_class_name` & `m_scripted_process_dictionary_sp` by a 
> `m_scripted_metadata` object, but I didn't want to process info hold scripted 
> process specific stuff. I don't have a strong opinion against it, either.
> I think you have a misunderstanding here: This is addition is not made in a 
> process plugin, but rather in the ProcessInfo class, the base class for 
> ProcessLaunchInfo and ProcessAttachInfo, so I don't think this violates in 
> any way the plugin architecture.

I know, but I would argue that the distinction between those two is pretty much 
irrelevant. That said, I do understand the need to pass this information, 
especially as more things become more scriptable (process, platform, etc). 
That's why I suggested using the `ScriptedMetadata` here because it's more 
generic.

Concretely my suggestion would be to have `{Set,Get}ScriptedMetadata` and if 
you think you really need it, maybe an `IsScripted` method that checks if the 
metadata is set (either by storing an optional ScriptedMetadata in the 
`ProcessInfo` or maybe through calling an `operator bool` on the 
`ScriptedMetadata` (which would check the name being empty like you do here). 

> but I didn't want to process info hold scripted process specific stuff. 

I totally agree, as of now `ScriptedMetadata` contains the exact two fields 
that you have here. If other patches are extending that for the scripted 
processes or platforms, we could keep the common stuff in a base class and have 
a ScriptedProcessMetadata and ScriptedPlatformMetadata respectively. WDYT?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143104

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

[Lldb-commits] [PATCH] D143104: [lldb/Plugins] Add Attach capabilities to ScriptedProcess

[Med Ismail Bennani via Phabricator via lldb-commits]

mib added inline comments.



Comment at: lldb/include/lldb/Utility/ProcessInfo.h:90-110
+  bool IsScriptedProcess() const {
+return !m_scripted_process_class_name.empty();
+  }
+
+  std::string GetScriptedProcessClassName() const {
+return m_scripted_process_class_name;
+  }

JDevlieghere wrote:
> mib wrote:
> > JDevlieghere wrote:
> > > What's the relationship between this an the `ScriptedMetadata`? It seems 
> > > like this is storing the same stuff. Also `IsScriptedProcess` is a pretty 
> > > blatant violation of the scripted process being a plugin... 
> > I think you have a misunderstanding here: This is addition is not made in a 
> > process plugin, but rather in the `ProcessInfo` class, the base class for 
> > `ProcessLaunchInfo` and `ProcessAttachInfo`, so I don't think this violates 
> > in any way the plugin architecture.
> > 
> > Regarding the relationship with `ScriptedMetadata`: I guess we could 
> > replace `m_scripted_process_class_name` & 
> > `m_scripted_process_dictionary_sp` by a `m_scripted_metadata` object, but I 
> > didn't want to process info hold scripted process specific stuff. I don't 
> > have a strong opinion against it, either.
> > I think you have a misunderstanding here: This is addition is not made in a 
> > process plugin, but rather in the ProcessInfo class, the base class for 
> > ProcessLaunchInfo and ProcessAttachInfo, so I don't think this violates in 
> > any way the plugin architecture.
> 
> I know, but I would argue that the distinction between those two is pretty 
> much irrelevant. That said, I do understand the need to pass this 
> information, especially as more things become more scriptable (process, 
> platform, etc). That's why I suggested using the `ScriptedMetadata` here 
> because it's more generic.
> 
> Concretely my suggestion would be to have `{Set,Get}ScriptedMetadata` and if 
> you think you really need it, maybe an `IsScripted` method that checks if the 
> metadata is set (either by storing an optional ScriptedMetadata in the 
> `ProcessInfo` or maybe through calling an `operator bool` on the 
> `ScriptedMetadata` (which would check the name being empty like you do here). 
> 
> > but I didn't want to process info hold scripted process specific stuff. 
> 
> I totally agree, as of now `ScriptedMetadata` contains the exact two fields 
> that you have here. If other patches are extending that for the scripted 
> processes or platforms, we could keep the common stuff in a base class and 
> have a ScriptedProcessMetadata and ScriptedPlatformMetadata respectively. 
> WDYT?
That sounds fair to me. Let's go with your suggestion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143104

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

[Lldb-commits] [PATCH] D143215: Separate Process::GetWatchpointSupportInfo into two methods to get the two separate pieces of information

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda created this revision.
jasonmolenda added a reviewer: jingham.
jasonmolenda added a project: LLDB.
Herald added subscribers: JDevlieghere, kristof.beyls.
Herald added a project: All.
jasonmolenda requested review of this revision.
Herald added a subscriber: lldb-commits.

There are two Process:GetWatchpointSupportInfo methods - one which (possibly) 
returns the number of watchpoints available in the target, and one which 
(possibly) returns the number of watchpoints in the target & whether watchpoint 
exceptions are received after the instruction has executed, or before.

The number of watchpoints in the target is determined by sending the 
`qWatchpointSupportInfo:` packet, an lldb extension.  We don't do anything with 
this except print it in the watchpoint commands, and return it in SBProcess.  
One user-requested watchpoint may need to use multiple actual watchpoints to 
implement - an unaligned buffer, or a buffer too large for a single watchpoint, 
so even if the target reports 4 watchpoints available, you may not be able to 
watch 4 different things simultaneously.

Whether watchpoint exception is received before or after is based on either the 
`qHostInfo` response from the remote stub (if it includes a k-v like 
`watchpoint_exceptions_received:before;`, an lldb extension, but maybe all of 
qHostInfo is now that I think of it), OR it is determined by the target CPU.  
realistically, I suspect it's always determined by the target CPU, but who 
knows, maybe there could exist a cu where it is selectable.

If the number of watchpoints could not be fetched with an lldb-extension 
packet, `Process:GetWatchpointSupportInfo` would not report whether breakpoints 
are received before or after the instruction has executed.  This latter is 
important to know to handle watchpoints; the former doesn't really matter.  So 
when we connect to a non-lldb-server/debugserver stub, and we are targetting an 
arm cpu (where watchpoint exceptions are received before the instruction 
executes), lldb would not correctly disable the watchpoint/insn-step/re-enable 
the watchpoint (v. StopInfoWatchpoint::ShouldStopSynchronous in StopInfo.cpp), 
you'd hit the watchpoint exception over and over without advancing.

This patch separates these into two different Process methods, and sets a 
default of "before" for arm targets if the qHostInfo packet didn't provide any 
hints.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D143215

Files:
  lldb/include/lldb/Target/Process.h
  lldb/include/lldb/lldb-defines.h
  lldb/source/API/SBProcess.cpp
  lldb/source/Commands/CommandObjectWatchpoint.cpp
  lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
  lldb/source/Plugins/Process/Windows/Common/ProcessWindows.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
  lldb/source/Target/StopInfo.cpp
  lldb/source/Target/Target.cpp

Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -790,13 +790,12 @@
 }
 
 static bool CheckIfWatchpointsSupported(Target *target, Status &error) {
-  uint32_t num_supported_hardware_watchpoints;
-  Status rc = target->GetProcessSP()->GetWatchpointSupportInfo(
-  num_supported_hardware_watchpoints);
+  uint32_t num_supported_hardware_watchpoints =
+  target->GetProcessSP()->GetWatchpointSlotCount();
 
   // If unable to determine the # of watchpoints available,
   // assume they are supported.
-  if (rc.Fail())
+  if (num_supported_hardware_watchpoints == LLDB_INVALID_WATCHPOINT_SLOTS)
 return true;
 
   if (num_supported_hardware_watchpoints == 0) {
Index: lldb/source/Target/StopInfo.cpp
===
--- lldb/source/Target/StopInfo.cpp
+++ lldb/source/Target/StopInfo.cpp
@@ -823,16 +823,8 @@
 // stop
 
 ProcessSP process_sp = exe_ctx.GetProcessSP();
-uint32_t num;
-bool wp_triggers_after;
+bool wp_triggers_after = process_sp->GetWatchpointReportedAfter();
 
-if (!process_sp->GetWatchpointSupportInfo(num, wp_triggers_after)
-.Success()) {
-  m_should_stop_is_valid = true;
-  m_should_stop = true;
-  return m_should_stop;
-}
-
 if (!wp_triggers_after) {
   // We have to step over the watchpoint before we know what to do:   
   StopInfoWatchpointSP me_as_siwp_sp 
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
@@ -159,7 +159,7 @@
 
   Status DisableWatchpoint(Watchpoint *wp, bool notify = true) ov

[Lldb-commits] [lldb] d980860 - Add usage info for backtick to the lldb tutorial.

[Jim Ingham via lldb-commits]

Author: Jim Ingham
Date: 2023-02-02T13:47:12-08:00
New Revision: d98086036a62e4e9c912c3b15bf16893d3df41da

URL: 
https://github.com/llvm/llvm-project/commit/d98086036a62e4e9c912c3b15bf16893d3df41da
DIFF: 
https://github.com/llvm/llvm-project/commit/d98086036a62e4e9c912c3b15bf16893d3df41da.diff

LOG: Add usage info for backtick to the lldb tutorial.

Added: 


Modified: 
lldb/docs/use/tutorial.rst

Removed: 




diff  --git a/lldb/docs/use/tutorial.rst b/lldb/docs/use/tutorial.rst
index d4da0bae5aaf0..9c807ce4bb55d 100644
--- a/lldb/docs/use/tutorial.rst
+++ b/lldb/docs/use/tutorial.rst
@@ -20,10 +20,23 @@ Unlike gdb's command set, which is rather free-form, we 
tried to make the lldb c
 The command line parsing is done before command execution, so it is uniform
 across all the commands. The command syntax for basic commands is very simple,
 arguments, options and option values are all white-space separated, and
-double-quotes are used to protect white-spaces in an argument. If you need to
-put a backslash or double-quote character in an argument you back-slash it in
-the argument. That makes the command syntax more regular, but it also means you
-may have to quote some arguments in lldb that you wouldn't in gdb.
+either single or double-quotes (in pairs) are used to protect white-spaces
+in an argument.  If you need to put a backslash or double-quote character in an
+argument you back-slash it in the argument. That makes the command syntax more
+regular, but it also means you may have to quote some arguments in lldb that
+you wouldn't in gdb.
+
+There is one other special quote character in lldb - the backtick.
+If you put backticks around an argument or option value, lldb will run the text
+of the value through the expression parser, and the result of the expression
+will be passed to the command.  So for instance, if "len" is a local
+int variable with the value 5, then the command:
+
+::
+
+   (lldb) memory read -c `len` 0x12345
+
+will receive the value 5 for the count option, rather than the string "len".
 
 
 Options can be placed anywhere on the command line, but if the arguments begin



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

[Lldb-commits] [PATCH] D143215: Separate Process::GetWatchpointSupportInfo into two methods to get the two separate pieces of information

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda updated this revision to Diff 494428.
jasonmolenda added a comment.

Ah, I wasn't paying close enough attention and got the logic for MIPS/PPC64 
watchpoint exceptions backwards in my update to 
GDBRemoteCommunicationClient::GetWatchpointReportedAfter, update patch for that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143215

Files:
  lldb/include/lldb/Target/Process.h
  lldb/include/lldb/lldb-defines.h
  lldb/source/API/SBProcess.cpp
  lldb/source/Commands/CommandObjectWatchpoint.cpp
  lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
  lldb/source/Plugins/Process/Windows/Common/ProcessWindows.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
  lldb/source/Target/StopInfo.cpp
  lldb/source/Target/Target.cpp

Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -790,13 +790,12 @@
 }
 
 static bool CheckIfWatchpointsSupported(Target *target, Status &error) {
-  uint32_t num_supported_hardware_watchpoints;
-  Status rc = target->GetProcessSP()->GetWatchpointSupportInfo(
-  num_supported_hardware_watchpoints);
+  uint32_t num_supported_hardware_watchpoints =
+  target->GetProcessSP()->GetWatchpointSlotCount();
 
   // If unable to determine the # of watchpoints available,
   // assume they are supported.
-  if (rc.Fail())
+  if (num_supported_hardware_watchpoints == LLDB_INVALID_WATCHPOINT_SLOTS)
 return true;
 
   if (num_supported_hardware_watchpoints == 0) {
Index: lldb/source/Target/StopInfo.cpp
===
--- lldb/source/Target/StopInfo.cpp
+++ lldb/source/Target/StopInfo.cpp
@@ -823,16 +823,8 @@
 // stop
 
 ProcessSP process_sp = exe_ctx.GetProcessSP();
-uint32_t num;
-bool wp_triggers_after;
+bool wp_triggers_after = process_sp->GetWatchpointReportedAfter();
 
-if (!process_sp->GetWatchpointSupportInfo(num, wp_triggers_after)
-.Success()) {
-  m_should_stop_is_valid = true;
-  m_should_stop = true;
-  return m_should_stop;
-}
-
 if (!wp_triggers_after) {
   // We have to step over the watchpoint before we know what to do:   
   StopInfoWatchpointSP me_as_siwp_sp 
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
@@ -159,7 +159,7 @@
 
   Status DisableWatchpoint(Watchpoint *wp, bool notify = true) override;
 
-  Status GetWatchpointSupportInfo(uint32_t &num) override;
+  uint32_t GetWatchpointSlotCount() override;
 
   llvm::Expected TraceSupported() override;
 
@@ -172,7 +172,7 @@
   llvm::Expected>
   TraceGetBinaryData(const TraceGetBinaryDataRequest &request) override;
 
-  Status GetWatchpointSupportInfo(uint32_t &num, bool &after) override;
+  bool GetWatchpointReportedAfter() override;
 
   bool StartNoticingNewThreads() override;
 
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -2818,16 +2818,13 @@
   return error;
 }
 
-Status ProcessGDBRemote::GetWatchpointSupportInfo(uint32_t &num) {
+uint32_t ProcessGDBRemote::GetWatchpointSlotCount() {
 
-  Status error(m_gdb_comm.GetWatchpointSupportInfo(num));
-  return error;
+  return m_gdb_comm.GetWatchpointSlotCount();
 }
 
-Status ProcessGDBRemote::GetWatchpointSupportInfo(uint32_t &num, bool &after) {
-  Status error(m_gdb_comm.GetWatchpointSupportInfo(
-  num, after, GetTarget().GetArchitecture()));
-  return error;
+bool ProcessGDBRemote::GetWatchpointReportedAfter() {
+  return m_gdb_comm.GetWatchpointReportedAfter(GetTarget().GetArchitecture());
 }
 
 Status ProcessGDBRemote::DoDeallocateMemory(lldb::addr_t addr) {
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
@@ -194,13 +194,9 @@
 
   Status GetMemoryRegionInfo(lldb::addr_t addr, MemoryRegionInfo &range_info);
 
-  Status GetWatchpointSupportInfo(uint32_t &num);
+  uint32_t GetWatchpointSlotCount();
 
-  Status GetWatchpointSupportInfo(uint32_t &num, bool &after,
-  const ArchSpec &arch);
-
-  Status GetWatc

[Lldb-commits] [PATCH] D143215: Separate Process::GetWatchpointSupportInfo into two methods to get the two separate pieces of information

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda added a comment.

tl;dr: watchpoints don't work on things like arm targets with a JTAG gdb stub 
that doesn't support `qWatchpointSupportInfo:` or `qHostInfo` with the 
`watchpoint_exceptions_received` key.

Nearly all of this patch is mechanical, and IMO the question is whether I 
should have stuck with `Process:GetWatchpointSupportInfo` but added some way to 
indicate that we cannot fetch the number of watchpoints, but we can report 
whether the exceptions come before/after.

Also this seems a little weak to have it encoded down in 
GDBRemoteCommunicationClient; it seems like something we could ask the ArchSpec 
about tbh.  I didn't go that far, but maybe that would be better to do while 
I'm here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143215

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

[Lldb-commits] [lldb] 35ef899 - [lldb/python] Fix scripted_platform python module creation

[Med Ismail Bennani via lldb-commits]

Author: Med Ismail Bennani
Date: 2023-02-02T15:22:52-08:00
New Revision: 35ef899f94eb3c52b9ba5671be606d755053d90e

URL: 
https://github.com/llvm/llvm-project/commit/35ef899f94eb3c52b9ba5671be606d755053d90e
DIFF: 
https://github.com/llvm/llvm-project/commit/35ef899f94eb3c52b9ba5671be606d755053d90e.diff

LOG: [lldb/python] Fix scripted_platform python module creation

This patch should fix the creation and addition of the `scripted_platform`
python module into the `lldb.plugins` module.

Previously, we were creating the `plugins` submodule, each time with a
different source file (either `scripted_process` or `scripted_platform`).

The removes the redundant `create_python_package` call and group both
python source files toghether.

Differential Revision: https://reviews.llvm.org/D143122

Signed-off-by: Med Ismail Bennani 

Added: 


Modified: 
lldb/bindings/python/CMakeLists.txt

Removed: 




diff  --git a/lldb/bindings/python/CMakeLists.txt 
b/lldb/bindings/python/CMakeLists.txt
index 528f9f47f17a8..2cc3ad1bb98b7 100644
--- a/lldb/bindings/python/CMakeLists.txt
+++ b/lldb/bindings/python/CMakeLists.txt
@@ -103,13 +103,7 @@ function(finish_swig_python swig_target 
lldb_python_bindings_dir lldb_python_tar
 ${lldb_python_target_dir}
 "plugins"
 FILES
-"${LLDB_SOURCE_DIR}/examples/python/scripted_process/scripted_process.py")
-
-  create_python_package(
-${swig_target}
-${lldb_python_target_dir}
-"plugins"
-FILES
+"${LLDB_SOURCE_DIR}/examples/python/scripted_process/scripted_process.py"
 "${LLDB_SOURCE_DIR}/examples/python/scripted_process/scripted_platform.py")
 
   if(APPLE)



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

[Lldb-commits] [lldb] 2403fa4 - [lldb] Fix typo in ScriptedProcess python docstrings (NFC)

[Med Ismail Bennani via lldb-commits]

Author: Med Ismail Bennani
Date: 2023-02-02T15:22:52-08:00
New Revision: 2403fa46b9e19b02f17ffd315e49110b7b2ebdea

URL: 
https://github.com/llvm/llvm-project/commit/2403fa46b9e19b02f17ffd315e49110b7b2ebdea
DIFF: 
https://github.com/llvm/llvm-project/commit/2403fa46b9e19b02f17ffd315e49110b7b2ebdea.diff

LOG: [lldb] Fix typo in ScriptedProcess python docstrings (NFC)

Signed-off-by: Med Ismail Bennani 

Added: 


Modified: 
lldb/examples/python/scripted_process/scripted_process.py

Removed: 




diff  --git a/lldb/examples/python/scripted_process/scripted_process.py 
b/lldb/examples/python/scripted_process/scripted_process.py
index a84e057edc5ec..758d0a47d1152 100644
--- a/lldb/examples/python/scripted_process/scripted_process.py
+++ b/lldb/examples/python/scripted_process/scripted_process.py
@@ -200,7 +200,7 @@ def get_process_metadata(self):
 
 Returns:
 Dict: A dictionary containing metadata for the scripted process.
-  None is the process as no metadata.
+  None if the process as no metadata.
 """
 return self.metadata
 
@@ -350,7 +350,7 @@ def get_extended_info(self):
 
 Returns:
 List: A list containing the extended information for the scripted 
process.
-  None is the thread as no extended information.
+  None if the thread as no extended information.
 """
 return self.extended_info
 



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

[Lldb-commits] [PATCH] D143122: [lldb/python] Fix scripted_platform python module creation

[Med Ismail Bennani via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG35ef899f94eb: [lldb/python] Fix scripted_platform python 
module creation (authored by mib).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143122

Files:
  lldb/bindings/python/CMakeLists.txt


Index: lldb/bindings/python/CMakeLists.txt
===
--- lldb/bindings/python/CMakeLists.txt
+++ lldb/bindings/python/CMakeLists.txt
@@ -103,13 +103,7 @@
 ${lldb_python_target_dir}
 "plugins"
 FILES
-"${LLDB_SOURCE_DIR}/examples/python/scripted_process/scripted_process.py")
-
-  create_python_package(
-${swig_target}
-${lldb_python_target_dir}
-"plugins"
-FILES
+"${LLDB_SOURCE_DIR}/examples/python/scripted_process/scripted_process.py"
 "${LLDB_SOURCE_DIR}/examples/python/scripted_process/scripted_platform.py")
 
   if(APPLE)


Index: lldb/bindings/python/CMakeLists.txt
===
--- lldb/bindings/python/CMakeLists.txt
+++ lldb/bindings/python/CMakeLists.txt
@@ -103,13 +103,7 @@
 ${lldb_python_target_dir}
 "plugins"
 FILES
-"${LLDB_SOURCE_DIR}/examples/python/scripted_process/scripted_process.py")
-
-  create_python_package(
-${swig_target}
-${lldb_python_target_dir}
-"plugins"
-FILES
+"${LLDB_SOURCE_DIR}/examples/python/scripted_process/scripted_process.py"
 "${LLDB_SOURCE_DIR}/examples/python/scripted_process/scripted_platform.py")
 
   if(APPLE)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D142926: [lldb][RFC] Replace SB swig interfaces with API headers

[Greg Clayton via Phabricator via lldb-commits]

clayborg added inline comments.



Comment at: lldb/include/lldb/API/SBAddress.h:14-16
+#ifdef SWIG
+%include "interface/SBAddressDocstrings.i"
+#endif

bulbazord wrote:
> clayborg wrote:
> > Do we want two different .i files? Right now we have 
> > both"interface/SB*Docstrings.i" and "interface/SB*Extensions.i". Can those 
> > be combined into just one "interface/SBAddress.i" and only included if we 
> > have anything in the .i file that isn't just a copy of the API itself? Or 
> > does the doc string stuff have to come first?
> What I discovered when working this patch is that the docstring information 
> needs to come before the declaration of what it wants to document. So 
> `SB*Docstrings.i` should come first. The `SB*Extensions.i` are there to 
> extend an existing class, so they should come after the class itself. I'm not 
> sure there's a good way to do this.
> 
> Alternatively, we could changes the `interfaces.swig` file to include 
> `./interface/SBAddressDocstrings.i`, then `lldb/API/SBAddress.h`, then 
> `./interface/SBAddressExtensions.i`. That way we don't need to have these 2 
> `#ifdef` blocks in the API headers themselves. 
Anything we can do to keep the headers clean would be great. I know that was 
the original reason we started using them.



Comment at: lldb/include/lldb/API/SBAddress.h:33
 
+#ifndef SWIG
   const lldb::SBAddress &operator=(const lldb::SBAddress &rhs);

bulbazord wrote:
> clayborg wrote:
> > Do we need all assignment operators left out for Swig? If so it might be 
> > good to add a comment explaining so it is clear to people. I can't remember 
> > if this is true or not in all cases?
> Yeah I can add a comment. When generating python bindings, swig will ignore 
> assignment operators as it doesn't map cleanly into python, which is why I 
> left this out. It may be more accurate to do something like `#ifndef 
> SWIGPYTHON` but seeing as the `SBAddress.i` class doesn't have an `operator=` 
> in it, I decided to leave it out for swig generation entirely.
Will it just ignore these if they are left in, or will it end up doing the 
wrong thing? Anything we can do to keep the header files clean is good. No 
worries if we have to keep the #ifndef stuff



Comment at: lldb/include/lldb/API/SBAddress.h:133
 
+#ifndef SWIG
 bool LLDB_API operator==(const SBAddress &lhs, const SBAddress &rhs);

bulbazord wrote:
> clayborg wrote:
> > Do we need all == operators left out of Swig stuff? Comment if so? Can't 
> > remember if this applies to all API objects or just to SBAddress?
> I don't think that all instances of `operator==` need to be left out. There 
> are some classes that have it in the interface files, e.g. SBBreakpoint. I 
> intentionally left this one out because it wasn't in `SBAddress.i` and I 
> wanted this to leave the python bindings roughly the same (no new 
> functions/methods, re-ordering them in the `lldb.py` should be ok though).
> 
Again, cleaner headers would be nice if we can get away with removing the 
#ifndef stuff


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142926

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

[Lldb-commits] [PATCH] D142926: [lldb][RFC] Replace SB swig interfaces with API headers

[Alex Langford via Phabricator via lldb-commits]

bulbazord added inline comments.



Comment at: lldb/include/lldb/API/SBAddress.h:14-16
+#ifdef SWIG
+%include "interface/SBAddressDocstrings.i"
+#endif

clayborg wrote:
> bulbazord wrote:
> > clayborg wrote:
> > > Do we want two different .i files? Right now we have 
> > > both"interface/SB*Docstrings.i" and "interface/SB*Extensions.i". Can 
> > > those be combined into just one "interface/SBAddress.i" and only included 
> > > if we have anything in the .i file that isn't just a copy of the API 
> > > itself? Or does the doc string stuff have to come first?
> > What I discovered when working this patch is that the docstring information 
> > needs to come before the declaration of what it wants to document. So 
> > `SB*Docstrings.i` should come first. The `SB*Extensions.i` are there to 
> > extend an existing class, so they should come after the class itself. I'm 
> > not sure there's a good way to do this.
> > 
> > Alternatively, we could changes the `interfaces.swig` file to include 
> > `./interface/SBAddressDocstrings.i`, then `lldb/API/SBAddress.h`, then 
> > `./interface/SBAddressExtensions.i`. That way we don't need to have these 2 
> > `#ifdef` blocks in the API headers themselves. 
> Anything we can do to keep the headers clean would be great. I know that was 
> the original reason we started using them.
I'll try to minimize the noise in the API headers when I do the complete thing.



Comment at: lldb/include/lldb/API/SBAddress.h:33
 
+#ifndef SWIG
   const lldb::SBAddress &operator=(const lldb::SBAddress &rhs);

clayborg wrote:
> bulbazord wrote:
> > clayborg wrote:
> > > Do we need all assignment operators left out for Swig? If so it might be 
> > > good to add a comment explaining so it is clear to people. I can't 
> > > remember if this is true or not in all cases?
> > Yeah I can add a comment. When generating python bindings, swig will ignore 
> > assignment operators as it doesn't map cleanly into python, which is why I 
> > left this out. It may be more accurate to do something like `#ifndef 
> > SWIGPYTHON` but seeing as the `SBAddress.i` class doesn't have an 
> > `operator=` in it, I decided to leave it out for swig generation entirely.
> Will it just ignore these if they are left in, or will it end up doing the 
> wrong thing? Anything we can do to keep the header files clean is good. No 
> worries if we have to keep the #ifndef stuff
It ignores them and emits a warning. I suppose we don't care about these 
specific warnings and swig has a way of disabling specified warnings so I'll 
just do that instead.



Comment at: lldb/include/lldb/API/SBAddress.h:133
 
+#ifndef SWIG
 bool LLDB_API operator==(const SBAddress &lhs, const SBAddress &rhs);

clayborg wrote:
> bulbazord wrote:
> > clayborg wrote:
> > > Do we need all == operators left out of Swig stuff? Comment if so? Can't 
> > > remember if this applies to all API objects or just to SBAddress?
> > I don't think that all instances of `operator==` need to be left out. There 
> > are some classes that have it in the interface files, e.g. SBBreakpoint. I 
> > intentionally left this one out because it wasn't in `SBAddress.i` and I 
> > wanted this to leave the python bindings roughly the same (no new 
> > functions/methods, re-ordering them in the `lldb.py` should be ok though).
> > 
> Again, cleaner headers would be nice if we can get away with removing the 
> #ifndef stuff
I'm going to keep this one for now. This change is supposed to be relatively 
NFC and I don't want to introduce extra functionality in the python bindings at 
the same time we do this. We can re-evaluate adding this to the python bindings 
at a later time.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142926

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

[Lldb-commits] [PATCH] D143232: Return an error when the CFA resolves to no known register, instead of segfaulting

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda created this revision.
jasonmolenda added a reviewer: bulbazord.
jasonmolenda added a project: LLDB.
Herald added a subscriber: JDevlieghere.
Herald added a project: All.
jasonmolenda requested review of this revision.
Herald added a subscriber: lldb-commits.

I'm working on a bug report where lldb crashes while trying to read the 
register that is used to calculate the canonical frame address for the first 
frame in a stack trace.  I haven't figured out how they're getting in this 
state yet, but I want to add a check for a failure to find a register in this 
case, and declare the stack frame as invalid to handle the error.  I also added 
an assert so we can catch it early in debug builds if it ever comes up here.

I think returning no valid stack frame for this thread is going to be a better 
failure mode than having the debugger crash out from under them, ending the 
debug session.  It's still a pretty bad failure, but hopefully we can collect 
some logging with this if it comes up again.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D143232

Files:
  lldb/source/Target/RegisterContextUnwind.cpp


Index: lldb/source/Target/RegisterContextUnwind.cpp
===
--- lldb/source/Target/RegisterContextUnwind.cpp
+++ lldb/source/Target/RegisterContextUnwind.cpp
@@ -37,6 +37,8 @@
 #include "lldb/Utility/RegisterValue.h"
 #include "lldb/Utility/VASPrintf.h"
 #include "lldb/lldb-private.h"
+
+#include 
 #include 
 
 using namespace lldb;
@@ -289,6 +291,13 @@
   } else
 ReadFrameAddress(row_register_kind, active_row->GetAFAValue(), m_afa);
 
+  if (m_cfa == LLDB_INVALID_ADDRESS && m_afa == LLDB_INVALID_ADDRESS) {
+UnwindLogMsg(
+"could not read CFA or AFA values for first frame, not valid.");
+m_frame_type = eNotAValidFrame;
+return;
+  }
+
   UnwindLogMsg("initialized frame current pc is 0x%" PRIx64 " cfa is 0x%" 
PRIx64
" afa is 0x%" PRIx64 " using %s UnwindPlan",
(uint64_t)m_current_pc.GetLoadAddress(exe_ctx.GetTargetPtr()),
@@ -2116,6 +2125,14 @@
   }
 
   const RegisterInfo *reg_info = GetRegisterInfoAtIndex(lldb_regnum);
+  assert(reg_info);
+  if (!reg_info) {
+UnwindLogMsg(
+"Could not find RegisterInfo definition for lldb register number %d",
+lldb_regnum);
+return false;
+  }
+
   RegisterValue reg_value;
   // if this is frame 0 (currently executing frame), get the requested reg
   // contents from the actual thread registers


Index: lldb/source/Target/RegisterContextUnwind.cpp
===
--- lldb/source/Target/RegisterContextUnwind.cpp
+++ lldb/source/Target/RegisterContextUnwind.cpp
@@ -37,6 +37,8 @@
 #include "lldb/Utility/RegisterValue.h"
 #include "lldb/Utility/VASPrintf.h"
 #include "lldb/lldb-private.h"
+
+#include 
 #include 
 
 using namespace lldb;
@@ -289,6 +291,13 @@
   } else
 ReadFrameAddress(row_register_kind, active_row->GetAFAValue(), m_afa);
 
+  if (m_cfa == LLDB_INVALID_ADDRESS && m_afa == LLDB_INVALID_ADDRESS) {
+UnwindLogMsg(
+"could not read CFA or AFA values for first frame, not valid.");
+m_frame_type = eNotAValidFrame;
+return;
+  }
+
   UnwindLogMsg("initialized frame current pc is 0x%" PRIx64 " cfa is 0x%" PRIx64
" afa is 0x%" PRIx64 " using %s UnwindPlan",
(uint64_t)m_current_pc.GetLoadAddress(exe_ctx.GetTargetPtr()),
@@ -2116,6 +2125,14 @@
   }
 
   const RegisterInfo *reg_info = GetRegisterInfoAtIndex(lldb_regnum);
+  assert(reg_info);
+  if (!reg_info) {
+UnwindLogMsg(
+"Could not find RegisterInfo definition for lldb register number %d",
+lldb_regnum);
+return false;
+  }
+
   RegisterValue reg_value;
   // if this is frame 0 (currently executing frame), get the requested reg
   // contents from the actual thread registers
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D142862: [Support] change StringMap hash function from djbHash to xxHash

[Fangrui Song via Phabricator via lldb-commits]

MaskRay added inline comments.



Comment at: clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp:638
 
-  EXPECT_THAT(Storage.keys(), ElementsAre(testPath("A.cc"), testPath("A.h")));
+  EXPECT_THAT(Storage.keys(), ElementsAre(testPath("A.h"), testPath("A.cc")));
   // Make sure we only store the Cmd for main file.

Rebase. I've changed this to `UnorderedElementsAre`



Comment at: clang-tools-extra/test/modularize/ProblemsInconsistent.modularize:6
 
-# CHECK: error: macro 'SYMBOL' defined at multiple locations:
-# CHECK-NEXT: {{.*}}{{[/\\]}}Inputs{{[/\\]}}InconsistentSubHeader.h:3:9
-# CHECK-NEXT: {{.*}}{{[/\\]}}Inputs{{[/\\]}}InconsistentSubHeader.h:7:9
-# CHECK-NEXT: error: macro 'FUNC_STYLE' defined at multiple locations:
+# CHECK: error: macro 'FUNC_STYLE' defined at multiple locations:
 # CHECK-NEXT: {{.*}}{{[/\\]}}Inputs{{[/\\]}}InconsistentSubHeader.h:4:9

Rebase. I've fixed modularize's misuse of StringMap 
https://llvm.org/docs/ProgrammersManual.html#llvm-adt-stringmap-h

> StringMap iteration order, however, is not guaranteed to be deterministic, so 
> any uses which require that should instead use a std::map.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142862

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

[Lldb-commits] [PATCH] D143232: Return an error when the CFA resolves to no known register, instead of segfaulting

[Alex Langford via Phabricator via lldb-commits]

bulbazord accepted this revision.
bulbazord added a comment.
This revision is now accepted and ready to land.

LGTM. I wonder if there's a good way to exercise this with a test? Like maybe 
we can create some bogus unwind information and see if LLDB falls over when 
consuming it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143232

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

[Lldb-commits] [PATCH] D143236: [lldb] Add a way to get a scripted process implementation from the SBAPI

[Med Ismail Bennani via Phabricator via lldb-commits]

mib created this revision.
mib added reviewers: bulbazord, JDevlieghere, jingham.
mib added a project: LLDB.
Herald added a project: All.
mib requested review of this revision.
Herald added a subscriber: lldb-commits.

This patch introduces a new `GetImplementation` method to the SBProcess
class in the SBAPI. It will allow users of Scripted Processes to fetch
the scripted implementation object from to script interpreter to be able
to interact with it directly (without having to go through lldb).

This allows to user to perform action that are not specified in the
scripted process interface, like calling un-specified methods, but also
to enrich the implementation, by passing it complex objects.

Signed-off-by: Med Ismail Bennani 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D143236

Files:
  lldb/bindings/interface/SBProcess.i
  lldb/bindings/python/python-typemaps.swig
  lldb/include/lldb/API/SBProcess.h
  lldb/include/lldb/Target/Process.h
  lldb/source/API/SBProcess.cpp
  lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
  lldb/source/Plugins/Process/scripted/ScriptedProcess.h
  lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
  lldb/test/API/functionalities/scripted_process/dummy_scripted_process.py

Index: lldb/test/API/functionalities/scripted_process/dummy_scripted_process.py
===
--- lldb/test/API/functionalities/scripted_process/dummy_scripted_process.py
+++ lldb/test/API/functionalities/scripted_process/dummy_scripted_process.py
@@ -43,6 +43,12 @@
 def get_scripted_thread_plugin(self):
 return DummyScriptedThread.__module__ + "." + DummyScriptedThread.__name__
 
+def my_super_secret_method(self):
+if hasattr(self, 'my_super_secret_member'):
+return self.my_super_secret_member
+else:
+return None
+
 
 class DummyScriptedThread(ScriptedThread):
 def __init__(self, process, args):
Index: lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
===
--- lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
+++ lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
@@ -10,6 +10,8 @@
 from lldbsuite.test import lldbutil
 from lldbsuite.test import lldbtest
 
+import dummy_scripted_process
+
 class ScriptedProcesTestCase(TestBase):
 
 NO_DEBUG_INFO_TESTCASE = True
@@ -118,6 +120,14 @@
 self.assertEqual(process.GetProcessID(), 42)
 self.assertEqual(process.GetNumThreads(), 1)
 
+py_impl = process.GetImplementation()
+self.assertTrue(py_impl)
+self.assertTrue(isinstance(py_impl, dummy_scripted_process.DummyScriptedProcess))
+self.assertFalse(hasattr(py_impl, 'my_super_secret_member'))
+py_impl.my_super_secret_member = 42
+self.assertTrue(hasattr(py_impl, 'my_super_secret_member'))
+self.assertEqual(py_impl.my_super_secret_method(), 42)
+
 addr = 0x5
 message = "Hello, world!"
 buff = process.ReadCStringFromMemory(addr, len(message) + 1, error)
Index: lldb/source/Plugins/Process/scripted/ScriptedProcess.h
===
--- lldb/source/Plugins/Process/scripted/ScriptedProcess.h
+++ lldb/source/Plugins/Process/scripted/ScriptedProcess.h
@@ -74,6 +74,8 @@
 
   void UpdateQueueListIfNeeded() override;
 
+  void *GetImplementation() override;
+
 protected:
   ScriptedProcess(lldb::TargetSP target_sp, lldb::ListenerSP listener_sp,
   const ScriptedMetadata &scripted_metadata, Status &error);
Index: lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
===
--- lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
+++ lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
@@ -512,3 +512,10 @@
 ScriptedProcessInterface &ScriptedProcess::GetInterface() const {
   return m_interpreter->GetScriptedProcessInterface();
 }
+
+void *ScriptedProcess::GetImplementation() {
+  if (m_script_object_sp &&
+  m_script_object_sp->GetType() == eStructuredDataTypeGeneric)
+return m_script_object_sp->GetAsGeneric()->GetValue();
+  return nullptr;
+}
Index: lldb/source/API/SBProcess.cpp
===
--- lldb/source/API/SBProcess.cpp
+++ lldb/source/API/SBProcess.cpp
@@ -1262,3 +1262,9 @@
   }
   return sb_error;
 }
+
+void *SBProcess::GetImplementation() {
+  LLDB_INSTRUMENT_VA(this);
+  ProcessSP process_sp(GetSP());
+  return (process_sp) ? process_sp->GetImplementation() : nullptr;
+}
Index: lldb/include/lldb/Target/Process.h
===
--- lldb/include/lldb/Target/Process.h
+++ lldb/include/lldb/Target/Process.h
@@ -2526,6 +2526,8 @@
   lldb::StructuredDataPluginSP
   GetStructuredDataPlugin(Co

[Lldb-commits] [PATCH] D143236: [lldb] Add a way to get a scripted process implementation from the SBAPI

[Alex Langford via Phabricator via lldb-commits]

bulbazord added a comment.

This is an interesting idea.

I suppose I find it a little strange that something from the SB API is passing 
a void * back to the caller instead of something more well defined (like 
another SB class or some primitive) but I suppose that's not the worst thing in 
the world. I'm not sure how else you would get a C++ method to return a type 
that is dependent on which language the method was called from...




Comment at: lldb/bindings/interface/SBProcess.i:347-348
 
+void*
+GetImplementation();
+

Maybe a docstring is in order? Something to explain how to use this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143236

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

[Lldb-commits] [PATCH] D143236: [lldb] Add a way to get a scripted process implementation from the SBAPI

[Med Ismail Bennani via Phabricator via lldb-commits]

mib added a comment.

In D143236#4101429 , @bulbazord wrote:

> This is an interesting idea.
>
> I suppose I find it a little strange that something from the SB API is 
> passing a void * back to the caller instead of something more well defined 
> (like another SB class or some primitive) but I suppose that's not the worst 
> thing in the world. I'm not sure how else you would get a C++ method to 
> return a type that is dependent on which language the method was called 
> from...

So, technically, we could also do something similar with `SBStructuredData` 
since now they don't allow holding generic objects. You can then image that the 
user will be able to unwrap the structured data to get the original object. I 
think @jingham also had other examples in mind, like for breakpoint python 
commands, if I'm not mistaken.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143236

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

[Lldb-commits] [PATCH] D143236: [lldb] Add a way to get a scripted process implementation from the SBAPI

[Med Ismail Bennani via Phabricator via lldb-commits]

mib updated this revision to Diff 494485.
mib marked an inline comment as done.
mib added a comment.

Add docstring.


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

https://reviews.llvm.org/D143236

Files:
  lldb/bindings/interface/SBProcess.i
  lldb/bindings/python/python-typemaps.swig
  lldb/include/lldb/API/SBProcess.h
  lldb/include/lldb/Target/Process.h
  lldb/source/API/SBProcess.cpp
  lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
  lldb/source/Plugins/Process/scripted/ScriptedProcess.h
  lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
  lldb/test/API/functionalities/scripted_process/dummy_scripted_process.py

Index: lldb/test/API/functionalities/scripted_process/dummy_scripted_process.py
===
--- lldb/test/API/functionalities/scripted_process/dummy_scripted_process.py
+++ lldb/test/API/functionalities/scripted_process/dummy_scripted_process.py
@@ -43,6 +43,12 @@
 def get_scripted_thread_plugin(self):
 return DummyScriptedThread.__module__ + "." + DummyScriptedThread.__name__
 
+def my_super_secret_method(self):
+if hasattr(self, 'my_super_secret_member'):
+return self.my_super_secret_member
+else:
+return None
+
 
 class DummyScriptedThread(ScriptedThread):
 def __init__(self, process, args):
Index: lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
===
--- lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
+++ lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
@@ -10,6 +10,8 @@
 from lldbsuite.test import lldbutil
 from lldbsuite.test import lldbtest
 
+import dummy_scripted_process
+
 class ScriptedProcesTestCase(TestBase):
 
 NO_DEBUG_INFO_TESTCASE = True
@@ -118,6 +120,14 @@
 self.assertEqual(process.GetProcessID(), 42)
 self.assertEqual(process.GetNumThreads(), 1)
 
+py_impl = process.GetImplementation()
+self.assertTrue(py_impl)
+self.assertTrue(isinstance(py_impl, dummy_scripted_process.DummyScriptedProcess))
+self.assertFalse(hasattr(py_impl, 'my_super_secret_member'))
+py_impl.my_super_secret_member = 42
+self.assertTrue(hasattr(py_impl, 'my_super_secret_member'))
+self.assertEqual(py_impl.my_super_secret_method(), 42)
+
 addr = 0x5
 message = "Hello, world!"
 buff = process.ReadCStringFromMemory(addr, len(message) + 1, error)
Index: lldb/source/Plugins/Process/scripted/ScriptedProcess.h
===
--- lldb/source/Plugins/Process/scripted/ScriptedProcess.h
+++ lldb/source/Plugins/Process/scripted/ScriptedProcess.h
@@ -74,6 +74,8 @@
 
   void UpdateQueueListIfNeeded() override;
 
+  void *GetImplementation() override;
+
 protected:
   ScriptedProcess(lldb::TargetSP target_sp, lldb::ListenerSP listener_sp,
   const ScriptedMetadata &scripted_metadata, Status &error);
Index: lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
===
--- lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
+++ lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
@@ -512,3 +512,10 @@
 ScriptedProcessInterface &ScriptedProcess::GetInterface() const {
   return m_interpreter->GetScriptedProcessInterface();
 }
+
+void *ScriptedProcess::GetImplementation() {
+  if (m_script_object_sp &&
+  m_script_object_sp->GetType() == eStructuredDataTypeGeneric)
+return m_script_object_sp->GetAsGeneric()->GetValue();
+  return nullptr;
+}
Index: lldb/source/API/SBProcess.cpp
===
--- lldb/source/API/SBProcess.cpp
+++ lldb/source/API/SBProcess.cpp
@@ -1262,3 +1262,9 @@
   }
   return sb_error;
 }
+
+void *SBProcess::GetImplementation() {
+  LLDB_INSTRUMENT_VA(this);
+  ProcessSP process_sp(GetSP());
+  return (process_sp) ? process_sp->GetImplementation() : nullptr;
+}
Index: lldb/include/lldb/Target/Process.h
===
--- lldb/include/lldb/Target/Process.h
+++ lldb/include/lldb/Target/Process.h
@@ -2526,6 +2526,8 @@
   lldb::StructuredDataPluginSP
   GetStructuredDataPlugin(ConstString type_name) const;
 
+  virtual void *GetImplementation() { return nullptr; }
+
 protected:
   friend class Trace;
 
Index: lldb/include/lldb/API/SBProcess.h
===
--- lldb/include/lldb/API/SBProcess.h
+++ lldb/include/lldb/API/SBProcess.h
@@ -423,6 +423,8 @@
   ///
   lldb::SBError DeallocateMemory(lldb::addr_t ptr);
 
+  void *GetImplementation();
+
 protected:
   friend class SBAddress;
   friend class SBBreakpoint;
Index: lldb/bindings/python/python-typemaps.swig
===

[Lldb-commits] [PATCH] D142926: [lldb][RFC] Replace SB swig interfaces with API headers

[Dave Lee via Phabricator via lldb-commits]

kastiglione added inline comments.



Comment at: lldb/include/lldb/API/SBAddress.h:14-16
+#ifdef SWIG
+%include "interface/SBAddressDocstrings.i"
+#endif

bulbazord wrote:
> clayborg wrote:
> > bulbazord wrote:
> > > clayborg wrote:
> > > > Do we want two different .i files? Right now we have 
> > > > both"interface/SB*Docstrings.i" and "interface/SB*Extensions.i". Can 
> > > > those be combined into just one "interface/SBAddress.i" and only 
> > > > included if we have anything in the .i file that isn't just a copy of 
> > > > the API itself? Or does the doc string stuff have to come first?
> > > What I discovered when working this patch is that the docstring 
> > > information needs to come before the declaration of what it wants to 
> > > document. So `SB*Docstrings.i` should come first. The `SB*Extensions.i` 
> > > are there to extend an existing class, so they should come after the 
> > > class itself. I'm not sure there's a good way to do this.
> > > 
> > > Alternatively, we could changes the `interfaces.swig` file to include 
> > > `./interface/SBAddressDocstrings.i`, then `lldb/API/SBAddress.h`, then 
> > > `./interface/SBAddressExtensions.i`. That way we don't need to have these 
> > > 2 `#ifdef` blocks in the API headers themselves. 
> > Anything we can do to keep the headers clean would be great. I know that 
> > was the original reason we started using them.
> I'll try to minimize the noise in the API headers when I do the complete 
> thing.
> Alternatively, we could changes the `interfaces.swig` file to include 
> `./interface/SBAddressDocstrings.i`, then `lldb/API/SBAddress.h`, then 
> `./interface/SBAddressExtensions.i`. That way we don't need to have these 2 
> `#ifdef` blocks in the API headers themselves. 

I was going to make this suggestion. I think it's better to do that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142926

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

[Lldb-commits] [PATCH] D142862: [Support] change StringMap hash function from djbHash to xxHash

[Erik Desjardins via Phabricator via lldb-commits]

erikdesjardins added inline comments.



Comment at: mlir/test/Transforms/print-op-graph.mlir:9
 //   DFG:   subgraph {{.*}} {
-//   DFG: v[[ARG0:.*]] [label = "arg0"
-//   DFG: v[[CONST10:.*]] [label ={{.*}}10 : i32
+//   DFG: v[[ARG0:.*]] [shape = ellipse, label = "arg0"
+//   DFG: v[[CONST10:.*]] [shape = ellipse, label ={{.*}}10 : i32

Opened https://reviews.llvm.org/D143239 for this test, I'll rebase once it 
lands.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142862

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

[Lldb-commits] [PATCH] D142862: [Support] change StringMap hash function from djbHash to xxHash

[Fangrui Song via Phabricator via lldb-commits]

MaskRay accepted this revision.
MaskRay added inline comments.
This revision is now accepted and ready to land.



Comment at: mlir/test/Transforms/print-op-graph.mlir:1
 // RUN: mlir-opt -allow-unregistered-dialect 
-mlir-elide-elementsattrs-if-larger=2 -view-op-graph %s -o %t 2>&1 | FileCheck 
-check-prefix=DFG %s
 // RUN: mlir-opt -allow-unregistered-dialect 
-mlir-elide-elementsattrs-if-larger=2 
-view-op-graph='print-data-flow-edges=false print-control-flow-edges=true' %s 
-o %t 2>&1 | FileCheck -check-prefix=CFG %s

I fixed `mlir/lib/Transforms/ViewOpGraph.cpp`. Please rebase and drop changes 
to this file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142862

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

[Lldb-commits] [PATCH] D142862: [Support] change StringMap hash function from djbHash to xxHash

[Fangrui Song via Phabricator via lldb-commits]

MaskRay added a comment.

> `For posterity, the tests that fail on main are:`
>
> `Skipped  : 43`
> [...]

You can remove these from the description. They are flaky tests unrelated to 
this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142862

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