[Lldb-commits] [lldb] 99b7b41 - [lldb][import-std-module] Do some basic file checks before trying to import a module

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

Author: Raphael Isemann
Date: 2021-01-21T12:32:51+01:00
New Revision: 99b7b41edf4fbc2d6e52bc4524c956e8f69042d9

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

LOG: [lldb][import-std-module] Do some basic file checks before trying to 
import a module

Currently when LLDB has enough data in the debug information to import the 
`std` module,
it will just try to import it. However when debugging libraries where the 
sources aren't
available anymore, importing the module will generate a confusing diagnostic 
that
the module couldn't be built.

For the fallback mode (where we retry failed expressions with the loaded 
module), this
will cause the second expression to fail with a module built error instead of 
the
actual parsing issue in the user expression.

This patch adds checks that ensures that we at least have any source files in 
the found
include paths before we try to import the module. This prevents the module from 
being
loaded in the situation described above which means we don't emit the bogus 
'can't
import module' diagnostic and also don't waste any time retrying the expression 
in the
fallback mode.

For the unit tests I did some refactoring as they now require a VFS with the 
files in it
and not just the paths. The Python test just builds a binary with a fake C++ 
module,
then deletes the module before debugging.

Fixes rdar://73264458

Reviewed By: JDevlieghere

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

Added: 

lldb/test/API/commands/expression/import-std-module/empty-module/root/usr/include/c++/v1/vector

lldb/test/API/commands/expression/import-std-module/empty-module/root/usr/include/stdio.h

lldb/test/API/commands/expression/import-std-module/forward_decl_from_module/root/usr/include/stdio.h

lldb/test/API/commands/expression/import-std-module/missing-module-sources/Makefile

lldb/test/API/commands/expression/import-std-module/missing-module-sources/TestStdModuleSourcesMissing.py

lldb/test/API/commands/expression/import-std-module/missing-module-sources/main.cpp

lldb/test/API/commands/expression/import-std-module/missing-module-sources/root/usr/include/c++/v1/module.modulemap

lldb/test/API/commands/expression/import-std-module/missing-module-sources/root/usr/include/c++/v1/vector

lldb/test/API/commands/expression/import-std-module/missing-module-sources/root/usr/include/stdio.h

lldb/test/API/commands/expression/import-std-module/sysroot/root/usr/include/c++/v1/vector

lldb/test/API/commands/expression/import-std-module/sysroot/root/usr/include/stdio.h

Modified: 
lldb/source/Plugins/ExpressionParser/Clang/CppModuleConfiguration.cpp
lldb/source/Plugins/ExpressionParser/Clang/CppModuleConfiguration.h

lldb/test/API/commands/expression/import-std-module/empty-module/root/usr/include/c++/v1/algorithm

lldb/test/API/commands/expression/import-std-module/forward_decl_from_module/root/usr/include/c++/v1/vector

lldb/test/API/commands/expression/import-std-module/sysroot/root/usr/include/c++/v1/algorithm
lldb/unittests/Expression/CppModuleConfigurationTest.cpp

Removed: 

lldb/test/API/commands/expression/import-std-module/empty-module/root/usr/include/libc_header.h

lldb/test/API/commands/expression/import-std-module/forward_decl_from_module/root/usr/include/libc_header.h

lldb/test/API/commands/expression/import-std-module/sysroot/root/usr/include/libc_header.h



diff  --git 
a/lldb/source/Plugins/ExpressionParser/Clang/CppModuleConfiguration.cpp 
b/lldb/source/Plugins/ExpressionParser/Clang/CppModuleConfiguration.cpp
index d2162cf4c574..ffab16b1682b 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/CppModuleConfiguration.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/CppModuleConfiguration.cpp
@@ -57,9 +57,38 @@ bool CppModuleConfiguration::analyzeFile(const FileSpec &f) {
   return true;
 }
 
+/// Utility function for just appending two paths.
+static std::string MakePath(llvm::StringRef lhs, llvm::StringRef rhs) {
+  llvm::SmallString<256> result(lhs);
+  llvm::sys::path::append(result, rhs);
+  return std::string(result);
+}
+
 bool CppModuleConfiguration::hasValidConfig() {
-  // We all these include directories to have a valid usable configuration.
-  return m_c_inc.Valid() && m_std_inc.Valid();
+  // We need to have a C and C++ include dir for a valid configuration.
+  if (!m_c_inc.Valid() || !m_std_inc.Valid())
+return false;
+
+  // Do some basic sanity checks on the directories that we don't activate
+  // the module when it's clear that it's not usable.
+  const std::vector files_to_check = {
+  // * Check that the C library contains at least one random C standard
+  //   library header.
+  MakePath(m_c_inc.Get(), "stdio.h"),
+  // * With

[Lldb-commits] [PATCH] D95096: [lldb][import-std-module] Do some basic file checks before trying to import a module

2021-01-21 Thread Raphael Isemann via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG99b7b41edf4f: [lldb][import-std-module] Do some basic file 
checks before trying to import a… (authored by teemperor).
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Changed prior to commit:
  https://reviews.llvm.org/D95096?vs=318068&id=318145#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95096

Files:
  lldb/source/Plugins/ExpressionParser/Clang/CppModuleConfiguration.cpp
  lldb/source/Plugins/ExpressionParser/Clang/CppModuleConfiguration.h
  
lldb/test/API/commands/expression/import-std-module/empty-module/root/usr/include/c++/v1/algorithm
  
lldb/test/API/commands/expression/import-std-module/empty-module/root/usr/include/c++/v1/vector
  
lldb/test/API/commands/expression/import-std-module/empty-module/root/usr/include/libc_header.h
  
lldb/test/API/commands/expression/import-std-module/empty-module/root/usr/include/stdio.h
  
lldb/test/API/commands/expression/import-std-module/forward_decl_from_module/root/usr/include/c++/v1/vector
  
lldb/test/API/commands/expression/import-std-module/forward_decl_from_module/root/usr/include/libc_header.h
  
lldb/test/API/commands/expression/import-std-module/forward_decl_from_module/root/usr/include/stdio.h
  
lldb/test/API/commands/expression/import-std-module/missing-module-sources/Makefile
  
lldb/test/API/commands/expression/import-std-module/missing-module-sources/TestStdModuleSourcesMissing.py
  
lldb/test/API/commands/expression/import-std-module/missing-module-sources/main.cpp
  
lldb/test/API/commands/expression/import-std-module/missing-module-sources/root/usr/include/c++/v1/module.modulemap
  
lldb/test/API/commands/expression/import-std-module/missing-module-sources/root/usr/include/c++/v1/vector
  
lldb/test/API/commands/expression/import-std-module/missing-module-sources/root/usr/include/stdio.h
  
lldb/test/API/commands/expression/import-std-module/sysroot/root/usr/include/c++/v1/algorithm
  
lldb/test/API/commands/expression/import-std-module/sysroot/root/usr/include/c++/v1/vector
  
lldb/test/API/commands/expression/import-std-module/sysroot/root/usr/include/libc_header.h
  
lldb/test/API/commands/expression/import-std-module/sysroot/root/usr/include/stdio.h
  lldb/unittests/Expression/CppModuleConfigurationTest.cpp

Index: lldb/unittests/Expression/CppModuleConfigurationTest.cpp
===
--- lldb/unittests/Expression/CppModuleConfigurationTest.cpp
+++ lldb/unittests/Expression/CppModuleConfigurationTest.cpp
@@ -11,6 +11,7 @@
 #include "TestingSupport/SubsystemRAII.h"
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Host/HostInfo.h"
+#include "llvm/Support/SmallVectorMemoryBuffer.h"
 
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
@@ -19,7 +20,33 @@
 
 namespace {
 struct CppModuleConfigurationTest : public testing::Test {
-  SubsystemRAII subsystems;
+  llvm::MemoryBufferRef m_empty_buffer;
+  llvm::IntrusiveRefCntPtr m_fs;
+
+  CppModuleConfigurationTest()
+  : m_empty_buffer("", ""),
+m_fs(new llvm::vfs::InMemoryFileSystem()) {}
+
+  void SetUp() override {
+FileSystem::Initialize(m_fs);
+HostInfo::Initialize();
+  }
+
+  void TearDown() override {
+HostInfo::Terminate();
+FileSystem::Terminate();
+  }
+
+  /// Utility function turning a list of paths into a FileSpecList.
+  FileSpecList makeFiles(llvm::ArrayRef paths) {
+FileSpecList result;
+for (const std::string &path : paths) {
+  result.Append(FileSpec(path, FileSpec::Style::posix));
+  if (!m_fs->addFileNoOwn(path, static_cast(0), m_empty_buffer))
+llvm_unreachable("Invalid test configuration?");
+}
+return result;
+  }
 };
 } // namespace
 
@@ -31,20 +58,18 @@
   return std::string(resource_dir);
 }
 
-/// Utility function turningn a list of paths into a FileSpecList.
-static FileSpecList makeFiles(llvm::ArrayRef paths) {
-  FileSpecList result;
-  for (const std::string &path : paths)
-result.Append(FileSpec(path, FileSpec::Style::posix));
-  return result;
-}
 
 TEST_F(CppModuleConfigurationTest, Linux) {
   // Test the average Linux configuration.
-  std::string libcpp = "/usr/include/c++/v1";
+
   std::string usr = "/usr/include";
-  CppModuleConfiguration config(
-  makeFiles({usr + "/bits/types.h", libcpp + "/vector"}));
+  std::string libcpp = "/usr/include/c++/v1";
+  std::vector files = {// C library
+usr + "/stdio.h",
+// C++ library
+libcpp + "/vector",
+libcpp + "/module.modulemap"};
+  CppModuleConfiguration config(makeFiles(files));
   EXPECT_THAT(config.GetImportedModules(), testing::ElementsAre("std"));
   EXPECT_THAT(config.GetIncludeDirs(),
   testing::ElementsAre(libcpp, ResourceI

[Lldb-commits] [PATCH] D94890: Makefile.rules: Avoid redundant .d generation (make restart) and inline archive rule to the only test

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

This is breaking the ` functionalities/archives/TestBSDArchives.py` test on 
macOS. It seems the MAKE_DSYM flag somehow looses its effect when the dsym 
version of the test is running (and then we fail generating a dsym without 
input files): 
http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/27711/testReport/junit/lldb-api/functionalities_archives/TestBSDArchives_py/


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94890

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

[Lldb-commits] [lldb] 060b51e - [lldb] Make TestBSDArchives a no-debug-info-test

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

Author: Raphael Isemann
Date: 2021-01-21T13:06:48+01:00
New Revision: 060b51e0524aed6b6cc452baa8eb6d663a580eee

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

LOG: [lldb] Make TestBSDArchives a no-debug-info-test

The DSYM variant of this test is failing since D94890. But as we explicitly
try to disable the DSYM generation in the makefile and build the archive on
our own, I don't see why we even need to run the DSYM version of the test.

This patch disables the generated derived versions of this test for the
different debug information containers (which includes the failing DSYM one).

Added: 


Modified: 
lldb/test/API/functionalities/archives/TestBSDArchives.py

Removed: 




diff  --git a/lldb/test/API/functionalities/archives/TestBSDArchives.py 
b/lldb/test/API/functionalities/archives/TestBSDArchives.py
index 4464edd96bea..500c1763b690 100644
--- a/lldb/test/API/functionalities/archives/TestBSDArchives.py
+++ b/lldb/test/API/functionalities/archives/TestBSDArchives.py
@@ -19,6 +19,8 @@ def setUp(self):
 self.line = line_number(
 'a.c', '// Set file and line breakpoint inside a().')
 
+# Doesn't depend on any specific debug information.
+@no_debug_info_test
 @expectedFailureAll(
 oslist=["windows"],
 bugnumber="llvm.org/pr24527.  Makefile.rules doesn't know how to build 
static libs on Windows")



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

[Lldb-commits] [PATCH] D94890: Makefile.rules: Avoid redundant .d generation (make restart) and inline archive rule to the only test

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

Not sure why we even run the DSYM variant if the test disables building DSYM. I 
just made this a no-debug-info-test in 060b51e0524aed6b6cc452baa8eb6d663a580eee 
 which 
gets it running again on the bots for now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94890

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

[Lldb-commits] [lldb] ed2853d - Reland [lldb] Fix TestThreadStepOut.py after "Flush local value map on every instruction"

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

Author: Raphael Isemann
Date: 2021-01-21T13:35:13+01:00
New Revision: ed2853d2c82d7286ba510c8f65049d6f649017f0

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

LOG: Reland [lldb] Fix TestThreadStepOut.py after "Flush local value map on 
every instruction"

The original patch got reverted as a dependency of cf1c774d6ace59c5adc9ab71b31e 
.
That patch got relanded so it's also necessary to reland this patch.

Original summary:

After cf1c774d6ace59c5adc9ab71b31e762c1be695b1, Clang seems to generate code
that is more similar to icc/Clang, so we can use the same line numbers for
all compilers in this test.

Added: 


Modified: 
lldb/test/API/functionalities/thread/step_out/TestThreadStepOut.py
lldb/test/API/functionalities/thread/step_out/main.cpp

Removed: 




diff  --git 
a/lldb/test/API/functionalities/thread/step_out/TestThreadStepOut.py 
b/lldb/test/API/functionalities/thread/step_out/TestThreadStepOut.py
index e273cc4be31b..2ab36b57eaee 100644
--- a/lldb/test/API/functionalities/thread/step_out/TestThreadStepOut.py
+++ b/lldb/test/API/functionalities/thread/step_out/TestThreadStepOut.py
@@ -70,12 +70,8 @@ def setUp(self):
 self.bkpt_string = '// Set breakpoint here'
 self.breakpoint = line_number('main.cpp', self.bkpt_string)   
 
-if "gcc" in self.getCompiler() or self.isIntelCompiler():
-self.step_out_destination = line_number(
-'main.cpp', '// Expect to stop here after step-out (icc and 
gcc)')
-else:
-self.step_out_destination = line_number(
-'main.cpp', '// Expect to stop here after step-out (clang)')
+self.step_out_destination = line_number(
+ 'main.cpp', '// Expect to stop here after step-out.')
 
 def step_out_single_thread_with_cmd(self):
 self.step_out_with_cmd("this-thread")

diff  --git a/lldb/test/API/functionalities/thread/step_out/main.cpp 
b/lldb/test/API/functionalities/thread/step_out/main.cpp
index 14d84010de8a..e7dd230d239c 100644
--- a/lldb/test/API/functionalities/thread/step_out/main.cpp
+++ b/lldb/test/API/functionalities/thread/step_out/main.cpp
@@ -19,10 +19,10 @@ thread_func ()
 pseudo_barrier_wait(g_barrier);
 
 // Do something
-step_out_of_here(); // Expect to stop here after step-out (clang)
+step_out_of_here();
 
 // Return
-return NULL;  // Expect to stop here after step-out (icc and gcc)
+return NULL;  // Expect to stop here after step-out.
 }
 
 int main ()



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

[Lldb-commits] [lldb] 37510f6 - [lldb][NFC] Fix build with GCC<6

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

Author: Raphael Isemann
Date: 2021-01-21T15:04:41+01:00
New Revision: 37510f69b4cb8d76064f108d57bebe95984a23ae

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

LOG: [lldb][NFC] Fix build with GCC<6

GCC/libstdc++ before 6.1 can't handle scoped enums as unordered_map keys. LLVM
(and some build) bots officially support some GCC 5.x versions, so this patch
just makes the enum unscoped until we can require GCC 6.x.

Added: 


Modified: 
lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h

Removed: 




diff  --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h 
b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
index d24c5958204f..24c6d90694b7 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
@@ -1125,7 +1125,7 @@ class ScratchTypeSystemClang : public TypeSystemClang {
   /// These ASTs are isolated from the main scratch AST and are each
   /// dedicated to a special language option/feature that makes the contained
   /// AST nodes incompatible with other AST nodes.
-  enum class IsolatedASTKind {
+  enum IsolatedASTKind {
 /// The isolated AST for declarations/types from expressions that imported
 /// type information from a C++ module. The templates from a C++ module
 /// often conflict with the templates we generate from debug information,



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

[Lldb-commits] [PATCH] D95164: Make SBDebugger::CreateTargetWithFileAndArch accept lldb.LLDB_DEFAULT_ARCH

2021-01-21 Thread Jim Ingham via Phabricator via lldb-commits 
jingham created this revision.
jingham added reviewers: clayborg, jasonmolenda, labath.
jingham requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

The header docs in SBDebugger.i suggest:

  target = debugger.CreateTargetWithFileAndArch (exe, lldb.LLDB_ARCH_DEFAULT)

but that call doesn't actually produce a valid target.  The equivalent API - 
FindTargetWithFileAndArch does work, because it uses the current platform to 
translate LLDB_ARCH_DEFAULT.  This patch just adds that same step to 
CreateTargetWithFileAndArch, and adds a test.

This API was untested so I also added a test for this one and the similar 
CreateTargetWithFileAndTargetTriple.

I don't see anywhere where we say what the difference between an "Arch" and a 
"TargetTriple" is.  If anybody has some good verbiage for that, I'll add it to 
the docs for the API.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D95164

Files:
  lldb/source/API/SBDebugger.cpp
  lldb/test/API/python_api/target/TestTargetAPI.py


Index: lldb/test/API/python_api/target/TestTargetAPI.py
===
--- lldb/test/API/python_api/target/TestTargetAPI.py
+++ lldb/test/API/python_api/target/TestTargetAPI.py
@@ -476,3 +476,15 @@
 desc2 = get_description(symbol2)
 self.assertTrue(desc1 and desc2 and desc1 == desc2,
 "The two addresses should resolve to the same symbol")
+def test_default_arch(self):
+""" Test the other two target create methods using LLDB_ARCH_DEFAULT. 
"""
+self.build()
+exe = self.getBuildArtifact("a.out")
+target = self.dbg.CreateTargetWithFileAndArch(exe, 
lldb.LLDB_ARCH_DEFAULT)
+self.assertTrue(target.IsValid(), "Default arch made a valid target.")
+# This should also work with the target's triple:
+target2 = self.dbg.CreateTargetWithFileAndArch(exe, target.GetTriple())
+self.assertTrue(target2.IsValid(), "Round trip with triple works")
+# And this triple should work for the FileAndTriple API:
+target3 = self.dbg.CreateTargetWithFileAndTargetTriple(exe, 
target.GetTriple())
+self.assertTrue(target3.IsValid())
Index: lldb/source/API/SBDebugger.cpp
===
--- lldb/source/API/SBDebugger.cpp
+++ lldb/source/API/SBDebugger.cpp
@@ -805,11 +805,13 @@
   if (m_opaque_sp) {
 Status error;
 const bool add_dependent_modules = true;
-
+PlatformSP platform_sp = 
m_opaque_sp->GetPlatformList().GetSelectedPlatform();
+ArchSpec arch = Platform::GetAugmentedArchSpec(
+platform_sp.get(), arch_cstr);
 error = m_opaque_sp->GetTargetList().CreateTarget(
-*m_opaque_sp, filename, arch_cstr,
-add_dependent_modules ? eLoadDependentsYes : eLoadDependentsNo, 
nullptr,
-target_sp);
+*m_opaque_sp, filename, arch,
+add_dependent_modules ? eLoadDependentsYes : eLoadDependentsNo, 
+platform_sp, target_sp);
 
 if (error.Success())
   sb_target.SetSP(target_sp);


Index: lldb/test/API/python_api/target/TestTargetAPI.py
===
--- lldb/test/API/python_api/target/TestTargetAPI.py
+++ lldb/test/API/python_api/target/TestTargetAPI.py
@@ -476,3 +476,15 @@
 desc2 = get_description(symbol2)
 self.assertTrue(desc1 and desc2 and desc1 == desc2,
 "The two addresses should resolve to the same symbol")
+def test_default_arch(self):
+""" Test the other two target create methods using LLDB_ARCH_DEFAULT. """
+self.build()
+exe = self.getBuildArtifact("a.out")
+target = self.dbg.CreateTargetWithFileAndArch(exe, lldb.LLDB_ARCH_DEFAULT)
+self.assertTrue(target.IsValid(), "Default arch made a valid target.")
+# This should also work with the target's triple:
+target2 = self.dbg.CreateTargetWithFileAndArch(exe, target.GetTriple())
+self.assertTrue(target2.IsValid(), "Round trip with triple works")
+# And this triple should work for the FileAndTriple API:
+target3 = self.dbg.CreateTargetWithFileAndTargetTriple(exe, target.GetTriple())
+self.assertTrue(target3.IsValid())
Index: lldb/source/API/SBDebugger.cpp
===
--- lldb/source/API/SBDebugger.cpp
+++ lldb/source/API/SBDebugger.cpp
@@ -805,11 +805,13 @@
   if (m_opaque_sp) {
 Status error;
 const bool add_dependent_modules = true;
-
+PlatformSP platform_sp = m_opaque_sp->GetPlatformList().GetSelectedPlatform();
+ArchSpec arch = Platform::GetAugmentedArchSpec(
+platform_sp.get(), arch_cstr);
 error = m_opaque_sp->GetTargetList().CreateTarget(
-*m_opaque_sp, filename, arch_cstr,
-add_dependent_modules ? eLoadDependentsYes : eLoadDependentsNo, nullptr,
-target

[Lldb-commits] [PATCH] D93874: [process] fix exec support on Linux

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

Sorry for returning late to this diff, but I have some additional information. 
This is what's happening:

- Before the exec, the base thread plan holds a reference to the thread pointer
- During the exec, the original thread is destroyed in 
ProcessGDBRemote::SetLastStopPacket, and later a new Thread pointer is created 
for the same tid. Notice that the base thread plan is still holding a reference 
to the previous pointer even though there's a new Thread pointer for the same 
tid. On Linux, exec preserves the tid. This step is where I think there's a 
wrong invariant.
- Then, at some point, the base thread plan is asked its stop reason, and it'll 
try to use the destroyed pointer, thus crashing.

I've found three possible fixes for this (I'm pretty sure that you can come up 
with more):

- The first one is what this diff does, which is clearing all the thread plans 
once the gdbremote client knows there's an exec.
- The second one is modifying the ThreadPlan::GetThread method, which currently 
looks like this

  if (m_thread)
  return *m_thread;
  
ThreadSP thread_sp = m_process.GetThreadList().FindThreadByID(m_tid);
m_thread = thread_sp.get();
return *m_thread;

Changing it into

  if (m_thread && m_thread->IsValid()) // IsValid will return false for a 
deleted thread
  return *m_thread;
  
ThreadSP thread_sp = m_process.GetThreadList().FindThreadByID(m_tid);
m_thread = thread_sp.get();
return *m_thread;

also works, as it will make the ThreadPlan grab the new thread pointer.

- Another solution is to create a method ThreadPlan::PrepareForExec() which 
just clears the m_thread pointer in the the thread plan, and call it from 
ProcessGDBRemote::SetLastStopPacket, where we handle the exec.

I prefer the first solution, as the thread plans are all cleared in preparation 
for the new process.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93874

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

[Lldb-commits] [PATCH] D95165: Implement workaround for issue that shows between libedit and low-level terminal routines on Linux

2021-01-21 Thread Augusto Noronha via Phabricator via lldb-commits 
augusto2112 created this revision.
augusto2112 added reviewers: teemperor, aprantl, jingham, mib.
Herald added a subscriber: krytarowski.
augusto2112 requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

On some Linux distributions, after setting up the EditLine object, the eventual 
call to set_curterm, which happens when calculating terminal properties, breaks 
the libedit configuration, causing characters that have functions bound to them 
not to show up. As a workaround, we calculate these terminal properties 
eagerly, before initializing the EditLine object.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D95165

Files:
  lldb/include/lldb/Host/File.h
  lldb/source/Core/IOHandler.cpp
  lldb/source/Host/common/File.cpp


Index: lldb/source/Host/common/File.cpp
===
--- lldb/source/Host/common/File.cpp
+++ lldb/source/Host/common/File.cpp
@@ -184,6 +184,11 @@
 #endif
 }
 
+void File::EagerlyCalculateInteractiveAndTerminalProperties() {
+  if (m_supports_colors == eLazyBoolCalculate)
+CalculateInteractiveAndTerminal();
+}
+
 bool File::GetIsInteractive() {
   if (m_is_interactive == eLazyBoolCalculate)
 CalculateInteractiveAndTerminal();
Index: lldb/source/Core/IOHandler.cpp
===
--- lldb/source/Core/IOHandler.cpp
+++ lldb/source/Core/IOHandler.cpp
@@ -262,6 +262,18 @@
  m_input_sp && m_input_sp->GetIsRealTerminal();
 
   if (use_editline) {
+#if defined(__linux__)
+// On some Linux distributions, after setting up the EditLine object,
+// the eventual call to set_curterm,
+// which happens when calculating terminal properties,
+// breaks the libedit configuration, causing characters that have
+// functions bound to them not to show up. As a workaround, we calculate
+// these terminal properties eagerly, before initializing the EditLine
+// object.
+m_output_sp->GetFile().EagerlyCalculateInteractiveAndTerminalProperties();
+m_error_sp->GetFile().EagerlyCalculateInteractiveAndTerminalProperties();
+#endif
+
 m_editline_up = std::make_unique(editline_name, GetInputFILE(),
GetOutputFILE(), GetErrorFILE(),
m_color_prompts);
Index: lldb/include/lldb/Host/File.h
===
--- lldb/include/lldb/Host/File.h
+++ lldb/include/lldb/Host/File.h
@@ -327,6 +327,11 @@
   /// in lldb_private::File::Permissions.
   uint32_t GetPermissions(Status &error) const;
 
+  /// Ensures that the properties which represent if this file is interactive,
+  /// a real terminal, and a terminal with colors are calculated eagerly.
+  void EagerlyCalculateInteractiveAndTerminalProperties();
+
+
   /// Return true if this file is interactive.
   ///
   /// \return


Index: lldb/source/Host/common/File.cpp
===
--- lldb/source/Host/common/File.cpp
+++ lldb/source/Host/common/File.cpp
@@ -184,6 +184,11 @@
 #endif
 }
 
+void File::EagerlyCalculateInteractiveAndTerminalProperties() {
+  if (m_supports_colors == eLazyBoolCalculate)
+CalculateInteractiveAndTerminal();
+}
+
 bool File::GetIsInteractive() {
   if (m_is_interactive == eLazyBoolCalculate)
 CalculateInteractiveAndTerminal();
Index: lldb/source/Core/IOHandler.cpp
===
--- lldb/source/Core/IOHandler.cpp
+++ lldb/source/Core/IOHandler.cpp
@@ -262,6 +262,18 @@
  m_input_sp && m_input_sp->GetIsRealTerminal();
 
   if (use_editline) {
+#if defined(__linux__)
+// On some Linux distributions, after setting up the EditLine object,
+// the eventual call to set_curterm,
+// which happens when calculating terminal properties,
+// breaks the libedit configuration, causing characters that have
+// functions bound to them not to show up. As a workaround, we calculate
+// these terminal properties eagerly, before initializing the EditLine
+// object.
+m_output_sp->GetFile().EagerlyCalculateInteractiveAndTerminalProperties();
+m_error_sp->GetFile().EagerlyCalculateInteractiveAndTerminalProperties();
+#endif
+
 m_editline_up = std::make_unique(editline_name, GetInputFILE(),
GetOutputFILE(), GetErrorFILE(),
m_color_prompts);
Index: lldb/include/lldb/Host/File.h
===
--- lldb/include/lldb/Host/File.h
+++ lldb/include/lldb/Host/File.h
@@ -327,6 +327,11 @@
   /// in lldb_private::File::Permissions.
   uint32_t GetPermissions(Status &error) const;
 
+  /// Ensures that the properties which represent if this file is interactive,
+  /// a real terminal

[Lldb-commits] [PATCH] D95165: Implement workaround for issue that shows between libedit and low-level terminal routines on Linux

2021-01-21 Thread Augusto Noronha via Phabricator via lldb-commits 
augusto2112 added a comment.
Herald added a subscriber: JDevlieghere.

This issue was found on the Swift REPL. However, I tested @teemperor's C REPL 
patch (https://reviews.llvm.org/D87281) and the same problem shows up.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95165

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

[Lldb-commits] [PATCH] D94937: [lldb] change SBStructuredData GetStringValue signature

2021-01-21 Thread Pedro Tammela via Phabricator via lldb-commits 
tammela updated this revision to Diff 318298.
tammela added a comment.

Addressing comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94937

Files:
  lldb/bindings/lua/lua-typemaps.swig

Index: lldb/bindings/lua/lua-typemaps.swig
===
--- lldb/bindings/lua/lua-typemaps.swig
+++ lldb/bindings/lua/lua-typemaps.swig
@@ -1 +1,97 @@
 %include 
+
+// FIXME: We need to port more typemaps from Python
+
+//===--===//
+
+// In 5.3 and beyond the VM supports integers, so we need to remap
+// SWIG's internal handling to integers.
+
+
+%define LLDB_NUMBER_TYPEMAP(TYPE)
+
+// Primitive integer mapping
+%typemap(in,checkfn="lua_isinteger") TYPE
+%{ $1 = (TYPE)lua_tointeger(L, $input); %}
+%typemap(in,checkfn="lua_isinteger") const TYPE&($basetype temp)
+%{ temp=($basetype)lua_tointeger(L,$input); $1=&temp;%}
+%typemap(out) TYPE
+%{ lua_pushinteger(L, (lua_Integer) $1); SWIG_arg++;%}
+%typemap(out) const TYPE&
+%{ lua_pushinteger(L, (lua_Integer) $1); SWIG_arg++;%}
+
+// Pointer and reference mapping
+%typemap(in,checkfn="lua_isinteger") TYPE *INPUT($*ltype temp), TYPE &INPUT($*ltype temp)
+%{ temp = ($*ltype)lua_tointeger(L,$input);
+   $1 = &temp; %}
+%typemap(in, numinputs=0) TYPE *OUTPUT ($*ltype temp)
+%{ $1 = &temp; %}
+%typemap(argout) TYPE *OUTPUT
+%{  lua_pushinteger(L, (lua_Integer) *$1); SWIG_arg++;%}
+%typemap(in) TYPE *INOUT = TYPE *INPUT;
+%typemap(argout) TYPE *INOUT = TYPE *OUTPUT;
+%typemap(in) TYPE &OUTPUT = TYPE *OUTPUT;
+%typemap(argout) TYPE &OUTPUT = TYPE *OUTPUT;
+%typemap(in) TYPE &INOUT = TYPE *INPUT;
+%typemap(argout) TYPE &INOUT = TYPE *OUTPUT;
+// const version (the $*ltype is the basic number without ptr or const's)
+%typemap(in,checkfn="lua_isinteger") const TYPE *INPUT($*ltype temp)
+%{ temp = ($*ltype)lua_tointeger(L,$input);
+   $1 = &temp; %}
+
+%enddef // LLDB_NUMBER_TYPEMAP
+
+LLDB_NUMBER_TYPEMAP(unsigned char);
+LLDB_NUMBER_TYPEMAP(signed char);
+LLDB_NUMBER_TYPEMAP(short);
+LLDB_NUMBER_TYPEMAP(unsigned short);
+LLDB_NUMBER_TYPEMAP(signed short);
+LLDB_NUMBER_TYPEMAP(int);
+LLDB_NUMBER_TYPEMAP(unsigned int);
+LLDB_NUMBER_TYPEMAP(signed int);
+LLDB_NUMBER_TYPEMAP(long);
+LLDB_NUMBER_TYPEMAP(unsigned long);
+LLDB_NUMBER_TYPEMAP(signed long);
+LLDB_NUMBER_TYPEMAP(long long);
+LLDB_NUMBER_TYPEMAP(unsigned long long);
+LLDB_NUMBER_TYPEMAP(signed long long);
+
+%apply unsigned long { size_t };
+%apply const unsigned long & { const size_t & };
+%apply long { ssize_t };
+%apply const long & { const ssize_t & };
+
+//===--===//
+
+/* Typemap definitions to allow SWIG to properly handle char buffer. */
+
+// typemap for a char buffer
+%typemap(in) (char *dst, size_t dst_len) {
+   $2 = luaL_checkinteger(L, $input);
+   if ($2 <= 0) {
+   return luaL_error(L, "Positive integer expected");
+   }
+   $1 = (char *) malloc($2);
+}
+
+// SBProcess::ReadCStringFromMemory() uses a void*, but needs to be treated
+// as char data instead of byte data.
+%typemap(in) (void *char_buf, size_t size) = (char *dst, size_t dst_len);
+
+// Return the char buffer.  Discarding any previous return result
+%typemap(argout) (char *dst, size_t dst_len) {
+   lua_pop(L, 1); // Blow away the previous result
+   if ($result == 0) {
+  lua_pushliteral(L, "");
+   } else {
+  lua_pushlstring(L, (const char *)$1, $result);
+   }
+   free($1);
+   // SWIG_arg was already incremented
+}
+
+// SBProcess::ReadCStringFromMemory() uses a void*, but needs to be treated
+// as char data instead of byte data.
+%typemap(argout) (void *char_buf, size_t size) = (char *dst, size_t dst_len);
+
+//===--===//
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 39239f9 - [lldb-vscode] improve modules request

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

Author: Walter Erquinigo
Date: 2021-01-21T13:18:50-08:00
New Revision: 39239f9b5666bebb059fa562badeffb9f1c3afab

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

LOG: [lldb-vscode] improve modules request

lldb-vsdode was communicating the list of modules to the IDE with events, which 
in practice ended up having some drawbacks
- when debugging large targets, the number of these events were easily 10k, 
which polluted the messages being transmitted, which caused the following: a 
harder time debugging the messages, a lag after terminated the process because 
of these messages being processes (this could easily take several seconds). The 
latter was specially bad, as users were complaining about it even when they 
didn't check the modules view.
- these events were rarely used, as users only check the modules view when 
something is wrong and they try to debug things.

After getting some feedback from users, we realized that it's better to not 
used events but make this simply a request and is triggered by users whenever 
they needed.

This diff achieves that and does some small clean up in the existing code.

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

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py
lldb/tools/lldb-vscode/VSCode.cpp
lldb/tools/lldb-vscode/lldb-vscode.cpp

Removed: 




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 70f29cdd3d75..0962e3612929 100644
--- a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
+++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
@@ -113,7 +113,6 @@ def __init__(self, recv, send, init_commands):
 self.initialize_body = None
 self.thread_stop_reasons = {}
 self.breakpoint_events = []
-self.module_events = {}
 self.sequence = 1
 self.threads = None
 self.recv_thread.start()
@@ -134,8 +133,12 @@ def validate_response(cls, command, response):
 if command['seq'] != response['request_seq']:
 raise ValueError('seq mismatch in response')
 
-def get_active_modules(self):
-return self.module_events
+def get_modules(self):
+module_list = self.request_modules()['body']['modules']
+modules = {}
+for module in module_list:
+modules[module['name']] = module
+return modules
 
 def get_output(self, category, timeout=0.0, clear=True):
 self.output_condition.acquire()
@@ -222,14 +225,6 @@ def handle_recv_packet(self, packet):
 self.breakpoint_events.append(packet)
 # no need to add 'breakpoint' event packets to our packets list
 return keepGoing
-elif event == 'module':
-reason = body['reason']
-if (reason == 'new' or reason == 'changed'):
-self.module_events[body['module']['name']] = body['module']
-elif reason == 'removed':
-if body['module']['name'] in self.module_events:
-self.module_events.pop(body['module']['name'])
-return keepGoing
 
 elif packet_type == 'response':
 if packet['command'] == 'disconnect':
@@ -782,10 +777,10 @@ def request_setFunctionBreakpoints(self, names, 
condition=None,
 }
 return self.send_recv(command_dict)
 
-def request_getCompileUnits(self, moduleId):
+def request_compileUnits(self, moduleId):
 args_dict = {'moduleId': moduleId}
 command_dict = {
-'command': 'getCompileUnits',
+'command': 'compileUnits',
 'type': 'request',
 'arguments': args_dict
 }
@@ -804,6 +799,12 @@ def request_completions(self, text):
 }
 return self.send_recv(command_dict)
 
+def request_modules(self):
+return self.send_recv({
+'command': 'modules',
+'type': 'request'
+})
+
 def request_stackTrace(self, threadId=None, startFrame=None, levels=None,
dump=False):
 if threadId is None:

diff  --git a/lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py 
b/lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py
index 7fa5f7d45267..40ac7c4e9d19 100644
--- a/lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py
+++ b/lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py
@@ -24,7 +24,7 @@ def run_test(self, symbol_basename, expect_debug_info_size):
 breakpoint_ids = self.set_function_breakpoints(functions)
 self.assert

[Lldb-commits] [PATCH] D94033: [lldb-vscode] improve modules request

2021-01-21 Thread walter erquinigo via 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 rG39239f9b5666: [lldb-vscode] improve modules request 
(authored by wallace).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94033

Files:
  lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
  lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py
  lldb/tools/lldb-vscode/VSCode.cpp
  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
@@ -440,30 +440,6 @@
 g_vsc.SendJSON(llvm::json::Value(std::move(bp_event)));
   }
 }
-  } else if (lldb::SBTarget::EventIsTargetEvent(event)) {
-if (event_mask & lldb::SBTarget::eBroadcastBitModulesLoaded ||
-event_mask & lldb::SBTarget::eBroadcastBitModulesUnloaded ||
-event_mask & lldb::SBTarget::eBroadcastBitSymbolsLoaded) {
-  int num_modules = lldb::SBTarget::GetNumModulesFromEvent(event);
-  for (int i = 0; i < num_modules; i++) {
-auto module = lldb::SBTarget::GetModuleAtIndexFromEvent(i, event);
-auto module_event = CreateEventObject("module");
-llvm::json::Value module_value = CreateModule(module);
-llvm::json::Object body;
-if (event_mask & lldb::SBTarget::eBroadcastBitModulesLoaded) {
-  body.try_emplace("reason", "new");
-} else if (event_mask &
-   lldb::SBTarget::eBroadcastBitModulesUnloaded) {
-  body.try_emplace("reason", "removed");
-} else if (event_mask &
-   lldb::SBTarget::eBroadcastBitSymbolsLoaded) {
-  body.try_emplace("reason", "changed");
-}
-body.try_emplace("module", module_value);
-module_event.try_emplace("body", std::move(body));
-g_vsc.SendJSON(llvm::json::Value(std::move(module_event)));
-  }
-}
   } else if (event.BroadcasterMatchesRef(g_vsc.broadcaster)) {
 if (event_mask & eBroadcastBitStopEventThread) {
   done = true;
@@ -1196,26 +1172,26 @@
   g_vsc.SendJSON(llvm::json::Value(std::move(response)));
 }
 
-// "getCompileUnitsRequest": {
+// "compileUnitsRequest": {
 //   "allOf": [ { "$ref": "#/definitions/Request" }, {
 // "type": "object",
 // "description": "Compile Unit request; value of command field is
-// 'getCompileUnits'.",
+// 'compileUnits'.",
 // "properties": {
 //   "command": {
 // "type": "string",
-// "enum": [ "getCompileUnits" ]
+// "enum": [ "compileUnits" ]
 //   },
 //   "arguments": {
-// "$ref": "#/definitions/getCompileUnitRequestArguments"
+// "$ref": "#/definitions/compileUnitRequestArguments"
 //   }
 // },
 // "required": [ "command", "arguments" ]
 //   }]
 // },
-// "getCompileUnitsRequestArguments": {
+// "compileUnitsRequestArguments": {
 //   "type": "object",
-//   "description": "Arguments for 'getCompileUnits' request.",
+//   "description": "Arguments for 'compileUnits' request.",
 //   "properties": {
 // "moduleId": {
 //   "type": "string",
@@ -1224,23 +1200,21 @@
 //   },
 //   "required": [ "moduleId" ]
 // },
-// "getCompileUnitsResponse": {
+// "compileUnitsResponse": {
 //   "allOf": [ { "$ref": "#/definitions/Response" }, {
 // "type": "object",
-// "description": "Response to 'getCompileUnits' request.",
+// "description": "Response to 'compileUnits' request.",
 // "properties": {
 //   "body": {
-// "description": "Response to 'getCompileUnits' request. Array of
+// "description": "Response to 'compileUnits' request. Array of
 // paths of compile units."
 //   }
 // }
 //   }]
 // }
-
-void request_getCompileUnits(const llvm::json::Object &request) {
+void request_compileUnits(const llvm::json::Object &request) {
   llvm::json::Object response;
   FillResponse(request, response);
-  lldb::SBProcess process = g_vsc.target.GetProcess();
   llvm::json::Object body;
   llvm::json::Array units;
   auto arguments = request.getObject("arguments");
@@ -1262,6 +1236,48 @@
   g_vsc.SendJSON(llvm::json::Value(std::move(response)));
 }
 
+// "modulesRequest": {
+//   "allOf": [ { "$ref": "#/definitions/Request" }, {
+// "type": "object",
+// "description": "Modules request; value of command field is
+// 'modules'.",
+// "properties": {
+//   "command": {
+// "type": "string",
+// "enum": [ "modules" ]
+//   },
+// },
+// "required": [ "command" ]
+//   }]
+// },
+// "modulesResponse": {
+//   "allOf":

[Lldb-commits] [PATCH] D94937: [lldb/Lua] add initial Lua typemaps

2021-01-21 Thread Pedro Tammela via Phabricator via lldb-commits 
tammela updated this revision to Diff 318300.
tammela added a comment.

Typos


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94937

Files:
  lldb/bindings/lua/lua-typemaps.swig

Index: lldb/bindings/lua/lua-typemaps.swig
===
--- lldb/bindings/lua/lua-typemaps.swig
+++ lldb/bindings/lua/lua-typemaps.swig
@@ -1 +1,96 @@
 %include 
+
+// FIXME: We need to port more typemaps from Python
+
+//===--===//
+
+// In 5.3 and beyond the VM supports integers, so we need to remap
+// SWIG's internal handling of integers.
+
+
+%define LLDB_NUMBER_TYPEMAP(TYPE)
+
+// Primitive integer mapping
+%typemap(in,checkfn="lua_isinteger") TYPE
+%{ $1 = (TYPE)lua_tointeger(L, $input); %}
+%typemap(in,checkfn="lua_isinteger") const TYPE&($basetype temp)
+%{ temp=($basetype)lua_tointeger(L,$input); $1=&temp;%}
+%typemap(out) TYPE
+%{ lua_pushinteger(L, (lua_Integer) $1); SWIG_arg++;%}
+%typemap(out) const TYPE&
+%{ lua_pushinteger(L, (lua_Integer) $1); SWIG_arg++;%}
+
+// Pointer and reference mapping
+%typemap(in,checkfn="lua_isinteger") TYPE *INPUT($*ltype temp), TYPE &INPUT($*ltype temp)
+%{ temp = ($*ltype)lua_tointeger(L,$input);
+   $1 = &temp; %}
+%typemap(in, numinputs=0) TYPE *OUTPUT ($*ltype temp)
+%{ $1 = &temp; %}
+%typemap(argout) TYPE *OUTPUT
+%{  lua_pushinteger(L, (lua_Integer) *$1); SWIG_arg++;%}
+%typemap(in) TYPE *INOUT = TYPE *INPUT;
+%typemap(argout) TYPE *INOUT = TYPE *OUTPUT;
+%typemap(in) TYPE &OUTPUT = TYPE *OUTPUT;
+%typemap(argout) TYPE &OUTPUT = TYPE *OUTPUT;
+%typemap(in) TYPE &INOUT = TYPE *INPUT;
+%typemap(argout) TYPE &INOUT = TYPE *OUTPUT;
+%typemap(in,checkfn="lua_isinteger") const TYPE *INPUT($*ltype temp)
+%{ temp = ($*ltype)lua_tointeger(L,$input);
+   $1 = &temp; %}
+
+%enddef // LLDB_NUMBER_TYPEMAP
+
+LLDB_NUMBER_TYPEMAP(unsigned char);
+LLDB_NUMBER_TYPEMAP(signed char);
+LLDB_NUMBER_TYPEMAP(short);
+LLDB_NUMBER_TYPEMAP(unsigned short);
+LLDB_NUMBER_TYPEMAP(signed short);
+LLDB_NUMBER_TYPEMAP(int);
+LLDB_NUMBER_TYPEMAP(unsigned int);
+LLDB_NUMBER_TYPEMAP(signed int);
+LLDB_NUMBER_TYPEMAP(long);
+LLDB_NUMBER_TYPEMAP(unsigned long);
+LLDB_NUMBER_TYPEMAP(signed long);
+LLDB_NUMBER_TYPEMAP(long long);
+LLDB_NUMBER_TYPEMAP(unsigned long long);
+LLDB_NUMBER_TYPEMAP(signed long long);
+
+%apply unsigned long { size_t };
+%apply const unsigned long & { const size_t & };
+%apply long { ssize_t };
+%apply const long & { const ssize_t & };
+
+//===--===//
+
+// Typemap definitions to allow SWIG to properly handle char buffer.
+
+// typemap for a char buffer
+%typemap(in) (char *dst, size_t dst_len) {
+   $2 = luaL_checkinteger(L, $input);
+   if ($2 <= 0) {
+   return luaL_error(L, "Positive integer expected");
+   }
+   $1 = (char *) malloc($2);
+}
+
+// SBProcess::ReadCStringFromMemory() uses a void*, but needs to be treated
+// as char data instead of byte data.
+%typemap(in) (void *char_buf, size_t size) = (char *dst, size_t dst_len);
+
+// Return the char buffer.  Discarding any previous return result
+%typemap(argout) (char *dst, size_t dst_len) {
+   lua_pop(L, 1); // Blow away the previous result
+   if ($result == 0) {
+  lua_pushliteral(L, "");
+   } else {
+  lua_pushlstring(L, (const char *)$1, $result);
+   }
+   free($1);
+   // SWIG_arg was already incremented
+}
+
+// SBProcess::ReadCStringFromMemory() uses a void*, but needs to be treated
+// as char data instead of byte data.
+%typemap(argout) (void *char_buf, size_t size) = (char *dst, size_t dst_len);
+
+//===--===//
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D95164: Make SBDebugger::CreateTargetWithFileAndArch accept lldb.LLDB_DEFAULT_ARCH

2021-01-21 Thread Greg Clayton via Phabricator via lldb-commits 
clayborg added a comment.
Herald added a subscriber: JDevlieghere.

I think the only difference is the triple is supposed to be a complete triple 
and the arch can be just "arm64" and like you added, we let the currently 
selected platform help fill it out? I don't remember why this was added. 
Nothing in the source history?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95164

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

Re: [Lldb-commits] [PATCH] D93874: [process] fix exec support on Linux

2021-01-21 Thread Jim Ingham via lldb-commits 


> On Jan 21, 2021, at 12:51 PM, walter erquinigo via Phabricator 
>  wrote:
> 
> wallace added a comment.
> 
> Sorry for returning late to this diff, but I have some additional 
> information. This is what's happening:
> 
> - Before the exec, the base thread plan holds a reference to the thread 
> pointer
> - During the exec, the original thread is destroyed in 
> ProcessGDBRemote::SetLastStopPacket, and later a new Thread pointer is 
> created for the same tid. Notice that the base thread plan is still holding a 
> reference to the previous pointer even though there's a new Thread pointer 
> for the same tid. On Linux, exec preserves the tid. This step is where I 
> think there's a wrong invariant.
> - Then, at some point, the base thread plan is asked its stop reason, and 
> it'll try to use the destroyed pointer, thus crashing.

Thanks for looking at this some more.

Sadly, I think I'm still missing something.  Here's how I think things should 
happen:

1) The program was stopped somewhere in the original program (before exec), and 
the user continued the process
   a) WillResume is called and all the thread plans clear their cached thread 
pointers.  So they will have to look them up by ID when next asked a question.
   b) Then the process resumes.
2) We get a stop packet with a reason of "exec" from the stub.
   a) We process the stop packet, and get the new thread list
   b) We start to build the new thread list
  1) From what you are saying, in the case of exec, regardless of whether 
the TID matches an extant one or not, we make a new Thread *, put it into the 
new ThreadList and delete the old Thread *.
   c) Then we start figuring out what to do after the stop.
  i) If the TID of the new thread is the same as any of the TID's in the 
old thread list (and thus of the ThreadPlanStack that was managing it) when the 
new thread gets asked what to do, it will route the question to the 
ThreadPlanStack for that TID.
  ii) If this happens after the new thread list is in place, FindThreadByID 
will return the new, valid thread, and there should be no problem.

So for this to fail as you described, somewhere between the time that we 
stopped for exec and the time we finalized the new thread list and discarded 
the old threads, somebody must have asked the thread plan stack for that TID a 
question that triggered it to cache the old thread's Thread *.

That really shouldn't happen, precisely for the reason you see here.  When we 
stop, we have to finish setting up the current ThreadList before we start 
asking questions of the ThreadPlanStack for threads, like ShouldStop or 
whatever.  It makes no sense to ask the pre-stop thread list questions for the 
current stop, and we shouldn't do it.

Do you know where we're calling ThreadPlan::GetThread while the thread list 
still has the old threads in it?  I'd really like to stop that if we can.  And 
if you can move those queries till after the new thread list is established, 
then we shouldn't need any of the alternatives you suggest.

About the alternatives:

First one: I'd rather not artificially clear out the ThreadPlanStack after 
exec.  After all, if you knew that exec on Linux preserves the TID, you could 
write a thread plan that carried over the exec and did something interesting.  
Be a shame to prevent that for implementation reasons.

Second one: Adding the IsValid check won't do any harm, but it implicitly lets 
something happen that I don't think should happen, so it doesn't feel right...

Third one: If there's some good reason why you have to first check through the 
old thread list for stop reasons, then update the thread list, then ask again 
with no Resume in between, I think the cleanest solution is to announce that by 
explicitly clearing the ThreadPlans' cached thread pointer.  So I think this 
one's the best option.  But again, I'd like to understand why it was necessary.

Jim


> 
> I've found three possible fixes for this (I'm pretty sure that you can come 
> up with more):
> 
> - The first one is what this diff does, which is clearing all the thread 
> plans once the gdbremote client knows there's an exec.
> - The second one is modifying the ThreadPlan::GetThread method, which 
> currently looks like this
> 
>  if (m_thread)
>  return *m_thread;
> 
>ThreadSP thread_sp = m_process.GetThreadList().FindThreadByID(m_tid);
>m_thread = thread_sp.get();
>return *m_thread;
> 
> Changing it into
> 
>  if (m_thread && m_thread->IsValid()) // IsValid will return false for a 
> deleted thread
>  return *m_thread;
> 
>ThreadSP thread_sp = m_process.GetThreadList().FindThreadByID(m_tid);
>m_thread = thread_sp.get();
>return *m_thread;
> 
> also works, as it will make the ThreadPlan grab the new thread pointer.
> 
> - Another solution is to create a method ThreadPlan::PrepareForExec() which 
> just clears the m_thread pointer in the the thread plan, and call it from 
> ProcessGDBRemote::SetLastStopPacket,

Re: [Lldb-commits] [PATCH] D93874: [process] fix exec support on Linux

2021-01-21 Thread Jim Ingham via lldb-commits 


> On Jan 21, 2021, at 2:33 PM, Jim Ingham  wrote:
> 
> 
> 
>> On Jan 21, 2021, at 12:51 PM, walter erquinigo via Phabricator 
>>  wrote:
>> 
>> wallace added a comment.
>> 
>> Sorry for returning late to this diff, but I have some additional 
>> information. This is what's happening:
>> 
>> - Before the exec, the base thread plan holds a reference to the thread 
>> pointer
>> - During the exec, the original thread is destroyed in 
>> ProcessGDBRemote::SetLastStopPacket, and later a new Thread pointer is 
>> created for the same tid. Notice that the base thread plan is still holding 
>> a reference to the previous pointer even though there's a new Thread pointer 
>> for the same tid. On Linux, exec preserves the tid. This step is where I 
>> think there's a wrong invariant.
>> - Then, at some point, the base thread plan is asked its stop reason, and 
>> it'll try to use the destroyed pointer, thus crashing.
> 
> Thanks for looking at this some more.
> 
> Sadly, I think I'm still missing something.  Here's how I think things should 
> happen:
> 
> 1) The program was stopped somewhere in the original program (before exec), 
> and the user continued the process
>   a) WillResume is called and all the thread plans clear their cached thread 
> pointers.  So they will have to look them up by ID when next asked a question.
>   b) Then the process resumes.
> 2) We get a stop packet with a reason of "exec" from the stub.
>   a) We process the stop packet, and get the new thread list
>   b) We start to build the new thread list

Ack, that was confusing.  Item b) should be "We start to build the new 
ThreadList".  The object in a) is the thread list the stub tells about, either 
in the stop packet or through subsequent queries.  The object in b) is the 
ThreadList in the Process that will describe the state at this particular stop 
point.

>  1) From what you are saying, in the case of exec, regardless of whether 
> the TID matches an extant one or not, we make a new Thread *, put it into the 
> new ThreadList and delete the old Thread *.
>   c) Then we start figuring out what to do after the stop.
>  i) If the TID of the new thread is the same as any of the TID's in the 
> old thread list (and thus of the ThreadPlanStack that was managing it) when 
> the new thread gets asked what to do, it will route the question to the 
> ThreadPlanStack for that TID.
>  ii) If this happens after the new thread list is in place, 
> FindThreadByID will return the new, valid thread, and there should be no 
> problem.
> 
> So for this to fail as you described, somewhere between the time that we 
> stopped for exec and the time we finalized the new thread list and discarded 
> the old threads, somebody must have asked the thread plan stack for that TID 
> a question that triggered it to cache the old thread's Thread *.
> 
> That really shouldn't happen, precisely for the reason you see here.  When we 
> stop, we have to finish setting up the current ThreadList before we start 
> asking questions of the ThreadPlanStack for threads, like ShouldStop or 
> whatever.  It makes no sense to ask the pre-stop thread list questions for 
> the current stop, and we shouldn't do it.
> 
> Do you know where we're calling ThreadPlan::GetThread while the thread list 
> still has the old threads in it?  I'd really like to stop that if we can.  
> And if you can move those queries till after the new thread list is 
> established, then we shouldn't need any of the alternatives you suggest.
> 
> About the alternatives:
> 
> First one: I'd rather not artificially clear out the ThreadPlanStack after 
> exec.  After all, if you knew that exec on Linux preserves the TID, you could 
> write a thread plan that carried over the exec and did something interesting. 
>  Be a shame to prevent that for implementation reasons.
> 
> Second one: Adding the IsValid check won't do any harm, but it implicitly 
> lets something happen that I don't think should happen, so it doesn't feel 
> right...
> 
> Third one: If there's some good reason why you have to first check through 
> the old thread list for stop reasons, then update the thread list, then ask 
> again with no Resume in between, I think the cleanest solution is to announce 
> that by explicitly clearing the ThreadPlans' cached thread pointer.  So I 
> think this one's the best option.  But again, I'd like to understand why it 
> was necessary.
> 
> Jim
> 
> 
>> 
>> I've found three possible fixes for this (I'm pretty sure that you can come 
>> up with more):
>> 
>> - The first one is what this diff does, which is clearing all the thread 
>> plans once the gdbremote client knows there's an exec.
>> - The second one is modifying the ThreadPlan::GetThread method, which 
>> currently looks like this
>> 
>> if (m_thread)
>> return *m_thread;
>> 
>>   ThreadSP thread_sp = m_process.GetThreadList().FindThreadByID(m_tid);
>>   m_thread = thread_sp.get();
>>   return *m_thread;
>> 
>> Changing it into
>>

[Lldb-commits] [PATCH] D95164: Make SBDebugger::CreateTargetWithFileAndArch accept lldb.LLDB_DEFAULT_ARCH

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

In D95164#2513644 , @clayborg wrote:

> I think the only difference is the triple is supposed to be a complete triple 
> and the arch can be just "arm64" and like you added, we let the currently 
> selected platform help fill it out? I don't remember why this was added. 
> Nothing in the source history?

The TargetTriple doesn't seem to need to be a full triple:

>>> target = lldb.debugger.CreateTargetWithFileAndTargetTriple("/tmp/foo", 
>>> "x86_64")
>>> target.GetTriple()

'x86_64-apple-macosx11.0.0'

Looking at the FindTargetWithFileAndArch, it looks like the only difference is 
we call the Platform::GetAugmentedArchSpec in the Arch case, and that 
translates some symbolic constants (systemArch, systemArch32, systemArch64).

This code was from way before the dread reformatting and I don't feel like 
spelunking past that today...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95164

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

Re: [Lldb-commits] [PATCH] D93874: [process] fix exec support on Linux

2021-01-21 Thread Walter via lldb-commits 
I've tried to find a way to move the calls the way you mentioned, but it
doesn't seem trivial.

Some more information:
- The invocation to the thread plan is done by Thread::ShouldStop, where it
does

```
// We're starting from the base plan, so just let it decide;
if (current_plan->IsBasePlan()) {
  should_stop = current_plan->ShouldStop(event_ptr);
```

The interesting part of this call-stack starts
at Process::HandlePrivateEvent, which seems to be handling the Stop Event.
Where I think there's some inconsistent code is in
ThreadPlanBase::ShouldStop, because it tries to get the stop reason, which
fails for Linux, and then it processes eStopReasonExec a few lines below.
And see what it does:

```
  case eStopReasonExec:
  // If we crashed, discard thread plans and stop.  Don't force the
  // discard, however, since on rerun the target may clean up this
  // exception and continue normally from there.
  LLDB_LOGF(
  log,
  "Base plan discarding thread plans for thread tid = 0x%4.4" PRIx64
  " (exec.)",
  m_tid);
  GetThread().DiscardThreadPlans(false);
  return true;
```

It does discard the thread plans for that thread! This makes me believe
that it should be fine to delete the thread plans in the first place. I
wasn't able to figure out more, but I can dig deeper if you give me a few
pointers. In any case, this last code of block makes me believe that
deleting the thread plans or reseting the thread pointer in the base thread
plan might be fine.


Btw, this is a bunch of logs I got, which start at a breakpoint, then
there's a "continue", which triggers what I'm saying. The backtrace is below

(lldb) bt
* thread #1, name = 'runner', stop reason = instruction step into
  * frame #0: 0x7f1fe674838d libc.so.6`raise + 61
frame #1: 0x00400a3c runner`main(argc=2,
argv=0x7fff4b78fc08) at runner.cpp:20
frame #2: 0x7f1fe6734555 libc.so.6`__libc_start_main + 245
frame #3: 0x00400919 runner`_start + 41
(lldb) thread plan list /// See that
only the base plan is there
thread #1: tid = 0x2b72f1:
  Active plan stack:
Element 0: Base thread plan.
  Completed plan stack:
Element 0: Stepping one instruction past 0x7f1fe6748387 stepping
into calls
Element 1: Stepping one instruction past 0x7f1fe6748387 stepping
into calls
(lldb) c
lldb Process::Resume -- locking run lock
lldb Process::PrivateResume() m_stop_id = 3, public state:
stopped private state: stopped
lldb WillResume Thread #1 (0x0x7fcd3da0): tid = 0x2b72f1,
pc = 0x7f1fe674838d, sp = 0x7fff4b78fad8, fp = 0x7fff4b78fb20, plan = 'base
plan', state = running, stop others = 0
lldb Resuming thread: 2b72f1 with state: running.
lldb 0x4e7020
Listener::Listener('gdb-remote.resume-packet-sent')
lldb 0x4e7020 Listener::StartListeningForEvents (broadcaster =
0x4787a8, mask = 0x0001) acquired_mask = 0x0001 for
gdb-remote.resume-packet-sent
lldb 0x4e7020 Listener::StartListeningForEvents (broadcaster =
0x478ff8, mask = 0x0004) acquired_mask = 0x0004 for
gdb-remote.resume-packet-sent
lldb 0x494ab0
Broadcaster("lldb.process.gdb-remote.async-broadcaster")::BroadcastEvent
(event_sp = {0x7fcd40007cd0 Event: broadcaster = 0x478ff8
(lldb.process.gdb-remote.async-broadcaster), type = 0x0001 (async
thread continue), data = {"c"}}, unique =0) hijack = (nil)
lldb 0x494bf0
Listener('lldb.process.gdb-remote.async-listener')::AddEvent (event_sp =
{0x7fcd40007cd0})
lldb this = 0x004E7020, timeout = 500 us for
gdb-remote.resume-packet-sent
b-remote.async>  0x494bf0 'lldb.process.gdb-remote.async-listener'
Listener::FindNextEventInternal(broadcaster=(nil),
broadcaster_names=(nil)[0], event_type_mask=0x, remove=1) event
0x7fcd40007cd0
b-remote.async>  Process::SetPrivateState (running)
b-remote.async>  0x483f30
Broadcaster("lldb.process.internal_state_broadcaster")::BroadcastEvent
(event_sp = {0x7fcd40005380 Event: broadcaster = 0x477dd0
(lldb.process.internal_state_broadcaster), type = 0x0001, data = {
process = 0x477ce0 (pid = 2847473), state = running}}, unique =0) hijack =
(nil)
b-remote.async>  0x4841b0
Listener('lldb.process.internal_state_listener')::AddEvent (event_sp =
{0x7fcd40005380})
b-remote.async>  0x485cf0 Broadcaster("gdb-remote.client")::BroadcastEvent
(event_sp = {0x7fcd40005500 Event: broadcaster = 0x4787a8
(gdb-remote.client), type = 0x0001, data = }, unique =0) hijack =
(nil)
b-remote.async>  0x4e7020
Listener('gdb-remote.resume-packet-sent')::AddEvent (event_sp =
{0x7fcd40005500})
intern-state 0x4841b0 'lldb.process.internal_state_listener'
Listener::FindNextEventInternal(broadcaster=(nil),
broadcaster_names=(nil)[0], event_type_mask=0x, remove=1) event
0x7fcd40005380
lldb 0x4e7020 'gdb-remote.resume-packet-sent'
Listener::Fi

[Lldb-commits] [PATCH] D95185: lldb: repair the standalone build for Windows

2021-01-21 Thread Saleem Abdulrasool via Phabricator via lldb-commits 
compnerd created this revision.
compnerd added a reviewer: JDevlieghere.
compnerd added a project: LLDB.
Herald added a subscriber: mgorny.
compnerd requested review of this revision.
Herald added a subscriber: lldb-commits.

The previous code path only happened to work incidentally.  The
`file(MAKE_DIRECTORY)` is executed early, without the generator
expression being evaluated.  The result is that the literal value is
being treated as a path.  However, on Windows `:` is not a valid
character for a file name.  This would cause the operation to fail.  The
subsequent commands are delayed until runtime, and the operations will
expand the value at generation time yielding the correct result.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D95185

Files:
  lldb/source/API/CMakeLists.txt


Index: lldb/source/API/CMakeLists.txt
===
--- lldb/source/API/CMakeLists.txt
+++ lldb/source/API/CMakeLists.txt
@@ -207,13 +207,12 @@
   # When building the LLDB framework, this isn't necessary as there we copy 
everything we need into
   # the framework (including the Clang resourece directory).
   if(NOT LLDB_BUILD_FRAMEWORK)
-set(LLDB_CLANG_RESOURCE_DIR_PARENT "$/clang")
-file(MAKE_DIRECTORY "${LLDB_CLANG_RESOURCE_DIR_PARENT}")
+get_target_property(liblldb_TARGET_FILE_DIR liblldb TARGET_FILE_DIR)
+file(MAKE_DIRECTORY "${liblldb_TARGET_FILE_DIR}/clang")
 add_custom_command(TARGET liblldb POST_BUILD
-  COMMENT "Linking Clang resource dir into LLDB build directory: 
${LLDB_CLANG_RESOURCE_DIR_PARENT}"
-  COMMAND ${CMAKE_COMMAND} -E make_directory 
"${LLDB_CLANG_RESOURCE_DIR_PARENT}"
-  COMMAND ${CMAKE_COMMAND} -E create_symlink 
"${LLDB_EXTERNAL_CLANG_RESOURCE_DIR}"
-  
"${LLDB_CLANG_RESOURCE_DIR_PARENT}/${LLDB_CLANG_RESOURCE_DIR_NAME}"
+  COMMENT "Linking Clang resource dir into LLDB build directory: 
${liblldb_TARGET_FILE_DIR}/clang"
+  COMMAND ${CMAKE_COMMAND} -E make_directory 
"${liblldb_TARGET_FILE_DIR}/clang"
+  COMMAND ${CMAKE_COMMAND} -E create_symlink 
"${LLDB_EXTERNAL_CLANG_RESOURCE_DIR}" 
"${liblldb_TARGET_FILE_DIR}/clang/${LLDB_CLANG_RESOURCE_DIR_NAME}"
 )
   endif()
 endif()


Index: lldb/source/API/CMakeLists.txt
===
--- lldb/source/API/CMakeLists.txt
+++ lldb/source/API/CMakeLists.txt
@@ -207,13 +207,12 @@
   # When building the LLDB framework, this isn't necessary as there we copy everything we need into
   # the framework (including the Clang resourece directory).
   if(NOT LLDB_BUILD_FRAMEWORK)
-set(LLDB_CLANG_RESOURCE_DIR_PARENT "$/clang")
-file(MAKE_DIRECTORY "${LLDB_CLANG_RESOURCE_DIR_PARENT}")
+get_target_property(liblldb_TARGET_FILE_DIR liblldb TARGET_FILE_DIR)
+file(MAKE_DIRECTORY "${liblldb_TARGET_FILE_DIR}/clang")
 add_custom_command(TARGET liblldb POST_BUILD
-  COMMENT "Linking Clang resource dir into LLDB build directory: ${LLDB_CLANG_RESOURCE_DIR_PARENT}"
-  COMMAND ${CMAKE_COMMAND} -E make_directory "${LLDB_CLANG_RESOURCE_DIR_PARENT}"
-  COMMAND ${CMAKE_COMMAND} -E create_symlink "${LLDB_EXTERNAL_CLANG_RESOURCE_DIR}"
-  "${LLDB_CLANG_RESOURCE_DIR_PARENT}/${LLDB_CLANG_RESOURCE_DIR_NAME}"
+  COMMENT "Linking Clang resource dir into LLDB build directory: ${liblldb_TARGET_FILE_DIR}/clang"
+  COMMAND ${CMAKE_COMMAND} -E make_directory "${liblldb_TARGET_FILE_DIR}/clang"
+  COMMAND ${CMAKE_COMMAND} -E create_symlink "${LLDB_EXTERNAL_CLANG_RESOURCE_DIR}" "${liblldb_TARGET_FILE_DIR}/clang/${LLDB_CLANG_RESOURCE_DIR_NAME}"
 )
   endif()
 endif()
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D95185: lldb: repair the standalone build for Windows

2021-01-21 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere accepted this revision.
JDevlieghere added a comment.
This revision is now accepted and ready to land.

LGTM but I'd like @teemperor to have a look too as he authored the original 
change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95185

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

Re: [Lldb-commits] [PATCH] D93874: [process] fix exec support on Linux

2021-01-21 Thread Jim Ingham via lldb-commits 
The ThreadPlanStack always has one base thread plan.  You can't get rid of that 
one, the system relies on its always being there.  Despite it's name, 
DiscardThreadPlans doesn't actually discard all the thread plans, just all but 
the base thread plan...  So I think that's neither here nor there...

The thing I want to see is the sequence of the events "Make the new thread list 
after stopping for exec" and "Call ThreadPlan::GetThread()" the first time 
after the stop for the exec where we go from no cached thread to caching the 
thread from the old thread list.

The former is ProcessGDBRemote::UpdateThreadList.

You can figure out the latter by running the debugger on an lldb that is 
debugging a program that exec's.  Have the inferior lldb drive its inferior to 
the state just before the exec.  In the superior lldb, set a breakpoint on 
ThreadPlan::GetThread().  Keep continuing in the inferior lldb if you have to 
till you see it get the stop packet that corresponds to the exec.  At this 
point all the thread plans should have no cached threads, so they are going to 
have to look them up.  Keep going from there to the first time that 
ThreadPlan::GetThread is called and has to look up the thread with 
FindThreadByID call.  If I understand what you are describing, that first call 
should pull up the old thread pointer and cache it.  

If that happens before the call to ProcessGDBRemote::UpdateThreadList finishes 
that would be bad, and we have to stop it doing that.  If what's going on is 
that we set up the new thread list, broadcast the event, and then change the 
thread list again in response to detecting that this is an exec, that would 
also be bad.  We really should have a finalized thread list for the stop before 
we start trying to figure out what to do with the stop.


Jim


 

> On Jan 21, 2021, at 4:28 PM, Walter  wrote:
> 
> I've tried to find a way to move the calls the way you mentioned, but it 
> doesn't seem trivial.
> 
> Some more information:
> - The invocation to the thread plan is done by Thread::ShouldStop, where it 
> does
> 
> ```
> // We're starting from the base plan, so just let it decide;
> if (current_plan->IsBasePlan()) {
>   should_stop = current_plan->ShouldStop(event_ptr);
> ```
> 
> The interesting part of this call-stack starts at 
> Process::HandlePrivateEvent, which seems to be handling the Stop Event. Where 
> I think there's some inconsistent code is in ThreadPlanBase::ShouldStop, 
> because it tries to get the stop reason, which fails for Linux, and then it 
> processes eStopReasonExec a few lines below. And see what it does:
> 
> ```
>   case eStopReasonExec:
>   // If we crashed, discard thread plans and stop.  Don't force the
>   // discard, however, since on rerun the target may clean up this
>   // exception and continue normally from there.
>   LLDB_LOGF(
>   log,
>   "Base plan discarding thread plans for thread tid = 0x%4.4" PRIx64
>   " (exec.)",
>   m_tid);
>   GetThread().DiscardThreadPlans(false);
>   return true;
> ```
> 
> It does discard the thread plans for that thread! This makes me believe that 
> it should be fine to delete the thread plans in the first place. I wasn't 
> able to figure out more, but I can dig deeper if you give me a few pointers. 
> In any case, this last code of block makes me believe that deleting the 
> thread plans or reseting the thread pointer in the base thread plan might be 
> fine.
> 
> 
> Btw, this is a bunch of logs I got, which start at a breakpoint, then there's 
> a "continue", which triggers what I'm saying. The backtrace is below
> 
> (lldb) bt
> * thread #1, name = 'runner', stop reason = instruction step into
>   * frame #0: 0x7f1fe674838d libc.so.6`raise + 61
> frame #1: 0x00400a3c runner`main(argc=2, argv=0x7fff4b78fc08) 
> at runner.cpp:20
> frame #2: 0x7f1fe6734555 libc.so.6`__libc_start_main + 245
> frame #3: 0x00400919 runner`_start + 41
> (lldb) thread plan list /// See that only 
> the base plan is there
> thread #1: tid = 0x2b72f1:
>   Active plan stack:
> Element 0: Base thread plan.
>   Completed plan stack:
> Element 0: Stepping one instruction past 0x7f1fe6748387 stepping into 
> calls
> Element 1: Stepping one instruction past 0x7f1fe6748387 stepping into 
> calls
> (lldb) c
> lldb Process::Resume -- locking run lock
> lldb Process::PrivateResume() m_stop_id = 3, public state: 
> stopped private state: stopped
> lldb WillResume Thread #1 (0x0x7fcd3da0): tid = 0x2b72f1, pc 
> = 0x7f1fe674838d, sp = 0x7fff4b78fad8, fp = 0x7fff4b78fb20, plan = 'base 
> plan', state = running, stop others = 0
> lldb Resuming thread: 2b72f1 with state: running.
> lldb 0x4e7020 Listener::Listener('gdb-remote.resume-packet-sent')
> lldb 0x4e7020 Listener::StartListeningForEvents (broadcaste

[Lldb-commits] [PATCH] D94888: [lldb] Add -Wl, -rpath to make tests run with fresh built libc++

2021-01-21 Thread Fangrui Song via Phabricator via lldb-commits 
MaskRay updated this revision to Diff 318414.
MaskRay marked an inline comment as done.
MaskRay added a comment.

Adopt rupprecht's suggestion


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94888

Files:
  lldb/packages/Python/lldbsuite/test/dotest.py
  lldb/packages/Python/lldbsuite/test/make/Makefile.rules


Index: lldb/packages/Python/lldbsuite/test/make/Makefile.rules
===
--- lldb/packages/Python/lldbsuite/test/make/Makefile.rules
+++ lldb/packages/Python/lldbsuite/test/make/Makefile.rules
@@ -279,11 +279,6 @@
 LD = $(CC)
 LDFLAGS ?= $(CFLAGS)
 LDFLAGS += $(LD_EXTRAS) $(ARCH_LDFLAGS)
-ifneq (,$(LLVM_LIBS_DIR))
-   ifeq ($(OS),NetBSD)
-   LDFLAGS += -L$(LLVM_LIBS_DIR) -Wl,-rpath,$(LLVM_LIBS_DIR)
-   endif
-endif
 ifeq (,$(filter $(OS), Windows_NT Android Darwin))
ifneq (,$(filter YES,$(ENABLE_THREADS)))
LDFLAGS += -pthread
@@ -393,21 +388,18 @@
 
 ifeq (1,$(USE_LIBCPP))
CXXFLAGS += -DLLDB_USING_LIBCPP
-   ifeq "$(OS)" "Linux"
-   ifneq (,$(findstring clang,$(CC)))
-   CXXFLAGS += -stdlib=libc++
-   LDFLAGS += -stdlib=libc++
-   else
-   CXXFLAGS += -isystem /usr/include/c++/v1
-   LDFLAGS += -lc++
-   endif
-   else ifeq "$(OS)" "Android"
+   ifeq "$(OS)" "Android"
# Nothing to do, this is already handled in
# Android.rules.
else
CXXFLAGS += -stdlib=libc++
LDFLAGS += -stdlib=libc++
endif
+   ifneq (,$(filter $(OS), FreeBSD Linux NetBSD))
+   ifneq (,$(LLVM_LIBS_DIR))
+   LDFLAGS += -Wl,-rpath,$(LLVM_LIBS_DIR)
+   endif
+   endif
 endif
 
 #--
Index: lldb/packages/Python/lldbsuite/test/dotest.py
===
--- lldb/packages/Python/lldbsuite/test/dotest.py
+++ lldb/packages/Python/lldbsuite/test/dotest.py
@@ -761,15 +761,15 @@
 return True, "libc++ always present"
 
 if platform == "linux":
-if os.path.isdir("/usr/include/c++/v1"):
-return True, "Headers found, let's hope they work"
 with tempfile.NamedTemporaryFile() as f:
 cmd = [configuration.compiler, "-xc++", "-stdlib=libc++", "-o", 
f.name, "-"]
 p = subprocess.Popen(cmd, stdin=subprocess.PIPE, 
stdout=subprocess.PIPE, stderr=subprocess.PIPE, universal_newlines=True)
 _, stderr = p.communicate("#include \nint main() {}")
-if not p.returncode:
-return True, "Compiling with -stdlib=libc++ works"
-return False, "Compiling with -stdlib=libc++ fails with the error: 
%s" % stderr
+if p.returncode != 0:
+return False, "Compiling with -stdlib=libc++ fails with the 
error: %s" % stderr
+if subprocess.call([f.name]) != 0:
+return False, "Compiling with -stdlib=libc++ works, but fails 
to run"
+return True, "Compiling with -stdlib=libc++ works"
 
 return False, "Don't know how to build with libc++ on %s" % platform
 


Index: lldb/packages/Python/lldbsuite/test/make/Makefile.rules
===
--- lldb/packages/Python/lldbsuite/test/make/Makefile.rules
+++ lldb/packages/Python/lldbsuite/test/make/Makefile.rules
@@ -279,11 +279,6 @@
 LD = $(CC)
 LDFLAGS ?= $(CFLAGS)
 LDFLAGS += $(LD_EXTRAS) $(ARCH_LDFLAGS)
-ifneq (,$(LLVM_LIBS_DIR))
-	ifeq ($(OS),NetBSD)
-		LDFLAGS += -L$(LLVM_LIBS_DIR) -Wl,-rpath,$(LLVM_LIBS_DIR)
-	endif
-endif
 ifeq (,$(filter $(OS), Windows_NT Android Darwin))
 	ifneq (,$(filter YES,$(ENABLE_THREADS)))
 		LDFLAGS += -pthread
@@ -393,21 +388,18 @@
 
 ifeq (1,$(USE_LIBCPP))
 	CXXFLAGS += -DLLDB_USING_LIBCPP
-	ifeq "$(OS)" "Linux"
-		ifneq (,$(findstring clang,$(CC)))
-			CXXFLAGS += -stdlib=libc++
-			LDFLAGS += -stdlib=libc++
-		else
-			CXXFLAGS += -isystem /usr/include/c++/v1
-			LDFLAGS += -lc++
-		endif
-	else ifeq "$(OS)" "Android"
+	ifeq "$(OS)" "Android"
 		# Nothing to do, this is already handled in
 		# Android.rules.
 	else
 		CXXFLAGS += -stdlib=libc++
 		LDFLAGS += -stdlib=libc++
 	endif
+	ifneq (,$(filter $(OS), FreeBSD Linux NetBSD))
+		ifneq (,$(LLVM_LIBS_DIR))
+			LDFLAGS += -Wl,-rpath,$(LLVM_LIBS_DIR)
+		endif
+	endif
 endif
 
 #--
Index: lldb/packages/Python/lldbsuite/test/dotest.py
===
--- lldb/packages/Python/lldbsuite/test/dotest.py
+++ lldb/packages/Python/lldbsuite/test/dotest.py
@@ -761,15 +761,15 @@
 return True, "libc++ always present"
 
 if platform == "linux":