[Lldb-commits] [lldb] 96a7359 - [lldb] Add support for demangling D symbols

[Raphael Isemann via lldb-commits]

Author: Luís Ferreira
Date: 2021-11-11T11:11:21+01:00
New Revision: 96a7359908397d8db3ac6f8e10fb9f6dc5756a44

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

LOG: [lldb] Add support for demangling D symbols

This is part of https://github.com/dlang/projects/issues/81 .

This patch enables support for D programming language demangler by using a
pretty printed stacktrace with demangled D symbols, when present.

Signed-off-by: Luís Ferreira 

Reviewed By: JDevlieghere, teemperor

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

Added: 


Modified: 
lldb/include/lldb/Core/Mangled.h
lldb/source/Core/Mangled.cpp
lldb/source/Symbol/Symtab.cpp
lldb/unittests/Core/MangledTest.cpp

Removed: 




diff  --git a/lldb/include/lldb/Core/Mangled.h 
b/lldb/include/lldb/Core/Mangled.h
index d11d13b63cfcf..c0542157f85db 100644
--- a/lldb/include/lldb/Core/Mangled.h
+++ b/lldb/include/lldb/Core/Mangled.h
@@ -44,7 +44,8 @@ class Mangled {
 eManglingSchemeNone = 0,
 eManglingSchemeMSVC,
 eManglingSchemeItanium,
-eManglingSchemeRustV0
+eManglingSchemeRustV0,
+eManglingSchemeD
   };
 
   /// Default constructor.

diff  --git a/lldb/source/Core/Mangled.cpp b/lldb/source/Core/Mangled.cpp
index e36d412896a93..20f4dbdb419fb 100644
--- a/lldb/source/Core/Mangled.cpp
+++ b/lldb/source/Core/Mangled.cpp
@@ -45,6 +45,9 @@ Mangled::ManglingScheme 
Mangled::GetManglingScheme(llvm::StringRef const name) {
   if (name.startswith("_R"))
 return Mangled::eManglingSchemeRustV0;
 
+  if (name.startswith("_D"))
+return Mangled::eManglingSchemeD;
+
   if (name.startswith("_Z"))
 return Mangled::eManglingSchemeItanium;
 
@@ -185,6 +188,19 @@ static char *GetRustV0DemangledStr(const char *M) {
   return demangled_cstr;
 }
 
+static char *GetDLangDemangledStr(const char *M) {
+  char *demangled_cstr = llvm::dlangDemangle(M);
+
+  if (Log *log = lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_DEMANGLE)) 
{
+if (demangled_cstr && demangled_cstr[0])
+  LLDB_LOG(log, "demangled dlang: {0} -> \"{1}\"", M, demangled_cstr);
+else
+  LLDB_LOG(log, "demangled dlang: {0} -> error: failed to demangle", M);
+  }
+
+  return demangled_cstr;
+}
+
 // Explicit demangling for scheduled requests during batch processing. This
 // makes use of ItaniumPartialDemangler's rich demangle info
 bool Mangled::DemangleWithRichManglingInfo(
@@ -244,7 +260,8 @@ bool Mangled::DemangleWithRichManglingInfo(
   }
 
   case eManglingSchemeRustV0:
-// Rich demangling scheme is not supported for Rust
+  case eManglingSchemeD:
+// Rich demangling scheme is not supported
 return false;
   }
   llvm_unreachable("Fully covered switch above!");
@@ -278,6 +295,9 @@ ConstString Mangled::GetDemangledName() const {
   case eManglingSchemeRustV0:
 demangled_name = GetRustV0DemangledStr(mangled_name);
 break;
+  case eManglingSchemeD:
+demangled_name = GetDLangDemangledStr(mangled_name);
+break;
   case eManglingSchemeNone:
 llvm_unreachable("eManglingSchemeNone was handled already");
   }

diff  --git a/lldb/source/Symbol/Symtab.cpp b/lldb/source/Symbol/Symtab.cpp
index 69887034a9fb0..19c1fee2bb381 100644
--- a/lldb/source/Symbol/Symtab.cpp
+++ b/lldb/source/Symbol/Symtab.cpp
@@ -248,10 +248,8 @@ static bool lldb_skip_name(llvm::StringRef mangled,
 
   // No filters for this scheme yet. Include all names in indexing.
   case Mangled::eManglingSchemeMSVC:
-return false;
-
-  // No filters for this scheme yet. Include all names in indexing.
   case Mangled::eManglingSchemeRustV0:
+  case Mangled::eManglingSchemeD:
 return false;
 
   // Don't try and demangle things we can't categorize.

diff  --git a/lldb/unittests/Core/MangledTest.cpp 
b/lldb/unittests/Core/MangledTest.cpp
index 431993fccb1a6..4c1bb0cc45c27 100644
--- a/lldb/unittests/Core/MangledTest.cpp
+++ b/lldb/unittests/Core/MangledTest.cpp
@@ -72,6 +72,24 @@ TEST(MangledTest, EmptyForInvalidRustV0Name) {
   EXPECT_STREQ("", the_demangled.GetCString());
 }
 
+TEST(MangledTest, ResultForValidDLangName) {
+  ConstString mangled_name("_Dmain");
+  Mangled the_mangled(mangled_name);
+  ConstString the_demangled = the_mangled.GetDemangledName();
+
+  ConstString expected_result("D main");
+  EXPECT_STREQ(expected_result.GetCString(), the_demangled.GetCString());
+}
+
+TEST(MangledTest, EmptyForInvalidDLangName) {
+  ConstString mangled_name("_DDD");
+  Mangled the_mangled(mangled_name);
+  ConstString the_demangled = the_mangled.GetDemangledName();
+
+  EXPECT_STREQ("", the_demangled.GetCString());
+}
+
+
 TEST(MangledTest, NameIndexes_FindFunctionSymbols) {
   SubsystemRAII
   subsystems;



___
lldb-commits mai

[Lldb-commits] [PATCH] D110578: [lldb] Add support for D programming language

[Raphael Isemann via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG96a735990839: [lldb] Add support for demangling D symbols 
(authored by ljmf00, committed by teemperor).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110578

Files:
  lldb/include/lldb/Core/Mangled.h
  lldb/source/Core/Mangled.cpp
  lldb/source/Symbol/Symtab.cpp
  lldb/unittests/Core/MangledTest.cpp

Index: lldb/unittests/Core/MangledTest.cpp
===
--- lldb/unittests/Core/MangledTest.cpp
+++ lldb/unittests/Core/MangledTest.cpp
@@ -72,6 +72,24 @@
   EXPECT_STREQ("", the_demangled.GetCString());
 }
 
+TEST(MangledTest, ResultForValidDLangName) {
+  ConstString mangled_name("_Dmain");
+  Mangled the_mangled(mangled_name);
+  ConstString the_demangled = the_mangled.GetDemangledName();
+
+  ConstString expected_result("D main");
+  EXPECT_STREQ(expected_result.GetCString(), the_demangled.GetCString());
+}
+
+TEST(MangledTest, EmptyForInvalidDLangName) {
+  ConstString mangled_name("_DDD");
+  Mangled the_mangled(mangled_name);
+  ConstString the_demangled = the_mangled.GetDemangledName();
+
+  EXPECT_STREQ("", the_demangled.GetCString());
+}
+
+
 TEST(MangledTest, NameIndexes_FindFunctionSymbols) {
   SubsystemRAII
   subsystems;
Index: lldb/source/Symbol/Symtab.cpp
===
--- lldb/source/Symbol/Symtab.cpp
+++ lldb/source/Symbol/Symtab.cpp
@@ -248,10 +248,8 @@
 
   // No filters for this scheme yet. Include all names in indexing.
   case Mangled::eManglingSchemeMSVC:
-return false;
-
-  // No filters for this scheme yet. Include all names in indexing.
   case Mangled::eManglingSchemeRustV0:
+  case Mangled::eManglingSchemeD:
 return false;
 
   // Don't try and demangle things we can't categorize.
Index: lldb/source/Core/Mangled.cpp
===
--- lldb/source/Core/Mangled.cpp
+++ lldb/source/Core/Mangled.cpp
@@ -45,6 +45,9 @@
   if (name.startswith("_R"))
 return Mangled::eManglingSchemeRustV0;
 
+  if (name.startswith("_D"))
+return Mangled::eManglingSchemeD;
+
   if (name.startswith("_Z"))
 return Mangled::eManglingSchemeItanium;
 
@@ -185,6 +188,19 @@
   return demangled_cstr;
 }
 
+static char *GetDLangDemangledStr(const char *M) {
+  char *demangled_cstr = llvm::dlangDemangle(M);
+
+  if (Log *log = lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_DEMANGLE)) {
+if (demangled_cstr && demangled_cstr[0])
+  LLDB_LOG(log, "demangled dlang: {0} -> \"{1}\"", M, demangled_cstr);
+else
+  LLDB_LOG(log, "demangled dlang: {0} -> error: failed to demangle", M);
+  }
+
+  return demangled_cstr;
+}
+
 // Explicit demangling for scheduled requests during batch processing. This
 // makes use of ItaniumPartialDemangler's rich demangle info
 bool Mangled::DemangleWithRichManglingInfo(
@@ -244,7 +260,8 @@
   }
 
   case eManglingSchemeRustV0:
-// Rich demangling scheme is not supported for Rust
+  case eManglingSchemeD:
+// Rich demangling scheme is not supported
 return false;
   }
   llvm_unreachable("Fully covered switch above!");
@@ -278,6 +295,9 @@
   case eManglingSchemeRustV0:
 demangled_name = GetRustV0DemangledStr(mangled_name);
 break;
+  case eManglingSchemeD:
+demangled_name = GetDLangDemangledStr(mangled_name);
+break;
   case eManglingSchemeNone:
 llvm_unreachable("eManglingSchemeNone was handled already");
   }
Index: lldb/include/lldb/Core/Mangled.h
===
--- lldb/include/lldb/Core/Mangled.h
+++ lldb/include/lldb/Core/Mangled.h
@@ -44,7 +44,8 @@
 eManglingSchemeNone = 0,
 eManglingSchemeMSVC,
 eManglingSchemeItanium,
-eManglingSchemeRustV0
+eManglingSchemeRustV0,
+eManglingSchemeD
   };
 
   /// Default constructor.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D110578: [lldb] Add support for D programming language

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added a comment.

I changed the commit name from "Add support for D programming language" to 
"[lldb] Add support for demangling D symbols" when landing (which is more 
accurate).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110578

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

[Lldb-commits] [PATCH] D113634: [lldb] Add support for DW_TAG_immutable_type

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added a comment.

Are the DWARFASTParserClang changes meant as a step towards making it parse D?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113634

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

[Lldb-commits] [PATCH] D111409: proposed support for Java interface to Scripting Bridge

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added a comment.

> Are you asking for dedicated physical resources for running nightly builds?

I don't think any of the current bots have a Java installation so I think it's 
either that or we get someone with a bot to setup the required Java 
installation.

FWIW, if no one wants to host a bot for this then I won't mind testing this in 
own CI , but I am not using buildbot so we'll have to 
see if that is acceptable for the community (I could also migrate it to 
buildbot, but the buildbot interface is just painful to use and I would prefer 
not to).

> Do you really have a separate build machines for the Python and Lua scripting 
> environments?

Python is more or less mandatory so all LLDB build bots have that. Lua is 
tested here: https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/

On a more general note: I haven't followed this thread very closely, but I am 
surprised that this is now adding a Java REPL to LLDB. I think the original 
"Just add Java bindings" seemed like a reasonable patch but I am not so sure 
about this. Could we split out the changes for just adding Java bindings to 
SWIG (which anyway seems like a standalone patch) and then we can talk about 
the whole Java scripting stuff in a separate review. I don't expect the first 
one to be controversial so this should also speed things up a bit.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D111409

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

[Lldb-commits] [PATCH] D112069: [lldb][AArch64] Add UnwindPlan for Linux sigreturn

[David Spickett via Phabricator via lldb-commits]

DavidSpickett updated this revision to Diff 386466.
DavidSpickett added a comment.

- Set x registers to a different pattern in the signal handler
- Don't save frames before and after, just check for the right pattern when 
unwinding out of the handler to sigill()
- Set register kind when we first make the unwind plan


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112069

Files:
  lldb/include/lldb/Target/Platform.h
  lldb/source/Plugins/Platform/Linux/PlatformLinux.cpp
  lldb/source/Plugins/Platform/Linux/PlatformLinux.h
  lldb/source/Target/RegisterContextUnwind.cpp
  lldb/test/API/functionalities/signal/handle-abrt/TestHandleAbort.py
  lldb/test/API/linux/aarch64/unwind_signal/Makefile
  lldb/test/API/linux/aarch64/unwind_signal/TestUnwindSignal.py
  lldb/test/API/linux/aarch64/unwind_signal/main.c

Index: lldb/test/API/linux/aarch64/unwind_signal/main.c
===
--- /dev/null
+++ lldb/test/API/linux/aarch64/unwind_signal/main.c
@@ -0,0 +1,64 @@
+#include 
+#include 
+#include 
+
+void handler(int sig) {
+  // The kernel only changes a few registers so set them all to something other
+  // than the values in sigill() so that we can't fall back to real registers
+  // and still pass the test.
+#define SETREG(N) "mov x" N ", #" N "+1\n\t"
+  asm volatile(
+  /* clang-format off */
+  /* x0 is used for a parameter */
+   SETREG("1")  SETREG("2")  SETREG("3")
+  SETREG("4")  SETREG("5")  SETREG("6")  SETREG("7")
+  SETREG("8")  SETREG("9")  SETREG("10") SETREG("11")
+  SETREG("12") SETREG("13") SETREG("14") SETREG("15")
+  SETREG("16") SETREG("17") SETREG("18") SETREG("19")
+  SETREG("20") SETREG("21") SETREG("22") SETREG("23")
+  SETREG("24") SETREG("25") SETREG("26") SETREG("27")
+  SETREG("28") // fp/x29 needed for unwiding
+  SETREG("30") // 31 is xzr/sp
+  /* clang-format on */
+  ::
+  : /* skipped x0 */ "x1", "x2", "x3", "x4", "x5", "x6", "x7", "x8",
+"x9", "x10", "x11", "x12", "x13", "x14", "x15", "x16", "x17", "x18",
+"x19", "x20", "x21", "x22", "x23", "x24", "x25", "x26", "x27",
+"x28",
+/* skipped fp/x29 */ "x30");
+  printf("Set a breakpoint here.\n");
+  exit(0);
+}
+
+static void sigill() {
+  // Set all general registers to known values to check
+  // that the signal unwind plan sets their locations correctly.
+#define SETREG(N) "mov x" N ", #" N "\n\t"
+  asm volatile(
+  /* clang-format off */
+  SETREG("0")  SETREG("1")  SETREG("2")  SETREG("3")
+  SETREG("4")  SETREG("5")  SETREG("6")  SETREG("7")
+  SETREG("8")  SETREG("9")  SETREG("10") SETREG("11")
+  SETREG("12") SETREG("13") SETREG("14") SETREG("15")
+  SETREG("16") SETREG("17") SETREG("18") SETREG("19")
+  SETREG("20") SETREG("21") SETREG("22") SETREG("23")
+  SETREG("24") SETREG("25") SETREG("26") SETREG("27")
+  SETREG("28") SETREG("29") SETREG("30") /* 31 is xzr/sp */
+  /* clang-format on */
+  ".inst   0x\n\t" // udf #0 (old binutils don't support udf)
+  ::
+  : "x0", "x1", "x2", "x3", "x4", "x5", "x6", "x7", "x8", "x9", "x10",
+"x11", "x12", "x13", "x14", "x15", "x16", "x17", "x18", "x19",
+"x20", "x21", "x22", "x23", "x24", "x25", "x26", "x27", "x28",
+"x29", "x30");
+}
+
+int main() {
+  if (signal(SIGILL, handler) == SIG_ERR) {
+perror("signal");
+return 1;
+  }
+
+  sigill();
+  return 2;
+}
Index: lldb/test/API/linux/aarch64/unwind_signal/TestUnwindSignal.py
===
--- lldb/test/API/linux/aarch64/unwind_signal/TestUnwindSignal.py
+++ lldb/test/API/linux/aarch64/unwind_signal/TestUnwindSignal.py
@@ -1,6 +1,5 @@
-"""Test that we can unwind out of a SIGABRT handler"""
-
-
+"""Test that we can unwind out of a signal handler.
+   Which for AArch64 Linux requires a specific unwind plan."""
 
 
 import lldb
@@ -9,33 +8,29 @@
 from lldbsuite.test import lldbutil
 
 
-class HandleAbortTestCase(TestBase):
+class UnwindSignalTestCase(TestBase):
 
 mydir = TestBase.compute_mydir(__file__)
 
 NO_DEBUG_INFO_TESTCASE = True
 
-@skipIfWindows  # signals do not exist on Windows
-# Fails on Ubuntu Focal
-@skipIf(archs=["aarch64"], oslist=["linux"])
-@expectedFailureNetBSD
-def test_inferior_handle_sigabrt(self):
-"""Inferior calls abort() and handles the resultant SIGABRT.
-   Stopped at a breakpoint in the handler, verify that the backtrace
-   includes the function that called abort()."""
+@skipUnlessArch("aarch64")
+@skipUnlessPlatform(["linux"])
+def test_unwind_signal(self):
+"""Inferior calls sigill() and handles the resultant SIGILL.
+   Stopped at a breakpoint in the handler, check that we can unwind
+   back to sigil

[Lldb-commits] [lldb] 9db2541 - [lldb][AArch64] Add UnwindPlan for Linux sigreturn

[David Spickett via lldb-commits]

Author: David Spickett
Date: 2021-11-11T11:32:06Z
New Revision: 9db2541d4c30100d7ccc6cc9db717df102b302d9

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

LOG: [lldb][AArch64] Add UnwindPlan for Linux sigreturn

This adds a specific unwind plan for AArch64 Linux sigreturn frames.
Previously we assumed that the fp would be valid here but it is not.

https://github.com/torvalds/linux/blob/master/arch/arm64/kernel/vdso/sigreturn.S

On Ubuntu Bionic it happened to point to an old frame info which meant
you got what looked like a correct backtrace. On Focal, the info is
completely invalid. (probably due to some code shuffling in libc)

This adds an UnwindPlan that knows that the sp in a sigreturn frame
points to an rt_sigframe from which we can offset to get saved
sp and pc values to backtrace correctly.

Based on LibUnwind's change: https://reviews.llvm.org/D90898

A new test is added that sets all compares the frames from the initial
signal catch to the handler break. Ensuring that the stack/frame pointer,
function name and register values match.
(this test is AArch64 Linux specific because it's the only one
with a specific unwind plan for this situation)

Fixes https://bugs.llvm.org/show_bug.cgi?id=52165

Reviewed By: omjavaid, labath

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

Added: 
lldb/test/API/linux/aarch64/unwind_signal/Makefile
lldb/test/API/linux/aarch64/unwind_signal/TestUnwindSignal.py
lldb/test/API/linux/aarch64/unwind_signal/main.c

Modified: 
lldb/include/lldb/Target/Platform.h
lldb/source/Plugins/Platform/Linux/PlatformLinux.cpp
lldb/source/Plugins/Platform/Linux/PlatformLinux.h
lldb/source/Target/RegisterContextUnwind.cpp
lldb/test/API/functionalities/signal/handle-abrt/TestHandleAbort.py

Removed: 




diff  --git a/lldb/include/lldb/Target/Platform.h 
b/lldb/include/lldb/Target/Platform.h
index 1106ce868761c..86c62140ba1ff 100644
--- a/lldb/include/lldb/Target/Platform.h
+++ b/lldb/include/lldb/Target/Platform.h
@@ -719,6 +719,24 @@ class Platform : public PluginInterface {
   /// A list of symbol names.  The list may be empty.
   virtual const std::vector &GetTrapHandlerSymbolNames();
 
+  /// Try to get a specific unwind plan for a named trap handler.
+  /// The default is not to have specific unwind plans for trap handlers.
+  ///
+  /// \param[in] triple
+  /// Triple of the current target.
+  ///
+  /// \param[in] name
+  /// Name of the trap handler function.
+  ///
+  /// \return
+  /// A specific unwind plan for that trap handler, or an empty
+  /// shared pointer. The latter means there is no specific plan,
+  /// unwind as normal.
+  virtual lldb::UnwindPlanSP
+  GetTrapHandlerUnwindPlan(const llvm::Triple &triple, ConstString name) {
+return {};
+  }
+
   /// Find a support executable that may not live within in the standard
   /// locations related to LLDB.
   ///

diff  --git a/lldb/source/Plugins/Platform/Linux/PlatformLinux.cpp 
b/lldb/source/Plugins/Platform/Linux/PlatformLinux.cpp
index 52c86d7a1328b..940ecc5c5e831 100644
--- a/lldb/source/Plugins/Platform/Linux/PlatformLinux.cpp
+++ b/lldb/source/Plugins/Platform/Linux/PlatformLinux.cpp
@@ -14,9 +14,11 @@
 #include 
 #endif
 
+#include "Utility/ARM64_DWARF_Registers.h"
 #include "lldb/Core/Debugger.h"
 #include "lldb/Core/PluginManager.h"
 #include "lldb/Host/HostInfo.h"
+#include "lldb/Symbol/UnwindPlan.h"
 #include "lldb/Target/Process.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Utility/FileSpec.h"
@@ -251,6 +253,96 @@ void PlatformLinux::CalculateTrapHandlerSymbolNames() {
   m_trap_handlers.push_back(ConstString("__restore_rt"));
 }
 
+static lldb::UnwindPlanSP GetAArch64TrapHanlderUnwindPlan(ConstString name) {
+  UnwindPlanSP unwind_plan_sp;
+  if (name != "__kernel_rt_sigreturn")
+return unwind_plan_sp;
+
+  UnwindPlan::RowSP row = std::make_shared();
+  row->SetOffset(0);
+
+  // In the signal trampoline frame, sp points to an rt_sigframe[1], which is:
+  //  - 128-byte siginfo struct
+  //  - ucontext struct:
+  // - 8-byte long (uc_flags)
+  // - 8-byte pointer (uc_link)
+  // - 24-byte stack_t
+  // - 128-byte signal set
+  // - 8 bytes of padding because sigcontext has 16-byte alignment
+  // - sigcontext/mcontext_t
+  // [1]
+  // https://github.com/torvalds/linux/blob/master/arch/arm64/kernel/signal.c
+  int32_t offset = 128 + 8 + 8 + 24 + 128 + 8;
+  // Then sigcontext[2] is:
+  // - 8 byte fault address
+  // - 31 8 byte registers
+  // - 8 byte sp
+  // - 8 byte pc
+  // [2]
+  // 
https://github.com/torvalds/linux/blob/master/arch/arm64/include/uapi/asm/sigcontext.h
+
+  // Skip fault address
+  offset += 8;
+  row->GetCFAValue().SetIsRegisterPlusOffset(arm64_dwarf::sp, offset

[Lldb-commits] [PATCH] D112069: [lldb][AArch64] Add UnwindPlan for Linux sigreturn

[David Spickett via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG9db2541d4c30: [lldb][AArch64] Add UnwindPlan for Linux 
sigreturn (authored by DavidSpickett).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112069

Files:
  lldb/include/lldb/Target/Platform.h
  lldb/source/Plugins/Platform/Linux/PlatformLinux.cpp
  lldb/source/Plugins/Platform/Linux/PlatformLinux.h
  lldb/source/Target/RegisterContextUnwind.cpp
  lldb/test/API/functionalities/signal/handle-abrt/TestHandleAbort.py
  lldb/test/API/linux/aarch64/unwind_signal/Makefile
  lldb/test/API/linux/aarch64/unwind_signal/TestUnwindSignal.py
  lldb/test/API/linux/aarch64/unwind_signal/main.c

Index: lldb/test/API/linux/aarch64/unwind_signal/main.c
===
--- /dev/null
+++ lldb/test/API/linux/aarch64/unwind_signal/main.c
@@ -0,0 +1,64 @@
+#include 
+#include 
+#include 
+
+void handler(int sig) {
+  // The kernel only changes a few registers so set them all to something other
+  // than the values in sigill() so that we can't fall back to real registers
+  // and still pass the test.
+#define SETREG(N) "mov x" N ", #" N "+1\n\t"
+  asm volatile(
+  /* clang-format off */
+  /* x0 is used for a parameter */
+   SETREG("1")  SETREG("2")  SETREG("3")
+  SETREG("4")  SETREG("5")  SETREG("6")  SETREG("7")
+  SETREG("8")  SETREG("9")  SETREG("10") SETREG("11")
+  SETREG("12") SETREG("13") SETREG("14") SETREG("15")
+  SETREG("16") SETREG("17") SETREG("18") SETREG("19")
+  SETREG("20") SETREG("21") SETREG("22") SETREG("23")
+  SETREG("24") SETREG("25") SETREG("26") SETREG("27")
+  SETREG("28") // fp/x29 needed for unwiding
+  SETREG("30") // 31 is xzr/sp
+  /* clang-format on */
+  ::
+  : /* skipped x0 */ "x1", "x2", "x3", "x4", "x5", "x6", "x7", "x8",
+"x9", "x10", "x11", "x12", "x13", "x14", "x15", "x16", "x17", "x18",
+"x19", "x20", "x21", "x22", "x23", "x24", "x25", "x26", "x27",
+"x28",
+/* skipped fp/x29 */ "x30");
+  printf("Set a breakpoint here.\n");
+  exit(0);
+}
+
+static void sigill() {
+  // Set all general registers to known values to check
+  // that the signal unwind plan sets their locations correctly.
+#define SETREG(N) "mov x" N ", #" N "\n\t"
+  asm volatile(
+  /* clang-format off */
+  SETREG("0")  SETREG("1")  SETREG("2")  SETREG("3")
+  SETREG("4")  SETREG("5")  SETREG("6")  SETREG("7")
+  SETREG("8")  SETREG("9")  SETREG("10") SETREG("11")
+  SETREG("12") SETREG("13") SETREG("14") SETREG("15")
+  SETREG("16") SETREG("17") SETREG("18") SETREG("19")
+  SETREG("20") SETREG("21") SETREG("22") SETREG("23")
+  SETREG("24") SETREG("25") SETREG("26") SETREG("27")
+  SETREG("28") SETREG("29") SETREG("30") /* 31 is xzr/sp */
+  /* clang-format on */
+  ".inst   0x\n\t" // udf #0 (old binutils don't support udf)
+  ::
+  : "x0", "x1", "x2", "x3", "x4", "x5", "x6", "x7", "x8", "x9", "x10",
+"x11", "x12", "x13", "x14", "x15", "x16", "x17", "x18", "x19",
+"x20", "x21", "x22", "x23", "x24", "x25", "x26", "x27", "x28",
+"x29", "x30");
+}
+
+int main() {
+  if (signal(SIGILL, handler) == SIG_ERR) {
+perror("signal");
+return 1;
+  }
+
+  sigill();
+  return 2;
+}
Index: lldb/test/API/linux/aarch64/unwind_signal/TestUnwindSignal.py
===
--- lldb/test/API/linux/aarch64/unwind_signal/TestUnwindSignal.py
+++ lldb/test/API/linux/aarch64/unwind_signal/TestUnwindSignal.py
@@ -1,6 +1,5 @@
-"""Test that we can unwind out of a SIGABRT handler"""
-
-
+"""Test that we can unwind out of a signal handler.
+   Which for AArch64 Linux requires a specific unwind plan."""
 
 
 import lldb
@@ -9,33 +8,29 @@
 from lldbsuite.test import lldbutil
 
 
-class HandleAbortTestCase(TestBase):
+class UnwindSignalTestCase(TestBase):
 
 mydir = TestBase.compute_mydir(__file__)
 
 NO_DEBUG_INFO_TESTCASE = True
 
-@skipIfWindows  # signals do not exist on Windows
-# Fails on Ubuntu Focal
-@skipIf(archs=["aarch64"], oslist=["linux"])
-@expectedFailureNetBSD
-def test_inferior_handle_sigabrt(self):
-"""Inferior calls abort() and handles the resultant SIGABRT.
-   Stopped at a breakpoint in the handler, verify that the backtrace
-   includes the function that called abort()."""
+@skipUnlessArch("aarch64")
+@skipUnlessPlatform(["linux"])
+def test_unwind_signal(self):
+"""Inferior calls sigill() and handles the resultant SIGILL.
+   Stopped at a breakpoint in the handler, check that we can unwind
+   back to sigill() and get the expected register contents there."""
 self.build()
 exe = self.getBuildArtifact("a.out")
 
-#

[Lldb-commits] [PATCH] D112069: [lldb][AArch64] Add UnwindPlan for Linux sigreturn

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

Thanks for the help, really helped get a high quality fix!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112069

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

[Lldb-commits] [lldb] 2a0e773 - [lldb][NFC] Remove no longer valid comment for TypeSystem::SetSymbolFile

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2021-11-11T12:39:54+01:00
New Revision: 2a0e77362e3ac1f4de1290640ad1122f9d4e208b

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

LOG: [lldb][NFC] Remove no longer valid comment for TypeSystem::SetSymbolFile

Added: 


Modified: 
lldb/include/lldb/Symbol/TypeSystem.h

Removed: 




diff  --git a/lldb/include/lldb/Symbol/TypeSystem.h 
b/lldb/include/lldb/Symbol/TypeSystem.h
index e695f6580767..be5783596897 100644
--- a/lldb/include/lldb/Symbol/TypeSystem.h
+++ b/lldb/include/lldb/Symbol/TypeSystem.h
@@ -91,7 +91,6 @@ class TypeSystem : public PluginInterface {
 
   virtual SymbolFile *GetSymbolFile() const { return m_sym_file; }
 
-  // Returns true if the symbol file changed during the set accessor.
   virtual void SetSymbolFile(SymbolFile *sym_file) { m_sym_file = sym_file; }
 
   // CompilerDecl functions



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

[Lldb-commits] [lldb] b72727a - [lldb][NFC] Remove commented out code in SymbolFileDWARF

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2021-11-11T12:45:38+01:00
New Revision: b72727a75a64df355c18b38465324cdf81893d3c

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

LOG: [lldb][NFC] Remove commented out code in SymbolFileDWARF

Added: 


Modified: 
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index 8d9b9bf86e31..2a5888038a03 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -100,24 +100,6 @@ LLDB_PLUGIN_DEFINE(SymbolFileDWARF)
 
 char SymbolFileDWARF::ID;
 
-// static inline bool
-// child_requires_parent_class_union_or_struct_to_be_completed (dw_tag_t tag)
-//{
-//switch (tag)
-//{
-//default:
-//break;
-//case DW_TAG_subprogram:
-//case DW_TAG_inlined_subroutine:
-//case DW_TAG_class_type:
-//case DW_TAG_structure_type:
-//case DW_TAG_union_type:
-//return true;
-//}
-//return false;
-//}
-//
-
 namespace {
 
 #define LLDB_PROPERTIES_symbolfiledwarf



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

[Lldb-commits] [PATCH] D111409: proposed support for Java interface to Scripting Bridge

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D111409#3124075 , @teemperor wrote:

>> Are you asking for dedicated physical resources for running nightly builds?
>
> I don't think any of the current bots have a Java installation so I think 
> it's either that or we get someone with a bot to setup the required Java 
> installation.

I don't have a problem with installing the necessary packages on the bot I 
manage, but I cannot subscribe to tracking down any failures (or flaky tests!) 
for this configuration (and, in my experience, any new feature like this is 
going to have flaky tests). Flaky tests (probably just one) are the reason that 
lua integration is not enabled on this bot.

> FWIW, if no one wants to host a bot for this then I won't mind testing this 
> in own CI , but I am not using buildbot so we'll 
> have to see if that is acceptable for the community (I could also migrate it 
> to buildbot, but the buildbot interface is just painful to use and I would 
> prefer not to).

I would rather not proliferate test infrastructures.

I'm not sure which pain points are you referring to, but setting up a buildbot 
instance is a lot simpler these days than it used to be (in particular, you 
don't need to track down and install any outdated packages).

>> Do you really have a separate build machines for the Python and Lua 
>> scripting environments?
>
> Python is more or less mandatory so all LLDB build bots have that. Lua is 
> tested here: https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/
>
> On a more general note: I haven't followed this thread very closely, but I am 
> surprised that this is now adding a Java REPL to LLDB. I think the original 
> "Just add Java bindings" seemed like a reasonable patch but I am not so sure 
> about this. Could we split out the changes for just adding Java bindings to 
> SWIG (which anyway seems like a standalone patch) and then we can talk about 
> the whole Java scripting stuff in a separate review. I don't expect the first 
> one to be controversial so this should also speed things up a bit.

+1


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D111409

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

[Lldb-commits] [PATCH] D111409: proposed support for Java interface to Scripting Bridge

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added a comment.

In D111409#3124194 , @labath wrote:

> In D111409#3124075 , @teemperor 
> wrote:
>
>>> Are you asking for dedicated physical resources for running nightly builds?
>>
>> I don't think any of the current bots have a Java installation so I think 
>> it's either that or we get someone with a bot to setup the required Java 
>> installation.
>
> I don't have a problem with installing the necessary packages on the bot I 
> manage, but I cannot subscribe to tracking down any failures (or flaky 
> tests!) for this configuration (and, in my experience, any new feature like 
> this is going to have flaky tests). Flaky tests (probably just one) are the 
> reason that lua integration is not enabled on this bot.

Sure, I think it should anyway not be up to any bot owner to track down flaky 
tests. And that nested_sessions lua test is anyway randomly failing everywhere 
from what I know.

>> FWIW, if no one wants to host a bot for this then I won't mind testing this 
>> in own CI , but I am not using buildbot so we'll 
>> have to see if that is acceptable for the community (I could also migrate it 
>> to buildbot, but the buildbot interface is just painful to use and I would 
>> prefer not to).
>
> I would rather not proliferate test infrastructures.
>
> I'm not sure which pain points are you referring to, but setting up a 
> buildbot instance is a lot simpler these days than it used to be (in 
> particular, you don't need to track down and install any outdated packages).

It's not the setup, it's just that the lab.llvm.org interface is far less 
usable than Jenkins for tracking down regressions in tests (and Jenkins is 
already not great, so that says something). But that's just my personal 
preference and I agree that lab.llvm.org should be the central place for 
infrastructure.

Anyway, if @labath can run them on his fancy build bot then I would prefer that 
over having it on my bot (because I pay out of my own pocket and CPU cycles are 
$$$).


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D111409

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

[Lldb-commits] [PATCH] D113521: Allow lldb to launch a remote executable when there isn't a local copy

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D113521#3122654 , @jingham wrote:

> In D113521#3120703 , @labath wrote:
>
>> I have two high-level questions about this:
>>
>> - what should be the relative priority of executable ModuleSP vs the launch 
>> info? IIUC, in the current implementation the module takes precedence, but 
>> it seems to me it would be more natural if the launch info came out on top. 
>> One of the reasons I'm thinking about this is because you currently cannot 
>> re-run executables that use exec(2) (I mean, you can, but it will rarely 
>> produce the desired results). This happens because we use the post-exec 
>> executable, but the rest of the launch info refers to the main one. If the 
>> executable module was not the primary source of truth here, then maybe the 
>> we could somehow use this mechanism to improve the exec story as well (by 
>> storing the original executable in the launch info or something).



> Anyway, I think you are asking the question "What should we do if the 
> Target's ProcessLaunchInfo::GetExecutableFile differs from the one in the 
> Target's Executable Module".  Or rather, should we keep the Target's 
> ProcessLaunchInfo as the truth of what that target wants to launch, rather 
> than looking at the Executable module.
>
> That question is orthogonal to the one this patch is determining, which is 
> just about the case where there isn't an executable file in the target so 
> that the user needs to provide this info from the outside.  So while I agree 
> that yours is an interesting question, it isn't directly germane to this 
> patch.

Yes, that is the question I am asking. I didn't want to go off into designing 
an exec feature. I only mentioned because it seemed potentially related. We can 
put away that for now.

While my question may not be "directly germane" to this patch, I wouldn't say 
it's orthogonal either. Right now (IIUC) the executable module is always the 
source of truth for the launch operation. Now you're adding a second source, so 
one has to choose how to handle the situation when both of them are present. 
You may not care what happens in that case, but _something_ is going to happen 
nonetheless. I guess we basically have three options:
a) prefer the executable module (this is what the patch implements, and it's 
also the smallest deviation from status quo of completely ignoring launch info)
b) prefer the launch info (that is what I proposed)
c) make it an error (I think that's something we could all agree on)

The reason I proposed (b) is because of the principle of specific overriding 
general. The executable module is embedded into the target, so I would consider 
it more general than the launch info, which can be provided directly to the 
Launch method. Greg's use case (launching a remote binary that you already have 
a copy of) seems like it could benefit from that. However, maybe I have an even 
better one. What would happen with reruns? On the first run, the user would 
create a executable-less target, and provide a binary through the launch info. 
The binary would launch fine and exit. Then the user would try to launch again 
using the same target and launch info, but the code would go down a different 
path (I'm not sure which one) because the target would already have an 
executable. (This is actually kind of similar to the exec scenario, because the 
executable module would change between the two runs.)

>> - what happens if the launch_info path happens to refer to a host 
>> executable? Will we still launch the remote one or will we try to copy the 
>> local one instead? I'm not sure of the current behavior, but I would like to 
>> avoid the behavior depending on the presence of local files.
>
> Everything in Process::Launch, which is what does the real work for this part 
> of the Launch, uses the return from GetTarget().GetExecutableModulePointer() 
> for its "host side" work.  That's always going to be null in the scenario I'm 
> adding support for.  So we won't look at the local file system at all till 
> the launch succeeds and the process stops and we start querying the dynamic 
> loader for executables, and go looking for local copies like we always do.

Ok, that sounds great.




Comment at: lldb/source/Commands/CommandObjectProcess.cpp:268-269
+  exe_module_sp = target->GetExecutableModule();
+if (!exe_module_sp) {
+  result.AppendError("Could not get executable module after launch.");
+  return false;

jingham wrote:
> clayborg wrote:
> > I agree with Pavel here, unless you have a case where you need this to work 
> > this way?
> If we couldn't get the executable module, that would mean that we queried a 
> process stopped in the remote server after launch, and the dynamic loader 
> wasn't able to determine the main executable.  That's something we do all the 
> time, so it would be we

[Lldb-commits] [PATCH] D113634: [lldb] Add support for DW_TAG_immutable_type

[Luís Ferreira via Phabricator via lldb-commits]

ljmf00 added a comment.

In D113634#3124042 , @teemperor wrote:

> Are the DWARFASTParserClang changes meant as a step towards making it parse D?

Yes, not only D but any language that currently falls here 
(https://github.com/llvm/llvm-project/blob/main/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp#L106-L112).
 AFAIK, rust have immutable variables too. Since I'm working on a 
DWARFFASTParser for D this won't affect it in the future, but for languages 
like Rust that uses the Python API and rely on Clang DWARFParser, this could be 
beneficial.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113634

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

[Lldb-commits] [PATCH] D113634: [lldb] Add support for DW_TAG_immutable_type

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added a comment.

In D113634#3124401 , @ljmf00 wrote:

> In D113634#3124042 , @teemperor 
> wrote:
>
>> Are the DWARFASTParserClang changes meant as a step towards making it parse 
>> D?
>
> Yes, not only D but any language that currently falls here 
> (https://github.com/llvm/llvm-project/blob/main/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp#L106-L112).
>  AFAIK, rust have immutable variables too. Since I'm working on a 
> DWARFFASTParser for D this won't affect it in the future, but for languages 
> like Rust that uses the Python API and rely on Clang DWARFParser, this could 
> be beneficial.

I know the change is well intended, but the Rust support is 100% untested and 
completely broken (it can't deal with pretty much any non-trivial program from 
my recollections). So I would split out the Clang changes (which probably 
require some longer discussion) and just keep the Type/DWARFDIe changes (which 
look good to me).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113634

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

[Lldb-commits] [PATCH] D113605: [lldb] Fix documentation for EncodingDataType

[Raphael Isemann via Phabricator via lldb-commits]

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

LGTM, thanks!




Comment at: lldb/include/lldb/Symbol/Type.h:69
   enum EncodingDataType {
+/// Invalid encoding
 eEncodingInvalid,

nit: LLVM comments end in a period.


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

https://reviews.llvm.org/D113605

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

[Lldb-commits] [PATCH] D113673: [lldb] Unwrap the type when dereferencing the value

[Andy Yankovsky via Phabricator via lldb-commits]

werat created this revision.
werat requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

The value type can be a typedef of a reference (e.g. `typedef int& myint`).
In this case `GetQualType(type)` will return `clang::Typedef`, which cannot
be casted to `clang::ReferenceType`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D113673

Files:
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
  
lldb/test/API/lang/cpp/dereferencing_references/TestCPPDereferencingReferences.py
  lldb/test/API/lang/cpp/dereferencing_references/main.cpp


Index: lldb/test/API/lang/cpp/dereferencing_references/main.cpp
===
--- lldb/test/API/lang/cpp/dereferencing_references/main.cpp
+++ lldb/test/API/lang/cpp/dereferencing_references/main.cpp
@@ -1,8 +1,13 @@
 typedef int TTT;
+typedef int &td_int_ref;
 
 int main() {
   int i = 0;
+  // references to typedefs
   TTT &l_ref = i;
   TTT &&r_ref = static_cast(i);
+  // typedef of a reference
+  td_int_ref td = i;
+
   return l_ref; // break here
 }
Index: 
lldb/test/API/lang/cpp/dereferencing_references/TestCPPDereferencingReferences.py
===
--- 
lldb/test/API/lang/cpp/dereferencing_references/TestCPPDereferencingReferences.py
+++ 
lldb/test/API/lang/cpp/dereferencing_references/TestCPPDereferencingReferences.py
@@ -21,3 +21,7 @@
 # Same as above for rvalue references.
 rref_val = self.expect_var_path("r_ref", type="TTT &&")
 self.assertEqual(rref_val.Dereference().GetType().GetName(), "TTT")
+
+# Typedef to a reference should dereference to the underlying type.
+td_val = self.expect_var_path("td", type="td_int_ref")
+self.assertEqual(td_val.Dereference().GetType().GetName(), "int")
Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
===
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -6492,7 +6492,8 @@
   case clang::Type::RValueReference:
 if (idx_is_valid) {
   const clang::ReferenceType *reference_type =
-  llvm::cast(GetQualType(type).getTypePtr());
+  llvm::cast(
+  RemoveWrappingTypes(GetQualType(type)).getTypePtr());
   CompilerType pointee_clang_type =
   GetType(reference_type->getPointeeType());
   if (transparent_pointers && pointee_clang_type.IsAggregateType()) {


Index: lldb/test/API/lang/cpp/dereferencing_references/main.cpp
===
--- lldb/test/API/lang/cpp/dereferencing_references/main.cpp
+++ lldb/test/API/lang/cpp/dereferencing_references/main.cpp
@@ -1,8 +1,13 @@
 typedef int TTT;
+typedef int &td_int_ref;
 
 int main() {
   int i = 0;
+  // references to typedefs
   TTT &l_ref = i;
   TTT &&r_ref = static_cast(i);
+  // typedef of a reference
+  td_int_ref td = i;
+
   return l_ref; // break here
 }
Index: lldb/test/API/lang/cpp/dereferencing_references/TestCPPDereferencingReferences.py
===
--- lldb/test/API/lang/cpp/dereferencing_references/TestCPPDereferencingReferences.py
+++ lldb/test/API/lang/cpp/dereferencing_references/TestCPPDereferencingReferences.py
@@ -21,3 +21,7 @@
 # Same as above for rvalue references.
 rref_val = self.expect_var_path("r_ref", type="TTT &&")
 self.assertEqual(rref_val.Dereference().GetType().GetName(), "TTT")
+
+# Typedef to a reference should dereference to the underlying type.
+td_val = self.expect_var_path("td", type="td_int_ref")
+self.assertEqual(td_val.Dereference().GetType().GetName(), "int")
Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
===
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -6492,7 +6492,8 @@
   case clang::Type::RValueReference:
 if (idx_is_valid) {
   const clang::ReferenceType *reference_type =
-  llvm::cast(GetQualType(type).getTypePtr());
+  llvm::cast(
+  RemoveWrappingTypes(GetQualType(type)).getTypePtr());
   CompilerType pointee_clang_type =
   GetType(reference_type->getPointeeType());
   if (transparent_pointers && pointee_clang_type.IsAggregateType()) {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D111409: proposed support for Java interface to Scripting Bridge

[David Millar via lldb-commits]

Am sorry I can't commit build resources, but am certainly willing to commit 
time to solving test problems.  In a very general sense, I think our project 
(Ghidra) will effectively be a test platform for issues with the Java bindings 
to SWIG.


With regard to the set-up for testing, I have noticed differences in behavior 
between the "ninja check-lldb-xxx" suite-testing environment, the llvm-lit test 
sets, and actual lldb runs / individual tests.  I believe (I should never say 
this out loud, but...) the current code handles these environments correctly.


The Java scripting framework requires the ability to find two things:  the Java 
install (and within it libjvm) and the SWIG.jar built from the JNI wrappers.  
If either LD_LIBRARY_PATH or JAVA_HOME is set in whichever environment is in 
use, the code should be able to find libjvm.  If CLASSPATH includes the 
SWIG.jar or we're running out of build, finding it should be handled.  Libjvm 
is loaded dynamically, so the inclusion of my code "should" have no effect on 
environments without Java installed.


I was thinking I would also spend a couple of days isolating the swig logic in 
case supporting the Java interface becomes untenable for you.  Am hoping I can 
pare it down so that our users could point-and-click at an existing lldb repo, 
without necessarily having to build llvm/lldb.


Best, Dave


From: Raphael Isemann via Phabricator 
Sent: Thursday, November 11, 2021 7:23:11 AM
To: David Millar; anoro...@apple.com; fallk...@yahoo.com; kkle...@redhat.com; 
medismail.benn...@gmail.com; jo...@devlieghere.com; tedw...@quicinc.com; 
jmole...@apple.com; syaghm...@apple.com; jing...@apple.com; v...@apple.com; 
boris.ulasev...@gmail.com; lldb-commits@lists.llvm.org; h.imai@nitech.jp; 
bruce.mitche...@gmail.com; david.spick...@linaro.org; 
quic_soura...@quicinc.com; djordje.todoro...@syrmia.com; 
serhiy.re...@gmail.com; liburd1...@outlook.com
Cc: pa...@labath.sk; mgo...@gentoo.org
Subject: [PATCH] D111409: proposed support for Java interface to Scripting 
Bridge

teemperor added a comment.

In D111409#3124194 , @labath wrote:

> In D111409#3124075 , @teemperor 
> wrote:
>
>>> Are you asking for dedicated physical resources for running nightly builds?
>>
>> I don't think any of the current bots have a Java installation so I think 
>> it's either that or we get someone with a bot to setup the required Java 
>> installation.
>
> I don't have a problem with installing the necessary packages on the bot I 
> manage, but I cannot subscribe to tracking down any failures (or flaky 
> tests!) for this configuration (and, in my experience, any new feature like 
> this is going to have flaky tests). Flaky tests (probably just one) are the 
> reason that lua integration is not enabled on this bot.

Sure, I think it should anyway not be up to any bot owner to track down flaky 
tests. And that nested_sessions lua test is anyway randomly failing everywhere 
from what I know.

>> FWIW, if no one wants to host a bot for this then I won't mind testing this 
>> in own CI , but I am not using buildbot so we'll 
>> have to see if that is acceptable for the community (I could also migrate it 
>> to buildbot, but the buildbot interface is just painful to use and I would 
>> prefer not to).
>
> I would rather not proliferate test infrastructures.
>
> I'm not sure which pain points are you referring to, but setting up a 
> buildbot instance is a lot simpler these days than it used to be (in 
> particular, you don't need to track down and install any outdated packages).

It's not the setup, it's just that the lab.llvm.org interface is far less 
usable than Jenkins for tracking down regressions in tests (and Jenkins is 
already not great, so that says something). But that's just my personal 
preference and I agree that lab.llvm.org should be the central place for 
infrastructure.

Anyway, if @labath can run them on his fancy build bot then I would prefer that 
over having it on my bot (because I pay out of my own pocket and CPU cycles are 
$$$).


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D111409

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

[Lldb-commits] [PATCH] D113498: [lldb] Constant-resolve operands to `getelementptr`

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added a reviewer: LLDB.
teemperor added a comment.

I really like the solution, but I think by fixing the `CanInterpret` you also 
made the test case no longer reach the actual changed interpreting logic?

So, `CanInterpret` says "we can't interpret this" (which is correct), but then 
the interpreter won't run it and your change to `ResolveConstantValue` isn't 
actually tested. There is no other test that touches that logic from what I can 
see. You could try throwing in some other expression at this that tests that 
new code? Maybe some kind of pointer arithmetic on a variable defined in your 
expression itself (so it can be constant evaluated). We could also split out 
the interpreting change and then this is good to go.

Also I think a second set of eyes on this would be nice as I rarely touch the 
IRInterpreter, but not sure who the best person for that is. I'll add the LLDB 
group and let's see if anyone has a second opinion on this, but in general this 
LGTM module the test situation.




Comment at: lldb/source/Expression/IRInterpreter.cpp:286
   SmallVector indices(op_cursor, op_end);
+  SmallVector const_indices;
+

Maybe `resolved_indices`? `const_` always sounds a bit like it's meaning 
'const' qualified version of indices or so.



Comment at: lldb/source/Expression/IRInterpreter.cpp:288
+
+  for (Value *idx : indices) {
+Constant *constant_idx = dyn_cast(idx);

I think this deserves a comment that `getIndexedOffsetInType` can only handle 
ConstantInt indices (and that's why we're resolving them here).



Comment at: lldb/source/Expression/IRInterpreter.cpp:490
+ constant_expr->op_end());
+for (Value *op : operands) {
+  Constant *constant_op = dyn_cast(op);

`for (Value *op : constant_expr->ops())` ?



Comment at: lldb/source/Expression/IRInterpreter.cpp:494
+return false;
+  if (!CanResolveConstant(constant_op)) {
+return false;

nit no `{}` for single line ifs



Comment at: lldb/test/API/lang/cpp/static_members/TestCPPStaticMembers.py:71
+value = self.target().EvaluateExpression(expr)
+self.assertTrue(value.GetError().Success())

`self.assertSuccess` (that will print the error on failure to the test log)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113498

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

[Lldb-commits] [PATCH] D113608: [lldb] Simplify specifying of platform supported architectures

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: lldb/include/lldb/Target/Platform.h:330-331
+  /// NB: This implementation is mutually recursive with
+  /// GetSupportedArchitectureAtIndex. Subclasses should implement at least one
+  /// of them.
+  virtual std::vector GetSupportedArchitectures();

JDevlieghere wrote:
> Do you expect any platform to implement both?
Not really, though it wouldn't necessarily be an error if they did. I'll just 
remove the "at least" part.



Comment at: lldb/source/Target/Platform.cpp:1217-1218
+std::vector
+Platform::CreateArchList(llvm::Triple::OSType os,
+ llvm::ArrayRef archs) {
+  std::vector list;

JDevlieghere wrote:
> In PlatformDarwin we set the vendor too. Could we have this take an optional 
> OSType and Vendor maybe that sets the ones that are not None? That would also 
> make migrating PlatformDarwin easier where we currently share the code and 
> don't set the OS. Later we could move this out of PlatformDarwin into the 
> child classes and have the OS set correctly. 
Umm.. maybe? I don't think this function is set in stone, but the nice thing 
about it being a helper function is that it's usage is completely optional. I 
haven't tried porting the darwin platforms yet, but, from a cursory glance I 
did see that they have a lot more complex usage patterns:
- they pass subarchitectures as well, so we'd either also need to have an 
Optional subarch field, or switch to passing architectures as strings (I mean, 
I suppose I could switch to strings now, but it feels more correct to work with 
named constants)
- a few of them also pass environments 
- and some don't even have matching OS fields (`x86_64-apple-macosx` and 
`arm64-apple-ios` in the same list)

So, I'd leave it to those patches to determine whether it's better to extend 
this function or create a new one.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113608

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

[Lldb-commits] [PATCH] D113019: [lldb][NFC] Inclusive Language: rename master plan to controlling plan

[Quinn Pham via Phabricator via lldb-commits]

quinnp added a comment.

@jingham could you take a look at the new changes when you have time? Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113019

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

[Lldb-commits] [PATCH] D113608: [lldb] Simplify specifying of platform supported architectures

[Pavel Labath via Phabricator via lldb-commits]

labath updated this revision to Diff 386516.
labath added a comment.

- remove "at least"
- change CreateArchList argument order to match the triple format. This would 
be particularly important if new arguments are added later.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113608

Files:
  lldb/include/lldb/Target/Platform.h
  lldb/source/Plugins/Platform/FreeBSD/PlatformFreeBSD.cpp
  lldb/source/Plugins/Platform/FreeBSD/PlatformFreeBSD.h
  lldb/source/Plugins/Platform/Linux/PlatformLinux.cpp
  lldb/source/Plugins/Platform/Linux/PlatformLinux.h
  lldb/source/Plugins/Platform/NetBSD/PlatformNetBSD.cpp
  lldb/source/Plugins/Platform/NetBSD/PlatformNetBSD.h
  lldb/source/Target/Platform.cpp

Index: lldb/source/Target/Platform.cpp
===
--- lldb/source/Target/Platform.cpp
+++ lldb/source/Target/Platform.cpp
@@ -1213,6 +1213,35 @@
   return platform_sp;
 }
 
+std::vector
+Platform::CreateArchList(llvm::ArrayRef archs,
+ llvm::Triple::OSType os) {
+  std::vector list;
+  for(auto arch : archs) {
+llvm::Triple triple;
+triple.setArch(arch);
+triple.setOS(os);
+list.push_back(ArchSpec(triple));
+  }
+  return list;
+}
+
+bool Platform::GetSupportedArchitectureAtIndex(uint32_t idx, ArchSpec &arch) {
+  const auto &archs = GetSupportedArchitectures();
+  if (idx > archs.size())
+return false;
+  arch = archs[idx];
+  return true;
+}
+
+std::vector Platform::GetSupportedArchitectures() {
+  std::vector result;
+  ArchSpec arch;
+  for (uint32_t idx = 0; GetSupportedArchitectureAtIndex(idx, arch); ++idx)
+result.push_back(arch);
+  return result;
+}
+
 /// Lets a platform answer if it is compatible with a given
 /// architecture and the target triple contained within.
 bool Platform::IsCompatibleArchitecture(const ArchSpec &arch,
Index: lldb/source/Plugins/Platform/NetBSD/PlatformNetBSD.h
===
--- lldb/source/Plugins/Platform/NetBSD/PlatformNetBSD.h
+++ lldb/source/Plugins/Platform/NetBSD/PlatformNetBSD.h
@@ -42,7 +42,7 @@
 
   void GetStatus(Stream &strm) override;
 
-  bool GetSupportedArchitectureAtIndex(uint32_t idx, ArchSpec &arch) override;
+  std::vector GetSupportedArchitectures() override;
 
   uint32_t GetResumeCountForLaunchInfo(ProcessLaunchInfo &launch_info) override;
 
@@ -54,6 +54,8 @@
   lldb::addr_t length, unsigned prot,
   unsigned flags, lldb::addr_t fd,
   lldb::addr_t offset) override;
+
+  std::vector m_supported_architectures;
 };
 
 } // namespace platform_netbsd
Index: lldb/source/Plugins/Platform/NetBSD/PlatformNetBSD.cpp
===
--- lldb/source/Plugins/Platform/NetBSD/PlatformNetBSD.cpp
+++ lldb/source/Plugins/Platform/NetBSD/PlatformNetBSD.cpp
@@ -100,54 +100,24 @@
 /// Default Constructor
 PlatformNetBSD::PlatformNetBSD(bool is_host)
 : PlatformPOSIX(is_host) // This is the local host platform
-{}
-
-bool PlatformNetBSD::GetSupportedArchitectureAtIndex(uint32_t idx,
- ArchSpec &arch) {
-  if (IsHost()) {
+{
+  if (is_host) {
 ArchSpec hostArch = HostInfo::GetArchitecture(HostInfo::eArchKindDefault);
-if (hostArch.GetTriple().isOSNetBSD()) {
-  if (idx == 0) {
-arch = hostArch;
-return arch.IsValid();
-  } else if (idx == 1) {
-// If the default host architecture is 64-bit, look for a 32-bit
-// variant
-if (hostArch.IsValid() && hostArch.GetTriple().isArch64Bit()) {
-  arch = HostInfo::GetArchitecture(HostInfo::eArchKind32);
-  return arch.IsValid();
-}
-  }
+m_supported_architectures.push_back(hostArch);
+if (hostArch.GetTriple().isArch64Bit()) {
+  m_supported_architectures.push_back(
+  HostInfo::GetArchitecture(HostInfo::eArchKind32));
 }
   } else {
-if (m_remote_platform_sp)
-  return m_remote_platform_sp->GetSupportedArchitectureAtIndex(idx, arch);
-
-llvm::Triple triple;
-// Set the OS to NetBSD
-triple.setOS(llvm::Triple::NetBSD);
-// Set the architecture
-switch (idx) {
-case 0:
-  triple.setArchName("x86_64");
-  break;
-case 1:
-  triple.setArchName("i386");
-  break;
-default:
-  return false;
-}
-// Leave the vendor as "llvm::Triple:UnknownVendor" and don't specify the
-// vendor by calling triple.SetVendorName("unknown") so that it is a
-// "unspecified unknown". This means when someone calls
-// triple.GetVendorName() it will return an empty string which indicates
-// that the vendor can be set when two architectures are merged
-
-// Now set the triple into "arch" and return true
-arch.SetTriple(triple);
-re

[Lldb-commits] [PATCH] D113673: [lldb] Unwrap the type when dereferencing the value

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added a comment.

I was trying to fix this a while back in D108717 
 but got distracted and did not get back to 
it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113673

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

[Lldb-commits] [PATCH] D113673: [lldb] Unwrap the type when dereferencing the value

[Raphael Isemann via Phabricator via lldb-commits]

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

Thanks a lot for fixing this, could you point to D103532 
 as the cause for the regression in the 
commit message?

This LGTM in general, but I think there could still be some obscure cases where 
this could fail.

So the switch statement this code is in (and which detects whether its a 
reference type) is defined as:

  clang::QualType parent_qual_type(
  RemoveWrappingTypes(GetCanonicalQualType(type)));
  switch (parent_qual_type->getTypeClass()) {

`GetCanonicalQualType` should strip all the outer and inner type sugar there 
is. And `RemoveWrappingTypes` removes *most* of the outer type sugar (which 
isn't necessary anymore as it should be all gone), but also makes types 
non-atomic (for simplicity reasons).

Meanwhile in our reference-case code we try to remove the outer type sugar (and 
atomic) via `RemoveWrappingTypes`. And if this doesn't end up in a 
ReferenceType the cast fails.

So IIUC there is still a possibility that we fail to cast here in case 
`GetCanonicalQualType` removes more outer sugar than `RemoveWrappingTypes`, 
which I think can only happen for type sugar classes not implemented by 
`RemoveWrappingTypes` (e.g., a quick grep tells me AttributedType and some 
template stuff is sugar that we don't handle).

So I would propose we do the following:

1. This patch can land now as-is to fix the regression.
2. It would be nice if we could expand the test with a few more variations of 
sugar. E.g., typedef to a reference of a typedef type and all that stuff.
3. I think we can simplify `RemoveWrappingTypes` to remove all type sugar (not 
just our small list we maintain there). That should eliminate the obscure cases 
and we no longer need to maintain a list of type sugar there.




Comment at: lldb/test/API/lang/cpp/dereferencing_references/main.cpp:10
+  // typedef of a reference
+  td_int_ref td = i;
+

nit: Could you give this a more unique name such as `td_to_ref_type`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113673

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

[Lldb-commits] [PATCH] D113608: [lldb] Simplify specifying of platform supported architectures

[Pavel Labath via Phabricator via lldb-commits]

labath updated this revision to Diff 386543.
labath added a comment.

Fix a rather embarrasing off-by-one in the base implementation of
GetSupportedArchitectureAtIndex, which I guess also means I did not run tests
before uploading this (I have now).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113608

Files:
  lldb/include/lldb/Target/Platform.h
  lldb/source/Plugins/Platform/FreeBSD/PlatformFreeBSD.cpp
  lldb/source/Plugins/Platform/FreeBSD/PlatformFreeBSD.h
  lldb/source/Plugins/Platform/Linux/PlatformLinux.cpp
  lldb/source/Plugins/Platform/Linux/PlatformLinux.h
  lldb/source/Plugins/Platform/NetBSD/PlatformNetBSD.cpp
  lldb/source/Plugins/Platform/NetBSD/PlatformNetBSD.h
  lldb/source/Target/Platform.cpp

Index: lldb/source/Target/Platform.cpp
===
--- lldb/source/Target/Platform.cpp
+++ lldb/source/Target/Platform.cpp
@@ -1213,6 +1213,35 @@
   return platform_sp;
 }
 
+std::vector
+Platform::CreateArchList(llvm::ArrayRef archs,
+ llvm::Triple::OSType os) {
+  std::vector list;
+  for(auto arch : archs) {
+llvm::Triple triple;
+triple.setArch(arch);
+triple.setOS(os);
+list.push_back(ArchSpec(triple));
+  }
+  return list;
+}
+
+bool Platform::GetSupportedArchitectureAtIndex(uint32_t idx, ArchSpec &arch) {
+  const auto &archs = GetSupportedArchitectures();
+  if (idx >= archs.size())
+return false;
+  arch = archs[idx];
+  return true;
+}
+
+std::vector Platform::GetSupportedArchitectures() {
+  std::vector result;
+  ArchSpec arch;
+  for (uint32_t idx = 0; GetSupportedArchitectureAtIndex(idx, arch); ++idx)
+result.push_back(arch);
+  return result;
+}
+
 /// Lets a platform answer if it is compatible with a given
 /// architecture and the target triple contained within.
 bool Platform::IsCompatibleArchitecture(const ArchSpec &arch,
Index: lldb/source/Plugins/Platform/NetBSD/PlatformNetBSD.h
===
--- lldb/source/Plugins/Platform/NetBSD/PlatformNetBSD.h
+++ lldb/source/Plugins/Platform/NetBSD/PlatformNetBSD.h
@@ -42,7 +42,7 @@
 
   void GetStatus(Stream &strm) override;
 
-  bool GetSupportedArchitectureAtIndex(uint32_t idx, ArchSpec &arch) override;
+  std::vector GetSupportedArchitectures() override;
 
   uint32_t GetResumeCountForLaunchInfo(ProcessLaunchInfo &launch_info) override;
 
@@ -54,6 +54,8 @@
   lldb::addr_t length, unsigned prot,
   unsigned flags, lldb::addr_t fd,
   lldb::addr_t offset) override;
+
+  std::vector m_supported_architectures;
 };
 
 } // namespace platform_netbsd
Index: lldb/source/Plugins/Platform/NetBSD/PlatformNetBSD.cpp
===
--- lldb/source/Plugins/Platform/NetBSD/PlatformNetBSD.cpp
+++ lldb/source/Plugins/Platform/NetBSD/PlatformNetBSD.cpp
@@ -100,54 +100,24 @@
 /// Default Constructor
 PlatformNetBSD::PlatformNetBSD(bool is_host)
 : PlatformPOSIX(is_host) // This is the local host platform
-{}
-
-bool PlatformNetBSD::GetSupportedArchitectureAtIndex(uint32_t idx,
- ArchSpec &arch) {
-  if (IsHost()) {
+{
+  if (is_host) {
 ArchSpec hostArch = HostInfo::GetArchitecture(HostInfo::eArchKindDefault);
-if (hostArch.GetTriple().isOSNetBSD()) {
-  if (idx == 0) {
-arch = hostArch;
-return arch.IsValid();
-  } else if (idx == 1) {
-// If the default host architecture is 64-bit, look for a 32-bit
-// variant
-if (hostArch.IsValid() && hostArch.GetTriple().isArch64Bit()) {
-  arch = HostInfo::GetArchitecture(HostInfo::eArchKind32);
-  return arch.IsValid();
-}
-  }
+m_supported_architectures.push_back(hostArch);
+if (hostArch.GetTriple().isArch64Bit()) {
+  m_supported_architectures.push_back(
+  HostInfo::GetArchitecture(HostInfo::eArchKind32));
 }
   } else {
-if (m_remote_platform_sp)
-  return m_remote_platform_sp->GetSupportedArchitectureAtIndex(idx, arch);
-
-llvm::Triple triple;
-// Set the OS to NetBSD
-triple.setOS(llvm::Triple::NetBSD);
-// Set the architecture
-switch (idx) {
-case 0:
-  triple.setArchName("x86_64");
-  break;
-case 1:
-  triple.setArchName("i386");
-  break;
-default:
-  return false;
-}
-// Leave the vendor as "llvm::Triple:UnknownVendor" and don't specify the
-// vendor by calling triple.SetVendorName("unknown") so that it is a
-// "unspecified unknown". This means when someone calls
-// triple.GetVendorName() it will return an empty string which indicates
-// that the vendor can be set when two architectures are merged
-
-// Now set the triple into "arch" and return true
-arch.SetT

[Lldb-commits] [PATCH] D113687: [lldb][NFC] Inclusive language: replace master/slave names for ptys

[Quinn Pham via Phabricator via lldb-commits]

quinnp created this revision.
quinnp requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

[NFC] This patch replaces master and slave with primary and secondary
respectively when referening to pseudoterminals/file descriptors.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D113687

Files:
  
lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm
  lldb/source/Plugins/Process/Windows/Common/NativeProcessWindows.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Target/Platform.cpp
  lldb/source/Target/Process.cpp
  lldb/test/API/functionalities/gdb_remote_client/TestPty.py
  lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py
  lldb/test/API/tools/lldb-server/TestPtyServer.py
  lldb/third_party/Python/module/ptyprocess-0.6.0/ptyprocess/ptyprocess.py
  lldb/unittests/Editline/EditlineTest.cpp

Index: lldb/unittests/Editline/EditlineTest.cpp
===
--- lldb/unittests/Editline/EditlineTest.cpp
+++ lldb/unittests/Editline/EditlineTest.cpp
@@ -87,24 +87,24 @@
   std::unique_ptr _editline_sp;
 
   PseudoTerminal _pty;
-  int _pty_master_fd;
+  int _pty_primary_fd;
   int _pty_secondary_fd;
 
   std::unique_ptr _el_secondary_file;
 };
 
 EditlineAdapter::EditlineAdapter()
-: _editline_sp(), _pty(), _pty_master_fd(-1), _pty_secondary_fd(-1),
+: _editline_sp(), _pty(), _pty_primary_fd(-1), _pty_secondary_fd(-1),
   _el_secondary_file() {
   lldb_private::Status error;
 
-  // Open the first master pty available.
+  // Open the first primary pty available.
   EXPECT_THAT_ERROR(_pty.OpenFirstAvailablePrimary(O_RDWR), llvm::Succeeded());
 
-  // Grab the master fd.  This is a file descriptor we will:
+  // Grab the primary fd.  This is a file descriptor we will:
   // (1) write to when we want to send input to editline.
   // (2) read from when we want to see what editline sends back.
-  _pty_master_fd = _pty.GetPrimaryFileDescriptor();
+  _pty_primary_fd = _pty.GetPrimaryFileDescriptor();
 
   // Open the corresponding secondary pty.
   EXPECT_THAT_ERROR(_pty.OpenSecondary(O_RDWR), llvm::Succeeded());
@@ -140,13 +140,13 @@
 
   // Write the line out to the pipe connected to editline's input.
   ssize_t input_bytes_written =
-  ::write(_pty_master_fd, line.c_str(),
+  ::write(_pty_primary_fd, line.c_str(),
   line.length() * sizeof(std::string::value_type));
 
   const char *eoln = "\n";
   const size_t eoln_length = strlen(eoln);
   input_bytes_written =
-  ::write(_pty_master_fd, eoln, eoln_length * sizeof(char));
+  ::write(_pty_primary_fd, eoln, eoln_length * sizeof(char));
 
   EXPECT_NE(-1, input_bytes_written) << strerror(errno);
   EXPECT_EQ(eoln_length * sizeof(char), size_t(input_bytes_written));
@@ -205,7 +205,7 @@
 }
 
 void EditlineAdapter::ConsumeAllOutput() {
-  FilePointer output_file(fdopen(_pty_master_fd, "r"));
+  FilePointer output_file(fdopen(_pty_primary_fd, "r"));
 
   int ch;
   while ((ch = fgetc(output_file)) != EOF) {
Index: lldb/third_party/Python/module/ptyprocess-0.6.0/ptyprocess/ptyprocess.py
===
--- lldb/third_party/Python/module/ptyprocess-0.6.0/ptyprocess/ptyprocess.py
+++ lldb/third_party/Python/module/ptyprocess-0.6.0/ptyprocess/ptyprocess.py
@@ -229,7 +229,7 @@
 pid, fd = _fork_pty.fork_pty()
 
 # Some platforms must call setwinsize() and setecho() from the
-# child process, and others from the master process. We do both,
+# child process, and others from the primary process. We do both,
 # allowing IOError for either.
 
 if pid == CHILD:
Index: lldb/test/API/tools/lldb-server/TestPtyServer.py
===
--- lldb/test/API/tools/lldb-server/TestPtyServer.py
+++ lldb/test/API/tools/lldb-server/TestPtyServer.py
@@ -15,10 +15,10 @@
 super().setUp()
 import pty
 import tty
-master, slave = pty.openpty()
-tty.setraw(master)
-self._master = io.FileIO(master, 'r+b')
-self._slave = io.FileIO(slave, 'r+b')
+primary, secondary = pty.openpty()
+tty.setraw(primary)
+self._primary = io.FileIO(primary, 'r+b')
+self._secondary = io.FileIO(secondary, 'r+b')
 
 def get_debug_monitor_command_line_args(self, attach_pid=None):
 commandline_args = self.debug_monitor_extra_args
@@ -28,7 +28,7 @@
 libc = ctypes.CDLL(None)
 libc.ptsname.argtypes = (ctypes.c_int,)
 libc.ptsname.restype = ctypes.c_char_p
-pty_path = libc.ptsname(self._master.fileno()).decode()
+pty_path = libc.ptsname(self._primary.fileno()).decode()
 commandline_args += ["serial://%s" % (pty_path,)]
 return commandline_args
 
@@ -48,7 +48,7 @@
 def

[Lldb-commits] [PATCH] D113650: [lldb] fix -print-script-interpreter-info on windows

[Stella Stamenova via Phabricator via lldb-commits]

stella.stamenova added inline comments.



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp:413
 
+static const char GetInterpreterInfoScript[] = R"(
+import os

Does `sys.executable` not work for some reason?



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp:432
+  elif os.name == 'posix':
+  exename = "python" + str(sys.version_info[0])
+  info['executable'] = os.path.join(sys.prefix, 'bin', exename)

Python on windows could have a version too (e.g. python3.exe)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113650

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

[Lldb-commits] [PATCH] D113650: [lldb] fix -print-script-interpreter-info on windows

[Lawrence D'Anna via Phabricator via lldb-commits]

lawrence_danna added inline comments.



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp:413
 
+static const char GetInterpreterInfoScript[] = R"(
+import os

stella.stamenova wrote:
> Does `sys.executable` not work for some reason?
Yes, unfortunately it will point to `lldb.exe`, not `python.exe`



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp:432
+  elif os.name == 'posix':
+  exename = "python" + str(sys.version_info[0])
+  info['executable'] = os.path.join(sys.prefix, 'bin', exename)

stella.stamenova wrote:
> Python on windows could have a version too (e.g. python3.exe)
Under what circumstances?   How do we determine if it does? The one from 
python.org doesn't.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113650

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

[Lldb-commits] [PATCH] D113650: [lldb] fix -print-script-interpreter-info on windows

[Lawrence D'Anna via Phabricator via lldb-commits]

lawrence_danna added a comment.

comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113650

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

[Lldb-commits] [PATCH] D113650: [lldb] fix -print-script-interpreter-info on windows

[Stella Stamenova via Phabricator via lldb-commits]

stella.stamenova added inline comments.



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp:432
+  elif os.name == 'posix':
+  exename = "python" + str(sys.version_info[0])
+  info['executable'] = os.path.join(sys.prefix, 'bin', exename)

lawrence_danna wrote:
> stella.stamenova wrote:
> > Python on windows could have a version too (e.g. python3.exe)
> Under what circumstances?   How do we determine if it does? The one from 
> python.org doesn't.
I am not sure. I *think* it depends on whether you have python 2 and python 3 
installed at the same time.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113650

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

[Lldb-commits] [PATCH] D113650: [lldb] fix -print-script-interpreter-info on windows

[Lawrence D'Anna via Phabricator via lldb-commits]

lawrence_danna added inline comments.



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp:432
+  elif os.name == 'posix':
+  exename = "python" + str(sys.version_info[0])
+  info['executable'] = os.path.join(sys.prefix, 'bin', exename)

stella.stamenova wrote:
> lawrence_danna wrote:
> > stella.stamenova wrote:
> > > Python on windows could have a version too (e.g. python3.exe)
> > Under what circumstances?   How do we determine if it does? The one 
> > from python.org doesn't.
> I am not sure. I *think* it depends on whether you have python 2 and python 3 
> installed at the same time.
yikes, OK I think I have a better idea. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113650

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

[Lldb-commits] [PATCH] D113019: [lldb][NFC] Inclusive Language: rename master plan to controlling plan

[Jim Ingham via Phabricator via lldb-commits]

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

LOKTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113019

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

[Lldb-commits] [PATCH] D113687: [lldb][NFC] Inclusive language: replace master/slave names for ptys

[Raphael Isemann via Phabricator via lldb-commits]

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

LGTM, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113687

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

[Lldb-commits] [PATCH] D113634: [lldb] Add support for DW_TAG_immutable_type

[Luís Ferreira via Phabricator via lldb-commits]

ljmf00 added a comment.

In D113634#3124428 , @teemperor wrote:

> In D113634#3124401 , @ljmf00 wrote:
>
>> In D113634#3124042 , @teemperor 
>> wrote:
>>
>>> Are the DWARFASTParserClang changes meant as a step towards making it parse 
>>> D?
>>
>> Yes, not only D but any language that currently falls here 
>> (https://github.com/llvm/llvm-project/blob/main/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp#L106-L112).
>>  AFAIK, rust have immutable variables too. Since I'm working on a 
>> DWARFFASTParser for D this won't affect it in the future, but for languages 
>> like Rust that uses the Python API and rely on Clang DWARFParser, this could 
>> be beneficial.
>
> I know the change is well intended, but the Rust support is 100% untested and 
> completely broken (it can't deal with pretty much any non-trivial program 
> from my recollections). So I would split out the Clang changes (which 
> probably require some longer discussion) and just keep the Type/DWARFDIe 
> changes (which look good to me).

Ok, makes sense to me. I'm going to make a separate patch, mark you as a 
subscriber and discuss this there.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113634

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

[Lldb-commits] [PATCH] D113650: [lldb] fix -print-script-interpreter-info on windows

[Lawrence D'Anna via Phabricator via lldb-commits]

lawrence_danna updated this revision to Diff 386599.
lawrence_danna added a comment.
Herald added a subscriber: mgorny.

maybe this approach is better


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113650

Files:
  lldb/CMakeLists.txt
  lldb/bindings/python/get-python-config.py
  lldb/source/Plugins/ScriptInterpreter/Python/CMakeLists.txt
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp

Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -410,30 +410,31 @@
   return g_spec;
 }
 
+static const char GetInterpreterInfoScript[] = R"(
+import os
+import sys
+
+def main(lldb_python_dir, python_exe_relative_path):
+  info = {
+"lldb-pythonpath": lldb_python_dir,
+"language": "python",
+"prefix": sys.prefix,
+"executable": os.path.join(sys.prefix, python_exe_relative_path)
+  }
+  return info
+)";
+
+static const char python_exe_relative_path[] = LLDB_PYTHON_EXE_RELATIVE_PATH;
+
 StructuredData::DictionarySP ScriptInterpreterPython::GetInterpreterInfo() {
   GIL gil;
   FileSpec python_dir_spec = GetPythonDir();
   if (!python_dir_spec)
 return nullptr;
-  PythonString python_dir(python_dir_spec.GetPath());
-  PythonDictionary info(PyInitialValue::Empty);
-  llvm::Error error = info.SetItem("lldb-pythonpath", python_dir);
-  if (error)
-return nullptr;
-  static const char script[] = R"(
-def main(info):
-  import sys
-  import os
-  name = 'python' + str(sys.version_info.major)
-  info.update({
-"language": "python",
-"prefix": sys.prefix,
-"executable": os.path.join(sys.prefix, "bin", name),
-  })
-  return info
-)";
-  PythonScript get_info(script);
-  auto info_json = unwrapIgnoringErrors(As(get_info(info)));
+  PythonScript get_info(GetInterpreterInfoScript);
+  auto info_json = unwrapIgnoringErrors(
+  As(get_info(PythonString(python_dir_spec.GetPath()),
+PythonString(python_exe_relative_path;
   if (!info_json)
 return nullptr;
   return info_json.CreateStructuredDictionary();
Index: lldb/source/Plugins/ScriptInterpreter/Python/CMakeLists.txt
===
--- lldb/source/Plugins/ScriptInterpreter/Python/CMakeLists.txt
+++ lldb/source/Plugins/ScriptInterpreter/Python/CMakeLists.txt
@@ -3,6 +3,12 @@
 endif()
 add_definitions(-DLLDB_PYTHON_RELATIVE_LIBDIR="${LLDB_PYTHON_RELATIVE_PATH}")
 
+if(NOT LLDB_PYTHON_EXE_RELATIVE_PATH)
+  message(FATAL_ERROR "LLDB_PYTHON_EXE_RELATIVE_PATH is not set.")
+endif()
+add_definitions(-DLLDB_PYTHON_EXE_RELATIVE_PATH="${LLDB_PYTHON_EXE_RELATIVE_PATH}")
+
+
 if (LLDB_ENABLE_LIBEDIT)
   list(APPEND LLDB_LIBEDIT_LIBS ${LibEdit_LIBRARIES})
 endif()
Index: lldb/bindings/python/get-python-config.py
===
--- /dev/null
+++ lldb/bindings/python/get-python-config.py
@@ -0,0 +1,47 @@
+#!/usr/bin/env python3
+
+import os
+import sys
+import argparse
+import sysconfig
+import distutils.sysconfig
+
+
+def relpath_nodots(path, base):
+rel = os.path.normpath(os.path.relpath(path, base))
+assert not os.path.isabs(rel)
+parts = rel.split(os.path.sep)
+if parts and parts[0] == '..':
+raise ValueError(f"{path} is not under {base}")
+return rel
+
+def main():
+parser = argparse.ArgumentParser(description="extract cmake variables from python")
+parser.add_argument("variable_name")
+args = parser.parse_args()
+if args.variable_name == "LLDB_PYTHON_DEFAULT_RELATIVE_PATH":
+print(distutils.sysconfig.get_python_lib(True, False, ''))
+elif args.variable_name == "LLDB_PYTHON_EXE_RELATIVE_PATH":
+tried = list()
+exe = sys.executable
+while True:
+try:
+print(relpath_nodots(exe, sys.prefix))
+break
+except ValueError:
+tried.append(exe)
+if os.path.islink(exe):
+exe = os.path.join(os.path.dirname(exe), os.readlink(exe))
+continue
+else:
+print("Could not find a relative path to sys.executable under sys.prefix", file=sys.stderr)
+for e in tried:
+print("tried:", e, file=sys.stderr)
+print("sys.prefix:", sys.prefix, file=sys.stderr)
+sys.exit(1)
+
+else:
+parser.error(f"unknown variable {args.variable_name}")
+
+if __name__ == '__main__':
+main()
\ No newline at end of file
Index: lldb/CMakeLists.txt
===
--- lldb/CMakeLists.txt
+++ lldb/CMakeLists.tx

[Lldb-commits] [PATCH] D113650: [lldb] fix -print-script-interpreter-info on windows

[Lawrence D'Anna via Phabricator via lldb-commits]

lawrence_danna marked 3 inline comments as done.
lawrence_danna added a comment.

OK this should work in all configs I think, as long as sys.executable 
ultimately points at something under sys.prefix


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113650

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

[Lldb-commits] [PATCH] D113521: Allow lldb to launch a remote executable when there isn't a local copy

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

I'm good with this to get things started. Pavel?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113521

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

[Lldb-commits] [PATCH] D113019: [lldb][NFC] Inclusive Language: rename master plan to controlling plan

[Quinn Pham via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG04cbfa950e02: [lldb][NFC] Inclusive Language: rename master 
plan to controlling plan (authored by quinnp).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113019

Files:
  lldb/docs/use/python-reference.rst
  lldb/include/lldb/Target/Thread.h
  lldb/include/lldb/Target/ThreadPlan.h
  lldb/include/lldb/Target/ThreadPlanStack.h
  lldb/source/API/SBThread.cpp
  lldb/source/Commands/CommandObjectThread.cpp
  lldb/source/Expression/FunctionCaller.cpp
  lldb/source/Target/Process.cpp
  lldb/source/Target/StopInfo.cpp
  lldb/source/Target/Thread.cpp
  lldb/source/Target/ThreadPlan.cpp
  lldb/source/Target/ThreadPlanBase.cpp
  lldb/source/Target/ThreadPlanCallFunction.cpp
  lldb/source/Target/ThreadPlanCallOnFunctionExit.cpp
  lldb/source/Target/ThreadPlanCallUserExpression.cpp
  lldb/source/Target/ThreadPlanPython.cpp
  lldb/source/Target/ThreadPlanStack.cpp

Index: lldb/source/Target/ThreadPlanStack.cpp
===
--- lldb/source/Target/ThreadPlanStack.cpp
+++ lldb/source/Target/ThreadPlanStack.cpp
@@ -213,35 +213,35 @@
   return;
 }
 
-void ThreadPlanStack::DiscardConsultingMasterPlans() {
+void ThreadPlanStack::DiscardConsultingControllingPlans() {
   std::lock_guard guard(m_stack_mutex);
   while (true) {
-int master_plan_idx;
+int controlling_plan_idx;
 bool discard = true;
 
-// Find the first master plan, see if it wants discarding, and if yes
+// Find the first controlling plan, see if it wants discarding, and if yes
 // discard up to it.
-for (master_plan_idx = m_plans.size() - 1; master_plan_idx >= 0;
- master_plan_idx--) {
-  if (m_plans[master_plan_idx]->IsMasterPlan()) {
-discard = m_plans[master_plan_idx]->OkayToDiscard();
+for (controlling_plan_idx = m_plans.size() - 1; controlling_plan_idx >= 0;
+ controlling_plan_idx--) {
+  if (m_plans[controlling_plan_idx]->IsControllingPlan()) {
+discard = m_plans[controlling_plan_idx]->OkayToDiscard();
 break;
   }
 }
 
-// If the master plan doesn't want to get discarded, then we're done.
+// If the controlling plan doesn't want to get discarded, then we're done.
 if (!discard)
   return;
 
 // First pop all the dependent plans:
-for (int i = m_plans.size() - 1; i > master_plan_idx; i--) {
+for (int i = m_plans.size() - 1; i > controlling_plan_idx; i--) {
   DiscardPlan();
 }
 
-// Now discard the master plan itself.
+// Now discard the controlling plan itself.
 // The bottom-most plan never gets discarded.  "OkayToDiscard" for it
 // means discard it's dependent plans, but not it...
-if (master_plan_idx > 0) {
+if (controlling_plan_idx > 0) {
   DiscardPlan();
 }
   }
Index: lldb/source/Target/ThreadPlanPython.cpp
===
--- lldb/source/Target/ThreadPlanPython.cpp
+++ lldb/source/Target/ThreadPlanPython.cpp
@@ -31,7 +31,7 @@
  eVoteNoOpinion, eVoteNoOpinion),
   m_class_name(class_name), m_args_data(args_data), m_did_push(false),
   m_stop_others(false) {
-  SetIsMasterPlan(true);
+  SetIsControllingPlan(true);
   SetOkayToDiscard(true);
   SetPrivate(false);
 }
Index: lldb/source/Target/ThreadPlanCallUserExpression.cpp
===
--- lldb/source/Target/ThreadPlanCallUserExpression.cpp
+++ lldb/source/Target/ThreadPlanCallUserExpression.cpp
@@ -39,7 +39,7 @@
   m_user_expression_sp(user_expression_sp) {
   // User expressions are generally "User generated" so we should set them up
   // to stop when done.
-  SetIsMasterPlan(true);
+  SetIsControllingPlan(true);
   SetOkayToDiscard(false);
 }
 
Index: lldb/source/Target/ThreadPlanCallOnFunctionExit.cpp
===
--- lldb/source/Target/ThreadPlanCallOnFunctionExit.cpp
+++ lldb/source/Target/ThreadPlanCallOnFunctionExit.cpp
@@ -18,7 +18,7 @@
  ),
   m_callback(callback) {
   // We are not a user-generated plan.
-  SetIsMasterPlan(false);
+  SetIsControllingPlan(false);
 }
 
 void ThreadPlanCallOnFunctionExit::DidPush() {
Index: lldb/source/Target/ThreadPlanCallFunction.cpp
===
--- lldb/source/Target/ThreadPlanCallFunction.cpp
+++ lldb/source/Target/ThreadPlanCallFunction.cpp
@@ -33,7 +33,7 @@
 bool ThreadPlanCallFunction::ConstructorSetup(
 Thread &thread, ABI *&abi, lldb::addr_t &start_load_addr,
 lldb::addr_t &function_load_addr) {
-  SetIsMasterPlan(true);
+  SetIsControllingPlan(true);
   SetOkayToDiscard(false);
   SetPrivate(true);
 
Index: lldb/source/Target/ThreadPlanBase.cpp

[Lldb-commits] [lldb] 04cbfa9 - [lldb][NFC] Inclusive Language: rename master plan to controlling plan

[Quinn Pham via lldb-commits]

Author: Quinn Pham
Date: 2021-11-11T15:04:44-06:00
New Revision: 04cbfa950e0221ac334f802407a9b766df33eee5

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

LOG: [lldb][NFC] Inclusive Language: rename master plan to controlling plan

[NFC] As part of using inclusive language within the llvm project, this patch
renames master plan to controlling plan in lldb.

Reviewed By: jingham

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

Added: 


Modified: 
lldb/docs/use/python-reference.rst
lldb/include/lldb/Target/Thread.h
lldb/include/lldb/Target/ThreadPlan.h
lldb/include/lldb/Target/ThreadPlanStack.h
lldb/source/API/SBThread.cpp
lldb/source/Commands/CommandObjectThread.cpp
lldb/source/Expression/FunctionCaller.cpp
lldb/source/Target/Process.cpp
lldb/source/Target/StopInfo.cpp
lldb/source/Target/Thread.cpp
lldb/source/Target/ThreadPlan.cpp
lldb/source/Target/ThreadPlanBase.cpp
lldb/source/Target/ThreadPlanCallFunction.cpp
lldb/source/Target/ThreadPlanCallOnFunctionExit.cpp
lldb/source/Target/ThreadPlanCallUserExpression.cpp
lldb/source/Target/ThreadPlanPython.cpp
lldb/source/Target/ThreadPlanStack.cpp

Removed: 




diff  --git a/lldb/docs/use/python-reference.rst 
b/lldb/docs/use/python-reference.rst
index 993ae3a0bc8e1..d5ab09cbf7c22 100644
--- a/lldb/docs/use/python-reference.rst
+++ b/lldb/docs/use/python-reference.rst
@@ -442,7 +442,7 @@ And for a MUCH fuller discussion of the whole state 
machine, see:
 
https://github.com/llvm/llvm-project/blob/main/lldb/include/lldb/Target/ThreadPlan.h
 
 If you are reading those comments it is useful to know that scripted thread
-plans are set to be "MasterPlans", and not "OkayToDiscard".
+plans are set to be "ControllingPlans", and not "OkayToDiscard".
 
 To implement a scripted step, you define a python class that has the following
 methods:

diff  --git a/lldb/include/lldb/Target/Thread.h 
b/lldb/include/lldb/Target/Thread.h
index a48be19005153..91feed310eb97 100644
--- a/lldb/include/lldb/Target/Thread.h
+++ b/lldb/include/lldb/Target/Thread.h
@@ -1015,7 +1015,8 @@ class Thread : public 
std::enable_shared_from_this,
 
   /// Discards the plans queued on the plan stack of the current thread.  This
   /// is
-  /// arbitrated by the "Master" ThreadPlans, using the "OkayToDiscard" call.
+  /// arbitrated by the "Controlling" ThreadPlans, using the "OkayToDiscard"
+  /// call.
   //  But if \a force is true, all thread plans are discarded.
   void DiscardThreadPlans(bool force);
 

diff  --git a/lldb/include/lldb/Target/ThreadPlan.h 
b/lldb/include/lldb/Target/ThreadPlan.h
index 58d6ef5fef8ba..616939f89fc8e 100644
--- a/lldb/include/lldb/Target/ThreadPlan.h
+++ b/lldb/include/lldb/Target/ThreadPlan.h
@@ -144,39 +144,42 @@ namespace lldb_private {
 //  implement DoPlanExplainsStop, the result is cached in PlanExplainsStop so
 //  the DoPlanExplainsStop itself will only get called once per stop.
 //
-//  Master plans:
+//  Controlling plans:
 //
 //  In the normal case, when we decide to stop, we will  collapse the plan
 //  stack up to the point of the plan that understood the stop reason.
 //  However, if a plan wishes to stay on the stack after an event it didn't
-//  directly handle it can designate itself a "Master" plan by responding true
-//  to IsMasterPlan, and then if it wants not to be discarded, it can return
-//  false to OkayToDiscard, and it and all its dependent plans will be
+//  directly handle it can designate itself a "Controlling" plan by responding
+//  true to IsControllingPlan, and then if it wants not to be discarded, it can
+//  return false to OkayToDiscard, and it and all its dependent plans will be
 //  preserved when we resume execution.
 //
-//  The other effect of being a master plan is that when the Master plan is
+//  The other effect of being a controlling plan is that when the Controlling
+//  plan is
 //  done , if it has set "OkayToDiscard" to false, then it will be popped &
 //  execution will stop and return to the user.  Remember that if OkayToDiscard
 //  is false, the plan will be popped and control will be given to the next
 //  plan above it on the stack  So setting OkayToDiscard to false means the
-//  user will regain control when the MasterPlan is completed.
+//  user will regain control when the ControllingPlan is completed.
 //
 //  Between these two controls this allows things like: a
-//  MasterPlan/DontDiscard Step Over to hit a breakpoint, stop and return
+//  ControllingPlan/DontDiscard Step Over to hit a breakpoint, stop and return
 //  control to the user, but then when the user continues, the step out
 //  succeeds.  Even more tricky, when the breakpoint is hit, the user can
 //  continue to step in/step over/etc, 

[Lldb-commits] [PATCH] D113521: Allow lldb to launch a remote executable when there isn't a local copy

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

In D113521#3124239 , @labath wrote:

> In D113521#3122654 , @jingham wrote:
>
>> In D113521#3120703 , @labath wrote:
>>
>>> I have two high-level questions about this:
>>>
>>> - what should be the relative priority of executable ModuleSP vs the launch 
>>> info? IIUC, in the current implementation the module takes precedence, but 
>>> it seems to me it would be more natural if the launch info came out on top. 
>>> One of the reasons I'm thinking about this is because you currently cannot 
>>> re-run executables that use exec(2) (I mean, you can, but it will rarely 
>>> produce the desired results). This happens because we use the post-exec 
>>> executable, but the rest of the launch info refers to the main one. If the 
>>> executable module was not the primary source of truth here, then maybe the 
>>> we could somehow use this mechanism to improve the exec story as well (by 
>>> storing the original executable in the launch info or something).
>
>
>
>> Anyway, I think you are asking the question "What should we do if the 
>> Target's ProcessLaunchInfo::GetExecutableFile differs from the one in the 
>> Target's Executable Module".  Or rather, should we keep the Target's 
>> ProcessLaunchInfo as the truth of what that target wants to launch, rather 
>> than looking at the Executable module.
>>
>> That question is orthogonal to the one this patch is determining, which is 
>> just about the case where there isn't an executable file in the target so 
>> that the user needs to provide this info from the outside.  So while I agree 
>> that yours is an interesting question, it isn't directly germane to this 
>> patch.
>
> Yes, that is the question I am asking. I didn't want to go off into designing 
> an exec feature. I only mentioned because it seemed potentially related. We 
> can put away that for now.
>
> While my question may not be "directly germane" to this patch, I wouldn't say 
> it's orthogonal either. Right now (IIUC) the executable module is always the 
> source of truth for the launch operation. Now you're adding a second source, 
> so one has to choose how to handle the situation when both of them are 
> present. You may not care what happens in that case, but _something_ is going 
> to happen nonetheless. I guess we basically have three options:
> a) prefer the executable module (this is what the patch implements, and it's 
> also the smallest deviation from status quo of completely ignoring launch 
> info)
> b) prefer the launch info (that is what I proposed)
> c) make it an error (I think that's something we could all agree on)
>
> The reason I proposed (b) is because of the principle of specific overriding 
> general. The executable module is embedded into the target, so I would 
> consider it more general than the launch info, which can be provided directly 
> to the Launch method. Greg's use case (launching a remote binary that you 
> already have a copy of) seems like it could benefit from that. However, maybe 
> I have an even better one. What would happen with reruns? On the first run, 
> the user would create a executable-less target, and provide a binary through 
> the launch info. The binary would launch fine and exit. Then the user would 
> try to launch again using the same target and launch info, but the code would 
> go down a different path (I'm not sure which one) because the target would 
> already have an executable. (This is actually kind of similar to the exec 
> scenario, because the executable module would change between the two runs.)

This is the same situation as if you had attached to a PID on a remote with no 
local binary, then reran, right?  That should also work w/o the user having to 
intervene with a LaunchInfo for the second run, and so should re-running in 
this case.  We shouldn't NEED the extra launch info to make rerun in your case 
work.  And in general we don't ask users to respecify elements of the 
LaunchInfo like arguments and environments on rerun - we always reuse what you 
set the first time. That seems like a good principle to follow here as well.  
So if this doesn't work, it's really a bug in our handling of the information 
we have after the first run exits, and doesn't seem like a very strong argument 
for case (b).

I don't think Greg's use case is instructive here either.  If you already have 
a binary locally, you can set its remote executable path directly, which is the 
extant way of specifying the remote path.  The other part of his ask was "give 
me a way not to install it" but the fact that you've set an ExecutableFile in 
your LaunchInfo doesn't seem like a good way to tell us that.  If we want to 
add that, then we should do it by adding a {Get/Set}ShouldInstallBinary to the 
LaunchInfo (*).

It seems to me the meaning of SetExecutableFile is clear if you don't have an 
executable module.  

[Lldb-commits] [PATCH] D113720: [lldb][NFC] Inclusive language: rename m_master in ASTImporterDelegate

[Quinn Pham via Phabricator via lldb-commits]

quinnp created this revision.
Herald added a subscriber: martong.
Herald added a reviewer: a.sidorin.
Herald added a reviewer: shafik.
quinnp requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

[NFC] As part of using inclusive language within the llvm project, this patch
replaces `m_master` in `ASTImporterDelegate` with `m_main`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D113720

Files:
  lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.h

Index: lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.h
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.h
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.h
@@ -259,11 +259,11 @@
   /// CxxModuleHandler to replace any missing or malformed declarations with
   /// their counterpart from a C++ module.
   struct ASTImporterDelegate : public clang::ASTImporter {
-ASTImporterDelegate(ClangASTImporter &master, clang::ASTContext *target_ctx,
+ASTImporterDelegate(ClangASTImporter &main, clang::ASTContext *target_ctx,
 clang::ASTContext *source_ctx)
-: clang::ASTImporter(*target_ctx, master.m_file_manager, *source_ctx,
- master.m_file_manager, true /*minimal*/),
-  m_master(master), m_source_ctx(source_ctx) {
+: clang::ASTImporter(*target_ctx, main.m_file_manager, *source_ctx,
+ main.m_file_manager, true /*minimal*/),
+  m_main(main), m_source_ctx(source_ctx) {
   // Target and source ASTContext shouldn't be identical. Importing AST
   // nodes within the same AST doesn't make any sense as the whole idea
   // is to import them to a different AST.
@@ -329,7 +329,7 @@
 /// were created from the 'std' C++ module to prevent that the Importer
 /// tries to sync them with the broken equivalent in the debug info AST.
 llvm::SmallPtrSet m_decls_to_ignore;
-ClangASTImporter &m_master;
+ClangASTImporter &m_main;
 clang::ASTContext *m_source_ctx;
 CxxModuleHandler *m_std_handler = nullptr;
 /// The currently attached listener.
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
@@ -824,7 +824,7 @@
   }
 
   // Check which ASTContext this declaration originally came from.
-  DeclOrigin origin = m_master.GetDeclOrigin(From);
+  DeclOrigin origin = m_main.GetDeclOrigin(From);
 
   // Prevent infinite recursion when the origin tracking contains a cycle.
   assert(origin.decl != From && "Origin points to itself?");
@@ -853,7 +853,7 @@
   // though all these different source ASTContexts just got a copy from
   // one source AST).
   if (origin.Valid()) {
-auto R = m_master.CopyDecl(&getToContext(), origin.decl);
+auto R = m_main.CopyDecl(&getToContext(), origin.decl);
 if (R) {
   RegisterImportedDecl(From, R);
   return R;
@@ -862,7 +862,7 @@
 
   // If we have a forcefully completed type, try to find an actual definition
   // for it in other modules.
-  const ClangASTMetadata *md = m_master.GetDeclMetadata(From);
+  const ClangASTMetadata *md = m_main.GetDeclMetadata(From);
   auto *td = dyn_cast(From);
   if (td && md && md->IsForcefullyCompleted()) {
 Log *log = GetLogIfAllCategoriesSet(LIBLLDB_LOG_EXPRESSIONS);
@@ -1045,7 +1045,7 @@
   }
 
   lldb::user_id_t user_id = LLDB_INVALID_UID;
-  ClangASTMetadata *metadata = m_master.GetDeclMetadata(from);
+  ClangASTMetadata *metadata = m_main.GetDeclMetadata(from);
   if (metadata)
 user_id = metadata->GetUserID();
 
@@ -1069,9 +1069,9 @@
   }
 
   ASTContextMetadataSP to_context_md =
-  m_master.GetContextMetadata(&to->getASTContext());
+  m_main.GetContextMetadata(&to->getASTContext());
   ASTContextMetadataSP from_context_md =
-  m_master.MaybeGetContextMetadata(m_source_ctx);
+  m_main.MaybeGetContextMetadata(m_source_ctx);
 
   if (from_context_md) {
 DeclOrigin origin = from_context_md->getOrigin(from);
@@ -1082,7 +1082,7 @@
   to_context_md->setOrigin(to, origin);
 
 ImporterDelegateSP direct_completer =
-m_master.GetDelegate(&to->getASTContext(), origin.ctx);
+m_main.GetDelegate(&to->getASTContext(), origin.ctx);
 
 if (direct_completer.get() != this)
   direct_completer->ASTImporter::Imported(origin.decl, to);
@@ -1143,7 +1143,7 @@
   }
 
   if (auto *to_namespace_decl = dyn_cast(to)) {
-m_master.BuildNamespaceMap(to_namespace_decl);
+m_main.BuildNamespaceMap(to_namespace_decl);
 to_namespace_decl->setHasExternalVisibleStorage();
   }
 
@@ -1172,10 +1172,10 @@
   }
 
   if (clang:

[Lldb-commits] [lldb] ac33e65 - [lldb][NFC] Delete commented out code in AddressRange

[Alex Langford via lldb-commits]

Author: Alex Langford
Date: 2021-11-11T15:42:27-08:00
New Revision: ac33e65d2169260364e3e92fed2ba81c58d5ce33

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

LOG: [lldb][NFC] Delete commented out code in AddressRange

Added: 


Modified: 
lldb/include/lldb/Core/AddressRange.h
lldb/source/Core/AddressRange.cpp

Removed: 




diff  --git a/lldb/include/lldb/Core/AddressRange.h 
b/lldb/include/lldb/Core/AddressRange.h
index 6fbdc35c91684..4a33c2d795876 100644
--- a/lldb/include/lldb/Core/AddressRange.h
+++ b/lldb/include/lldb/Core/AddressRange.h
@@ -242,8 +242,6 @@ class AddressRange {
   lldb::addr_t m_byte_size = 0; ///< The size in bytes of this address range.
 };
 
-// bool operator== (const AddressRange& lhs, const AddressRange& rhs);
-
 } // namespace lldb_private
 
 #endif // LLDB_CORE_ADDRESSRANGE_H

diff  --git a/lldb/source/Core/AddressRange.cpp 
b/lldb/source/Core/AddressRange.cpp
index af6e31a67da30..66dcda574890a 100644
--- a/lldb/source/Core/AddressRange.cpp
+++ b/lldb/source/Core/AddressRange.cpp
@@ -59,15 +59,6 @@ bool AddressRange::Contains(const Address &addr) const {
   return ContainsFileAddress(addr);
 }
 
-//
-// bool
-// AddressRange::Contains (const Address *addr) const
-//{
-//if (addr)
-//return Contains (*addr);
-//return false;
-//}
-
 bool AddressRange::ContainsFileAddress(const Address &addr) const {
   if (addr.GetSection() == m_base_addr.GetSection())
 return (addr.GetOffset() - m_base_addr.GetOffset()) < GetByteSize();
@@ -212,11 +203,3 @@ void AddressRange::DumpDebug(Stream *s) const {
 static_cast(m_base_addr.GetSection().get()),
 m_base_addr.GetOffset(), GetByteSize());
 }
-//
-// bool
-// lldb::operator==(const AddressRange& lhs, const AddressRange& rhs)
-//{
-//if (lhs.GetBaseAddress() == rhs.GetBaseAddress())
-//return lhs.GetByteSize() == rhs.GetByteSize();
-//return false;
-//}



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

[Lldb-commits] [PATCH] D113722: [lldb] fix test expectation broken by clang fix at D110216

[Matheus Izvekov via Phabricator via lldb-commits]

mizvekov created this revision.
mizvekov requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Signed-off-by: Matheus Izvekov 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D113722

Files:
  lldb/test/Shell/SymbolFile/NativePDB/function-types-builtins.cpp


Index: lldb/test/Shell/SymbolFile/NativePDB/function-types-builtins.cpp
===
--- lldb/test/Shell/SymbolFile/NativePDB/function-types-builtins.cpp
+++ lldb/test/Shell/SymbolFile/NativePDB/function-types-builtins.cpp
@@ -118,7 +118,7 @@
 auto aac = &unary;
 // CHECK: (void (*)(int (&&)[5])) aac = {{.*}}
 auto aad = &unary;
-// CHECK: (void (*)(int (*)[5])) aad = {{.*}}
+// CHECK: (void (*)(int (*const)[5])) aad = {{.*}}
 
 
 // same test cases with return values, note we can't overload on return type


Index: lldb/test/Shell/SymbolFile/NativePDB/function-types-builtins.cpp
===
--- lldb/test/Shell/SymbolFile/NativePDB/function-types-builtins.cpp
+++ lldb/test/Shell/SymbolFile/NativePDB/function-types-builtins.cpp
@@ -118,7 +118,7 @@
 auto aac = &unary;
 // CHECK: (void (*)(int (&&)[5])) aac = {{.*}}
 auto aad = &unary;
-// CHECK: (void (*)(int (*)[5])) aad = {{.*}}
+// CHECK: (void (*)(int (*const)[5])) aad = {{.*}}
 
 
 // same test cases with return values, note we can't overload on return type
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D113722: [lldb] fix test expectation broken by clang fix at D110216

[Matheus Izvekov via Phabricator via lldb-commits]

mizvekov added a comment.
Herald added a subscriber: JDevlieghere.

Fixes buildbot failure seen at 
http://lab.llvm.org:8011/#/builders/68/builds/21662/steps/6/logs/stdio
Caused by a fix in D110216 .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113722

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

[Lldb-commits] [lldb] 5508595 - [lldb] fix test expectation broken by clang fix at D110216

[Matheus Izvekov via lldb-commits]

Author: Matheus Izvekov
Date: 2021-11-12T01:47:29+01:00
New Revision: 55085952175ed3b029097a0594acc4e34a5df218

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

LOG: [lldb] fix test expectation broken by clang fix at D110216

Signed-off-by: Matheus Izvekov 

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

Added: 


Modified: 
lldb/test/Shell/SymbolFile/NativePDB/function-types-builtins.cpp

Removed: 




diff  --git a/lldb/test/Shell/SymbolFile/NativePDB/function-types-builtins.cpp 
b/lldb/test/Shell/SymbolFile/NativePDB/function-types-builtins.cpp
index 75f9a029d448c..69d8d17179fe9 100644
--- a/lldb/test/Shell/SymbolFile/NativePDB/function-types-builtins.cpp
+++ b/lldb/test/Shell/SymbolFile/NativePDB/function-types-builtins.cpp
@@ -118,7 +118,7 @@ auto aab = &unary;
 auto aac = &unary;
 // CHECK: (void (*)(int (&&)[5])) aac = {{.*}}
 auto aad = &unary;
-// CHECK: (void (*)(int (*)[5])) aad = {{.*}}
+// CHECK: (void (*)(int (*const)[5])) aad = {{.*}}
 
 
 // same test cases with return values, note we can't overload on return type



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

[Lldb-commits] [PATCH] D113722: [lldb] fix test expectation broken by clang fix at D110216

[Matheus Izvekov via Phabricator via lldb-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rG55085952175e: [lldb] fix test expectation broken by clang 
fix at D110216 (authored by mizvekov).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113722

Files:
  lldb/test/Shell/SymbolFile/NativePDB/function-types-builtins.cpp


Index: lldb/test/Shell/SymbolFile/NativePDB/function-types-builtins.cpp
===
--- lldb/test/Shell/SymbolFile/NativePDB/function-types-builtins.cpp
+++ lldb/test/Shell/SymbolFile/NativePDB/function-types-builtins.cpp
@@ -118,7 +118,7 @@
 auto aac = &unary;
 // CHECK: (void (*)(int (&&)[5])) aac = {{.*}}
 auto aad = &unary;
-// CHECK: (void (*)(int (*)[5])) aad = {{.*}}
+// CHECK: (void (*)(int (*const)[5])) aad = {{.*}}
 
 
 // same test cases with return values, note we can't overload on return type


Index: lldb/test/Shell/SymbolFile/NativePDB/function-types-builtins.cpp
===
--- lldb/test/Shell/SymbolFile/NativePDB/function-types-builtins.cpp
+++ lldb/test/Shell/SymbolFile/NativePDB/function-types-builtins.cpp
@@ -118,7 +118,7 @@
 auto aac = &unary;
 // CHECK: (void (*)(int (&&)[5])) aac = {{.*}}
 auto aad = &unary;
-// CHECK: (void (*)(int (*)[5])) aad = {{.*}}
+// CHECK: (void (*)(int (*const)[5])) aad = {{.*}}
 
 
 // same test cases with return values, note we can't overload on return type
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D113724: [LLDB][NativePDB] Fix missing symbol info when backtrace minidump

[Zequan Wu via Phabricator via lldb-commits]

zequanwu created this revision.
zequanwu added reviewers: rnk, labath, teemperor, shafik.
zequanwu requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added subscribers: lldb-commits, sstefan1.
Herald added a project: LLDB.

I don't know how to add tests for this. I tested locally with a minidump, a pdb
and the executable.
Before this change, the following comand fail to give symbol information for
backtrace because the `m_comp_units` in `m_index->compilands()` was always
empty, changed to use `GetCompileUnitAtIndex` to create CU when failed to find
one at given index.

  lldb -O 'target create GoogleDriveFS.exe --core GoogleDriveFS.dmp' -o 'target 
symbols add GoogleDriveFS.pdb' -o 'bt'


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D113724

Files:
  lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp


Index: lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
===
--- lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
+++ lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
@@ -945,11 +945,11 @@
 llvm::Optional modi = m_index->GetModuleIndexForVa(file_addr);
 if (!modi)
   return 0;
-CompilandIndexItem *cci = m_index->compilands().GetCompiland(*modi);
-if (!cci)
+CompUnitSP cu_sp = GetCompileUnitAtIndex(modi.getValue());
+if (!cu_sp)
   return 0;
 
-sc.comp_unit = GetOrCreateCompileUnit(*cci).get();
+sc.comp_unit = cu_sp.get();
 resolved_flags |= eSymbolContextCompUnit;
   }
 
Index: lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
===
--- lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
+++ lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
@@ -1013,9 +1013,22 @@
   llvm::StringRef proc_name = proc.Name;
   proc_name.consume_front(context_name);
   proc_name.consume_front("::");
-
-  clang::FunctionDecl *function_decl = m_clang.CreateFunctionDeclaration(
-  parent, OptionalClangModuleID(), proc_name, func_ct, storage, false);
+  clang::FunctionDecl *function_decl = nullptr;
+  if (parent->isRecord()) {
+clang::QualType parent_qt = llvm::dyn_cast(parent)
+->getTypeForDecl()
+->getCanonicalTypeInternal();
+// TODO: Get other information for this method.
+function_decl = m_clang.AddMethodToCXXRecordType(
+ToCompilerType(parent_qt).GetOpaqueQualType(), proc_name,
+/*mangled_name=*/nullptr, func_ct, lldb::AccessType::eAccessPublic,
+/*is_virtual=*/false, /*is_static=*/false,
+/*is_inline=*/false, /*is_explicit=*/false,
+/*is_attr_used=*/false, /*is_artificial=*/false);
+  } else {
+function_decl = m_clang.CreateFunctionDeclaration(
+parent, OptionalClangModuleID(), proc_name, func_ct, storage, false);
+  }
 
   lldbassert(m_uid_to_decl.count(toOpaqueUid(func_id)) == 0);
   m_uid_to_decl[toOpaqueUid(func_id)] = function_decl;
@@ -1023,8 +1036,9 @@
   status.resolved = true;
   status.uid = toOpaqueUid(func_id);
   m_decl_to_status.insert({function_decl, status});
-
-  CreateFunctionParameters(func_id, *function_decl, func_type->getNumParams());
+  if (!parent->isRecord())
+CreateFunctionParameters(func_id, *function_decl,
+ func_type->getNumParams());
 
   return function_decl;
 }


Index: lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
===
--- lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
+++ lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
@@ -945,11 +945,11 @@
 llvm::Optional modi = m_index->GetModuleIndexForVa(file_addr);
 if (!modi)
   return 0;
-CompilandIndexItem *cci = m_index->compilands().GetCompiland(*modi);
-if (!cci)
+CompUnitSP cu_sp = GetCompileUnitAtIndex(modi.getValue());
+if (!cu_sp)
   return 0;
 
-sc.comp_unit = GetOrCreateCompileUnit(*cci).get();
+sc.comp_unit = cu_sp.get();
 resolved_flags |= eSymbolContextCompUnit;
   }
 
Index: lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
===
--- lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
+++ lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
@@ -1013,9 +1013,22 @@
   llvm::StringRef proc_name = proc.Name;
   proc_name.consume_front(context_name);
   proc_name.consume_front("::");
-
-  clang::FunctionDecl *function_decl = m_clang.CreateFunctionDeclaration(
-  parent, OptionalClangModuleID(), proc_name, func_ct, storage, false);
+  clang::FunctionDecl *function_decl = nullptr;
+  if (parent->isRecord()) {
+clang::QualType parent_qt = llvm::dyn_cast(parent)
+   

[Lldb-commits] [PATCH] D113608: [lldb] Simplify specifying of platform supported architectures

[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




Comment at: lldb/source/Target/Platform.cpp:1217-1218
+std::vector
+Platform::CreateArchList(llvm::Triple::OSType os,
+ llvm::ArrayRef archs) {
+  std::vector list;

labath wrote:
> JDevlieghere wrote:
> > In PlatformDarwin we set the vendor too. Could we have this take an 
> > optional OSType and Vendor maybe that sets the ones that are not None? That 
> > would also make migrating PlatformDarwin easier where we currently share 
> > the code and don't set the OS. Later we could move this out of 
> > PlatformDarwin into the child classes and have the OS set correctly. 
> Umm.. maybe? I don't think this function is set in stone, but the nice thing 
> about it being a helper function is that it's usage is completely optional. I 
> haven't tried porting the darwin platforms yet, but, from a cursory glance I 
> did see that they have a lot more complex usage patterns:
> - they pass subarchitectures as well, so we'd either also need to have an 
> Optional subarch field, or switch to passing architectures as strings (I 
> mean, I suppose I could switch to strings now, but it feels more correct to 
> work with named constants)
> - a few of them also pass environments 
> - and some don't even have matching OS fields (`x86_64-apple-macosx` and 
> `arm64-apple-ios` in the same list)
> 
> So, I'd leave it to those patches to determine whether it's better to extend 
> this function or create a new one.
Okay, I'm happy to modify the helper when I start using it for the darwin 
platfomrs. I think at least some of the complexity/inconsistencies is purely 
historic such as the one where we were specifying an OS in the list of 
supported architectures in PlatformDarwin. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113608

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

[Lldb-commits] [PATCH] D113724: [LLDB][NativePDB] Fix missing symbol info when backtrace minidump

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

IIUC, these are two separate fixes:
a) fix clang ast creation for methods
b) fix symbol context lookup
It would be better to split them into two patches, as each will require a 
different kind of a test. The ast thing could be tested by running "image dump 
ast" on an appropriate piece of debug info. I'm hoping that the second one 
could be tested through "image lookup -a", but I don't know if it'll be as 
simple as running just that command. I guess you might need to play around with 
it a bit to find a way to invoke it without first instantiating the compile 
unit. It's possible you will need more than one compile unit in the test 
executable.

I don't think either of the fixes is really tied to minidumps, so I hope it 
will be possible to reproduce the issues by just compiling some c++ (without 
even running the program).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113724

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