[Lldb-commits] [lldb] 9e0a39f - [lldb] Add a test for class loading via member typedefs

2021-11-01 Thread Raphael Isemann via lldb-commits

Author: Raphael Isemann
Date: 2021-11-01T15:58:45+01:00
New Revision: 9e0a39f3787ac055631891be9062dcd561cf4501

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

LOG: [lldb] Add a test for class loading via member typedefs

This is currently only tested indirectly in the huge stdlibc++ formatter tests.

Added: 
lldb/test/API/lang/cpp/class-loading-via-member-typedef/Makefile

lldb/test/API/lang/cpp/class-loading-via-member-typedef/TestClassLoadingViaMemberTypedef.py
lldb/test/API/lang/cpp/class-loading-via-member-typedef/main.cpp

Modified: 


Removed: 




diff  --git a/lldb/test/API/lang/cpp/class-loading-via-member-typedef/Makefile 
b/lldb/test/API/lang/cpp/class-loading-via-member-typedef/Makefile
new file mode 100644
index 0..8b20bcb05
--- /dev/null
+++ b/lldb/test/API/lang/cpp/class-loading-via-member-typedef/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules

diff  --git 
a/lldb/test/API/lang/cpp/class-loading-via-member-typedef/TestClassLoadingViaMemberTypedef.py
 
b/lldb/test/API/lang/cpp/class-loading-via-member-typedef/TestClassLoadingViaMemberTypedef.py
new file mode 100644
index 0..6d1a85fff7214
--- /dev/null
+++ 
b/lldb/test/API/lang/cpp/class-loading-via-member-typedef/TestClassLoadingViaMemberTypedef.py
@@ -0,0 +1,41 @@
+"""
+Tests loading of classes when the loading is triggered via a typedef inside the
+class (and not via the normal LLDB lookup that first resolves the surrounding
+class).
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+@no_debug_info_test
+def test(self):
+self.build()
+self.createTestTarget()
+
+# Print the top-level typedef which triggers the loading of the class
+# that the typedef is defined inside.
+self.expect_expr(
+"pull_in_classes",
+result_type="StructWithMember::MemberTypedef",
+result_value="0",
+)
+
+# Print the classes and check their types.
+self.expect_expr(
+"struct_to_print",
+result_type="StructWithMember",
+result_children=[
+ValueCheck(
+name="m",
+type="StructWithNested::Nested::OtherTypedef",
+children=[ValueCheck(name="i", value="0", type="int")],
+)
+],
+)

diff  --git a/lldb/test/API/lang/cpp/class-loading-via-member-typedef/main.cpp 
b/lldb/test/API/lang/cpp/class-loading-via-member-typedef/main.cpp
new file mode 100644
index 0..ba08d3bcbfd59
--- /dev/null
+++ b/lldb/test/API/lang/cpp/class-loading-via-member-typedef/main.cpp
@@ -0,0 +1,31 @@
+struct TopLevelStruct {
+  int i;
+};
+
+// Contains a templated nested class with a typedef.
+struct StructWithNested {
+  template 
+  struct Nested {
+// Typedef in a class. Intended to be referenced directly so that it can
+// trigger the loading of the surrounding classes.
+typedef TopLevelStruct OtherTypedef;
+  };
+};
+
+// Contains a typedef.
+struct StructWithMember {
+  // This member pulls in the typedef (and classes) above.
+  StructWithNested::Nested::OtherTypedef m;
+  // Typedef in a class. Intended to be referenced directly so that it can
+  // trigger the loading of the surrounding class.
+  typedef int MemberTypedef;
+};
+
+// This is printed and will pull in the typedef in StructWithmember.
+StructWithMember::MemberTypedef pull_in_classes;
+
+
+StructWithMember struct_to_print;
+
+
+int main() {}



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


[Lldb-commits] [PATCH] D112931: Fix mixed disassembly showing source lines for "line 0"

2021-11-01 Thread Ted Woodward via Phabricator via lldb-commits
ted created this revision.
ted requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

"line 0" in a DWARF linetable means something that doesn't have associated
source. The code for mixed disassembly has a comment indicating that
"line 0" should be skipped, but the wrong value was returned. Fix the return
value and add a test to check that we don't incorrectly show source lines
from the beginning of the file.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D112931

Files:
  lldb/source/Core/Disassembler.cpp
  lldb/test/Shell/Commands/command-disassemble-mixed.c


Index: lldb/test/Shell/Commands/command-disassemble-mixed.c
===
--- /dev/null
+++ lldb/test/Shell/Commands/command-disassemble-mixed.c
@@ -0,0 +1,18 @@
+// invalid mixed disassembly line
+
+// RUN: %clang -g %s -o %t
+// RUN: %lldb %t -o "dis -m -n main" | FileCheck %s
+
+// CHECK: int main
+// CHECK: int i
+// CHECK-NOT: invalid mixed disassembly line
+// CHECK: return 0;
+
+int main(int argc, char **argv)
+{
+  int i;
+
+  for (i=0; i < 10; ++i) ;
+
+  return 0;
+}
Index: lldb/source/Core/Disassembler.cpp
===
--- lldb/source/Core/Disassembler.cpp
+++ lldb/source/Core/Disassembler.cpp
@@ -243,7 +243,7 @@
 
   // Skip any line #0 entries - they are implementation details
   if (line.line == 0)
-return false;
+return true;
 
   ThreadSP thread_sp = exe_ctx.GetThreadSP();
   if (thread_sp) {


Index: lldb/test/Shell/Commands/command-disassemble-mixed.c
===
--- /dev/null
+++ lldb/test/Shell/Commands/command-disassemble-mixed.c
@@ -0,0 +1,18 @@
+// invalid mixed disassembly line
+
+// RUN: %clang -g %s -o %t
+// RUN: %lldb %t -o "dis -m -n main" | FileCheck %s
+
+// CHECK: int main
+// CHECK: int i
+// CHECK-NOT: invalid mixed disassembly line
+// CHECK: return 0;
+
+int main(int argc, char **argv)
+{
+  int i;
+
+  for (i=0; i < 10; ++i) ;
+
+  return 0;
+}
Index: lldb/source/Core/Disassembler.cpp
===
--- lldb/source/Core/Disassembler.cpp
+++ lldb/source/Core/Disassembler.cpp
@@ -243,7 +243,7 @@
 
   // Skip any line #0 entries - they are implementation details
   if (line.line == 0)
-return false;
+return true;
 
   ThreadSP thread_sp = exe_ctx.GetThreadSP();
   if (thread_sp) {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D112931: Fix mixed disassembly showing source lines for "line 0"

2021-11-01 Thread Ted Woodward via Phabricator via lldb-commits
ted updated this revision to Diff 383810.
ted added a comment.

Fix test to exit lldb


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112931

Files:
  lldb/source/Core/Disassembler.cpp
  lldb/test/Shell/Commands/command-disassemble-mixed.c


Index: lldb/test/Shell/Commands/command-disassemble-mixed.c
===
--- /dev/null
+++ lldb/test/Shell/Commands/command-disassemble-mixed.c
@@ -0,0 +1,18 @@
+// invalid mixed disassembly line
+
+// RUN: %clang -g %s -o %t
+// RUN: %lldb %t -o "dis -m -n main" -o "exit" | FileCheck %s
+
+// CHECK: int main
+// CHECK: int i
+// CHECK-NOT: invalid mixed disassembly line
+// CHECK: return 0;
+
+int main(int argc, char **argv)
+{
+  int i;
+
+  for (i=0; i < 10; ++i) ;
+
+  return 0;
+}
Index: lldb/source/Core/Disassembler.cpp
===
--- lldb/source/Core/Disassembler.cpp
+++ lldb/source/Core/Disassembler.cpp
@@ -243,7 +243,7 @@
 
   // Skip any line #0 entries - they are implementation details
   if (line.line == 0)
-return false;
+return true;
 
   ThreadSP thread_sp = exe_ctx.GetThreadSP();
   if (thread_sp) {


Index: lldb/test/Shell/Commands/command-disassemble-mixed.c
===
--- /dev/null
+++ lldb/test/Shell/Commands/command-disassemble-mixed.c
@@ -0,0 +1,18 @@
+// invalid mixed disassembly line
+
+// RUN: %clang -g %s -o %t
+// RUN: %lldb %t -o "dis -m -n main" -o "exit" | FileCheck %s
+
+// CHECK: int main
+// CHECK: int i
+// CHECK-NOT: invalid mixed disassembly line
+// CHECK: return 0;
+
+int main(int argc, char **argv)
+{
+  int i;
+
+  for (i=0; i < 10; ++i) ;
+
+  return 0;
+}
Index: lldb/source/Core/Disassembler.cpp
===
--- lldb/source/Core/Disassembler.cpp
+++ lldb/source/Core/Disassembler.cpp
@@ -243,7 +243,7 @@
 
   // Skip any line #0 entries - they are implementation details
   if (line.line == 0)
-return false;
+return true;
 
   ThreadSP thread_sp = exe_ctx.GetThreadSP();
   if (thread_sp) {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] 1c05c52 - [lldb-vscode] Fix coredump load source mapping for first file

2021-11-01 Thread Ted Woodward via lldb-commits

Author: Ted Woodward
Date: 2021-11-01T10:47:42-05:00
New Revision: 1c05c52de2177a328b7d2d07b697af67eb9f8122

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

LOG: [lldb-vscode] Fix coredump load source mapping for first file

SetSourceMapFromArguments is called after the core is loaded. This means
that the source file for the crashing code won't have the source map applied.
Move the call to SetSourceMapFromArguments in request_attach to just after
the call to RunInitCommands, matching request_launch behavior.

Reviewed By: clayborg, wallace

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

Added: 
lldb/test/API/tools/lldb-vscode/coreFile/main.c

Modified: 
lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
lldb/test/API/tools/lldb-vscode/coreFile/TestVSCode_coreFile.py
lldb/tools/lldb-vscode/lldb-vscode.cpp

Removed: 




diff  --git 
a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
index 0a55fc0ead1e4..255a4805a9737 100644
--- 
a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
+++ 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
@@ -251,7 +251,7 @@ def attach(self, program=None, pid=None, waitFor=None, 
trace=None,
initCommands=None, preRunCommands=None, stopCommands=None,
exitCommands=None, attachCommands=None, coreFile=None,
disconnectAutomatically=True, terminateCommands=None,
-   postRunCommands=None):
+   postRunCommands=None, sourceMap=None):
 '''Build the default Makefile target, create the VSCode debug adaptor,
and attach to the process.
 '''
@@ -271,7 +271,8 @@ def cleanup():
 initCommands=initCommands, preRunCommands=preRunCommands,
 stopCommands=stopCommands, exitCommands=exitCommands,
 attachCommands=attachCommands, terminateCommands=terminateCommands,
-coreFile=coreFile, postRunCommands=postRunCommands)
+coreFile=coreFile, postRunCommands=postRunCommands,
+sourceMap=sourceMap)
 if not (response and response['success']):
 self.assertTrue(response['success'],
 'attach failed (%s)' % (response['message']))

diff  --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
index df057d5e63aa6..603b1545cd714 100644
--- a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
+++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
@@ -506,7 +506,8 @@ def request_attach(self, program=None, pid=None, 
waitFor=None, trace=None,
initCommands=None, preRunCommands=None,
stopCommands=None, exitCommands=None,
attachCommands=None, terminateCommands=None,
-   coreFile=None, postRunCommands=None):
+   coreFile=None, postRunCommands=None,
+   sourceMap=None):
 args_dict = {}
 if pid is not None:
 args_dict['pid'] = pid
@@ -533,6 +534,8 @@ def request_attach(self, program=None, pid=None, 
waitFor=None, trace=None,
 args_dict['coreFile'] = coreFile
 if postRunCommands:
 args_dict['postRunCommands'] = postRunCommands
+if sourceMap:
+args_dict['sourceMap'] = sourceMap
 command_dict = {
 'command': 'attach',
 'type': 'request',

diff  --git a/lldb/test/API/tools/lldb-vscode/coreFile/TestVSCode_coreFile.py 
b/lldb/test/API/tools/lldb-vscode/coreFile/TestVSCode_coreFile.py
index 55efd91d827a6..56a93ccd6c8ab 100644
--- a/lldb/test/API/tools/lldb-vscode/coreFile/TestVSCode_coreFile.py
+++ b/lldb/test/API/tools/lldb-vscode/coreFile/TestVSCode_coreFile.py
@@ -41,3 +41,18 @@ def test_core_file(self):
 
 self.vscode.request_next(threadId=32259)
 self.assertEquals(self.get_stackFrames(), expected_frames)
+
+@skipIfWindows
+@skipIfRemote
+def test_core_file_source_mapping(self):
+''' Test that sourceMap property is correctly applied when loading a 
core '''
+current_dir = os.path.dirname(os.path.realpath(__file__))
+exe_file = os.path.join(current_dir, "linux-x86_64.out")
+core_file = os.path.join(current_dir, "linux-x86_64.core")
+
+self.create_debug_adaptor()
+
+source_map = [["/home/labath/test", current_dir]]
+self.attach(exe_file, coreFile=core_file, sourceMap=source_map)
+
+

[Lldb-commits] [PATCH] D112834: [lldb-vscode] Fix coredump load source mapping for first file

2021-11-01 Thread Ted Woodward via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG1c05c52de217: [lldb-vscode] Fix coredump load source mapping 
for first file (authored by ted).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112834

Files:
  lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
  lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
  lldb/test/API/tools/lldb-vscode/coreFile/TestVSCode_coreFile.py
  lldb/test/API/tools/lldb-vscode/coreFile/main.c
  lldb/tools/lldb-vscode/lldb-vscode.cpp


Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -616,6 +616,8 @@
   // Run any initialize LLDB commands the user specified in the launch.json
   g_vsc.RunInitCommands();
 
+  SetSourceMapFromArguments(*arguments);
+
   lldb::SBError status;
   g_vsc.SetTarget(g_vsc.CreateTargetFromArguments(*arguments, status));
   if (status.Fail()) {
@@ -657,8 +659,6 @@
 g_vsc.target = g_vsc.debugger.GetSelectedTarget();
   }
 
-  SetSourceMapFromArguments(*arguments);
-
   if (error.Success() && core_file.empty()) {
 auto attached_pid = g_vsc.target.GetProcess().GetProcessID();
 if (attached_pid == LLDB_INVALID_PROCESS_ID) {
Index: lldb/test/API/tools/lldb-vscode/coreFile/main.c
===
--- /dev/null
+++ lldb/test/API/tools/lldb-vscode/coreFile/main.c
@@ -0,0 +1 @@
+/* Fake source file for core dump source mapping test */
Index: lldb/test/API/tools/lldb-vscode/coreFile/TestVSCode_coreFile.py
===
--- lldb/test/API/tools/lldb-vscode/coreFile/TestVSCode_coreFile.py
+++ lldb/test/API/tools/lldb-vscode/coreFile/TestVSCode_coreFile.py
@@ -41,3 +41,18 @@
 
 self.vscode.request_next(threadId=32259)
 self.assertEquals(self.get_stackFrames(), expected_frames)
+
+@skipIfWindows
+@skipIfRemote
+def test_core_file_source_mapping(self):
+''' Test that sourceMap property is correctly applied when loading a 
core '''
+current_dir = os.path.dirname(os.path.realpath(__file__))
+exe_file = os.path.join(current_dir, "linux-x86_64.out")
+core_file = os.path.join(current_dir, "linux-x86_64.core")
+
+self.create_debug_adaptor()
+
+source_map = [["/home/labath/test", current_dir]]
+self.attach(exe_file, coreFile=core_file, sourceMap=source_map)
+
+self.assertTrue(current_dir in 
self.get_stackFrames()[0]['source']['path'])
Index: lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
===
--- lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
+++ lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
@@ -506,7 +506,8 @@
initCommands=None, preRunCommands=None,
stopCommands=None, exitCommands=None,
attachCommands=None, terminateCommands=None,
-   coreFile=None, postRunCommands=None):
+   coreFile=None, postRunCommands=None,
+   sourceMap=None):
 args_dict = {}
 if pid is not None:
 args_dict['pid'] = pid
@@ -533,6 +534,8 @@
 args_dict['coreFile'] = coreFile
 if postRunCommands:
 args_dict['postRunCommands'] = postRunCommands
+if sourceMap:
+args_dict['sourceMap'] = sourceMap
 command_dict = {
 'command': 'attach',
 'type': 'request',
Index: 
lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
===
--- lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
+++ lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
@@ -251,7 +251,7 @@
initCommands=None, preRunCommands=None, stopCommands=None,
exitCommands=None, attachCommands=None, coreFile=None,
disconnectAutomatically=True, terminateCommands=None,
-   postRunCommands=None):
+   postRunCommands=None, sourceMap=None):
 '''Build the default Makefile target, create the VSCode debug adaptor,
and attach to the process.
 '''
@@ -271,7 +271,8 @@
 initCommands=initCommands, preRunCommands=preRunCommands,
 stopCommands=stopCommands, exitCommands=exitCommands,
 attachCommands=attachCommands, terminateCommands=terminateCommands,
-coreFile=coreFile, postRunCommands=postRunCommands)
+coreFile=coreFile, postRunCommands=postRunCommands,
+sourceMap=sourceMap)
 if not (re

[Lldb-commits] [lldb] 64cc073 - [lldb] Only specify LLVM_ENABLE_RUNTIMES in the libcxx error message.

2021-11-01 Thread Jonas Devlieghere via lldb-commits

Author: Jonas Devlieghere
Date: 2021-11-01T09:39:24-07:00
New Revision: 64cc073abd59d7ac2359d258f407254de6c79c84

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

LOG: [lldb] Only specify LLVM_ENABLE_RUNTIMES in the libcxx error message.

Now that passing libcxx via LLVM_ENABLE_PROJECTS has been deprecated,
update the error message and recommend using LLVM_ENABLE_RUNTIMES
instead. This patch also remove the error message for the old layout.

Differential revision: https://reviews.llvm.org/D112856

Added: 


Modified: 
lldb/test/CMakeLists.txt

Removed: 




diff  --git a/lldb/test/CMakeLists.txt b/lldb/test/CMakeLists.txt
index dcb59cfb6c1c..d03f17335a24 100644
--- a/lldb/test/CMakeLists.txt
+++ b/lldb/test/CMakeLists.txt
@@ -117,23 +117,12 @@ if(TARGET clang)
   endif()
 else()
   # We require libcxx for the test suite, so if we aren't building it,
-  # try to provide a helpful error about how to resolve the situation.
+  # provide a helpful error about how to resolve the situation.
   if(NOT TARGET cxx AND NOT libcxx IN_LIST LLVM_ENABLE_RUNTIMES)
-if(LLVM_ENABLE_PROJECTS STREQUAL "")
-  # If `LLVM_ENABLE_PROJECTS` is not being used (implying that we are
-  # using the old layout), suggest checking it out.
-  message(FATAL_ERROR
-"LLDB test suite requires libc++, but it is currently disabled. "
-"Please checkout `libcxx` in `llvm/projects` or disable tests "
-"via `LLDB_INCLUDE_TESTS=OFF`.")
-else()
-  # If `LLVM_ENABLE_PROJECTS` is being used, suggest adding it.
-  message(FATAL_ERROR
-"LLDB test suite requires libc++, but it is currently disabled. "
-"Please add `libcxx` to `LLVM_ENABLE_PROJECTS` or "
-"`LLVM_ENABLE_RUNTIMES`, or disable tests via "
-"`LLDB_INCLUDE_TESTS=OFF`.")
-endif()
+message(FATAL_ERROR
+  "LLDB test suite requires libc++, but it is currently disabled. "
+  "Please add `libcxx` to `LLVM_ENABLE_RUNTIMES` or disable tests via "
+  "`LLDB_INCLUDE_TESTS=OFF`.")
   endif()
 endif()
   endif()



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


[Lldb-commits] [PATCH] D112856: [lldb] Only specify LLVM_ENABLE_RUNTIMES in the libcxx error message.

2021-11-01 Thread Jonas Devlieghere via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG64cc073abd59: [lldb] Only specify LLVM_ENABLE_RUNTIMES in 
the libcxx error message. (authored by JDevlieghere).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112856

Files:
  lldb/test/CMakeLists.txt


Index: lldb/test/CMakeLists.txt
===
--- lldb/test/CMakeLists.txt
+++ lldb/test/CMakeLists.txt
@@ -117,23 +117,12 @@
   endif()
 else()
   # We require libcxx for the test suite, so if we aren't building it,
-  # try to provide a helpful error about how to resolve the situation.
+  # provide a helpful error about how to resolve the situation.
   if(NOT TARGET cxx AND NOT libcxx IN_LIST LLVM_ENABLE_RUNTIMES)
-if(LLVM_ENABLE_PROJECTS STREQUAL "")
-  # If `LLVM_ENABLE_PROJECTS` is not being used (implying that we are
-  # using the old layout), suggest checking it out.
-  message(FATAL_ERROR
-"LLDB test suite requires libc++, but it is currently disabled. "
-"Please checkout `libcxx` in `llvm/projects` or disable tests "
-"via `LLDB_INCLUDE_TESTS=OFF`.")
-else()
-  # If `LLVM_ENABLE_PROJECTS` is being used, suggest adding it.
-  message(FATAL_ERROR
-"LLDB test suite requires libc++, but it is currently disabled. "
-"Please add `libcxx` to `LLVM_ENABLE_PROJECTS` or "
-"`LLVM_ENABLE_RUNTIMES`, or disable tests via "
-"`LLDB_INCLUDE_TESTS=OFF`.")
-endif()
+message(FATAL_ERROR
+  "LLDB test suite requires libc++, but it is currently disabled. "
+  "Please add `libcxx` to `LLVM_ENABLE_RUNTIMES` or disable tests via "
+  "`LLDB_INCLUDE_TESTS=OFF`.")
   endif()
 endif()
   endif()


Index: lldb/test/CMakeLists.txt
===
--- lldb/test/CMakeLists.txt
+++ lldb/test/CMakeLists.txt
@@ -117,23 +117,12 @@
   endif()
 else()
   # We require libcxx for the test suite, so if we aren't building it,
-  # try to provide a helpful error about how to resolve the situation.
+  # provide a helpful error about how to resolve the situation.
   if(NOT TARGET cxx AND NOT libcxx IN_LIST LLVM_ENABLE_RUNTIMES)
-if(LLVM_ENABLE_PROJECTS STREQUAL "")
-  # If `LLVM_ENABLE_PROJECTS` is not being used (implying that we are
-  # using the old layout), suggest checking it out.
-  message(FATAL_ERROR
-"LLDB test suite requires libc++, but it is currently disabled. "
-"Please checkout `libcxx` in `llvm/projects` or disable tests "
-"via `LLDB_INCLUDE_TESTS=OFF`.")
-else()
-  # If `LLVM_ENABLE_PROJECTS` is being used, suggest adding it.
-  message(FATAL_ERROR
-"LLDB test suite requires libc++, but it is currently disabled. "
-"Please add `libcxx` to `LLVM_ENABLE_PROJECTS` or "
-"`LLVM_ENABLE_RUNTIMES`, or disable tests via "
-"`LLDB_INCLUDE_TESTS=OFF`.")
-endif()
+message(FATAL_ERROR
+  "LLDB test suite requires libc++, but it is currently disabled. "
+  "Please add `libcxx` to `LLVM_ENABLE_RUNTIMES` or disable tests via "
+  "`LLDB_INCLUDE_TESTS=OFF`.")
   endif()
 endif()
   endif()
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D112165: Cleanup a few more PR36048 skips

2021-11-01 Thread Raphael Isemann via Phabricator via lldb-commits
teemperor added a comment.

Small note: gmodules test are never run on Linux, so you actually have to run 
them on macOS (or I think FreeBSD) to know whether the tests work.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112165

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


[Lldb-commits] [lldb] 68bb4e1 - [lldb][NFC] Inclusive Language: Replace master with main

2021-11-01 Thread Quinn Pham via lldb-commits

Author: Quinn Pham
Date: 2021-11-01T12:25:41-05:00
New Revision: 68bb4e16482b765148993cbb912f5be1b01d5b57

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

LOG: [lldb][NFC] Inclusive Language: Replace master with main

[NFC] This patch fixes a URL within a git repo whose master branch was renamed
to main.

Added: 


Modified: 
lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp

Removed: 




diff  --git a/lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp 
b/lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp
index 2b5a038fbc91c..ccfbeec3d5891 100644
--- a/lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp
+++ b/lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp
@@ -402,7 +402,7 @@ bool ABIMacOSX_arm64::CreateDefaultUnwindPlan(UnwindPlan 
&unwind_plan) {
 // volatile (and specifically only the lower 8 bytes of these regs), the rest
 // of the fp/SIMD registers are volatile.
 //
-// v. https://github.com/ARM-software/abi-aa/blob/master/aapcs64/
+// v. https://github.com/ARM-software/abi-aa/blob/main/aapcs64/
 
 // We treat x29 as callee preserved also, else the unwinder won't try to
 // retrieve fp saves.



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


[Lldb-commits] [lldb] ac7c880 - [lldb][gmodules] Fix TestDataFormatterCpp with gmodules on macOS

2021-11-01 Thread Raphael Isemann via lldb-commits

Author: Raphael Isemann
Date: 2021-11-01T18:44:32+01:00
New Revision: ac7c8808ba89fb6188d5b1bb83bdd08e5c39d71e

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

LOG: [lldb][gmodules] Fix TestDataFormatterCpp with gmodules on macOS

'Point' is actually a system type defined on macOS.

Added: 


Modified: 

lldb/test/API/functionalities/data-formatter/data-formatter-cpp/TestDataFormatterCpp.py
lldb/test/API/functionalities/data-formatter/data-formatter-cpp/main.cpp

Removed: 




diff  --git 
a/lldb/test/API/functionalities/data-formatter/data-formatter-cpp/TestDataFormatterCpp.py
 
b/lldb/test/API/functionalities/data-formatter/data-formatter-cpp/TestDataFormatterCpp.py
index bc681825d4a3b..e6b1d43599989 100644
--- 
a/lldb/test/API/functionalities/data-formatter/data-formatter-cpp/TestDataFormatterCpp.py
+++ 
b/lldb/test/API/functionalities/data-formatter/data-formatter-cpp/TestDataFormatterCpp.py
@@ -120,32 +120,32 @@ def cleanup():
 ' = ptr = ',
 ' 
"1234567890123456789012345678901234567890123456789012345678901234ABC"'])
 
-self.runCmd("type summary add -c Point")
+self.runCmd("type summary add -c TestPoint")
 
 self.expect("frame variable iAmSomewhere",
 substrs=['x = 4',
  'y = 6'])
 
 self.expect("type summary list",
-substrs=['Point',
+substrs=['TestPoint',
  'one-line'])
 
-self.runCmd("type summary add --summary-string \"y=${var.y%x}\" Point")
+self.runCmd("type summary add --summary-string \"y=${var.y%x}\" 
TestPoint")
 
 self.expect("frame variable iAmSomewhere",
 substrs=['y=0x'])
 
 self.runCmd(
-"type summary add --summary-string \"y=${var.y},x=${var.x}\" 
Point")
+"type summary add --summary-string \"y=${var.y},x=${var.x}\" 
TestPoint")
 
 self.expect("frame variable iAmSomewhere",
 substrs=['y=6',
  'x=4'])
 
-self.runCmd("type summary add --summary-string \"hello\" Point -e")
+self.runCmd("type summary add --summary-string \"hello\" TestPoint -e")
 
 self.expect("type summary list",
-substrs=['Point',
+substrs=['TestPoint',
  'show children'])
 
 self.expect("frame variable iAmSomewhere",

diff  --git 
a/lldb/test/API/functionalities/data-formatter/data-formatter-cpp/main.cpp 
b/lldb/test/API/functionalities/data-formatter/data-formatter-cpp/main.cpp
index b22a3d8c732ad..c81a68fd2094a 100644
--- a/lldb/test/API/functionalities/data-formatter/data-formatter-cpp/main.cpp
+++ b/lldb/test/API/functionalities/data-formatter/data-formatter-cpp/main.cpp
@@ -22,10 +22,10 @@ typedef Type3 Type4; // should show as char
 typedef int ChildType; // should show as int
 typedef int AnotherChildType; // should show as int
 
-struct Point {
+struct TestPoint {
 int x;
 int y;
-Point(int X = 3, int Y = 2) : x(X), y(Y) {}
+TestPoint(int X = 3, int Y = 2) : x(X), y(Y) {}
 };
 
 typedef float ShowMyGuts;
@@ -85,7 +85,7 @@ int main (int argc, const char * argv[])
 
 Speed* SPPtrILookHex = new Speed(16);
 
-Point iAmSomewhere(4,6);
+TestPoint iAmSomewhere(4,6);
 
i_am_cool *cool_pointer = (i_am_cool*)malloc(sizeof(i_am_cool)*3);
cool_pointer[0] = i_am_cool(3,-3.141592,'E');



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


[Lldb-commits] [PATCH] D112945: [lldb] Improve error reporting in `lang objc tagged-pointer info`

2021-11-01 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere created this revision.
JDevlieghere added a reviewer: aprantl.
JDevlieghere requested review of this revision.

Improve error handling for the `lang objc tagged-pointer info`. Rather than 
failing silently, report an error if we couldn't convert an argument to an 
address or resolve the class descriptor.

  (lldb) lang objc tagged-pointer info 0xbb6404c47a587764
  could not get class descriptor for 0xbb6404c47a587764
  (lldb) lang objc tagged-pointer info n1
  could not convert 'n1' to a valid address


https://reviews.llvm.org/D112945

Files:
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp

Index: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
+++ lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
@@ -951,50 +951,70 @@
 
 Process *process = m_exe_ctx.GetProcessPtr();
 ExecutionContext exe_ctx(process);
+
 ObjCLanguageRuntime *objc_runtime = ObjCLanguageRuntime::Get(*process);
-if (objc_runtime) {
-  ObjCLanguageRuntime::TaggedPointerVendor *tagged_ptr_vendor =
-  objc_runtime->GetTaggedPointerVendor();
-  if (tagged_ptr_vendor) {
-for (size_t i = 0; i < command.GetArgumentCount(); i++) {
-  const char *arg_str = command.GetArgumentAtIndex(i);
-  if (!arg_str)
-continue;
-  Status error;
-  lldb::addr_t arg_addr = OptionArgParser::ToAddress(
-  &exe_ctx, arg_str, LLDB_INVALID_ADDRESS, &error);
-  if (arg_addr == 0 || arg_addr == LLDB_INVALID_ADDRESS || error.Fail())
-continue;
-  auto descriptor_sp = tagged_ptr_vendor->GetClassDescriptor(arg_addr);
-  if (!descriptor_sp)
-continue;
-  uint64_t info_bits = 0;
-  uint64_t value_bits = 0;
-  uint64_t payload = 0;
-  if (descriptor_sp->GetTaggedPointerInfo(&info_bits, &value_bits,
-  &payload)) {
-result.GetOutputStream().Printf(
-"0x%" PRIx64 " is tagged.\n\tpayload = 0x%" PRIx64
-"\n\tvalue = 0x%" PRIx64 "\n\tinfo bits = 0x%" PRIx64
-"\n\tclass = %s\n",
-(uint64_t)arg_addr, payload, value_bits, info_bits,
-descriptor_sp->GetClassName().AsCString(""));
-  } else {
-result.GetOutputStream().Printf("0x%" PRIx64 " is not tagged.\n",
-(uint64_t)arg_addr);
-  }
-}
+if (!objc_runtime) {
+  result.AppendError("current process has no Objective-C runtime loaded");
+  result.SetStatus(lldb::eReturnStatusFailed);
+  return false;
+}
+
+ObjCLanguageRuntime::TaggedPointerVendor *tagged_ptr_vendor =
+objc_runtime->GetTaggedPointerVendor();
+if (!tagged_ptr_vendor) {
+  result.AppendError("current process has no tagged pointer support");
+  result.SetStatus(lldb::eReturnStatusFailed);
+  return false;
+}
+
+bool success = false;
+for (size_t i = 0; i < command.GetArgumentCount(); i++) {
+  const char *arg_str = command.GetArgumentAtIndex(i);
+  if (!arg_str)
+continue;
+
+  Status error;
+  lldb::addr_t arg_addr = OptionArgParser::ToAddress(
+  &exe_ctx, arg_str, LLDB_INVALID_ADDRESS, &error);
+  if (arg_addr == 0 || arg_addr == LLDB_INVALID_ADDRESS || error.Fail()) {
+result.GetErrorStream().Printf(
+"could not convert '%s' to a valid address\n", arg_str);
+continue;
+  }
+
+  auto descriptor_sp = tagged_ptr_vendor->GetClassDescriptor(arg_addr);
+  if (!descriptor_sp) {
+result.GetErrorStream().Printf(
+"could not get class descriptor for 0x%" PRIx64 "\n",
+(uint64_t)arg_addr);
+continue;
+  }
+
+  uint64_t info_bits = 0;
+  uint64_t value_bits = 0;
+  uint64_t payload = 0;
+  if (descriptor_sp->GetTaggedPointerInfo(&info_bits, &value_bits,
+  &payload)) {
+result.GetOutputStream().Printf(
+"0x%" PRIx64 " is tagged.\n\tpayload = 0x%" PRIx64
+"\n\tvalue = 0x%" PRIx64 "\n\tinfo bits = 0x%" PRIx64
+"\n\tclass = %s\n",
+(uint64_t)arg_addr, payload, value_bits, info_bits,
+descriptor_sp->GetClassName().AsCString(""));
   } else {
-result.AppendError("current process has no tagged pointer support");
-result.SetStatus(lldb::eReturnStatusFailed);
-return false;
+result.GetOutputStream().Printf("0x%" PRIx64 " is not tagged.\n",
+(uint64_t)arg_addr);
   }
-  result.SetStatus(lldb::eReturnStatusSuccessFinishResult);
-  return true;
+  success

[Lldb-commits] [PATCH] D112945: [lldb] Improve error reporting in `lang objc tagged-pointer info`

2021-11-01 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere updated this revision to Diff 383851.
JDevlieghere added a comment.

Add test


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

https://reviews.llvm.org/D112945

Files:
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
  lldb/test/Shell/ObjC/Inputs/test.m
  lldb/test/Shell/ObjC/tagged_pointer_info.test

Index: lldb/test/Shell/ObjC/tagged_pointer_info.test
===
--- /dev/null
+++ lldb/test/Shell/ObjC/tagged_pointer_info.test
@@ -0,0 +1,9 @@
+REQUIRES: system-darwin
+
+RUN: %clang_host -g -framework Foundation -o %t.out %S/Inputs/test.m
+
+RUN: %lldb %t.out -o "b main" -o "r" -o "n" -o "lang objc tagged-pointer info bogus" 2>&1 | FileCheck %s -check-prefix ADDRESS
+ADDRESS: could not convert 'bogus' to a valid address
+
+RUN: %lldb %t.out -o "b main" -o "r" -o "n" -o "lang objc tagged-pointer info 0x1" 2>&1 | FileCheck %s --check-prefix DESCRIPTOR
+DESCRIPTOR: could not get class descriptor for 0x1
Index: lldb/test/Shell/ObjC/Inputs/test.m
===
--- /dev/null
+++ lldb/test/Shell/ObjC/Inputs/test.m
@@ -0,0 +1,6 @@
+#import 
+int main() {
+  id n1 = [NSNumber numberWithInt:1];
+  printf("%x", n1);
+  return 0;
+}
Index: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
+++ lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
@@ -951,50 +951,70 @@
 
 Process *process = m_exe_ctx.GetProcessPtr();
 ExecutionContext exe_ctx(process);
+
 ObjCLanguageRuntime *objc_runtime = ObjCLanguageRuntime::Get(*process);
-if (objc_runtime) {
-  ObjCLanguageRuntime::TaggedPointerVendor *tagged_ptr_vendor =
-  objc_runtime->GetTaggedPointerVendor();
-  if (tagged_ptr_vendor) {
-for (size_t i = 0; i < command.GetArgumentCount(); i++) {
-  const char *arg_str = command.GetArgumentAtIndex(i);
-  if (!arg_str)
-continue;
-  Status error;
-  lldb::addr_t arg_addr = OptionArgParser::ToAddress(
-  &exe_ctx, arg_str, LLDB_INVALID_ADDRESS, &error);
-  if (arg_addr == 0 || arg_addr == LLDB_INVALID_ADDRESS || error.Fail())
-continue;
-  auto descriptor_sp = tagged_ptr_vendor->GetClassDescriptor(arg_addr);
-  if (!descriptor_sp)
-continue;
-  uint64_t info_bits = 0;
-  uint64_t value_bits = 0;
-  uint64_t payload = 0;
-  if (descriptor_sp->GetTaggedPointerInfo(&info_bits, &value_bits,
-  &payload)) {
-result.GetOutputStream().Printf(
-"0x%" PRIx64 " is tagged.\n\tpayload = 0x%" PRIx64
-"\n\tvalue = 0x%" PRIx64 "\n\tinfo bits = 0x%" PRIx64
-"\n\tclass = %s\n",
-(uint64_t)arg_addr, payload, value_bits, info_bits,
-descriptor_sp->GetClassName().AsCString(""));
-  } else {
-result.GetOutputStream().Printf("0x%" PRIx64 " is not tagged.\n",
-(uint64_t)arg_addr);
-  }
-}
+if (!objc_runtime) {
+  result.AppendError("current process has no Objective-C runtime loaded");
+  result.SetStatus(lldb::eReturnStatusFailed);
+  return false;
+}
+
+ObjCLanguageRuntime::TaggedPointerVendor *tagged_ptr_vendor =
+objc_runtime->GetTaggedPointerVendor();
+if (!tagged_ptr_vendor) {
+  result.AppendError("current process has no tagged pointer support");
+  result.SetStatus(lldb::eReturnStatusFailed);
+  return false;
+}
+
+bool success = false;
+for (size_t i = 0; i < command.GetArgumentCount(); i++) {
+  const char *arg_str = command.GetArgumentAtIndex(i);
+  if (!arg_str)
+continue;
+
+  Status error;
+  lldb::addr_t arg_addr = OptionArgParser::ToAddress(
+  &exe_ctx, arg_str, LLDB_INVALID_ADDRESS, &error);
+  if (arg_addr == 0 || arg_addr == LLDB_INVALID_ADDRESS || error.Fail()) {
+result.GetErrorStream().Printf(
+"could not convert '%s' to a valid address\n", arg_str);
+continue;
+  }
+
+  auto descriptor_sp = tagged_ptr_vendor->GetClassDescriptor(arg_addr);
+  if (!descriptor_sp) {
+result.GetErrorStream().Printf(
+"could not get class descriptor for 0x%" PRIx64 "\n",
+(uint64_t)arg_addr);
+continue;
+  }
+
+  uint64_t info_bits = 0;
+  uint64_t value_bits = 0;
+  uint64_t payload = 0;
+  if (descriptor_sp->GetTaggedPointerInfo(&info_bits, &value_bits,
+  &payload)) {
+result.GetOutputStream().Printf(
+"0x%" PRIx64 " is tagg

[Lldb-commits] [PATCH] D112165: Cleanup a few more PR36048 skips

2021-11-01 Thread David Blaikie via Phabricator via lldb-commits
dblaikie added a comment.

In D112165#3100608 , @teemperor wrote:

> Small note: gmodules test are never run on Linux, so you actually have to run 
> them on macOS (or I think FreeBSD) to know whether the tests work.

Yeah, I'll admit I didn't test this, but seemed consistent with the other 
changes/cleanup - did this cause any breakage you know of?

Could gmodules be tested on Linux? (I had originally really hoped/tried to 
encourage gmodules to be implemented in a way that'd be compatible with Split 
DWARF but it never quite got there, unfortunately... would've made more overlap 
in functionality/testability/portability, I think)

(I should setup a build environment on my Macbook Pro, but haven't got around 
to it as yet)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112165

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


[Lldb-commits] [PATCH] D112931: Fix mixed disassembly showing source lines for "line 0"

2021-11-01 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment.

LGTM. This is in Jason's function so I will let him give the final ok.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112931

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


[Lldb-commits] [PATCH] D112931: Fix mixed disassembly showing source lines for "line 0"

2021-11-01 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added inline comments.



Comment at: lldb/test/Shell/Commands/command-disassemble-mixed.c:11-18
+int main(int argc, char **argv)
+{
+  int i;
+
+  for (i=0; i < 10; ++i) ;
+
+  return 0;

are we guaranteed to get some debug info with line zero in it with this example?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112931

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


[Lldb-commits] [PATCH] D112945: [lldb] Improve error reporting in `lang objc tagged-pointer info`

2021-11-01 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added a comment.

This is great, thanks! (one question inline)




Comment at: 
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp:1008
   }
-  result.SetStatus(lldb::eReturnStatusSuccessFinishResult);
-  return true;
+  success = true;
 }

I don't think `success` really works. Shouldn't the function only return `true` 
if none of the `arg_str`s failed?


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

https://reviews.llvm.org/D112945

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


[Lldb-commits] [PATCH] D112945: [lldb] Improve error reporting in `lang objc tagged-pointer info`

2021-11-01 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added inline comments.



Comment at: lldb/test/Shell/ObjC/tagged_pointer_info.test:3
+
+RUN: %clang_host -g -framework Foundation -o %t.out %S/Inputs/test.m
+

personal opinion without through reasoning behind it: This feels more like an 
API test to me.


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

https://reviews.llvm.org/D112945

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


[Lldb-commits] [PATCH] D112931: Fix mixed disassembly showing source lines for "line 0"

2021-11-01 Thread Ted Woodward via Phabricator via lldb-commits
ted added inline comments.



Comment at: lldb/test/Shell/Commands/command-disassemble-mixed.c:11-18
+int main(int argc, char **argv)
+{
+  int i;
+
+  for (i=0; i < 10; ++i) ;
+
+  return 0;

clayborg wrote:
> are we guaranteed to get some debug info with line zero in it with this 
> example?
I tried this with x86 Linux clang 12.0 and our internal Hexagon clang; both 
gave me a line zero.

I ran this test on Ubuntu 18 using x86 Linux clang 12.0 and the test failed 
without the change and passed with it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112931

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


[Lldb-commits] [PATCH] D112863: [lldb][NFC] avoid unnecessary roundtrips between different string types

2021-11-01 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl accepted this revision.
aprantl added a comment.

Thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112863

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


[Lldb-commits] [PATCH] D112439: normalize file path when searching the source map

2021-11-01 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl accepted this revision.
aprantl added a comment.

In D112439#3098307 , @xujuntwt95329 
wrote:

> Hi, @aprantl
>
> I've submitted a NFC patch here:
> https://reviews.llvm.org/D112863
>
> Seems that patch can't build by CI because it is based on this patch. In my 
> understanding we need to merge this patch firstly and rebase that NFC patch 
> to let CI work, right?

Yes, that sounds right!

> Please let me know if I misunderstand the workflow.
>
> Thanks a lot!




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112439

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


[Lldb-commits] [PATCH] D112972: [lldb] use EXT_SUFFIX for python extension

2021-11-01 Thread Lawrence D'Anna via Phabricator via lldb-commits
lawrence_danna created this revision.
lawrence_danna added reviewers: jasonmolenda, JDevlieghere, jingham.
Herald added a subscriber: mgorny.
lawrence_danna requested review of this revision.
Herald added a project: LLDB.

LLDB doesn't use only the python stable ABI, which means loading
it into an incompatible python can cause the process to crash.
_lldb.so should be named with the full EXT_SUFFIX from sysconfig

- such as _lldb.cpython-39-darwin.so -- so this doesn't happen.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D112972

Files:
  lldb/CMakeLists.txt
  lldb/bindings/python/CMakeLists.txt


Index: lldb/bindings/python/CMakeLists.txt
===
--- lldb/bindings/python/CMakeLists.txt
+++ lldb/bindings/python/CMakeLists.txt
@@ -138,13 +138,14 @@
 set(LIBLLDB_SYMLINK_DEST 
"${LLVM_SHLIB_OUTPUT_INTDIR}/liblldb${CMAKE_SHARED_LIBRARY_SUFFIX}")
   endif()
   if(WIN32)
+# FIXME should we use EXT_SUFFIX on windows?
 if(CMAKE_BUILD_TYPE STREQUAL Debug)
   set(LIBLLDB_SYMLINK_OUTPUT_FILE "_lldb_d.pyd")
 else()
   set(LIBLLDB_SYMLINK_OUTPUT_FILE "_lldb.pyd")
 endif()
   else()
-set(LIBLLDB_SYMLINK_OUTPUT_FILE "_lldb.so")
+set(LIBLLDB_SYMLINK_OUTPUT_FILE "_lldb${LLDB_PYTHON_EXT_SUFFIX}")
   endif()
   create_relative_symlink(${swig_target} ${LIBLLDB_SYMLINK_DEST}
   ${lldb_python_target_dir} 
${LIBLLDB_SYMLINK_OUTPUT_FILE})
Index: lldb/CMakeLists.txt
===
--- lldb/CMakeLists.txt
+++ lldb/CMakeLists.txt
@@ -37,18 +37,29 @@
   -c "import distutils.sysconfig; 
print(distutils.sysconfig.get_python_lib(True, False, ''))"
   OUTPUT_VARIABLE LLDB_PYTHON_DEFAULT_RELATIVE_PATH
   OUTPUT_STRIP_TRAILING_WHITESPACE)
-
 file(TO_CMAKE_PATH ${LLDB_PYTHON_DEFAULT_RELATIVE_PATH} 
LLDB_PYTHON_DEFAULT_RELATIVE_PATH)
+execute_process(
+  COMMAND ${Python3_EXECUTABLE}
+  -c "import sysconfig; print(sysconfig.get_config_var('EXT_SUFFIX'))"
+  OUTPUT_VARIABLE LLDB_PYTHON_DEFAULT_EXT_SUFFIX
+  OUTPUT_STRIP_TRAILING_WHITESPACE)
   else ()
 if ("${LLDB_PYTHON_RELATIVE_PATH}" STREQUAL "")
   message(FATAL_ERROR
 "Crosscompiling LLDB with Python requires manually setting
 LLDB_PYTHON_RELATIVE_PATH.")
 endif ()
+if ("${LLDB_PYTHON_EXT_SUFFIX}" STREQUAL "")
+  message(FATAL_ERROR
+"Crosscompiling LLDB with Python requires manually setting
+LLDB_PYTHON_EXT_SUFFIX.")
+endif ()
   endif ()
 
   set(LLDB_PYTHON_RELATIVE_PATH ${LLDB_PYTHON_DEFAULT_RELATIVE_PATH}
-CACHE STRING "Path where Python modules are installed, relative to install 
prefix")
+CACHE STRING "Path where Python modules are installed, relative to install 
prefix.")
+  set(LLDB_PYTHON_EXT_SUFFIX ${LLDB_PYTHON_DEFAULT_EXT_SUFFIX}
+CACHE STRING "Filename suffix for Python modules.")
 endif ()
 
 if (LLDB_ENABLE_LUA)


Index: lldb/bindings/python/CMakeLists.txt
===
--- lldb/bindings/python/CMakeLists.txt
+++ lldb/bindings/python/CMakeLists.txt
@@ -138,13 +138,14 @@
 set(LIBLLDB_SYMLINK_DEST "${LLVM_SHLIB_OUTPUT_INTDIR}/liblldb${CMAKE_SHARED_LIBRARY_SUFFIX}")
   endif()
   if(WIN32)
+# FIXME should we use EXT_SUFFIX on windows?
 if(CMAKE_BUILD_TYPE STREQUAL Debug)
   set(LIBLLDB_SYMLINK_OUTPUT_FILE "_lldb_d.pyd")
 else()
   set(LIBLLDB_SYMLINK_OUTPUT_FILE "_lldb.pyd")
 endif()
   else()
-set(LIBLLDB_SYMLINK_OUTPUT_FILE "_lldb.so")
+set(LIBLLDB_SYMLINK_OUTPUT_FILE "_lldb${LLDB_PYTHON_EXT_SUFFIX}")
   endif()
   create_relative_symlink(${swig_target} ${LIBLLDB_SYMLINK_DEST}
   ${lldb_python_target_dir} ${LIBLLDB_SYMLINK_OUTPUT_FILE})
Index: lldb/CMakeLists.txt
===
--- lldb/CMakeLists.txt
+++ lldb/CMakeLists.txt
@@ -37,18 +37,29 @@
   -c "import distutils.sysconfig; print(distutils.sysconfig.get_python_lib(True, False, ''))"
   OUTPUT_VARIABLE LLDB_PYTHON_DEFAULT_RELATIVE_PATH
   OUTPUT_STRIP_TRAILING_WHITESPACE)
-
 file(TO_CMAKE_PATH ${LLDB_PYTHON_DEFAULT_RELATIVE_PATH} LLDB_PYTHON_DEFAULT_RELATIVE_PATH)
+execute_process(
+  COMMAND ${Python3_EXECUTABLE}
+  -c "import sysconfig; print(sysconfig.get_config_var('EXT_SUFFIX'))"
+  OUTPUT_VARIABLE LLDB_PYTHON_DEFAULT_EXT_SUFFIX
+  OUTPUT_STRIP_TRAILING_WHITESPACE)
   else ()
 if ("${LLDB_PYTHON_RELATIVE_PATH}" STREQUAL "")
   message(FATAL_ERROR
 "Crosscompiling LLDB with Python requires manually setting
 LLDB_PYTHON_RELATIVE_PATH.")
 endif ()
+if ("${LLDB_PYTHON_EXT_SUFFIX}" STREQUAL "")
+  message(FATAL_ERROR
+"Crosscompiling LLDB with Python requires manually setting
+LLDB_PYTHON_EXT_SUFFIX.")
+endif ()
   endif ()
 
   set(LLDB_PYTHON_RELATIVE_PATH

[Lldb-commits] [PATCH] D112973: [lldb] make it easier to find LLDB's python

2021-11-01 Thread Lawrence D'Anna via Phabricator via lldb-commits
lawrence_danna created this revision.
lawrence_danna added reviewers: jasonmolenda, JDevlieghere, jingham.
Herald added subscribers: dang, mgorny.
lawrence_danna requested review of this revision.
Herald added a project: LLDB.

It is surprisingly difficult to write a simple python script that
can reliably `import lldb` without failing, or crashing.   I'm
currently resorting to convolutions like this:

def find_lldb(may_reexec=False):

if prefix := os.environ.get('LLDB_PYTHON_PREFIX'):
if os.path.realpath(prefix) != 
os.path.realpath(sys.prefix):
raise Exception("cannot import lldb.\n"
f"  sys.prefix should be: {prefix}\n"
f"  but it is: {sys.prefix}")
else:
line1, line2 = subprocess.run(
['lldb', '-x', '-b', '-o', 'script 
print(sys.prefix)'],
encoding='utf8', stdout=subprocess.PIPE,
check=True).stdout.strip().splitlines()
assert line1.strip() == '(lldb) script 
print(sys.prefix)'
prefix = line2.strip()
os.environ['LLDB_PYTHON_PREFIX'] = prefix
  
if sys.prefix != prefix:
if not may_reexec:
raise Exception(
"cannot import lldb.\n" +
f"  This python, at {sys.prefix}\n"
f"  does not math LLDB's python at 
{prefix}")
os.environ['LLDB_PYTHON_PREFIX'] = prefix
python_exe = os.path.join(prefix, 'bin', 'python3')
os.execl(python_exe, python_exe, *sys.argv)
  
lldb_path = subprocess.run(['lldb', '-P'],
check=True, stdout=subprocess.PIPE,
encoding='utf8').stdout.strip()
  
sys.path = [lldb_path] + sys.path

This patch aims to replace all that with:

  #!/usr/bin/env lldb-python
  import lldb
  ...

- by adding the following features:

- new lldb::PathType for SBHostOS::GetLLDBPath: ePathTypePythonPrefix

is the path to python's sys.prefix

- new command line option: lldb --python-prefix to print that prefix.

- new tool (unix only): lldb-python which finds python and exec's it.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D112973

Files:
  lldb/bindings/python/CMakeLists.txt
  lldb/docs/man/lldb.rst
  lldb/docs/python_api_enums.rst
  lldb/include/lldb/lldb-enumerations.h
  lldb/source/API/SBHostOS.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h
  lldb/test/API/functionalities/paths/TestPaths.py
  lldb/test/Shell/Driver/TestHelp.test
  lldb/tools/driver/Driver.cpp
  lldb/tools/driver/Driver.h
  lldb/tools/driver/Options.td

Index: lldb/tools/driver/Options.td
===
--- lldb/tools/driver/Options.td
+++ lldb/tools/driver/Options.td
@@ -48,6 +48,10 @@
   HelpText<"Alias for --python-path">,
   Group;
 
+def python_prefix: F<"python-prefix">,
+  HelpText<"Prints out the sys.prefix for the python used by this lldb.">,
+  Group;
+
 def script_language: Separate<["--", "-"], "script-language">,
   MetaVarName<"">,
   HelpText<"Tells the debugger to use the specified scripting language for user-defined scripts.">,
Index: lldb/tools/driver/Driver.h
===
--- lldb/tools/driver/Driver.h
+++ lldb/tools/driver/Driver.h
@@ -79,6 +79,7 @@
 bool m_source_quietly = false;
 bool m_print_version = false;
 bool m_print_python_path = false;
+bool m_print_python_prefix = false;
 bool m_wait_for = false;
 bool m_repl = false;
 bool m_batch = false;
Index: lldb/tools/driver/Driver.cpp
===
--- lldb/tools/driver/Driver.cpp
+++ lldb/tools/driver/Driver.cpp
@@ -43,7 +43,7 @@
 #include 
 #include 
 
-// Includes for pipe()
+// Includes for pipe(), execv()
 #if defined(_WIN32)
 #include 
 #include 
@@ -201,6 +201,9 @@
   if (args.hasArg(OPT_python_path)) {
 m_option_data.m_print_python_path = true;
   }
+  if (args.hasArg(OPT_python_prefix)) {
+m_option_data.m_print_python_prefix = true;
+  }
 
   if (args.hasArg(OPT_batch)) {
 m_option_data.m_batch = true;
@@ -398,6 +401,21 @@
 return error;
   }
 
+  if (m_option_data.m_print_python_prefix) {
+SBFileSpec python_file_spec = SBHostOS::GetLLDBPath(ePathTypePythonPrefix);
+if (python_file_spec.IsValid()) {
+  char python_path[PATH_MAX];
+  size_t num_chars = python_file_spec.GetPath(python_path, PATH_MAX);
+  if (num_chars < PATH

[Lldb-commits] [PATCH] D112973: [lldb] make it easier to find LLDB's python

2021-11-01 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere added a comment.

I feel like this puts too much Python specific logic in the driver compared to 
the convenience it brings. Even though we already leak python details (like 
`--python-path`), LLDB has always been designed to support several scripting 
languages. I'm not convinced this use case is compelling enough to make the 
situation worse.

FWIW `crashlog.py` has a good example of how to import LLDB. It's complicated 
slightly by us having to honor the `LLDB_DEFAULT_PYTHON_VERSION`, but otherwise 
it's pretty simple:

  try:
  import lldb
  except ImportError:
  lldb_python_path = subprocess.check_output(['xcrun', 'lldb', 
'-P']).decode("utf-8").strip()
  if os.path.exists(lldb_python_path) and not 
sys.path.__contains__(lldb_python_path):
  sys.path.append(lldb_python_path)
  import lldb

A potential alternative to this could be an `lldb-python` (python) script that 
reinvokes itself with the correct `PYTHONPATH` set?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112973

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


[Lldb-commits] [PATCH] D112973: [lldb] make it easier to find LLDB's python

2021-11-01 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment.

Since you do need to match the lldb module to the python it was built against, 
this seems a useful addition.

The original --python-path was a bit unfortunate, we're supposed to be able to 
support multiple scripting interpreters (currently python & lua) and it would 
be annoying to have to add one of these variables for each scripting language 
we support.  So it seems a shame to repeat that error.

Maybe it would be better to have:

--scripting-module-path {python, lua}

and

--scripting-executable-path {python, lua}

as the arguments to the driver (we'll have to keep the --python-path for 
compatibility.)

Seems like "where is your module" and "where is your executable" are also 
questions we should be asking the ScriptInterpreter base class, and not going 
straight to the Python interpreter.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112973

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


[Lldb-commits] [PATCH] D112973: [lldb] make it easier to find LLDB's python

2021-11-01 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment.

In D112973#3101602 , @JDevlieghere 
wrote:

> I feel like this puts too much Python specific logic in the driver compared 
> to the convenience it brings. Even though we already leak python details 
> (like `--python-path`), LLDB has always been designed to support several 
> scripting languages. I'm not convinced this use case is compelling enough to 
> make the situation worse.
>
> FWIW `crashlog.py` has a good example of how to import LLDB. It's complicated 
> slightly by us having to honor the `LLDB_DEFAULT_PYTHON_VERSION`, but 
> otherwise it's pretty simple:
>
>   try:
>   import lldb
>   except ImportError:
>   lldb_python_path = subprocess.check_output(['xcrun', 'lldb', 
> '-P']).decode("utf-8").strip()
>   if os.path.exists(lldb_python_path) and not 
> sys.path.__contains__(lldb_python_path):
>   sys.path.append(lldb_python_path)
>   import lldb
>
> A potential alternative to this could be an `lldb-python` (python) script 
> that reinvokes itself with the correct `PYTHONPATH` set?

I don't think that fits exactly with what Larry's patch is doing.  The lldb 
vrs. Python problem is two-fold, one problem is finding some lldb module 
somewhere, but the other is making sure that the python you pick (should you 
have multiple on your system) is the one that's going to work with the lldb 
module your lldb is going to find for you.

The solutions we have to this problem all assume you've ALREADY found the right 
python, now you just have to find some lldb module.  OTOH lldb-python uses the 
Python library that lldb loads to find the right python executable and match it 
to the right lldb module.

This isn't 100% necessary: once you've found your lldb, you could just continue 
to use lldb, employing the script command to feed the Python interpreter.  But 
I can see how you might want Python to be in control, and this solution makes 
that use case less error-prone.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112973

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


[Lldb-commits] [PATCH] D112973: [lldb] make it easier to find LLDB's python

2021-11-01 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment.

But I do agree with Jonas that we should find a less python-specific way to 
implement this.  I don't think you need to actually fill in the Lua parts, but 
whoever does that shouldn't have to copy everything that was done for python, a 
lot of this should be shareable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112973

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


[Lldb-commits] [PATCH] D112973: [lldb] make it easier to find LLDB's python

2021-11-01 Thread Lawrence D'Anna via Phabricator via lldb-commits
lawrence_danna added a comment.

In D112973#3101602 , @JDevlieghere 
wrote:

> FWIW `crashlog.py` has a good example of how to import LLDB. It's complicated 
> slightly by us having to honor the `LLDB_DEFAULT_PYTHON_VERSION`, but 
> otherwise it's pretty simple:

Unfortunately that is not enough to solve the problem, because if we run that 
script with the wrong version of python it will crash (see 
https://reviews.llvm.org/D112972)
Even if that bug is fixed, it will still raise an ImportError

> A potential alternative to this could be an `lldb-python` (python) script 
> that reinvokes itself with the correct `PYTHONPATH` set?

That approach would work, but I'd still need `--python-prefix` to find the 
right python.   I'll update this diff tomorrow doing it that way.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112973

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


[Lldb-commits] [PATCH] D112863: [lldb][NFC] avoid unnecessary roundtrips between different string types

2021-11-01 Thread walter erquinigo via Phabricator via lldb-commits
wallace added a comment.

could you rebase this on top of the main branch? I tried to 'arc patch' it 
locally and there are a few merge issues.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112863

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


[Lldb-commits] [PATCH] D112863: [lldb][NFC] avoid unnecessary roundtrips between different string types

2021-11-01 Thread Xu Jun via Phabricator via lldb-commits
xujuntwt95329 updated this revision to Diff 383965.
xujuntwt95329 added a comment.

rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112863

Files:
  lldb/include/lldb/Target/PathMappingList.h
  lldb/source/API/SBTarget.cpp
  lldb/source/Commands/CommandObjectTarget.cpp
  lldb/source/Core/Module.cpp
  lldb/source/Core/ModuleList.cpp
  lldb/source/Interpreter/OptionValuePathMappings.cpp
  lldb/source/Target/PathMappingList.cpp
  lldb/unittests/Target/FindFileTest.cpp
  lldb/unittests/Target/PathMappingListTest.cpp

Index: lldb/unittests/Target/PathMappingListTest.cpp
===
--- lldb/unittests/Target/PathMappingListTest.cpp
+++ lldb/unittests/Target/PathMappingListTest.cpp
@@ -66,16 +66,16 @@
 #endif
   };
   PathMappingList map;
-  map.Append(ConstString("."), ConstString("/tmp"), false);
+  map.Append(".", "/tmp", false);
   TestPathMappings(map, matches, fails);
   PathMappingList map2;
-  map2.Append(ConstString(""), ConstString("/tmp"), false);
+  map2.Append("", "/tmp", false);
   TestPathMappings(map, matches, fails);
 }
 
 TEST(PathMappingListTest, AbsoluteTests) {
   PathMappingList map;
-  map.Append(ConstString("/old"), ConstString("/new"), false);
+  map.Append("/old", "/new", false);
   Matches matches[] = {
 {"/old", "/new"},
 {"/old/", "/new"},
@@ -97,7 +97,7 @@
 
 TEST(PathMappingListTest, RemapRoot) {
   PathMappingList map;
-  map.Append(ConstString("/"), ConstString("/new"), false);
+  map.Append("/", "/new", false);
   Matches matches[] = {
 {"/old", "/new/old"},
 {"/old/", "/new/old"},
@@ -118,7 +118,7 @@
 #ifndef _WIN32
 TEST(PathMappingListTest, CrossPlatformTests) {
   PathMappingList map;
-  map.Append(ConstString(R"(C:\old)"), ConstString("/new"), false);
+  map.Append(R"(C:\old)", "/new", false);
   Matches matches[] = {
 {R"(C:\old)", llvm::sys::path::Style::windows, "/new"},
 {R"(C:\old\)", llvm::sys::path::Style::windows, "/new"},
Index: lldb/unittests/Target/FindFileTest.cpp
===
--- lldb/unittests/Target/FindFileTest.cpp
+++ lldb/unittests/Target/FindFileTest.cpp
@@ -75,8 +75,8 @@
   llvm::FileRemover file_remover(FileName);
   PathMappingList map;
 
-  map.Append(ConstString("/old"), ConstString(DirName.str()), false);
-  map.Append(ConstString(R"(C:\foo)"), ConstString(DirName.str()), false);
+  map.Append("/old", DirName.str(), false);
+  map.Append(R"(C:\foo)", DirName.str(), false);
 
   Matches matches[] = {
   {"/old", llvm::sys::path::Style::posix, DirName.c_str()},
Index: lldb/source/Target/PathMappingList.cpp
===
--- lldb/source/Target/PathMappingList.cpp
+++ lldb/source/Target/PathMappingList.cpp
@@ -30,11 +30,11 @@
   // with the raw path pair, which doesn't work anymore because the paths have
   // been normalized when the debug info was loaded. So we need to store
   // nomalized path pairs to ensure things match up.
-  ConstString NormalizePath(ConstString path) {
-// If we use "path" to construct a FileSpec, it will normalize the path for
-// us. We then grab the string and turn it back into a ConstString.
-return ConstString(FileSpec(path.GetStringRef()).GetPath());
-  }
+std::string NormalizePath(llvm::StringRef path) {
+  // If we use "path" to construct a FileSpec, it will normalize the path for
+  // us. We then grab the string.
+  return FileSpec(path).GetPath();
+}
 }
 // PathMappingList constructor
 PathMappingList::PathMappingList() : m_pairs() {}
@@ -59,8 +59,8 @@
 
 PathMappingList::~PathMappingList() = default;
 
-void PathMappingList::Append(ConstString path,
- ConstString replacement, bool notify) {
+void PathMappingList::Append(llvm::StringRef path, llvm::StringRef replacement,
+ bool notify) {
   ++m_mod_id;
   m_pairs.emplace_back(pair(NormalizePath(path), NormalizePath(replacement)));
   if (notify && m_callback)
@@ -78,9 +78,8 @@
   }
 }
 
-void PathMappingList::Insert(ConstString path,
- ConstString replacement, uint32_t index,
- bool notify) {
+void PathMappingList::Insert(llvm::StringRef path, llvm::StringRef replacement,
+ uint32_t index, bool notify) {
   ++m_mod_id;
   iterator insert_iter;
   if (index >= m_pairs.size())
@@ -93,9 +92,8 @@
 m_callback(*this, m_callback_baton);
 }
 
-bool PathMappingList::Replace(ConstString path,
-  ConstString replacement, uint32_t index,
-  bool notify) {
+bool PathMappingList::Replace(llvm::StringRef path, llvm::StringRef replacement,
+  uint32_t index, bool notify) {
   if (index >= m_pairs.size())
 return false;
   ++m_mod_id;
@@ -221,20 +219,19 @@
   // We

[Lldb-commits] [PATCH] D112439: normalize file path when searching the source map

2021-11-01 Thread Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGfe19ae352ce2: normalize file path when searching the source 
map (authored by xujuntwt95329, committed by Walter Erquinigo 
).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112439

Files:
  lldb/source/Target/PathMappingList.cpp
  lldb/unittests/Target/CMakeLists.txt
  lldb/unittests/Target/FindFileTest.cpp

Index: lldb/unittests/Target/FindFileTest.cpp
===
--- /dev/null
+++ lldb/unittests/Target/FindFileTest.cpp
@@ -0,0 +1,97 @@
+//===-- FindFileTest.cpp -===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "TestingSupport/TestUtilities.h"
+#include "lldb/Host/FileSystem.h"
+#include "lldb/Host/HostInfo.h"
+#include "lldb/Target/PathMappingList.h"
+#include "lldb/Utility/FileSpec.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/FileUtilities.h"
+#include "gtest/gtest.h"
+#include 
+
+using namespace llvm;
+using namespace llvm::sys::fs;
+using namespace lldb_private;
+
+namespace {
+struct Matches {
+  FileSpec original;
+  llvm::StringRef remapped;
+  Matches(const char *o, const char *r) : original(o), remapped(r) {}
+  Matches(const char *o, llvm::sys::path::Style style, const char *r)
+  : original(o, style), remapped(r) {}
+};
+
+class FindFileTest : public testing::Test {
+public:
+  void SetUp() override {
+FileSystem::Initialize();
+HostInfo::Initialize();
+  }
+  void TearDown() override {
+HostInfo::Terminate();
+FileSystem::Terminate();
+  }
+};
+} // namespace
+
+static void TestFileFindings(const PathMappingList &map,
+ llvm::ArrayRef matches,
+ llvm::ArrayRef fails) {
+  for (const auto &fail : fails) {
+SCOPED_TRACE(fail.GetCString());
+EXPECT_FALSE(map.FindFile(fail));
+  }
+
+  for (const auto &match : matches) {
+SCOPED_TRACE(match.original.GetPath() + " -> " + match.remapped);
+llvm::Optional remapped;
+
+EXPECT_TRUE(bool(remapped = map.FindFile(match.original)));
+EXPECT_TRUE(FileSpec(remapped.getValue()).GetPath() ==
+ConstString(match.remapped).GetStringRef());
+  }
+}
+
+TEST_F(FindFileTest, FindFileTests) {
+  const auto *Info = testing::UnitTest::GetInstance()->current_test_info();
+  llvm::SmallString<128> DirName, FileName;
+  int fd;
+
+  ASSERT_NO_ERROR(createUniqueDirectory(Info->name(), DirName));
+
+  sys::path::append(FileName, Twine(DirName), Twine("test"));
+  ASSERT_NO_ERROR(openFile(FileName, fd, CD_CreateAlways, FA_Read, OF_None));
+
+  llvm::FileRemover dir_remover(DirName);
+  llvm::FileRemover file_remover(FileName);
+  PathMappingList map;
+
+  map.Append(ConstString("/old"), ConstString(DirName.str()), false);
+  map.Append(ConstString(R"(C:\foo)"), ConstString(DirName.str()), false);
+
+  Matches matches[] = {
+  {"/old", llvm::sys::path::Style::posix, DirName.c_str()},
+  {"/old/test", llvm::sys::path::Style::posix, FileName.c_str()},
+  {R"(C:\foo)", llvm::sys::path::Style::windows, DirName.c_str()},
+  {R"(C:\foo\test)", llvm::sys::path::Style::windows, FileName.c_str()}};
+
+  ArrayRef fails{
+  // path not mapped
+  FileSpec("/foo", llvm::sys::path::Style::posix),
+  FileSpec("/new", llvm::sys::path::Style::posix),
+  FileSpec(R"(C:\new)", llvm::sys::path::Style::windows),
+  // path mapped, but file not exist
+  FileSpec("/old/test1", llvm::sys::path::Style::posix),
+  FileSpec(R"(C:\foo\test2)", llvm::sys::path::Style::windows)};
+
+  TestFileFindings(map, matches, fails);
+}
Index: lldb/unittests/Target/CMakeLists.txt
===
--- lldb/unittests/Target/CMakeLists.txt
+++ lldb/unittests/Target/CMakeLists.txt
@@ -7,6 +7,7 @@
   PathMappingListTest.cpp
   RemoteAwarePlatformTest.cpp
   StackFrameRecognizerTest.cpp
+  FindFileTest.cpp
 
   LINK_LIBS
   lldbCore
Index: lldb/source/Target/PathMappingList.cpp
===
--- lldb/source/Target/PathMappingList.cpp
+++ lldb/source/Target/PathMappingList.cpp
@@ -218,7 +218,12 @@
 }
 
 llvm::Optional PathMappingList::FindFile(const FileSpec &orig_spec) const {
-  if (auto remapped = RemapPath(orig_spec.GetPath(), /*only_if_exists=*/true))
+  // We must normalize the orig_spec again using the host's path style,
+  // otherwise there will be mismatch between the host and remote platform
+  // if they use different path styles.
+  if (auto r

[Lldb-commits] [lldb] fe19ae3 - normalize file path when searching the source map

2021-11-01 Thread Walter Erquinigo via lldb-commits

Author: Xu Jun
Date: 2021-11-01T22:13:55-07:00
New Revision: fe19ae352ce201e7bab8baac4587cc4334e742e4

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

LOG: normalize file path when searching the source map

The key stored in the source map is a normalized path string with host's
path style, so it is also necessary to normalize the file path during
searching the map

Reviewed By: wallace, aprantl

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

Added: 
lldb/unittests/Target/FindFileTest.cpp

Modified: 
lldb/source/Target/PathMappingList.cpp
lldb/unittests/Target/CMakeLists.txt

Removed: 




diff  --git a/lldb/source/Target/PathMappingList.cpp 
b/lldb/source/Target/PathMappingList.cpp
index b660c310ef31a..cd76421cec18a 100644
--- a/lldb/source/Target/PathMappingList.cpp
+++ b/lldb/source/Target/PathMappingList.cpp
@@ -218,7 +218,12 @@ bool PathMappingList::ReverseRemapPath(const FileSpec 
&file, FileSpec &fixed) co
 }
 
 llvm::Optional PathMappingList::FindFile(const FileSpec &orig_spec) 
const {
-  if (auto remapped = RemapPath(orig_spec.GetPath(), /*only_if_exists=*/true))
+  // We must normalize the orig_spec again using the host's path style,
+  // otherwise there will be mismatch between the host and remote platform
+  // if they use 
diff erent path styles.
+  if (auto remapped = RemapPath(
+  NormalizePath(ConstString(orig_spec.GetCString())).GetStringRef(),
+  /*only_if_exists=*/true))
 return remapped;
 
   return {};

diff  --git a/lldb/unittests/Target/CMakeLists.txt 
b/lldb/unittests/Target/CMakeLists.txt
index c126597c79daf..3b23550feaf9c 100644
--- a/lldb/unittests/Target/CMakeLists.txt
+++ b/lldb/unittests/Target/CMakeLists.txt
@@ -7,6 +7,7 @@ add_lldb_unittest(TargetTests
   PathMappingListTest.cpp
   RemoteAwarePlatformTest.cpp
   StackFrameRecognizerTest.cpp
+  FindFileTest.cpp
 
   LINK_LIBS
   lldbCore

diff  --git a/lldb/unittests/Target/FindFileTest.cpp 
b/lldb/unittests/Target/FindFileTest.cpp
new file mode 100644
index 0..30ac4dfc9f5d1
--- /dev/null
+++ b/lldb/unittests/Target/FindFileTest.cpp
@@ -0,0 +1,97 @@
+//===-- FindFileTest.cpp -===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "TestingSupport/TestUtilities.h"
+#include "lldb/Host/FileSystem.h"
+#include "lldb/Host/HostInfo.h"
+#include "lldb/Target/PathMappingList.h"
+#include "lldb/Utility/FileSpec.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/FileUtilities.h"
+#include "gtest/gtest.h"
+#include 
+
+using namespace llvm;
+using namespace llvm::sys::fs;
+using namespace lldb_private;
+
+namespace {
+struct Matches {
+  FileSpec original;
+  llvm::StringRef remapped;
+  Matches(const char *o, const char *r) : original(o), remapped(r) {}
+  Matches(const char *o, llvm::sys::path::Style style, const char *r)
+  : original(o, style), remapped(r) {}
+};
+
+class FindFileTest : public testing::Test {
+public:
+  void SetUp() override {
+FileSystem::Initialize();
+HostInfo::Initialize();
+  }
+  void TearDown() override {
+HostInfo::Terminate();
+FileSystem::Terminate();
+  }
+};
+} // namespace
+
+static void TestFileFindings(const PathMappingList &map,
+ llvm::ArrayRef matches,
+ llvm::ArrayRef fails) {
+  for (const auto &fail : fails) {
+SCOPED_TRACE(fail.GetCString());
+EXPECT_FALSE(map.FindFile(fail));
+  }
+
+  for (const auto &match : matches) {
+SCOPED_TRACE(match.original.GetPath() + " -> " + match.remapped);
+llvm::Optional remapped;
+
+EXPECT_TRUE(bool(remapped = map.FindFile(match.original)));
+EXPECT_TRUE(FileSpec(remapped.getValue()).GetPath() ==
+ConstString(match.remapped).GetStringRef());
+  }
+}
+
+TEST_F(FindFileTest, FindFileTests) {
+  const auto *Info = testing::UnitTest::GetInstance()->current_test_info();
+  llvm::SmallString<128> DirName, FileName;
+  int fd;
+
+  ASSERT_NO_ERROR(createUniqueDirectory(Info->name(), DirName));
+
+  sys::path::append(FileName, Twine(DirName), Twine("test"));
+  ASSERT_NO_ERROR(openFile(FileName, fd, CD_CreateAlways, FA_Read, OF_None));
+
+  llvm::FileRemover dir_remover(DirName);
+  llvm::FileRemover file_remover(FileName);
+  PathMappingList map;
+
+  map.Append(ConstString("/old"), ConstString(DirName.str()), false);
+  map.Append(ConstString(R"(C:\foo)"), ConstString(DirName.str()), false);
+
+  Matches matches[] = {
+  {

[Lldb-commits] [PATCH] D112863: [lldb][NFC] avoid unnecessary roundtrips between different string types

2021-11-01 Thread Xu Jun via Phabricator via lldb-commits
xujuntwt95329 added a comment.

@wallace rebased

But I guess you should apply this patch firstly?
https://reviews.llvm.org/D112439


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112863

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


[Lldb-commits] [lldb] dfd499a - [lldb][NFC] avoid unnecessary roundtrips between different string types

2021-11-01 Thread Walter Erquinigo via lldb-commits

Author: Xu Jun
Date: 2021-11-01T22:15:01-07:00
New Revision: dfd499a61c45778b7f01458d50ccc384343f53d5

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

LOG: [lldb][NFC] avoid unnecessary roundtrips between different string types

The amount of roundtrips between StringRefs, ConstStrings and std::strings is
getting a bit out of hand, this patch avoid the unnecessary roundtrips.

Reviewed By: wallace, aprantl

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

Added: 


Modified: 
lldb/include/lldb/Target/PathMappingList.h
lldb/source/API/SBTarget.cpp
lldb/source/Commands/CommandObjectTarget.cpp
lldb/source/Core/Module.cpp
lldb/source/Core/ModuleList.cpp
lldb/source/Interpreter/OptionValuePathMappings.cpp
lldb/source/Target/PathMappingList.cpp
lldb/unittests/Target/FindFileTest.cpp
lldb/unittests/Target/PathMappingListTest.cpp

Removed: 




diff  --git a/lldb/include/lldb/Target/PathMappingList.h 
b/lldb/include/lldb/Target/PathMappingList.h
index d788d120c47e9..f1cc779ea50fe 100644
--- a/lldb/include/lldb/Target/PathMappingList.h
+++ b/lldb/include/lldb/Target/PathMappingList.h
@@ -32,8 +32,7 @@ class PathMappingList {
 
   const PathMappingList &operator=(const PathMappingList &rhs);
 
-  void Append(ConstString path, ConstString replacement,
-  bool notify);
+  void Append(llvm::StringRef path, llvm::StringRef replacement, bool notify);
 
   void Append(const PathMappingList &rhs, bool notify);
 
@@ -49,17 +48,16 @@ class PathMappingList {
   bool GetPathsAtIndex(uint32_t idx, ConstString &path,
ConstString &new_path) const;
 
-  void Insert(ConstString path, ConstString replacement,
+  void Insert(llvm::StringRef path, llvm::StringRef replacement,
   uint32_t insert_idx, bool notify);
 
   bool Remove(size_t index, bool notify);
 
   bool Remove(ConstString path, bool notify);
 
-  bool Replace(ConstString path, ConstString replacement,
-   bool notify);
+  bool Replace(llvm::StringRef path, llvm::StringRef replacement, bool notify);
 
-  bool Replace(ConstString path, ConstString replacement,
+  bool Replace(llvm::StringRef path, llvm::StringRef replacement,
uint32_t index, bool notify);
   bool RemapPath(ConstString path, ConstString &new_path) const;
 
@@ -104,7 +102,7 @@ class PathMappingList {
   /// The newly remapped filespec that is guaranteed to exist.
   llvm::Optional FindFile(const FileSpec &orig_spec) const;
 
-  uint32_t FindIndexForPath(ConstString path) const;
+  uint32_t FindIndexForPath(llvm::StringRef path) const;
 
   uint32_t GetModificationID() const { return m_mod_id; }
 

diff  --git a/lldb/source/API/SBTarget.cpp b/lldb/source/API/SBTarget.cpp
index 9db5b6d03c3fc..98158f457a04f 100644
--- a/lldb/source/API/SBTarget.cpp
+++ b/lldb/source/API/SBTarget.cpp
@@ -214,8 +214,8 @@ SBStructuredData SBTarget::GetStatistics() {
   if (!target_sp)
 return LLDB_RECORD_RESULT(data);
   std::string json_str =
-  llvm::formatv("{0:2}", 
-  DebuggerStats::ReportStatistics(target_sp->GetDebugger(), 
+  llvm::formatv("{0:2}",
+  DebuggerStats::ReportStatistics(target_sp->GetDebugger(),
   target_sp.get())).str();
   data.m_impl_up->SetObjectSP(StructuredData::ParseJSON(json_str));
   return LLDB_RECORD_RESULT(data);
@@ -1586,13 +1586,13 @@ void SBTarget::AppendImageSearchPath(const char *from, 
const char *to,
   if (!target_sp)
 return error.SetErrorString("invalid target");
 
-  const ConstString csFrom(from), csTo(to);
-  if (!csFrom)
+  llvm::StringRef srFrom = from, srTo = to;
+  if (srFrom.empty())
 return error.SetErrorString(" path can't be empty");
-  if (!csTo)
+  if (srTo.empty())
 return error.SetErrorString(" path can't be empty");
 
-  target_sp->GetImageSearchPathList().Append(csFrom, csTo, true);
+  target_sp->GetImageSearchPathList().Append(srFrom, srTo, true);
 }
 
 lldb::SBModule SBTarget::AddModule(const char *path, const char *triple,

diff  --git a/lldb/source/Commands/CommandObjectTarget.cpp 
b/lldb/source/Commands/CommandObjectTarget.cpp
index e0a88a710fb97..2a42eb22938d7 100644
--- a/lldb/source/Commands/CommandObjectTarget.cpp
+++ b/lldb/source/Commands/CommandObjectTarget.cpp
@@ -1047,8 +1047,7 @@ class CommandObjectTargetModulesSearchPathsAdd : public 
CommandObjectParsed {
   }
   bool last_pair = ((argc - i) == 2);
   target->GetImageSearchPathList().Append(
-  ConstString(from), ConstString(to),
-  last_pair); // Notify if this is the last pair
+  from, to, last_pair); // Notify if this is the last pair
   result.SetStatus(eReturnStatusSuccessFinishNoResult);
 } else {

[Lldb-commits] [PATCH] D112863: [lldb][NFC] avoid unnecessary roundtrips between different string types

2021-11-01 Thread Phabricator via lldb-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGdfd499a61c45: [lldb][NFC] avoid unnecessary roundtrips 
between different string types (authored by xujuntwt95329, committed by Walter 
Erquinigo ).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112863

Files:
  lldb/include/lldb/Target/PathMappingList.h
  lldb/source/API/SBTarget.cpp
  lldb/source/Commands/CommandObjectTarget.cpp
  lldb/source/Core/Module.cpp
  lldb/source/Core/ModuleList.cpp
  lldb/source/Interpreter/OptionValuePathMappings.cpp
  lldb/source/Target/PathMappingList.cpp
  lldb/unittests/Target/FindFileTest.cpp
  lldb/unittests/Target/PathMappingListTest.cpp

Index: lldb/unittests/Target/PathMappingListTest.cpp
===
--- lldb/unittests/Target/PathMappingListTest.cpp
+++ lldb/unittests/Target/PathMappingListTest.cpp
@@ -66,16 +66,16 @@
 #endif
   };
   PathMappingList map;
-  map.Append(ConstString("."), ConstString("/tmp"), false);
+  map.Append(".", "/tmp", false);
   TestPathMappings(map, matches, fails);
   PathMappingList map2;
-  map2.Append(ConstString(""), ConstString("/tmp"), false);
+  map2.Append("", "/tmp", false);
   TestPathMappings(map, matches, fails);
 }
 
 TEST(PathMappingListTest, AbsoluteTests) {
   PathMappingList map;
-  map.Append(ConstString("/old"), ConstString("/new"), false);
+  map.Append("/old", "/new", false);
   Matches matches[] = {
 {"/old", "/new"},
 {"/old/", "/new"},
@@ -97,7 +97,7 @@
 
 TEST(PathMappingListTest, RemapRoot) {
   PathMappingList map;
-  map.Append(ConstString("/"), ConstString("/new"), false);
+  map.Append("/", "/new", false);
   Matches matches[] = {
 {"/old", "/new/old"},
 {"/old/", "/new/old"},
@@ -118,7 +118,7 @@
 #ifndef _WIN32
 TEST(PathMappingListTest, CrossPlatformTests) {
   PathMappingList map;
-  map.Append(ConstString(R"(C:\old)"), ConstString("/new"), false);
+  map.Append(R"(C:\old)", "/new", false);
   Matches matches[] = {
 {R"(C:\old)", llvm::sys::path::Style::windows, "/new"},
 {R"(C:\old\)", llvm::sys::path::Style::windows, "/new"},
Index: lldb/unittests/Target/FindFileTest.cpp
===
--- lldb/unittests/Target/FindFileTest.cpp
+++ lldb/unittests/Target/FindFileTest.cpp
@@ -75,8 +75,8 @@
   llvm::FileRemover file_remover(FileName);
   PathMappingList map;
 
-  map.Append(ConstString("/old"), ConstString(DirName.str()), false);
-  map.Append(ConstString(R"(C:\foo)"), ConstString(DirName.str()), false);
+  map.Append("/old", DirName.str(), false);
+  map.Append(R"(C:\foo)", DirName.str(), false);
 
   Matches matches[] = {
   {"/old", llvm::sys::path::Style::posix, DirName.c_str()},
Index: lldb/source/Target/PathMappingList.cpp
===
--- lldb/source/Target/PathMappingList.cpp
+++ lldb/source/Target/PathMappingList.cpp
@@ -30,11 +30,11 @@
   // with the raw path pair, which doesn't work anymore because the paths have
   // been normalized when the debug info was loaded. So we need to store
   // nomalized path pairs to ensure things match up.
-  ConstString NormalizePath(ConstString path) {
-// If we use "path" to construct a FileSpec, it will normalize the path for
-// us. We then grab the string and turn it back into a ConstString.
-return ConstString(FileSpec(path.GetStringRef()).GetPath());
-  }
+std::string NormalizePath(llvm::StringRef path) {
+  // If we use "path" to construct a FileSpec, it will normalize the path for
+  // us. We then grab the string.
+  return FileSpec(path).GetPath();
+}
 }
 // PathMappingList constructor
 PathMappingList::PathMappingList() : m_pairs() {}
@@ -59,8 +59,8 @@
 
 PathMappingList::~PathMappingList() = default;
 
-void PathMappingList::Append(ConstString path,
- ConstString replacement, bool notify) {
+void PathMappingList::Append(llvm::StringRef path, llvm::StringRef replacement,
+ bool notify) {
   ++m_mod_id;
   m_pairs.emplace_back(pair(NormalizePath(path), NormalizePath(replacement)));
   if (notify && m_callback)
@@ -78,9 +78,8 @@
   }
 }
 
-void PathMappingList::Insert(ConstString path,
- ConstString replacement, uint32_t index,
- bool notify) {
+void PathMappingList::Insert(llvm::StringRef path, llvm::StringRef replacement,
+ uint32_t index, bool notify) {
   ++m_mod_id;
   iterator insert_iter;
   if (index >= m_pairs.size())
@@ -93,9 +92,8 @@
 m_callback(*this, m_callback_baton);
 }
 
-bool PathMappingList::Replace(ConstString path,
-  ConstString replacement, uint32_t index,
-  bool notify) {
+bool PathMappingL

[Lldb-commits] [PATCH] D112973: [lldb] make it easier to find LLDB's python

2021-11-01 Thread Lawrence D'Anna via Phabricator via lldb-commits
lawrence_danna updated this revision to Diff 383978.
lawrence_danna added a comment.

change lldb-python into a script instead of a feature of Driver.cpp


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112973

Files:
  lldb/bindings/python/CMakeLists.txt
  lldb/bindings/python/lldb-python
  lldb/docs/man/lldb.rst
  lldb/docs/python_api_enums.rst
  lldb/include/lldb/lldb-enumerations.h
  lldb/source/API/SBHostOS.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h
  lldb/test/API/functionalities/paths/TestPaths.py
  lldb/test/Shell/Driver/TestHelp.test
  lldb/tools/driver/Driver.cpp
  lldb/tools/driver/Driver.h
  lldb/tools/driver/Options.td

Index: lldb/tools/driver/Options.td
===
--- lldb/tools/driver/Options.td
+++ lldb/tools/driver/Options.td
@@ -48,6 +48,10 @@
   HelpText<"Alias for --python-path">,
   Group;
 
+def python_prefix: F<"python-prefix">,
+  HelpText<"Prints out the sys.prefix for the python used by this lldb.">,
+  Group;
+
 def script_language: Separate<["--", "-"], "script-language">,
   MetaVarName<"">,
   HelpText<"Tells the debugger to use the specified scripting language for user-defined scripts.">,
Index: lldb/tools/driver/Driver.h
===
--- lldb/tools/driver/Driver.h
+++ lldb/tools/driver/Driver.h
@@ -79,6 +79,7 @@
 bool m_source_quietly = false;
 bool m_print_version = false;
 bool m_print_python_path = false;
+bool m_print_python_prefix = false;
 bool m_wait_for = false;
 bool m_repl = false;
 bool m_batch = false;
Index: lldb/tools/driver/Driver.cpp
===
--- lldb/tools/driver/Driver.cpp
+++ lldb/tools/driver/Driver.cpp
@@ -201,6 +201,9 @@
   if (args.hasArg(OPT_python_path)) {
 m_option_data.m_print_python_path = true;
   }
+  if (args.hasArg(OPT_python_prefix)) {
+m_option_data.m_print_python_prefix = true;
+  }
 
   if (args.hasArg(OPT_batch)) {
 m_option_data.m_batch = true;
@@ -398,6 +401,21 @@
 return error;
   }
 
+  if (m_option_data.m_print_python_prefix) {
+SBFileSpec python_file_spec = SBHostOS::GetLLDBPath(ePathTypePythonPrefix);
+if (python_file_spec.IsValid()) {
+  char python_path[PATH_MAX];
+  size_t num_chars = python_file_spec.GetPath(python_path, PATH_MAX);
+  if (num_chars < PATH_MAX) {
+llvm::outs() << python_path << '\n';
+  } else
+llvm::outs() << "\n";
+} else
+  llvm::outs() << "\n";
+exiting = true;
+return error;
+  }
+
   return error;
 }
 
Index: lldb/test/Shell/Driver/TestHelp.test
===
--- lldb/test/Shell/Driver/TestHelp.test
+++ lldb/test/Shell/Driver/TestHelp.test
@@ -63,5 +63,6 @@
 CHECK: SCRIPTING
 CHECK: -l
 CHECK: --python-path
+CHECK: --python-prefix
 CHECK: -P
 CHECK: --script-language
Index: lldb/test/API/functionalities/paths/TestPaths.py
===
--- lldb/test/API/functionalities/paths/TestPaths.py
+++ lldb/test/API/functionalities/paths/TestPaths.py
@@ -5,6 +5,7 @@
 
 import lldb
 import os
+import sys
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
@@ -22,10 +23,12 @@
   lldb.ePathTypeSupportExecutableDir,
   lldb.ePathTypeHeaderDir,
   lldb.ePathTypePythonDir,
+  lldb.ePathTypePythonPrefix,
   lldb.ePathTypeLLDBSystemPlugins,
   lldb.ePathTypeLLDBUserPlugins,
   lldb.ePathTypeLLDBTempSystemDir,
-  lldb.ePathTypeClangDir]
+  lldb.ePathTypeClangDir,
+  lldb.ePathTypePythonPrefix]
 
 for path_type in dir_path_types:
 f = lldb.SBHostOS.GetLLDBPath(path_type)
@@ -42,6 +45,10 @@
 self.assertTrue(any([os.path.exists(os.path.join(shlib_dir, f)) for f in
 filenames]), "shlib_dir = " + shlib_dir)
 
+@no_debug_info_test
+def test_prefix(self):
+prefix = lldb.SBHostOS.GetLLDBPath(lldb.ePathTypePythonPrefix).GetDirectory()
+self.assertEqual(os.path.realpath(sys.prefix), os.path.realpath(prefix))
 
 @no_debug_info_test
 def test_directory_doesnt_end_with_slash(self):
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h
@@ -51,6 +51,7 @@
   static llvm::