[Lldb-commits] [PATCH] D101237: [lldb] Fix [[no_unique_address]] and libstdc++ 11's std::unique_ptr

2021-04-26 Thread David Blaikie via Phabricator via lldb-commits
dblaikie added a comment.

In D101237#2718456 , @shafik wrote:

> I think @dblaikie original idea of adding a DWARF attribute for this case is 
> the right way to go here. AFAICT this will change the answer to basic 
> questions such as what size a `struct` is and this will likely lead to 
> confusion from our users who will expect the answers in expression parsing to 
> match what they are seeing elsewhere e.g.:
>
>   struct X {
>   int i;
>   [[no_unique_address]] Empty e;
>   };
>
>   struct X2 {
>   int i;
>   Empty e;
>   };
>
>   struct Z {
>   char c;
>   [[no_unique_address]] Empty e1, e2;
>   };
>   
>   struct Z2 {
>   char c;
>   Empty e1, e2;
>   };
>   
>   
>   struct W {
>   char c[2];
>   [[no_unique_address]] Empty e1, e2;
>   };
>   
>   struct W2 {
>   char c[2];
>   Empty e1, e2;
>   };
>   
>   
>   int main()
>   {
>   std::cout << sizeof(Empty) << "\n"
> << "X: " << sizeof(X) << "\n"
> << "X2: " << sizeof(X2) << "\n"
> << "Z: " << sizeof(Z) << "\n"
> << "Z2: " << sizeof(Z2) << "\n"
> << "W: " << sizeof(W) << "\n"
> << "W2: " << sizeof(W2) << "\n";
> 
>   }
>
> See godbolt  which shows this result:
>
>   1
>   X: 4
>   X2: 8
>   Z: 2
>   Z2: 3
>   W: 3
>   W2: 4

Hmm, but is this still a problem for the particular way lldb does things - 
where it simultaneiously marks all the fields as no_unique_address (in this 
patch), but then still tells clang exactly where the fields will go? (and it 
could tell clang exactly how large the structure is too - from the DWARF)

@jankratochvil - have you been able to use these examples from @shafik to 
demonstrate problems with this patch when applied to lldb? What sort of 
problems arise in lldb?

(that said, an extension flag attribute should be fairly low cost to add and 
nice to explicitly encode this parameter in any case, I think - but I'd like to 
understand the tradeoffs/justification/etc)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101237

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


[Lldb-commits] [PATCH] D101237: [lldb] Fix [[no_unique_address]] and libstdc++ 11's std::unique_ptr

2021-04-26 Thread Jan Kratochvil via Phabricator via lldb-commits
jankratochvil planned changes to this revision.
jankratochvil added a comment.

In D101237#2718456 , @shafik wrote:

> AFAICT this will change the answer to basic questions such as what size a 
> `struct` is and this will likely lead to confusion from our users who will 
> expect the answers in expression parsing to match what they are seeing 
> elsewhere e.g.:

OK, that justifies the new DW_AT_*. I did not notice there is this difference. 
For my needs the new DW_AT_* thus needs to be implemented also in GCC.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101237

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


[Lldb-commits] [PATCH] D101250: Wrap edit line configuration calls into helper functions

2021-04-26 Thread Neal via Phabricator via lldb-commits
nealsid added a comment.

In D101250#2718107 , @JDevlieghere 
wrote:

> LGTM. I wonder if we would want to wrap this in a macro to get rid of the 
> `EditLineConstString` duplication while keeping the type safety.

Thanks!  I looked into removing the EditLineConstString, but I wasn't a fan of 
having two preprocessor macro expansions.  Maybe it could be a template 
function wchar_t or char.  I also tried some template specialization where the 
functions to call into the edit line library could be instantiated when used, 
but it didn't really appear to save much because all the method signatures for 
edit line parameters have to be manually maintained, and some of them still 
take varargs even after specifying the edit line op parameter., e.g.:

template void editLineCaller();

template<>
void editLineCaller(param1Type param1, param2Type param2) {

  el_set(EL_ADFN, param1, param2);

. . .
}


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101250

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


[Lldb-commits] [PATCH] D101237: [lldb] Fix [[no_unique_address]] and libstdc++ 11's std::unique_ptr

2021-04-26 Thread Shafik Yaghmour via Phabricator via lldb-commits
shafik added a comment.

I think @dblaikie original idea of adding a DWARF attribute for this case is 
the right way to go here. AFAICT this will change the answer to basic questions 
such as what size a `struct` is and this will likely lead to confusion from our 
users who will expect the answers in expression parsing to match what they are 
seeing elsewhere e.g.:

  struct X {
  int i;
  [[no_unique_address]] Empty e;
  };
   
  struct X2 {
  int i;
  Empty e;
  };
   
  struct Z {
  char c;
  [[no_unique_address]] Empty e1, e2;
  };
  
  struct Z2 {
  char c;
  Empty e1, e2;
  };
  
  
  struct W {
  char c[2];
  [[no_unique_address]] Empty e1, e2;
  };
  
  struct W2 {
  char c[2];
  Empty e1, e2;
  };
  
  
  int main()
  {
  std::cout << sizeof(Empty) << "\n"
<< "X: " << sizeof(X) << "\n"
<< "X2: " << sizeof(X2) << "\n"
<< "Z: " << sizeof(Z) << "\n"
<< "Z2: " << sizeof(Z2) << "\n"
<< "W: " << sizeof(W) << "\n"
<< "W2: " << sizeof(W2) << "\n";

  }

See godbolt  which shows this result:

  1
  X: 4
  X2: 8
  Z: 2
  Z2: 3
  W: 3
  W2: 4


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101237

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


[Lldb-commits] [PATCH] D100965: [lldb] Refactor argument group by SourceLocationSpec (NFCI)

2021-04-26 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib added a comment.

In D100965#2716239 , @teemperor wrote:

> I am wondering how SourceLocationSpec is related to lldb's Declaration class 
> (which is FileSpec + line + column and describes a location in source code)? 
> It seems like the current SourceLocationSpec is just a Declaration with the 
> two additional search variables (and the first iteration was exactly the same 
> as Declaration).
> Could we maybe turn SourceLocationSpec to store a lldb::Declaration instead 
> of file/line/column? I'm aware that the Declaration class first needs some 
> cleanup (and a removal of that #ifdef around column), but right now we 
> are already using Declarations in a bunch of places to describe source 
> locations.

I don't think adding a level of indirection would serve much purpose and I 
don't like the fact `Declaration` is an exposed class (being part of the `lldb` 
namespace), and that it uses a macro-based system for invalid values. Also 
`SourceLocationSpec` instances are immutable and also be valid. May be I could 
try to replace all occurrences of `Declaration` by `SourceLocationSpec` in a 
follow-up patch ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100965

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


[Lldb-commits] [PATCH] D99947: [LLDB] AArch64 Linux PAC unwinder support

2021-04-26 Thread Muhammad Omair Javaid via Phabricator via lldb-commits
omjavaid updated this revision to Diff 340702.
omjavaid added a comment.

Fixed review comments and rebased.


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

https://reviews.llvm.org/D99947

Files:
  lldb/test/API/functionalities/unwind/aarch64_unwind_pac/Makefile
  
lldb/test/API/functionalities/unwind/aarch64_unwind_pac/TestAArch64UnwindPAC.py
  lldb/test/API/functionalities/unwind/aarch64_unwind_pac/main.c


Index: lldb/test/API/functionalities/unwind/aarch64_unwind_pac/main.c
===
--- /dev/null
+++ lldb/test/API/functionalities/unwind/aarch64_unwind_pac/main.c
@@ -0,0 +1,24 @@
+// This program makes a multi tier nested function call to test AArch64
+// Pointer Authentication feature.
+
+// To enable PAC return address signing compile with following clang arguments:
+// -march=armv8.5-a -mbranch-protection=pac-ret+leaf
+
+#include 
+
+static void __attribute__((noinline)) func_c(void) {
+  exit(0); // Frame func_c
+}
+
+static void __attribute__((noinline)) func_b(void) {
+  func_c(); // Frame func_b
+}
+
+static void __attribute__((noinline)) func_a(void) {
+  func_b(); // Frame func_a
+}
+
+int main(int argc, char *argv[]) {
+  func_a(); // Frame main
+  return 0;
+}
Index: 
lldb/test/API/functionalities/unwind/aarch64_unwind_pac/TestAArch64UnwindPAC.py
===
--- /dev/null
+++ 
lldb/test/API/functionalities/unwind/aarch64_unwind_pac/TestAArch64UnwindPAC.py
@@ -0,0 +1,43 @@
+"""
+Test that we can backtrace correctly when AArch64 PAC is enabled
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class AArch64UnwindPAC(TestBase):
+mydir = TestBase.compute_mydir(__file__)
+
+@skipIf(archs=no_match(["aarch64"]))
+@skipIf(oslist=no_match(['linux']))
+def test(self):
+"""Test that we can backtrace correctly when AArch64 PAC is enabled"""
+self.build()
+
+self.line = line_number('main.c', '// Frame func_c')
+
+exe = self.getBuildArtifact("a.out")
+self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
+
+lldbutil.run_break_set_by_file_and_line(
+self, "main.c", self.line, num_expected_locations=1)
+self.runCmd("run", RUN_SUCCEEDED)
+self.expect("thread backtrace", STOPPED_DUE_TO_BREAKPOINT,
+substrs=["stop reason = breakpoint 1."])
+
+target = self.dbg.GetSelectedTarget()
+process = target.GetProcess()
+thread = process.GetThreadAtIndex(0)
+
+backtrace = ["func_c", "func_b", "func_a", "main", 
"__libc_start_main", "_start"]
+self.assertEqual(thread.GetNumFrames(), len(backtrace))
+for frame_idx, frame in enumerate(thread.frames):
+frame = thread.GetFrameAtIndex(frame_idx)
+self.assertTrue(frame)
+self.assertEqual(frame.GetFunctionName(), backtrace[frame_idx])
+if (frame_idx < 4):
+self.assertEqual(frame.GetLineEntry().GetLine(),
+ line_number("main.c", "Frame " + 
backtrace[frame_idx]))
Index: lldb/test/API/functionalities/unwind/aarch64_unwind_pac/Makefile
===
--- /dev/null
+++ lldb/test/API/functionalities/unwind/aarch64_unwind_pac/Makefile
@@ -0,0 +1,5 @@
+C_SOURCES := main.c
+
+CFLAGS := -g -Os -march=armv8.5-a -mbranch-protection=pac-ret+leaf
+
+include Makefile.rules


Index: lldb/test/API/functionalities/unwind/aarch64_unwind_pac/main.c
===
--- /dev/null
+++ lldb/test/API/functionalities/unwind/aarch64_unwind_pac/main.c
@@ -0,0 +1,24 @@
+// This program makes a multi tier nested function call to test AArch64
+// Pointer Authentication feature.
+
+// To enable PAC return address signing compile with following clang arguments:
+// -march=armv8.5-a -mbranch-protection=pac-ret+leaf
+
+#include 
+
+static void __attribute__((noinline)) func_c(void) {
+  exit(0); // Frame func_c
+}
+
+static void __attribute__((noinline)) func_b(void) {
+  func_c(); // Frame func_b
+}
+
+static void __attribute__((noinline)) func_a(void) {
+  func_b(); // Frame func_a
+}
+
+int main(int argc, char *argv[]) {
+  func_a(); // Frame main
+  return 0;
+}
Index: lldb/test/API/functionalities/unwind/aarch64_unwind_pac/TestAArch64UnwindPAC.py
===
--- /dev/null
+++ lldb/test/API/functionalities/unwind/aarch64_unwind_pac/TestAArch64UnwindPAC.py
@@ -0,0 +1,43 @@
+"""
+Test that we can backtrace correctly when AArch64 PAC is enabled
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class AArch64UnwindPAC(TestBase):
+mydir = TestBase.compute_mydir(__file__)
+
+

[Lldb-commits] [PATCH] D100965: [lldb] Refactor argument group by SourceLocationSpec (NFCI)

2021-04-26 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib updated this revision to Diff 340701.
mib marked 2 inline comments as done.
mib added a comment.

Use `llvm::Expected` error when possible.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100965

Files:
  lldb/include/lldb/Breakpoint/BreakpointResolverFileLine.h
  lldb/include/lldb/Core/AddressResolverFileLine.h
  lldb/include/lldb/Symbol/CompileUnit.h
  lldb/include/lldb/Symbol/LineTable.h
  lldb/include/lldb/Symbol/SymbolFile.h
  lldb/source/API/SBThread.cpp
  lldb/source/Breakpoint/Breakpoint.cpp
  lldb/source/Breakpoint/BreakpointResolverFileLine.cpp
  lldb/source/Breakpoint/BreakpointResolverFileRegex.cpp
  lldb/source/Core/AddressResolverFileLine.cpp
  lldb/source/Core/Module.cpp
  lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
  lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
  lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
  lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
  lldb/source/Symbol/CompileUnit.cpp
  lldb/source/Symbol/LineTable.cpp
  lldb/source/Symbol/SymbolFile.cpp
  lldb/source/Target/Target.cpp
  lldb/unittests/Symbol/TestLineEntry.cpp

Index: lldb/unittests/Symbol/TestLineEntry.cpp
===
--- lldb/unittests/Symbol/TestLineEntry.cpp
+++ lldb/unittests/Symbol/TestLineEntry.cpp
@@ -53,8 +53,10 @@
 }
 
 llvm::Expected LineEntryTest::GetLineEntryForLine(uint32_t line) {
-  bool check_inlines = true;
-  bool exact = true;
+  // TODO: Handle SourceLocationSpec column information
+  const llvm::Optional column = llvm::None;
+  const bool check_inlines = true;
+  const bool exact_match = true;
   SymbolContextList sc_comp_units;
   SymbolContextList sc_line_entries;
   FileSpec file_spec("inlined-functions.cpp");
@@ -64,9 +66,14 @@
   if (sc_comp_units.GetSize() == 0)
 return llvm::createStringError(llvm::inconvertibleErrorCode(),
"No comp unit found on the test object.");
+
+  auto location_spec = SourceLocationSpec::Create(file_spec, line, column,
+  check_inlines, exact_match);
+  if (!location_spec)
+return location_spec.takeError();
+
   sc_comp_units[0].comp_unit->ResolveSymbolContext(
-  file_spec, line, check_inlines, exact, eSymbolContextLineEntry,
-  sc_line_entries);
+  *location_spec, eSymbolContextLineEntry, sc_line_entries);
   if (sc_line_entries.GetSize() == 0)
 return llvm::createStringError(llvm::inconvertibleErrorCode(),
"No line entry found on the test object.");
Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -371,9 +371,16 @@
   if (move_to_nearest_code == eLazyBoolCalculate)
 move_to_nearest_code = GetMoveToNearestCode() ? eLazyBoolYes : eLazyBoolNo;
 
+  auto location_spec =
+  SourceLocationSpec::Create(remapped_file, line_no, column, check_inlines,
+ !static_cast(move_to_nearest_code));
+  if (!location_spec) {
+llvm::errs() << llvm::toString(location_spec.takeError()) << "\n";
+return nullptr;
+  }
+
   BreakpointResolverSP resolver_sp(new BreakpointResolverFileLine(
-  nullptr, remapped_file, line_no, column, offset, check_inlines,
-  skip_prologue, !static_cast(move_to_nearest_code)));
+  nullptr, offset, skip_prologue, *location_spec));
   return CreateBreakpoint(filter_sp, resolver_sp, internal, hardware, true);
 }
 
Index: lldb/source/Symbol/SymbolFile.cpp
===
--- lldb/source/Symbol/SymbolFile.cpp
+++ lldb/source/Symbol/SymbolFile.cpp
@@ -97,10 +97,10 @@
   return type_system_or_err;
 }
 
-uint32_t SymbolFile::ResolveSymbolContext(const FileSpec _spec,
-  uint32_t line, bool check_inlines,
-  lldb::SymbolContextItem resolve_scope,
-  SymbolContextList _list) {
+uint32_t
+SymbolFile::ResolveSymbolContext(const SourceLocationSpec _location_spec,
+ lldb::SymbolContextItem resolve_scope,
+ SymbolContextList _list) {
   return 0;
 }
 
Index: lldb/source/Symbol/LineTable.cpp
===
--- lldb/source/Symbol/LineTable.cpp
+++ lldb/source/Symbol/LineTable.cpp
@@ -304,7 +304,7 @@
 
 

[Lldb-commits] [PATCH] D99944: [LLDB] AArch64 PAC elf-core stack unwinder support

2021-04-26 Thread Muhammad Omair Javaid via Phabricator via lldb-commits
omjavaid updated this revision to Diff 340686.
omjavaid added a comment.

Moved mask calculation to ABISysV_arm64 class.


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

https://reviews.llvm.org/D99944

Files:
  lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp
  lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.h
  lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
  lldb/test/API/functionalities/postmortem/elf-core/linux-aarch64-pac.out

Index: lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
===
--- lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
+++ lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
@@ -20,6 +20,7 @@
 mydir = TestBase.compute_mydir(__file__)
 
 _aarch64_pid = 37688
+_aarch64_pac_pid = 387
 _i386_pid = 32306
 _x86_64_pid = 32259
 _s390x_pid = 1045
@@ -135,7 +136,7 @@
 
 error = lldb.SBError()
 bytesread = process.ReadMemory(0x400ff0, 20, error)
-
+
 # read only 16 bytes without zero bytes filling
 self.assertEqual(len(bytesread), 16)
 self.dbg.DeleteTarget(target)
@@ -257,6 +258,18 @@
 
 self.dbg.DeleteTarget(target)
 
+@skipIfLLVMTargetMissing("AArch64")
+def test_aarch64_pac(self):
+"""Test that lldb can unwind stack for AArch64 elf core file with PAC enabled."""
+
+target = self.dbg.CreateTarget("linux-aarch64-pac.out")
+self.assertTrue(target, VALID_TARGET)
+process = target.LoadCore("linux-aarch64-pac.core")
+
+self.check_all(process, self._aarch64_pac_pid, self._aarch64_regions, "a.out")
+
+self.dbg.DeleteTarget(target)
+
 @skipIfLLVMTargetMissing("AArch64")
 @expectedFailureAll(archs=["aarch64"], oslist=["freebsd"],
 bugnumber="llvm.org/pr49415")
Index: lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.h
===
--- lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.h
+++ lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.h
@@ -85,6 +85,9 @@
 
   uint32_t GetPluginVersion() override;
 
+  lldb::addr_t FixCodeAddress(lldb::addr_t pc) override;
+  lldb::addr_t FixDataAddress(lldb::addr_t pc) override;
+
 protected:
   lldb::ValueObjectSP
   GetReturnValueObjectImpl(lldb_private::Thread ,
Index: lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp
===
--- lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp
+++ lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp
@@ -787,6 +787,56 @@
   return (pc & pac_sign_extension) ? pc | mask : pc & (~mask);
 }
 
+// Reads code or data address mask for the current Linux process.
+static lldb::addr_t ReadLinuxProcessAddressMask(lldb::ProcessSP process_sp,
+llvm::StringRef reg_name) {
+  // Linux configures user-space virtual addresses with top byte ignored.
+  // We set default value of mask such that top byte is masked out.
+  uint64_t address_mask = ~((1ULL << 56) - 1);
+  // If Pointer Authentication feature is enabled then Linux exposes
+  // PAC data and code mask register. Try reading relevant register
+  // below and merge it with default address mask calculated above.
+  lldb::ThreadSP thread_sp = process_sp->GetThreadList().GetSelectedThread();
+  if (thread_sp) {
+lldb::RegisterContextSP reg_ctx_sp = thread_sp->GetRegisterContext();
+if (reg_ctx_sp) {
+  const RegisterInfo *reg_info =
+  reg_ctx_sp->GetRegisterInfoByName(reg_name, 0);
+  if (reg_info) {
+lldb::addr_t mask_reg_val = reg_ctx_sp->ReadRegisterAsUnsigned(
+reg_info->kinds[eRegisterKindLLDB], LLDB_INVALID_ADDRESS);
+if (mask_reg_val != LLDB_INVALID_ADDRESS)
+  address_mask |= mask_reg_val;
+  }
+}
+  }
+  return address_mask;
+}
+
+lldb::addr_t ABISysV_arm64::FixCodeAddress(lldb::addr_t pc) {
+  if (lldb::ProcessSP process_sp = GetProcessSP()) {
+if (process_sp->GetTarget().GetArchitecture().GetTriple().isOSLinux() &&
+!process_sp->GetCodeAddressMask())
+  process_sp->SetCodeAddressMask(
+  ReadLinuxProcessAddressMask(process_sp, "code_mask"));
+
+return FixAddress(pc, process_sp->GetCodeAddressMask());
+  }
+  return pc;
+}
+
+lldb::addr_t ABISysV_arm64::FixDataAddress(lldb::addr_t pc) {
+  if (lldb::ProcessSP process_sp = GetProcessSP()) {
+if (process_sp->GetTarget().GetArchitecture().GetTriple().isOSLinux() &&
+!process_sp->GetDataAddressMask())
+  process_sp->SetDataAddressMask(
+  ReadLinuxProcessAddressMask(process_sp, "data_mask"));
+
+return FixAddress(pc, process_sp->GetDataAddressMask());
+  }
+  return pc;
+}
+
 void ABISysV_arm64::Initialize() {
   PluginManager::RegisterPlugin(GetPluginNameStatic(),
 "SysV ABI for AArch64 targets", CreateInstance);

[Lldb-commits] [PATCH] D101333: Also display the underlying error message when displaying a fixit

2021-04-26 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl created this revision.
aprantl added reviewers: teemperor, shafik, jingham.
aprantl requested review of this revision.

When the user running LLDB with default settings sees the fixit notification it 
means that the auto-applied fixit didn't work. This patch shows the underlying 
error message instead of just the fixit to make it easier to understand what 
the error in the expression was.


https://reviews.llvm.org/D101333

Files:
  lldb/source/Expression/UserExpression.cpp
  lldb/test/API/commands/expression/fixits/TestFixIts.py


Index: lldb/test/API/commands/expression/fixits/TestFixIts.py
===
--- lldb/test/API/commands/expression/fixits/TestFixIts.py
+++ lldb/test/API/commands/expression/fixits/TestFixIts.py
@@ -132,25 +132,25 @@
 # Disable retries which will fail.
 multiple_runs_options.SetRetriesWithFixIts(0)
 value = frame.EvaluateExpression(two_runs_expr, multiple_runs_options)
-self.assertIn("expression failed to parse, fixed expression 
suggested:",
-  value.GetError().GetCString())
-self.assertIn("using typename T::TypeDef",
-  value.GetError().GetCString())
+errmsg = value.GetError().GetCString()
+self.assertIn("expression failed to parse", errmsg)
+self.assertIn("using declaration resolved to type without 'typename'",
+  errmsg)
+self.assertIn("fixed expression suggested:", errmsg)
+self.assertIn("using typename T::TypeDef", errmsg)
 # The second Fix-It shouldn't be suggested here as Clang should have
 # aborted the parsing process.
-self.assertNotIn("i->m",
-  value.GetError().GetCString())
+self.assertNotIn("i->m", errmsg)
 
 # Retry once, but the expression needs two retries.
 multiple_runs_options.SetRetriesWithFixIts(1)
 value = frame.EvaluateExpression(two_runs_expr, multiple_runs_options)
-self.assertIn("expression failed to parse, fixed expression 
suggested:",
-  value.GetError().GetCString())
+errmsg = value.GetError().GetCString()
+self.assertIn("expression failed to parse", errmsg)
+self.assertIn("fixed expression suggested:", errmsg)
 # Both our fixed expressions should be in the suggested expression.
-self.assertIn("using typename T::TypeDef",
-  value.GetError().GetCString())
-self.assertIn("i->m",
-  value.GetError().GetCString())
+self.assertIn("using typename T::TypeDef", errmsg)
+self.assertIn("i->m", errmsg)
 
 # Retry twice, which will get the expression working.
 multiple_runs_options.SetRetriesWithFixIts(2)
Index: lldb/source/Expression/UserExpression.cpp
===
--- lldb/source/Expression/UserExpression.cpp
+++ lldb/source/Expression/UserExpression.cpp
@@ -302,19 +302,19 @@
 }
 
 if (!parse_success) {
-  if (!fixed_expression->empty() && target->GetEnableNotifyAboutFixIts()) {
-error.SetExpressionErrorWithFormat(
-execution_results,
-"expression failed to parse, fixed expression suggested:\n  %s",
-fixed_expression->c_str());
-  } else {
-if (!diagnostic_manager.Diagnostics().size())
-  error.SetExpressionError(execution_results,
-   "expression failed to parse, unknown 
error");
+  std::string msg;
+  {
+llvm::raw_string_ostream os(msg);
+os << "expression failed to parse:\n";
+if (diagnostic_manager.Diagnostics().size())
+  os << diagnostic_manager.GetString();
 else
-  error.SetExpressionError(execution_results,
-   diagnostic_manager.GetString().c_str());
+  os << "unknown error";
+if (target->GetEnableNotifyAboutFixIts() && fixed_expression &&
+!fixed_expression->empty())
+  os << "\nfixed expression suggested:\n  " << *fixed_expression;
   }
+  error.SetExpressionError(execution_results, msg.c_str());
 }
   }
 


Index: lldb/test/API/commands/expression/fixits/TestFixIts.py
===
--- lldb/test/API/commands/expression/fixits/TestFixIts.py
+++ lldb/test/API/commands/expression/fixits/TestFixIts.py
@@ -132,25 +132,25 @@
 # Disable retries which will fail.
 multiple_runs_options.SetRetriesWithFixIts(0)
 value = frame.EvaluateExpression(two_runs_expr, multiple_runs_options)
-self.assertIn("expression failed to parse, fixed expression suggested:",
-  value.GetError().GetCString())
-self.assertIn("using typename T::TypeDef",
-  value.GetError().GetCString())
+errmsg = value.GetError().GetCString()

[Lldb-commits] [PATCH] D99941: [LLDB] Support AArch64 PAC elf-core register read

2021-04-26 Thread Muhammad Omair Javaid via Phabricator via lldb-commits
omjavaid marked an inline comment as done.
omjavaid added inline comments.



Comment at: 
lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp:32
+  if (pac_data.GetByteSize() > sizeof(uint64_t))
+opt_regsets.Set(RegisterInfoPOSIX_arm64::eRegsetMaskPAuth);
+

DavidSpickett wrote:
> How is the `sizeof()` calculated here? Is it the size of the masks 
> themselves, or a header block. (SVE above is clearly a header of some kind)
added a comment to reflect reason behind pac data minimum size



Comment at: 
lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py:451
+self.assertTrue(target, VALID_TARGET)
+process = target.LoadCore("linux-aarch64-pac.core")
+

DavidSpickett wrote:
> Do you need anything special to generate the core file? Might be worth noting 
> in the commit message.
> 
> Though I guess you can just compile a hello world on a Linux system with PAC.
Updated commit message.


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

https://reviews.llvm.org/D99941

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


[Lldb-commits] [PATCH] D99941: [LLDB] Support AArch64 PAC elf-core register read

2021-04-26 Thread Muhammad Omair Javaid via Phabricator via lldb-commits
omjavaid updated this revision to Diff 340680.
omjavaid added a comment.

Resolved review comments.


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

https://reviews.llvm.org/D99941

Files:
  lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_arm64.cpp
  lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_arm64.h
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h
  lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp
  lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.h
  lldb/source/Plugins/Process/elf-core/RegisterUtilities.h
  lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
  lldb/test/API/functionalities/postmortem/elf-core/linux-aarch64-pac.core

Index: lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
===
--- lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
+++ lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
@@ -443,6 +443,21 @@
 
 self.expect("register read --all")
 
+@skipIfLLVMTargetMissing("AArch64")
+def test_aarch64_pac_regs(self):
+# Test AArch64/Linux Pointer Authenication register read
+target = self.dbg.CreateTarget(None)
+self.assertTrue(target, VALID_TARGET)
+process = target.LoadCore("linux-aarch64-pac.core")
+
+values = {"data_mask": "0x007f000", "code_mask": "0x007f000"}
+
+for regname, value in values.items():
+self.expect("register read {}".format(regname),
+substrs=["{} = {}".format(regname, value)])
+
+self.expect("register read --all")
+
 @skipIfLLVMTargetMissing("ARM")
 def test_arm_core(self):
 # check 32 bit ARM core file
Index: lldb/source/Plugins/Process/elf-core/RegisterUtilities.h
===
--- lldb/source/Plugins/Process/elf-core/RegisterUtilities.h
+++ lldb/source/Plugins/Process/elf-core/RegisterUtilities.h
@@ -111,6 +111,10 @@
 {llvm::Triple::Linux, llvm::Triple::aarch64, llvm::ELF::NT_ARM_SVE},
 };
 
+constexpr RegsetDesc AARCH64_PAC_Desc[] = {
+{llvm::Triple::Linux, llvm::Triple::aarch64, llvm::ELF::NT_ARM_PAC_MASK},
+};
+
 constexpr RegsetDesc PPC_VMX_Desc[] = {
 {llvm::Triple::FreeBSD, llvm::Triple::UnknownArch, llvm::ELF::NT_PPC_VMX},
 {llvm::Triple::Linux, llvm::Triple::UnknownArch, llvm::ELF::NT_PPC_VMX},
Index: lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.h
===
--- lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.h
+++ lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.h
@@ -42,7 +42,6 @@
   lldb_private::Thread ,
   std::unique_ptr register_info,
   const lldb_private::DataExtractor ,
-  const lldb_private::DataExtractor ,
   llvm::ArrayRef notes);
 
   bool ReadGPR() override;
@@ -54,10 +53,10 @@
   bool WriteFPR() override;
 
 private:
-  lldb::DataBufferSP m_gpr_buffer;
-  lldb_private::DataExtractor m_gpr;
-  lldb_private::DataExtractor m_fpregset;
-  lldb_private::DataExtractor m_sveregset;
+  lldb_private::DataExtractor m_gpr_data;
+  lldb_private::DataExtractor m_fpr_data;
+  lldb_private::DataExtractor m_sve_data;
+  lldb_private::DataExtractor m_pac_data;
 
   SVEState m_sve_state;
   uint16_t m_sve_vector_length = 0;
Index: lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp
===
--- lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp
+++ lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp
@@ -21,32 +21,43 @@
 RegisterContextCorePOSIX_arm64::Create(Thread , const ArchSpec ,
const DataExtractor ,
llvm::ArrayRef notes) {
-  DataExtractor sveregset =
-  getRegset(notes, arch.GetTriple(), AARCH64_SVE_Desc);
-
   Flags opt_regsets = RegisterInfoPOSIX_arm64::eRegsetMaskDefault;
-  if (sveregset.GetByteSize() > sizeof(sve::user_sve_header))
+
+  DataExtractor sve_data = getRegset(notes, arch.GetTriple(), AARCH64_SVE_Desc);
+  if (sve_data.GetByteSize() > sizeof(sve::user_sve_header))
 opt_regsets.Set(RegisterInfoPOSIX_arm64::eRegsetMaskSVE);
+
+  // Pointer Authentication register set data is based on struct
+  // user_pac_mask declared in ptrace.h. See reference implementation
+  // in Linux kernel source at arch/arm64/include/uapi/asm/ptrace.h.
+  DataExtractor pac_data = getRegset(notes, arch.GetTriple(), AARCH64_PAC_Desc);
+  if (pac_data.GetByteSize() >= sizeof(uint64_t) * 2)
+opt_regsets.Set(RegisterInfoPOSIX_arm64::eRegsetMaskPAuth);
+
   auto register_info_up =
   std::make_unique(arch, opt_regsets);
   return std::unique_ptr(
   new 

[Lldb-commits] [PATCH] D101131: [lldb-vscode] Follow up of D99989 - store some strings more safely

2021-04-26 Thread Greg Clayton via Phabricator via lldb-commits
clayborg requested changes to this revision.
clayborg added inline comments.
This revision now requires changes to proceed.



Comment at: lldb/tools/lldb-vscode/lldb-vscode.cpp:2939
 break;
   variable_name_counts[variable.GetName()]++;
 }

This will crash if "variable.GetName()" return nullptr. We should check and 
only add if not nullptr


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101131

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


[Lldb-commits] [PATCH] D101250: Wrap edit line configuration calls into helper functions

2021-04-26 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere accepted this revision.
JDevlieghere added a comment.

LGTM. I wonder if we would want to wrap this in a macro to get rid of the 
`EditLineConstString` duplication while keeping the type safety.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101250

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


[Lldb-commits] [PATCH] D101329: [lldb] Support SaveCore() from gdb-remote client [WIP]

2021-04-26 Thread Michał Górny via Phabricator via lldb-commits
mgorny added a comment.

I need to implement path hints and add tests still.


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

https://reviews.llvm.org/D101329

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


[Lldb-commits] [PATCH] D101329: [lldb] Support SaveCore() from gdb-remote client

2021-04-26 Thread Michał Górny via Phabricator via lldb-commits
mgorny created this revision.
mgorny added reviewers: labath, emaste, krytarowski.
mgorny requested review of this revision.

Extend PluginManager::SaveCore() to support saving core dumps
via Process plugins.  Implement the client-side part of qSaveCore
request in the gdb-remote plugin, that creates the core dump
on the remote host and then uses vFile packets to transfer it.


https://reviews.llvm.org/D101329

Files:
  lldb/include/lldb/Target/Process.h
  lldb/source/Core/PluginManager.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h

Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
@@ -230,6 +230,8 @@
   std::string HarmonizeThreadIdsForProfileData(
   StringExtractorGDBRemote );
 
+  llvm::Expected SaveCore(llvm::StringRef outfile) override;
+
 protected:
   friend class ThreadGDBRemote;
   friend class GDBRemoteCommunicationClient;
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -5062,6 +5062,54 @@
   BuildDynamicRegisterInfo(true);
 }
 
+llvm::Expected ProcessGDBRemote::SaveCore(llvm::StringRef outfile) {
+  if (!m_gdb_comm.GetSaveCoreSupported())
+return false;
+
+  // TODO: path-hint
+  std::string packet{"qSaveCore"};
+  StringExtractorGDBRemote response;
+  if (m_gdb_comm.SendPacketAndWaitForResponse(packet, response, false) ==
+  GDBRemoteCommunication::PacketResult::Success) {
+// TODO: grab error message from the packet?  StringExtractor seems to
+// be missing a method for that
+if (response.IsErrorResponse())
+  return llvm::createStringError(
+  llvm::inconvertibleErrorCode(),
+  llvm::formatv("qSaveCore returned an error"));
+
+std::string path;
+
+// process the response
+llvm::SmallVector reply_data;
+response.GetStringRef().split(reply_data, ';');
+for (auto x : reply_data) {
+  if (x.consume_front("core-path:"))
+StringExtractor(x).GetHexByteString(path);
+}
+
+// verify that we've gotten what we need
+if (path.empty())
+  return llvm::createStringError(llvm::inconvertibleErrorCode(),
+ "qSaveCore returned no core path");
+
+// now transfer the core file
+FileSpec remote_core{llvm::StringRef(path)};
+Platform  = *GetTarget().GetPlatform();
+Status error = platform.GetFile(remote_core, FileSpec(outfile));
+
+// NB: we unlink the file on error too
+platform.Unlink(remote_core);
+if (error.Fail())
+  return error.ToError();
+
+return true;
+  }
+
+  return llvm::createStringError(llvm::inconvertibleErrorCode(),
+ "Unable to send qSaveCore");
+}
+
 static const char *const s_async_json_packet_prefix = "JSON-async:";
 
 static StructuredData::ObjectSP
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
@@ -521,6 +521,8 @@
   llvm::Expected>
   SendTraceGetBinaryData(const TraceGetBinaryDataRequest );
 
+  bool GetSaveCoreSupported() const;
+
 protected:
   LazyBool m_supports_not_sending_acks;
   LazyBool m_supports_thread_suffix;
@@ -558,6 +560,7 @@
   LazyBool m_supports_QPassSignals;
   LazyBool m_supports_error_string_reply;
   LazyBool m_supports_multiprocess;
+  LazyBool m_supports_qSaveCore;
 
   bool m_supports_qProcessInfoPID : 1, m_supports_qfProcessInfo : 1,
   m_supports_qUserName : 1, m_supports_qGroupName : 1,
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -90,6 +90,7 @@
   m_supports_QPassSignals(eLazyBoolCalculate),
   m_supports_error_string_reply(eLazyBoolCalculate),
   m_supports_multiprocess(eLazyBoolCalculate),
+  m_supports_qSaveCore(eLazyBoolCalculate),
   m_supports_qProcessInfoPID(true), m_supports_qfProcessInfo(true),
   m_supports_qUserName(true), m_supports_qGroupName(true),
   m_supports_qThreadStopInfo(true), m_supports_z0(true),
@@ -294,6 +295,7 @@
 m_attach_or_wait_reply = eLazyBoolCalculate;
 

[Lldb-commits] [PATCH] D101237: [lldb] Fix [[no_unique_address]] and libstdc++ 11's std::unique_ptr

2021-04-26 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment.

A bit of history on laying out classes: layout used to be a problem before we 
were able to give all of the field offsets directly to clang to assist in 
laying out. The main issues were:

- #pragma pack directives are not in DWARF so we must use the 
DW_AT_data_member_location
- Some DWARF optimizations cause class definitions to be omitted and we can 
have a class A that inherits from class B but we have no real definition for 
class B, just a forward declaration. In this case we will create a class A that 
inherits from a class B that has nothing in it, but the field offsets will 
ensure we show all other instance variables of class A correctly. Furthermore, 
we have code that can detect when we have such a case and it can grab the right 
definition for class B when it is imported into another AST, such as when 
evaluating an expression. This will only happen if class B is in another shared 
library from class A, and if we do have debug info for class B.
- any other keywords or attributes such as [[no_unique_address]] that can 
affect layout that don't appear in DWARF.

It seems it would be a nice attribute to have add to DWARF in case the current 
solution affects things adversely.

I will let the expression parser experts comment on the viability of always 
setting "[[no_unique_address]]" on every field. Seems dangerous to me, but I 
will defer to expression experts.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101237

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


[Lldb-commits] [PATCH] D101250: Wrap edit line configuration calls into helper functions

2021-04-26 Thread 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!

I can to land this for you, but first let's wait a bit to see if anyone else 
has any comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101250

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


[Lldb-commits] [PATCH] D101285: [lldb] [llgs server] Support creating core dumps on NetBSD

2021-04-26 Thread Michał Górny via Phabricator via lldb-commits
mgorny updated this revision to Diff 340638.
mgorny added a comment.

Fix missing `+` in qSupported. Fix known stub features.


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

https://reviews.llvm.org/D101285

Files:
  lldb/include/lldb/Host/common/NativeProcessProtocol.h
  lldb/include/lldb/Utility/StringExtractorGDBRemote.h
  lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
  lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp
  lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
  lldb/source/Utility/StringExtractorGDBRemote.cpp
  lldb/test/API/tools/lldb-server/TestGdbRemoteSaveCore.py

Index: lldb/test/API/tools/lldb-server/TestGdbRemoteSaveCore.py
===
--- /dev/null
+++ lldb/test/API/tools/lldb-server/TestGdbRemoteSaveCore.py
@@ -0,0 +1,60 @@
+import gdbremote_testcase
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+import binascii
+import os
+
+class TestGdbSaveCore(gdbremote_testcase.GdbRemoteTestCaseBase):
+mydir = TestBase.compute_mydir(__file__)
+
+@skipUnlessPlatform(oslist=["netbsd"])
+def test_netbsd_path(self):
+self.build()
+self.set_inferior_startup_attach()
+procs = self.prep_debug_monitor_and_inferior()
+self.add_qSupported_packets()
+ret = self.expect_gdbremote_sequence()
+self.assertIn("qSaveCore+", ret["qSupported_response"])
+self.reset_test_sequence()
+
+core_path = lldbutil.append_to_process_working_directory(self, "core")
+self.test_sequence.add_log_lines([
+"read packet: $qSaveCore;path-hint:{}#00".format(
+binascii.b2a_hex(core_path.encode()).decode()),
+{"direction": "send", "regex": "[$]core-path:([0-9a-f]+)#.*",
+ "capture": {1: "path"}},
+], True)
+ret = self.expect_gdbremote_sequence()
+out_path = binascii.a2b_hex(ret["path"].encode()).decode()
+self.assertEqual(core_path, out_path)
+
+target = self.dbg.CreateTarget(None)
+process = target.LoadCore(out_path)
+self.assertTrue(process, PROCESS_IS_VALID)
+self.assertEqual(process.GetProcessID(), procs["inferior"].pid)
+
+@skipUnlessPlatform(oslist=["netbsd"])
+def test_netbsd_no_path(self):
+self.build()
+self.set_inferior_startup_attach()
+procs = self.prep_debug_monitor_and_inferior()
+self.add_qSupported_packets()
+ret = self.expect_gdbremote_sequence()
+self.assertIn("qSaveCore+", ret["qSupported_response"])
+self.reset_test_sequence()
+
+self.test_sequence.add_log_lines([
+"read packet: $qSaveCore#00",
+{"direction": "send", "regex": "[$]core-path:([0-9a-f]+)#.*",
+ "capture": {1: "path"}},
+], True)
+ret = self.expect_gdbremote_sequence()
+out_path = binascii.a2b_hex(ret["path"].encode()).decode()
+
+target = self.dbg.CreateTarget(None)
+process = target.LoadCore(out_path)
+self.assertTrue(process, PROCESS_IS_VALID)
+self.assertEqual(process.GetProcessID(), procs["inferior"].pid)
+os.unlink(out_path)
Index: lldb/source/Utility/StringExtractorGDBRemote.cpp
===
--- lldb/source/Utility/StringExtractorGDBRemote.cpp
+++ lldb/source/Utility/StringExtractorGDBRemote.cpp
@@ -253,6 +253,8 @@
   break;
 
 case 'S':
+  if (PACKET_STARTS_WITH("qSaveCore"))
+return eServerPacketType_qLLDBSaveCore;
   if (PACKET_STARTS_WITH("qSpeedTest:"))
 return eServerPacketType_qSpeedTest;
   if (PACKET_MATCHES("qShlibInfoAddr"))
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
@@ -214,6 +214,8 @@
 
   PacketResult Handle_QPassSignals(StringExtractorGDBRemote );
 
+  PacketResult Handle_qSaveCore(StringExtractorGDBRemote );
+
   PacketResult Handle_g(StringExtractorGDBRemote );
 
   void SetCurrentThreadID(lldb::tid_t tid);
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -217,6 +217,10 @@
   quit = true;
   return this->Handle_k(packet);
 });
+
+  

[Lldb-commits] [PATCH] D101198: [lldb-vscode] Read requests asynchronously

2021-04-26 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment.

This will be tricky to do right and I really don't know how much extra 
performance we will get out of this for all of the possible issues we will can 
introduce by multi-threading things. That being said, I am happy to help see if 
this will actually improve performance.

The first issue with adding threading to the packets is that this introduces a 
large overhead when processing packets. Having a read thread reading packets 
and using a mutex + condition variable slowed down lldb-server packets way too 
much. I know that the VS Code DAP packets aren't sent in the same volume so it 
may not make a difference, but we should make sure this doesn't slow things 
down.

The second issue is we need to understand the packets and do something 
intelligent with them based on what they are. For example if we get a packet 
sequence like:

- step
- threads
- scopes
- get locals from scopes
- step
- threads
- scopes
- get locals from scopes

We will need to know that we must do the "step" items, and as soon as another 
"step" is received, we could just return an error to the "threads", "scopes" 
and "get locals" so if we haven't processed the "step" yet, we could simplify 
this down to:

- step
- step
- threads
- scopes
- get locals from scopes

The other tricky issue is all things that control the process or require the 
process to stay stopped while the request happens (like "threads", "scopes" and 
"get locals") all need to happen on the same thread anyway. But this is almost 
all of the packets to begin with, so we really won't be able to multi-thread 
the handling of packets. Think of what would happen if you got the following 
packets:

- step
- continue
- kill

We can't grab each one of these and try each on on a separate thread because 
what if we run the "kill" first, then the "continue" then the "step". So the 
all packets we receive can't just be run on different threads without risking 
things not making sense and messing up the debug session.

That leads me to what we can do. We could receive all packets on a read thread, 
and parse them one by one on the main thread and possible return errors to the 
ones that don't need to be done because we know there is another process 
control packet after them. But I am not sure even if a debug adaptor should be 
doing this as I would hope the IDE would have a small timeout before requesting 
the locals after say stepping or resuming...




Comment at: lldb/tools/lldb-vscode/lldb-vscode.cpp:3196
+private:
+  std::list m_data;
+  std::mutex m_mutex;

Rename to packets?



Comment at: lldb/tools/lldb-vscode/lldb-vscode.cpp:3198
+  std::mutex m_mutex;
+  std::condition_variable m_can_consume;
+  llvm::Optional m_termination_status;

rename to "m_condition"? the name m_can_consume seems like a bool.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101198

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


[Lldb-commits] [PATCH] D101157: [lldb] [test] Add tests for coredumps with multiple threads

2021-04-26 Thread Michał Górny via Phabricator via lldb-commits
mgorny updated this revision to Diff 340586.
mgorny added a comment.

Add FreeBSD/aarch64 test.


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

https://reviews.llvm.org/D101157

Files:
  lldb/test/Shell/Register/Core/Inputs/aarch64-freebsd-multithread.core
  lldb/test/Shell/Register/Core/Inputs/multithread.cpp
  lldb/test/Shell/Register/Core/Inputs/x86-32-freebsd-multithread.core
  lldb/test/Shell/Register/Core/Inputs/x86-32-linux-multithread.core
  lldb/test/Shell/Register/Core/Inputs/x86-32-netbsd-multithread.core
  lldb/test/Shell/Register/Core/Inputs/x86-64-freebsd-multithread.core
  lldb/test/Shell/Register/Core/Inputs/x86-64-linux-multithread.core
  lldb/test/Shell/Register/Core/Inputs/x86-64-netbsd-multithread.core
  lldb/test/Shell/Register/Core/aarch64-freebsd-multithread.test
  lldb/test/Shell/Register/Core/x86-32-freebsd-multithread.test
  lldb/test/Shell/Register/Core/x86-32-linux-multithread.test
  lldb/test/Shell/Register/Core/x86-32-netbsd-multithread.test
  lldb/test/Shell/Register/Core/x86-64-freebsd-multithread.test
  lldb/test/Shell/Register/Core/x86-64-linux-multithread.test
  lldb/test/Shell/Register/Core/x86-64-netbsd-multithread.test

Index: lldb/test/Shell/Register/Core/x86-64-netbsd-multithread.test
===
--- /dev/null
+++ lldb/test/Shell/Register/Core/x86-64-netbsd-multithread.test
@@ -0,0 +1,41 @@
+# RUN: %lldb -b -s %s -c %p/Inputs/x86-64-netbsd-multithread.core | FileCheck %s
+
+thread list
+# CHECK: * thread #1: tid = 2, 0x00400f82, stop reason = signal SIGSEGV 
+# CHECK-NEXT:   thread #2: tid = 4, 0x00400f88, stop reason = signal 0 
+# CHECK-NEXT:   thread #3: tid = 3, 0x00400f88, stop reason = signal 0 
+# CHECK-NEXT:   thread #4: tid = 1, 0x791280ca1faa, stop reason = signal 0 
+
+register read --all
+# CHECK-DAG: ecx = 0x04040404
+# CHECK-DAG: edx = 0x03030303
+# CHECK-DAG: edi = 0x01010101
+# CHECK-DAG: esi = 0x02020202
+# CHECK-DAG: xmm0 = {0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x40 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00}
+# CHECK-DAG: xmm1 = {0x00 0x00 0x00 0x00 0x00 0x00 0x10 0x40 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00}
+# CHECK-DAG: xmm2 = {0x00 0x00 0x00 0x00 0x00 0x00 0x20 0x40 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00}
+# CHECK-DAG: xmm3 = {0x00 0x00 0x00 0x00 0x00 0x00 0x30 0x40 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00}
+
+thread select 3
+# CHECK: (lldb) thread select 3
+register read --all
+# CHECK-DAG: ecx = 0x14141414
+# CHECK-DAG: edx = 0x13131313
+# CHECK-DAG: edi = 0x
+# CHECK-DAG: esi = 0x12121212
+# CHECK-DAG: xmm0 = {0x00 0x00 0x00 0x00 0x00 0x00 0x08 0x40 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00}
+# CHECK-DAG: xmm1 = {0x00 0x00 0x00 0x00 0x00 0x00 0x18 0x40 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00}
+# CHECK-DAG: xmm2 = {0x00 0x00 0x00 0x00 0x00 0x00 0x22 0x40 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00}
+# CHECK-DAG: xmm3 = {0x00 0x00 0x00 0x00 0x00 0x00 0x28 0x40 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00}
+
+thread select 2
+# CHECK: (lldb) thread select 2
+register read --all
+# CHECK-DAG: ecx = 0x24242424
+# CHECK-DAG: edx = 0x23232323
+# CHECK-DAG: edi = 0x21212121
+# CHECK-DAG: esi = 0x
+# CHECK-DAG: xmm0 = {0x00 0x00 0x00 0x00 0x00 0x00 0x14 0x40 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00}
+# CHECK-DAG: xmm1 = {0x00 0x00 0x00 0x00 0x00 0x00 0x24 0x40 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00}
+# CHECK-DAG: xmm2 = {0x00 0x00 0x00 0x00 0x00 0x00 0x2e 0x40 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00}
+# CHECK-DAG: xmm3 = {0x00 0x00 0x00 0x00 0x00 0x00 0x34 0x40 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00}
Index: lldb/test/Shell/Register/Core/x86-64-linux-multithread.test
===
--- /dev/null
+++ lldb/test/Shell/Register/Core/x86-64-linux-multithread.test
@@ -0,0 +1,41 @@
+# RUN: %lldb -b -s %s -c %p/Inputs/x86-64-linux-multithread.core | FileCheck %s
+
+thread list
+# CHECK: * thread #1: tid = 329384, 0x00401262, name = 'a.out', stop reason = signal SIGSEGV
+# CHECK-NEXT:   thread #2: tid = 329385, 0x0040126d, stop reason = signal 0
+# CHECK-NEXT:   thread #3: tid = 329386, 0x0040126d, stop reason = signal 0
+# CHECK-NEXT:   thread #4: tid = 329383, 0x7fcf5582f762, stop reason = signal 0
+
+register read --all
+# CHECK-DAG: ecx = 0x04040404
+# CHECK-DAG: edx = 0x03030303
+# CHECK-DAG: edi = 0x01010101
+# CHECK-DAG: esi = 0x02020202
+# CHECK-DAG: xmm0 = {0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x40 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00}
+# CHECK-DAG: xmm1 = {0x00 0x00 0x00 0x00 0x00 0x00 0x10 0x40 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00}
+# CHECK-DAG: xmm2 = {0x00 0x00 0x00 0x00 0x00 0x00 0x20 0x40 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00}
+# CHECK-DAG: xmm3 = {0x00 0x00 0x00 0x00 0x00 0x00 0x30 0x40 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00}
+
+thread select 2
+# CHECK: (lldb) thread select 2
+register read --all
+# CHECK-DAG: ecx = 0x14141414
+# CHECK-DAG: edx = 

[Lldb-commits] [lldb] a0c735e - [lldb] Skip TestPointerToMemberTypeDependingOnParentSize on Windows and GCC

2021-04-26 Thread Raphael Isemann via lldb-commits

Author: Raphael Isemann
Date: 2021-04-26T18:55:54+02:00
New Revision: a0c735e29a4fd471fa4a9ee60b3bdea79a066e28

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

LOG: [lldb] Skip TestPointerToMemberTypeDependingOnParentSize on Windows and GCC

The test added in D100977 is failing to compile on these platforms. This seems
to be caused by GCC, MSVC and Clang@Windows rejecting the code because
`ToLayout` isn't complete when pointer_to_member_member is declared (even though
that seems to be valid code).

This also reverts the test changes in the lazy-loading test from D100977 as
that failed for the same reason.

Added: 


Modified: 
lldb/test/API/functionalities/lazy-loading/TestLazyLoading.py
lldb/test/API/functionalities/lazy-loading/main.cpp

lldb/test/API/lang/cpp/pointer_to_member_type_depending_on_parent_size/TestPointerToMemberTypeDependingOnParentSize.py

Removed: 




diff  --git a/lldb/test/API/functionalities/lazy-loading/TestLazyLoading.py 
b/lldb/test/API/functionalities/lazy-loading/TestLazyLoading.py
index de5154e66ae5..326315c838e5 100644
--- a/lldb/test/API/functionalities/lazy-loading/TestLazyLoading.py
+++ b/lldb/test/API/functionalities/lazy-loading/TestLazyLoading.py
@@ -41,7 +41,6 @@ def setUp(self):
 class_we_enter_decl = [class_decl_kind, "ClassWeEnter"]
 class_member_decl = [struct_decl_kind, "ClassMember"]
 class_static_member_decl = [struct_decl_kind, "StaticClassMember"]
-class_pointer_to_member_decl = [struct_decl_kind, "PointerToMember"]
 unused_class_member_decl = [struct_decl_kind, "UnusedClassMember"]
 unused_class_member_ptr_decl = [struct_decl_kind, "UnusedClassMemberPtr"]
 
@@ -59,7 +58,6 @@ def assert_no_decls_loaded(self):
 self.assert_decl_not_loaded(self.class_in_namespace_decl)
 self.assert_decl_not_loaded(self.class_member_decl)
 self.assert_decl_not_loaded(self.class_static_member_decl)
-self.assert_decl_not_loaded(self.class_pointer_to_member_decl)
 self.assert_decl_not_loaded(self.unused_class_member_decl)
 
 def get_ast_dump(self):
@@ -234,8 +232,6 @@ def test_class_function_access_member(self):
 self.assert_decl_loaded(self.class_member_decl)
 # We didn't load the type of the unused static member.
 self.assert_decl_not_completed(self.class_static_member_decl)
-# We didn't load the type of the unused pointer-to-member member.
-self.assert_decl_not_completed(self.class_pointer_to_member_decl)
 
 # This should not have loaded anything else.
 self.assert_decl_not_loaded(self.other_struct_decl)

diff  --git a/lldb/test/API/functionalities/lazy-loading/main.cpp 
b/lldb/test/API/functionalities/lazy-loading/main.cpp
index 013fb02eb88f..bb8f56e277ce 100644
--- a/lldb/test/API/functionalities/lazy-loading/main.cpp
+++ b/lldb/test/API/functionalities/lazy-loading/main.cpp
@@ -26,7 +26,6 @@ struct ClassMember { int i; };
 struct StaticClassMember { int i; };
 struct UnusedClassMember { int i; };
 struct UnusedClassMemberPtr { int i; };
-struct PointerToMember { int i; };
 
 namespace NS {
 class ClassInNamespace {
@@ -37,7 +36,6 @@ class ClassWeEnter {
   int dummy; // Prevent bug where LLDB always completes first member.
   ClassMember member;
   static StaticClassMember static_member;
-  int (PointerToMember::*ptr_to_member);
   UnusedClassMember unused_member;
   UnusedClassMemberPtr *unused_member_ptr;
   int enteredFunction() {

diff  --git 
a/lldb/test/API/lang/cpp/pointer_to_member_type_depending_on_parent_size/TestPointerToMemberTypeDependingOnParentSize.py
 
b/lldb/test/API/lang/cpp/pointer_to_member_type_depending_on_parent_size/TestPointerToMemberTypeDependingOnParentSize.py
index fd978158e716..cd8977c3a2e1 100644
--- 
a/lldb/test/API/lang/cpp/pointer_to_member_type_depending_on_parent_size/TestPointerToMemberTypeDependingOnParentSize.py
+++ 
b/lldb/test/API/lang/cpp/pointer_to_member_type_depending_on_parent_size/TestPointerToMemberTypeDependingOnParentSize.py
@@ -7,6 +7,12 @@ class TestCase(TestBase):
 
 mydir = TestBase.compute_mydir(__file__)
 
+# GCC rejects the test code because `ToLayout` is not complete when
+# pointer_to_member_member is declared.
+@skipIf(compiler="gcc")
+# On Windows both MSVC and Clang are rejecting the test code because
+# `ToLayout` is not complete when pointer_to_member_member is declared.
+@skipIfWindows
 @no_debug_info_test
 def test(self):
 """



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


[Lldb-commits] [PATCH] D100977: [lldb] Use forward type in pointer-to-member

2021-04-26 Thread Raphael Isemann via Phabricator via lldb-commits
teemperor added a comment.

Actually it seems Clang also rejects it when running on Windows? I would have 
assumed that only the `-fms-compatibility-version=19.0` or something like 
`-fdelayed-template-parsing` matters here but I'm not sure why this is 
different on Windows. I'm skipping for GCC and Windows then.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100977

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


[Lldb-commits] [PATCH] D100977: [lldb] Use forward type in pointer-to-member

2021-04-26 Thread Raphael Isemann via Phabricator via lldb-commits
teemperor added a comment.

Sorry and thanks for commenting Stella!

So it seems that using an incomplete type in the test is rejected by GCC and 
MSVC but accepted by Clang and ICC. I believe that should be accepted and 
that's also what the internet(TM) and Shafik are saying, so I'm going to skip 
the test for those compilers.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100977

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


[Lldb-commits] [lldb] 7d850db - [lldb] Don't use ::fork or ::vfork on watchOS or tvOS

2021-04-26 Thread Jonas Devlieghere via lldb-commits

Author: Jonas Devlieghere
Date: 2021-04-26T09:31:35-07:00
New Revision: 7d850db6b6438221555b800d31ef5dc63f50dc89

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

LOG: [lldb] Don't use ::fork or ::vfork on watchOS or tvOS

Update lldb-server to not use fork or vfork on watchOS and tvOS as these
functions are explicitly marked unavailable there.

llvm-project/lldb/test/API/tools/lldb-server/main.cpp:304:11:
error: 'fork' is unavailable: not available on watchOS
  if (fork() == 0)
  ^
WatchSimulator6.2.sdk/usr/include/unistd.h:447:8: note: 'fork' has been
explicitly marked unavailable here
pid_tfork(void) __WATCHOS_PROHIBITED __TVOS_PROHIBITED;
 ^
llvm-project/lldb/test/API/tools/lldb-server/main.cpp:307:11:
error: 'vfork' is unavailable: not available on watchOS
  if (vfork() == 0)
  ^
WatchSimulator6.2.sdk/usr/include/unistd.h:602:8: note: 'vfork' has been
explicitly marked unavailable here
pid_tvfork(void) __WATCHOS_PROHIBITED __TVOS_PROHIBITED;
 ^

Added: 


Modified: 
lldb/test/API/tools/lldb-server/main.cpp

Removed: 




diff  --git a/lldb/test/API/tools/lldb-server/main.cpp 
b/lldb/test/API/tools/lldb-server/main.cpp
index a161a95708ee..f113304cfd62 100644
--- a/lldb/test/API/tools/lldb-server/main.cpp
+++ b/lldb/test/API/tools/lldb-server/main.cpp
@@ -19,6 +19,9 @@
 #include 
 #include 
 #include 
+#if defined(__APPLE__)
+#include 
+#endif
 
 static const char *const PRINT_PID_COMMAND = "print-pid";
 
@@ -299,7 +302,7 @@ int main(int argc, char **argv) {
   else if (arg == "swap_chars")
 func_p = swap_chars;
   func_p();
-#if !defined(_WIN32)
+#if !defined(_WIN32) && !defined(TARGET_OS_WATCH) && !defined(TARGET_OS_TV)
 } else if (arg == "fork") {
   if (fork() == 0)
 _exit(0);



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


[Lldb-commits] [PATCH] D101285: [lldb] [llgs server] Support creating core dumps on NetBSD

2021-04-26 Thread Michał Górny via Phabricator via lldb-commits
mgorny updated this revision to Diff 340560.
mgorny added a comment.

Remove unnecessary `const_cast`, reformat.


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

https://reviews.llvm.org/D101285

Files:
  lldb/include/lldb/Host/common/NativeProcessProtocol.h
  lldb/include/lldb/Utility/StringExtractorGDBRemote.h
  lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp
  lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
  lldb/source/Utility/StringExtractorGDBRemote.cpp
  lldb/test/API/tools/lldb-server/TestGdbRemoteSaveCore.py

Index: lldb/test/API/tools/lldb-server/TestGdbRemoteSaveCore.py
===
--- /dev/null
+++ lldb/test/API/tools/lldb-server/TestGdbRemoteSaveCore.py
@@ -0,0 +1,60 @@
+import gdbremote_testcase
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+import binascii
+import os
+
+class TestGdbSaveCore(gdbremote_testcase.GdbRemoteTestCaseBase):
+mydir = TestBase.compute_mydir(__file__)
+
+@skipUnlessPlatform(oslist=["netbsd"])
+def test_netbsd_path(self):
+self.build()
+self.set_inferior_startup_attach()
+procs = self.prep_debug_monitor_and_inferior()
+self.add_qSupported_packets()
+ret = self.expect_gdbremote_sequence()
+self.assertIn("qSaveCore", ret["qSupported_response"])
+self.reset_test_sequence()
+
+core_path = lldbutil.append_to_process_working_directory(self, "core")
+self.test_sequence.add_log_lines([
+"read packet: $qSaveCore;path-hint:{}#00".format(
+binascii.b2a_hex(core_path.encode()).decode()),
+{"direction": "send", "regex": "[$]core-path:([0-9a-f]+)#.*",
+ "capture": {1: "path"}},
+], True)
+ret = self.expect_gdbremote_sequence()
+out_path = binascii.a2b_hex(ret["path"].encode()).decode()
+self.assertEqual(core_path, out_path)
+
+target = self.dbg.CreateTarget(None)
+process = target.LoadCore(out_path)
+self.assertTrue(process, PROCESS_IS_VALID)
+self.assertEqual(process.GetProcessID(), procs["inferior"].pid)
+
+@skipUnlessPlatform(oslist=["netbsd"])
+def test_netbsd_no_path(self):
+self.build()
+self.set_inferior_startup_attach()
+procs = self.prep_debug_monitor_and_inferior()
+self.add_qSupported_packets()
+ret = self.expect_gdbremote_sequence()
+self.assertIn("qSaveCore", ret["qSupported_response"])
+self.reset_test_sequence()
+
+self.test_sequence.add_log_lines([
+"read packet: $qSaveCore#00",
+{"direction": "send", "regex": "[$]core-path:([0-9a-f]+)#.*",
+ "capture": {1: "path"}},
+], True)
+ret = self.expect_gdbremote_sequence()
+out_path = binascii.a2b_hex(ret["path"].encode()).decode()
+
+target = self.dbg.CreateTarget(None)
+process = target.LoadCore(out_path)
+self.assertTrue(process, PROCESS_IS_VALID)
+self.assertEqual(process.GetProcessID(), procs["inferior"].pid)
+os.unlink(out_path)
Index: lldb/source/Utility/StringExtractorGDBRemote.cpp
===
--- lldb/source/Utility/StringExtractorGDBRemote.cpp
+++ lldb/source/Utility/StringExtractorGDBRemote.cpp
@@ -253,6 +253,8 @@
   break;
 
 case 'S':
+  if (PACKET_STARTS_WITH("qSaveCore"))
+return eServerPacketType_qLLDBSaveCore;
   if (PACKET_STARTS_WITH("qSpeedTest:"))
 return eServerPacketType_qSpeedTest;
   if (PACKET_MATCHES("qShlibInfoAddr"))
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
@@ -214,6 +214,8 @@
 
   PacketResult Handle_QPassSignals(StringExtractorGDBRemote );
 
+  PacketResult Handle_qSaveCore(StringExtractorGDBRemote );
+
   PacketResult Handle_g(StringExtractorGDBRemote );
 
   void SetCurrentThreadID(lldb::tid_t tid);
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -217,6 +217,10 @@
   quit = true;
   return this->Handle_k(packet);
 });
+
+  RegisterMemberFunctionHandler(
+  StringExtractorGDBRemote::eServerPacketType_qLLDBSaveCore,
+  

[Lldb-commits] [PATCH] D101285: [lldb] [llgs server] Support creating core dumps on NetBSD

2021-04-26 Thread Michał Górny via Phabricator via lldb-commits
mgorny updated this revision to Diff 340558.
mgorny retitled this revision from "[lldb] [llgs server] Support creating core 
dumps on NetBSD [WIP]" to "[lldb] [llgs server] Support creating core dumps on 
NetBSD".
mgorny added a comment.

Create a temporary file if `path-hint` is not provided.


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

https://reviews.llvm.org/D101285

Files:
  lldb/include/lldb/Host/common/NativeProcessProtocol.h
  lldb/include/lldb/Utility/StringExtractorGDBRemote.h
  lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp
  lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
  lldb/source/Utility/StringExtractorGDBRemote.cpp
  lldb/test/API/tools/lldb-server/TestGdbRemoteSaveCore.py

Index: lldb/test/API/tools/lldb-server/TestGdbRemoteSaveCore.py
===
--- /dev/null
+++ lldb/test/API/tools/lldb-server/TestGdbRemoteSaveCore.py
@@ -0,0 +1,60 @@
+import gdbremote_testcase
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+import binascii
+import os
+
+class TestGdbSaveCore(gdbremote_testcase.GdbRemoteTestCaseBase):
+mydir = TestBase.compute_mydir(__file__)
+
+@skipUnlessPlatform(oslist=["netbsd"])
+def test_netbsd_path(self):
+self.build()
+self.set_inferior_startup_attach()
+procs = self.prep_debug_monitor_and_inferior()
+self.add_qSupported_packets()
+ret = self.expect_gdbremote_sequence()
+self.assertIn("qSaveCore", ret["qSupported_response"])
+self.reset_test_sequence()
+
+core_path = lldbutil.append_to_process_working_directory(self, "core")
+self.test_sequence.add_log_lines([
+"read packet: $qSaveCore;path-hint:{}#00".format(
+binascii.b2a_hex(core_path.encode()).decode()),
+{"direction": "send", "regex": "[$]core-path:([0-9a-f]+)#.*",
+ "capture": {1: "path"}},
+], True)
+ret = self.expect_gdbremote_sequence()
+out_path = binascii.a2b_hex(ret["path"].encode()).decode()
+self.assertEqual(core_path, out_path)
+
+target = self.dbg.CreateTarget(None)
+process = target.LoadCore(out_path)
+self.assertTrue(process, PROCESS_IS_VALID)
+self.assertEqual(process.GetProcessID(), procs["inferior"].pid)
+
+@skipUnlessPlatform(oslist=["netbsd"])
+def test_netbsd_no_path(self):
+self.build()
+self.set_inferior_startup_attach()
+procs = self.prep_debug_monitor_and_inferior()
+self.add_qSupported_packets()
+ret = self.expect_gdbremote_sequence()
+self.assertIn("qSaveCore", ret["qSupported_response"])
+self.reset_test_sequence()
+
+self.test_sequence.add_log_lines([
+"read packet: $qSaveCore#00",
+{"direction": "send", "regex": "[$]core-path:([0-9a-f]+)#.*",
+ "capture": {1: "path"}},
+], True)
+ret = self.expect_gdbremote_sequence()
+out_path = binascii.a2b_hex(ret["path"].encode()).decode()
+
+target = self.dbg.CreateTarget(None)
+process = target.LoadCore(out_path)
+self.assertTrue(process, PROCESS_IS_VALID)
+self.assertEqual(process.GetProcessID(), procs["inferior"].pid)
+os.unlink(out_path)
Index: lldb/source/Utility/StringExtractorGDBRemote.cpp
===
--- lldb/source/Utility/StringExtractorGDBRemote.cpp
+++ lldb/source/Utility/StringExtractorGDBRemote.cpp
@@ -253,6 +253,8 @@
   break;
 
 case 'S':
+  if (PACKET_STARTS_WITH("qSaveCore"))
+return eServerPacketType_qLLDBSaveCore;
   if (PACKET_STARTS_WITH("qSpeedTest:"))
 return eServerPacketType_qSpeedTest;
   if (PACKET_MATCHES("qShlibInfoAddr"))
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
@@ -214,6 +214,8 @@
 
   PacketResult Handle_QPassSignals(StringExtractorGDBRemote );
 
+  PacketResult Handle_qSaveCore(StringExtractorGDBRemote );
+
   PacketResult Handle_g(StringExtractorGDBRemote );
 
   void SetCurrentThreadID(lldb::tid_t tid);
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -217,6 +217,10 @@
   quit = true;
 

[Lldb-commits] [PATCH] D100977: [lldb] Use forward type in pointer-to-member

2021-04-26 Thread Stella Stamenova via Phabricator via lldb-commits
stella.stamenova added a comment.

Looks like this is failing on the Windows LLDB bot:

https://lab.llvm.org/buildbot/#/builders/83/builds/6033


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100977

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


Re: [Lldb-commits] Buildbot failure in LLVM Buildbot on lldb-aarch64-ubuntu

2021-04-26 Thread Raphael Isemann via lldb-commits
You can ignore that, that seems to be either a flaky test or an issue
with the bot (the next test run passes just fine).

- Raphael

Am Mo., 26. Apr. 2021 um 17:04 Uhr schrieb Nalamothu, Venkata
Ramanaiah via lldb-commits :
>
> [AMD Public Use]
>
> Hi,
>
> The below commit I made is an NFC and moreover, AFAIU, it has got nothing to 
> do with LLDB.
> What should I do?
>
> Regards,
> Ram
>
> -Original Message-
> From: llvm.buildmas...@lab.llvm.org 
> Sent: Saturday, April 24, 2021 6:59 AM
> To: Nalamothu, Venkata Ramanaiah 
> Cc: gkistan...@gmail.com
> Subject: Buildbot failure in LLVM Buildbot on lldb-aarch64-ubuntu
>
> [CAUTION: External Email]
>
> The Buildbot has detected a failed build on builder lldb-aarch64-ubuntu while 
> building llvm.
>
> Full details are available at:
> 
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flab.llvm.org%2Fbuildbot%23builders%2F96%2Fbuilds%2F7033data=04%7C01%7CVenkataRamanaiah.Nalamothu%40amd.com%7C416cc1ebe2244a1ac69708d906c05824%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637548245462498918%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000sdata=Qu%2BQ2MK2x2Bfwxa2dcIWoxxnac65cC734LPqMfAlAqU%3Dreserved=0
>
> Worker for this Build: linaro-aarch64-lldb
> Blamelist:
> RamNalamothu 
>
> BUILD FAILED: failed 'ninja check-lldb ...' (failure)
>
> Step 4 (build) warnings: 'ninja -j16' (warnings)
> /usr/bin/ar: warning: 
> tools/clang/lib/Basic/CMakeFiles/obj.clangBasic.dir/CharInfo.cpp.o: 
> unsupported GNU_PROPERTY_TYPE (5) type: 0xc000
> /usr/bin/ranlib: warning: lib/libclangBasic.a(CharInfo.cpp.o): unsupported 
> GNU_PROPERTY_TYPE (5) type: 0xc000
>
> Step 6 (test) failure: 'ninja check-lldb ...' (failure) ...
> [39/41] Linking CXX executable tools/lldb/unittests/Utility/UtilityTests
> [40/41] Linking CXX executable tools/lldb/unittests/Thread/ThreadTests
> [40/41] Running lldb lit test suite
> -- Testing: 2320 tests, 8 workers --
> llvm-lit: 
> /home/tcwg-buildslave/worker/lldb-cmake-aarch64/llvm-project/llvm/utils/lit/lit/llvm/config.py:428:
>  note: using clang: 
> /home/tcwg-buildslave/worker/lldb-cmake-aarch64/build/bin/clang
> llvm-lit: 
> /home/tcwg-buildslave/worker/lldb-cmake-aarch64/llvm-project/llvm/utils/lit/lit/llvm/config.py:428:
>  note: using ld.lld: 
> /home/tcwg-buildslave/worker/lldb-cmake-aarch64/build/bin/ld.lld
> llvm-lit: 
> /home/tcwg-buildslave/worker/lldb-cmake-aarch64/llvm-project/llvm/utils/lit/lit/llvm/config.py:428:
>  note: using lld-link: 
> /home/tcwg-buildslave/worker/lldb-cmake-aarch64/build/bin/lld-link
> llvm-lit: 
> /home/tcwg-buildslave/worker/lldb-cmake-aarch64/llvm-project/llvm/utils/lit/lit/llvm/config.py:428:
>  note: using ld64.lld: 
> /home/tcwg-buildslave/worker/lldb-cmake-aarch64/build/bin/ld64.lld
> llvm-lit: 
> /home/tcwg-buildslave/worker/lldb-cmake-aarch64/llvm-project/llvm/utils/lit/lit/llvm/config.py:428:
>  note: using wasm-ld: 
> /home/tcwg-buildslave/worker/lldb-cmake-aarch64/build/bin/wasm-ld
> Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70..
> FAIL: lldb-api :: commands/platform/connect/TestPlatformConnect.py (478 of 
> 2320)
>  TEST 'lldb-api :: 
> commands/platform/connect/TestPlatformConnect.py' FAILED 
> Script:
> --
> /usr/bin/python3.6 
> /home/tcwg-buildslave/worker/lldb-cmake-aarch64/llvm-project/lldb/test/API/dotest.py
>  -u CXXFLAGS -u CFLAGS --env ARCHIVER=/usr/bin/ar --env 
> OBJCOPY=/usr/bin/objcopy --env 
> LLVM_LIBS_DIR=/home/tcwg-buildslave/worker/lldb-cmake-aarch64/build/./lib 
> --arch aarch64 --build-dir 
> /home/tcwg-buildslave/worker/lldb-cmake-aarch64/build/lldb-test-build.noindex 
> --lldb-module-cache-dir 
> /home/tcwg-buildslave/worker/lldb-cmake-aarch64/build/lldb-test-build.noindex/module-cache-lldb/lldb-api
>  --clang-module-cache-dir 
> /home/tcwg-buildslave/worker/lldb-cmake-aarch64/build/lldb-test-build.noindex/module-cache-clang/lldb-api
>  --executable 
> /home/tcwg-buildslave/worker/lldb-cmake-aarch64/build/./bin/lldb --compiler 
> /home/tcwg-buildslave/worker/lldb-cmake-aarch64/build/./bin/clang --dsymutil 
> /home/tcwg-buildslave/worker/lldb-cmake-aarch64/build/./bin/dsymutil 
> --llvm-tools-dir /home/tcwg-buildslave/worker/lldb-cmake-aarch64/build/./bin 
> --lldb-libs-dir /home/tcwg-buildslave/worker/lldb-cmake-a
>  arch64/build/./lib 
> /home/tcwg-buildslave/worker/lldb-cmake-aarch64/llvm-project/lldb/test/API/commands/platform/connect
>  -p TestPlatformConnect.py
> --
> Exit Code: 1
>
> Command Output (stdout):
> --
> lldb version 13.0.0 
> (https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fllvm%2Fllvm-project.gitdata=04%7C01%7CVenkataRamanaiah.Nalamothu%40amd.com%7C416cc1ebe2244a1ac69708d906c05824%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637548245462498918%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000sdata=asorfXmx8XbZcqGdPKVtixk%2FQnVtQiVO7ZcNx5Yn4GI%3Dreserved=0

Re: [Lldb-commits] Buildbot failure in LLVM Buildbot on lldb-aarch64-ubuntu

2021-04-26 Thread Nalamothu, Venkata Ramanaiah via lldb-commits
[AMD Public Use]

Hi,

The below commit I made is an NFC and moreover, AFAIU, it has got nothing to do 
with LLDB.
What should I do?

Regards,
Ram

-Original Message-
From: llvm.buildmas...@lab.llvm.org  
Sent: Saturday, April 24, 2021 6:59 AM
To: Nalamothu, Venkata Ramanaiah 
Cc: gkistan...@gmail.com
Subject: Buildbot failure in LLVM Buildbot on lldb-aarch64-ubuntu

[CAUTION: External Email]

The Buildbot has detected a failed build on builder lldb-aarch64-ubuntu while 
building llvm.

Full details are available at:

https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flab.llvm.org%2Fbuildbot%23builders%2F96%2Fbuilds%2F7033data=04%7C01%7CVenkataRamanaiah.Nalamothu%40amd.com%7C416cc1ebe2244a1ac69708d906c05824%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637548245462498918%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000sdata=Qu%2BQ2MK2x2Bfwxa2dcIWoxxnac65cC734LPqMfAlAqU%3Dreserved=0

Worker for this Build: linaro-aarch64-lldb
Blamelist:
RamNalamothu 

BUILD FAILED: failed 'ninja check-lldb ...' (failure)

Step 4 (build) warnings: 'ninja -j16' (warnings)
/usr/bin/ar: warning: 
tools/clang/lib/Basic/CMakeFiles/obj.clangBasic.dir/CharInfo.cpp.o: unsupported 
GNU_PROPERTY_TYPE (5) type: 0xc000
/usr/bin/ranlib: warning: lib/libclangBasic.a(CharInfo.cpp.o): unsupported 
GNU_PROPERTY_TYPE (5) type: 0xc000

Step 6 (test) failure: 'ninja check-lldb ...' (failure) ...
[39/41] Linking CXX executable tools/lldb/unittests/Utility/UtilityTests
[40/41] Linking CXX executable tools/lldb/unittests/Thread/ThreadTests
[40/41] Running lldb lit test suite
-- Testing: 2320 tests, 8 workers --
llvm-lit: 
/home/tcwg-buildslave/worker/lldb-cmake-aarch64/llvm-project/llvm/utils/lit/lit/llvm/config.py:428:
 note: using clang: 
/home/tcwg-buildslave/worker/lldb-cmake-aarch64/build/bin/clang
llvm-lit: 
/home/tcwg-buildslave/worker/lldb-cmake-aarch64/llvm-project/llvm/utils/lit/lit/llvm/config.py:428:
 note: using ld.lld: 
/home/tcwg-buildslave/worker/lldb-cmake-aarch64/build/bin/ld.lld
llvm-lit: 
/home/tcwg-buildslave/worker/lldb-cmake-aarch64/llvm-project/llvm/utils/lit/lit/llvm/config.py:428:
 note: using lld-link: 
/home/tcwg-buildslave/worker/lldb-cmake-aarch64/build/bin/lld-link
llvm-lit: 
/home/tcwg-buildslave/worker/lldb-cmake-aarch64/llvm-project/llvm/utils/lit/lit/llvm/config.py:428:
 note: using ld64.lld: 
/home/tcwg-buildslave/worker/lldb-cmake-aarch64/build/bin/ld64.lld
llvm-lit: 
/home/tcwg-buildslave/worker/lldb-cmake-aarch64/llvm-project/llvm/utils/lit/lit/llvm/config.py:428:
 note: using wasm-ld: 
/home/tcwg-buildslave/worker/lldb-cmake-aarch64/build/bin/wasm-ld
Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70..
FAIL: lldb-api :: commands/platform/connect/TestPlatformConnect.py (478 of 2320)
 TEST 'lldb-api :: 
commands/platform/connect/TestPlatformConnect.py' FAILED 
Script:
--
/usr/bin/python3.6 
/home/tcwg-buildslave/worker/lldb-cmake-aarch64/llvm-project/lldb/test/API/dotest.py
 -u CXXFLAGS -u CFLAGS --env ARCHIVER=/usr/bin/ar --env 
OBJCOPY=/usr/bin/objcopy --env 
LLVM_LIBS_DIR=/home/tcwg-buildslave/worker/lldb-cmake-aarch64/build/./lib 
--arch aarch64 --build-dir 
/home/tcwg-buildslave/worker/lldb-cmake-aarch64/build/lldb-test-build.noindex 
--lldb-module-cache-dir 
/home/tcwg-buildslave/worker/lldb-cmake-aarch64/build/lldb-test-build.noindex/module-cache-lldb/lldb-api
 --clang-module-cache-dir 
/home/tcwg-buildslave/worker/lldb-cmake-aarch64/build/lldb-test-build.noindex/module-cache-clang/lldb-api
 --executable /home/tcwg-buildslave/worker/lldb-cmake-aarch64/build/./bin/lldb 
--compiler /home/tcwg-buildslave/worker/lldb-cmake-aarch64/build/./bin/clang 
--dsymutil /home/tcwg-buildslave/worker/lldb-cmake-aarch64/build/./bin/dsymutil 
--llvm-tools-dir /home/tcwg-buildslave/worker/lldb-cmake-aarch64/build/./bin 
--lldb-libs-dir /home/tcwg-buildslave/worker/lldb-cmake-a
 arch64/build/./lib 
/home/tcwg-buildslave/worker/lldb-cmake-aarch64/llvm-project/lldb/test/API/commands/platform/connect
 -p TestPlatformConnect.py
--
Exit Code: 1

Command Output (stdout):
--
lldb version 13.0.0 
(https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fllvm%2Fllvm-project.gitdata=04%7C01%7CVenkataRamanaiah.Nalamothu%40amd.com%7C416cc1ebe2244a1ac69708d906c05824%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637548245462498918%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000sdata=asorfXmx8XbZcqGdPKVtixk%2FQnVtQiVO7ZcNx5Yn4GI%3Dreserved=0
 revision 4e87fdd78643f0fd26f1675c339c4de1dc8b1dcb)
  clang revision 4e87fdd78643f0fd26f1675c339c4de1dc8b1dcb
  llvm revision 4e87fdd78643f0fd26f1675c339c4de1dc8b1dcb
Skipping the following test categories: ['darwin-log', 'libc++', 'dsym', 
'gmodules', 'debugserver', 'objc']

--
Command Output (stderr):
--
failed to start gdbserver: timed out
SIGHUP received, exiting lldb-server...
PLEASE submit a bug report 

[Lldb-commits] [PATCH] D101153: [lldb][NFC] Specify guidelines for API tests

2021-04-26 Thread Raphael Isemann via Phabricator via lldb-commits
teemperor updated this revision to Diff 340520.
teemperor marked 5 inline comments as done.
teemperor added a comment.

- Update diff with feedback.


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

https://reviews.llvm.org/D101153

Files:
  lldb/docs/resources/test.rst

Index: lldb/docs/resources/test.rst
===
--- lldb/docs/resources/test.rst
+++ lldb/docs/resources/test.rst
@@ -196,6 +196,129 @@
 variants mean that more general tests should be API tests, so that they can be
 run against the different variants.
 
+Guidelines for API tests
+
+
+API tests are expected to be fast, reliable and maintainable. To achieve this
+goal, API tests should conform to the following guidelines in addition to normal
+good testing practices.
+
+**Don't unnecessarily launch the test executable.**
+Launching a process and running to a breakpoint can often be the most
+expensive part of a test and should be avoided if possible. A large part
+of LLDB's functionality is available directly after creating an `SBTarget`
+of the test executable.
+
+The part of the SB API that can be tested with just a target includes
+everything that just represents information about the executable and its
+debug information (e.g., `SBTarget`, `SBModule`, `SBSymbolContext`,
+`SBFunction`, `SBInstruction`, `SBCompileUnit`, etc.). For test executables
+written in languages with a type system that is mostly defined at compile
+time (e.g., C and C++) there is also usually no process necessary to test
+the `SBType`-related parts of the API. With those languages it's also
+possible to test `SBValue` by running expressions with
+`SBTarget.EvaluateExpression` or the `expect_expr` testing utility.
+
+Functionality that always requires a running process is everything that
+tests the `SBProcess`, `SBThread`, and `SBFrame` classes. The same is true
+for tests that exercise to breakpoints, watchpoints and sanitizers.
+Languages such as Objective-C that have a dependency on a runtime
+environment also always require a running process.
+
+**Don't unnecessarily include system headers in test sources.**
+Including external headers slows down the compilation of the test executable
+and it makes reproducing test failures on other operating systems or
+configurations harder.
+
+**Avoid specifying test-specific compiler flags when including system headers.**
+If a test requires including a system header (e.g., a test for a libc++
+formatter includes a libc++ header), try to avoid specifying custom compiler
+flags if possible. Certain debug information formats such as ``gmodules``
+use a cache that is shared between all API tests and that contains
+precompiled system headers. If you add or remove a specific compiler flag
+in your test (e.g., adding ``-DFOO`` to the ``Makefile`` or ``self.build``
+arguments), then the test will not use the shared precompiled header cache
+and expensively recompile all system headers from scratch. If you depend on
+a specific compiler flag for the test, you can avoid this issue by either
+removing all system header includes or decorating the test function with
+``@no_debug_info_test`` (which will avoid running all debug information
+variants including ``gmodules``).
+
+**Test programs should be kept simple.**
+Test executables should do the minimum amount of work to bring the process
+into the state that is required for the test. Simulating a 'real' program
+that actually tries to do some useful task rarely helps with catching bugs
+and makes the test much harder to debug and maintain. The test programs
+should always be deterministic (i.e. do not generate and check against
+random test values).
+
+**Identifiers in tests should be simple and descriptive.**
+Often test program need to declare functions and classes which require
+choosing some form of identifier for them. These identifiers should always
+either be kept simple for small tests (e.g., ``A``, ``B``, ...) or have some
+descriptive name (e.g., ``ClassWithTailPadding``, ``inlined_func``, ...).
+Never choose identifiers that are already used anywhere else in LLVM or
+other programs (e.g., don't name a class  ``VirtualFileSystem``, a function
+``llvm_unreachable``, or a namespace ``rapidxml``) as this will just mislead
+people ``grep``'ing the LLVM repository for those strings.
+
+**Prefer LLDB testing utilities over directly working with the SB API.**
+The ``lldbutil`` module and the ``TestBase`` class come with a large amount
+of utility functions that can do common test setup tasks (e.g., starting a
+test executable and running the process to a breakpoint). Using these
+functions not only keeps the test shorter and free of duplicated code, but
+they also follow best 

[Lldb-commits] [PATCH] D100977: [lldb] Use forward type in pointer-to-member

2021-04-26 Thread Raphael Isemann via Phabricator via lldb-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGe439a463a308: [lldb] Use forward type in pointer-to-member 
(authored by emrekultursay, committed by teemperor).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100977

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/test/API/functionalities/lazy-loading/TestLazyLoading.py
  lldb/test/API/functionalities/lazy-loading/main.cpp
  
lldb/test/API/lang/cpp/pointer_to_member_type_depending_on_parent_size/Makefile
  
lldb/test/API/lang/cpp/pointer_to_member_type_depending_on_parent_size/TestPointerToMemberTypeDependingOnParentSize.py
  
lldb/test/API/lang/cpp/pointer_to_member_type_depending_on_parent_size/main.cpp

Index: lldb/test/API/lang/cpp/pointer_to_member_type_depending_on_parent_size/main.cpp
===
--- /dev/null
+++ lldb/test/API/lang/cpp/pointer_to_member_type_depending_on_parent_size/main.cpp
@@ -0,0 +1,35 @@
+// This class just serves as an indirection between LLDB and Clang. LLDB might
+// be tempted to check the member type of DependsOnParam2 for whether it's
+// in some 'currently-loading' state before trying to produce the record layout.
+// By inheriting from ToLayout this will make LLDB just check if
+// DependsOnParam1 is currently being loaded (which it's not) but it won't
+// check if all the types DependsOnParam2 is depending on for its layout are
+// currently parsed.
+template  struct DependsOnParam1 : ToLayoutParam {};
+// This class forces the memory layout of it's type parameter to be created.
+template  struct DependsOnParam2 {
+  DependsOnParam1 m;
+};
+
+// This is the class that LLDB has to generate the record layout for.
+struct ToLayout {
+  // The class part of this pointer-to-member type has a memory layout that
+  // depends on the surrounding class. If LLDB eagerly tries to layout the
+  // class part of a pointer-to-member type while parsing, then layouting this
+  // type should cause a test failure (as we aren't done parsing ToLayout
+  // at this point).
+  int DependsOnParam2::* pointer_to_member_member;
+  // Some dummy member variable. This is only there so that Clang can detect
+  // that the record layout is inconsistent (i.e., the number of fields in the
+  // layout doesn't fit to the fields in the declaration).
+  int some_member;
+};
+
+// Emit the definition of DependsOnParam2. It seems Clang won't
+// emit the definition of a class template if it's only used in the class part
+// of a pointer-to-member type.
+DependsOnParam2 x;
+
+ToLayout test_var;
+
+int main() { return test_var.some_member; }
Index: lldb/test/API/lang/cpp/pointer_to_member_type_depending_on_parent_size/TestPointerToMemberTypeDependingOnParentSize.py
===
--- /dev/null
+++ lldb/test/API/lang/cpp/pointer_to_member_type_depending_on_parent_size/TestPointerToMemberTypeDependingOnParentSize.py
@@ -0,0 +1,24 @@
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+@no_debug_info_test
+def test(self):
+"""
+This tests a pointer-to-member member which class part is the
+surrounding class. LLDB should *not* try to generate the record layout
+of the class when parsing pointer-to-member types while parsing debug
+info (as the references class might not be complete when the type is
+parsed).
+"""
+self.build()
+self.dbg.CreateTarget(self.getBuildArtifact("a.out"))
+
+# Force the record layout for 'ToLayout' to be generated by printing
+# a value of it's type.
+self.expect("target variable test_var")
Index: lldb/test/API/lang/cpp/pointer_to_member_type_depending_on_parent_size/Makefile
===
--- /dev/null
+++ lldb/test/API/lang/cpp/pointer_to_member_type_depending_on_parent_size/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
Index: lldb/test/API/functionalities/lazy-loading/main.cpp
===
--- lldb/test/API/functionalities/lazy-loading/main.cpp
+++ lldb/test/API/functionalities/lazy-loading/main.cpp
@@ -26,6 +26,7 @@
 struct StaticClassMember { int i; };
 struct UnusedClassMember { int i; };
 struct UnusedClassMemberPtr { int i; };
+struct PointerToMember { int i; };
 
 namespace NS {
 class ClassInNamespace {
@@ -36,6 +37,7 @@
   int dummy; // Prevent bug where LLDB always completes first member.
   ClassMember member;
   static StaticClassMember static_member;
+  int (PointerToMember::*ptr_to_member);
   

[Lldb-commits] [lldb] e439a46 - [lldb] Use forward type in pointer-to-member

2021-04-26 Thread Raphael Isemann via lldb-commits

Author: Emre Kultursay
Date: 2021-04-26T15:23:58+02:00
New Revision: e439a463a30833f1c7d366ed722f0f12d1682638

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

LOG: [lldb] Use forward type in pointer-to-member

This change is similar in spirit to the change at:
https://reviews.llvm.org/rG34c697c85e9d0af11a72ac4df5578aac94a627b3

It fixes the problem where the layout of a type was being accessed
while its base classes were not populated yet; which caused an
incorrect layout to be produced and cached.

This fixes PR50054

Reviewed By: teemperor

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

Added: 

lldb/test/API/lang/cpp/pointer_to_member_type_depending_on_parent_size/Makefile

lldb/test/API/lang/cpp/pointer_to_member_type_depending_on_parent_size/TestPointerToMemberTypeDependingOnParentSize.py

lldb/test/API/lang/cpp/pointer_to_member_type_depending_on_parent_size/main.cpp

Modified: 
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
lldb/test/API/functionalities/lazy-loading/TestLazyLoading.py
lldb/test/API/functionalities/lazy-loading/main.cpp

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index c417f8055c88..55fd5897c370 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1361,7 +1361,7 @@ TypeSP DWARFASTParserClang::ParsePointerToMemberType(
   dwarf->ResolveTypeUID(attrs.containing_type.Reference(), true);
 
   CompilerType pointee_clang_type = pointee_type->GetForwardCompilerType();
-  CompilerType class_clang_type = class_type->GetLayoutCompilerType();
+  CompilerType class_clang_type = class_type->GetForwardCompilerType();
 
   CompilerType clang_type = TypeSystemClang::CreateMemberPointerType(
   class_clang_type, pointee_clang_type);

diff  --git a/lldb/test/API/functionalities/lazy-loading/TestLazyLoading.py 
b/lldb/test/API/functionalities/lazy-loading/TestLazyLoading.py
index 326315c838e5..de5154e66ae5 100644
--- a/lldb/test/API/functionalities/lazy-loading/TestLazyLoading.py
+++ b/lldb/test/API/functionalities/lazy-loading/TestLazyLoading.py
@@ -41,6 +41,7 @@ def setUp(self):
 class_we_enter_decl = [class_decl_kind, "ClassWeEnter"]
 class_member_decl = [struct_decl_kind, "ClassMember"]
 class_static_member_decl = [struct_decl_kind, "StaticClassMember"]
+class_pointer_to_member_decl = [struct_decl_kind, "PointerToMember"]
 unused_class_member_decl = [struct_decl_kind, "UnusedClassMember"]
 unused_class_member_ptr_decl = [struct_decl_kind, "UnusedClassMemberPtr"]
 
@@ -58,6 +59,7 @@ def assert_no_decls_loaded(self):
 self.assert_decl_not_loaded(self.class_in_namespace_decl)
 self.assert_decl_not_loaded(self.class_member_decl)
 self.assert_decl_not_loaded(self.class_static_member_decl)
+self.assert_decl_not_loaded(self.class_pointer_to_member_decl)
 self.assert_decl_not_loaded(self.unused_class_member_decl)
 
 def get_ast_dump(self):
@@ -232,6 +234,8 @@ def test_class_function_access_member(self):
 self.assert_decl_loaded(self.class_member_decl)
 # We didn't load the type of the unused static member.
 self.assert_decl_not_completed(self.class_static_member_decl)
+# We didn't load the type of the unused pointer-to-member member.
+self.assert_decl_not_completed(self.class_pointer_to_member_decl)
 
 # This should not have loaded anything else.
 self.assert_decl_not_loaded(self.other_struct_decl)

diff  --git a/lldb/test/API/functionalities/lazy-loading/main.cpp 
b/lldb/test/API/functionalities/lazy-loading/main.cpp
index bb8f56e277ce..013fb02eb88f 100644
--- a/lldb/test/API/functionalities/lazy-loading/main.cpp
+++ b/lldb/test/API/functionalities/lazy-loading/main.cpp
@@ -26,6 +26,7 @@ struct ClassMember { int i; };
 struct StaticClassMember { int i; };
 struct UnusedClassMember { int i; };
 struct UnusedClassMemberPtr { int i; };
+struct PointerToMember { int i; };
 
 namespace NS {
 class ClassInNamespace {
@@ -36,6 +37,7 @@ class ClassWeEnter {
   int dummy; // Prevent bug where LLDB always completes first member.
   ClassMember member;
   static StaticClassMember static_member;
+  int (PointerToMember::*ptr_to_member);
   UnusedClassMember unused_member;
   UnusedClassMemberPtr *unused_member_ptr;
   int enteredFunction() {

diff  --git 
a/lldb/test/API/lang/cpp/pointer_to_member_type_depending_on_parent_size/Makefile
 
b/lldb/test/API/lang/cpp/pointer_to_member_type_depending_on_parent_size/Makefile
new file mode 100644
index ..8b20bcb0
--- /dev/null
+++ 

[Lldb-commits] [PATCH] D101153: [lldb][NFC] Specify guidelines for API tests

2021-04-26 Thread Raphael Isemann via Phabricator via lldb-commits
teemperor marked 7 inline comments as done.
teemperor added a comment.

Addressing feedback.




Comment at: lldb/docs/resources/test.rst:206
+
+**Don't unnecessarily launch the test executable.**
+Launching a process and running to a breakpoint can often be the most

shafik wrote:
> While I agree with this, it also feels unhelpful because it does not give any 
> examples nor explain the alternatives.
> 
> I can see the problem with pointing at specific tests which may disappear or 
> change. I can also see the problem with attempting to enumerate all the 
> possibilities below this as well.
> 
> Maybe we need a set of example tests as well?
> 
> Most of the rest of advice stands alone pretty well though. 
Good point. We actually have (or had?) example tests but they always end up 
being forgotten about and are probably in a bad shape these days. I think by 
now many tests are in a good enough shape that people find a good test they can 
copy/extend for their own purpose, so I think we are relatively fine without 
explicit example tests that demonstrate this.

I updated the text with a list of functionality that I would expect to avoid 
creating a process in their tests (and a list of features where creating a test 
is unavoidable in 99% of the cases). I don't think this list will every change 
and I think it should give people a general idea whether they can avoid 
launching a process.



Comment at: lldb/docs/resources/test.rst:222
+``gmodules`` need to recompile external headers when they encounter
+test-specific flags (including defines) which can be very expensive.
+

DavidSpickett wrote:
> Does this mean only use the flags you really need, or to put those flags 
> somewhere other than the makefile? (I guess the former since they'd have to 
> be in the makefile for your test to work anyway)
> 
> Would be good to make it clear so the reader doesn't think that there's some 
> other place to put compiler flags, which isn't specified.
Thanks! I clarified that specifying them anywhere is the problem (and how to 
avoid it).



Comment at: lldb/docs/resources/test.rst:229
+and makes the test much harder to debug and maintain. The test programs
+should always be deterministic (i.e., do not generate and check against
+random test values).

JDevlieghere wrote:
> 
Done. I actually believe the comma is correct (at least the internet says it is 
and if that's not a reliable source than what is).



Comment at: lldb/docs/resources/test.rst:264
+::
+self.expect("expr 1 - 1", substrs=["0"])
+

rupprecht wrote:
> shafik wrote:
> > Maybe some more examples with alternatives would be helpful here.
> Also mentioning why this check is bad would help spell it out. IIUC, it's 
> because the full output will include `$0` (if it's the first expression, as 
> noted); in which case, a stronger example is something that's clearly wrong:
> 
> ```
> (lldb) expr 5-3
> (int) $0 = 2
> ```
> 
> In which case, `self.expect("expr 5 - 3", substrs=["0"])` passes, but 
> shouldn't.
Thanks!

I actually tried to avoid the formatting pain of having a code example in the 
middle a sphinx definition block. I put an explanation and a better alternative 
in the text now, but I don't think I can avoid that the non-inline code example 
terminates the first block. The only effect of this is just that the text 
following the definition blocks doesn't have the same indentation as the first 
text block (which actually doesn't look that bad, just a mild annoyance).



Comment at: lldb/docs/resources/test.rst:279-280
+self.assertTrue(expected_string in list_of_results)
+# Good. Will print expected_string and the contents of list_of_results.
+self.assertIn(expected_string, list_of_results) # Good.
+

rupprecht wrote:
> nit: extra "# Good"
Thanks!


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

https://reviews.llvm.org/D101153

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


[Lldb-commits] [PATCH] D101285: [lldb] [llgs server] Support creating core dumps on NetBSD [WIP]

2021-04-26 Thread Michał Górny via Phabricator via lldb-commits
mgorny created this revision.
mgorny added reviewers: labath, krytarowski, emaste.
mgorny requested review of this revision.

Add a new SaveCore() process method that can be used to request a core
dump.  This is currently implemented on NetBSD via the PT_DUMPCORE
ptrace(2) request, and enabled via 'savecore' extension.

Protocol-wise, a new qSaveCore packet is introduced.  It accepts zero
or more semicolon-separated key:value options, invokes the core dump
and returns a key:value response.  Currently the only option supported
is "path-hint", and the return value contains the "path" actually used.
The support for the feature is exposed via qSaveCore qSupported feature.


https://reviews.llvm.org/D101285

Files:
  lldb/include/lldb/Host/common/NativeProcessProtocol.h
  lldb/include/lldb/Utility/StringExtractorGDBRemote.h
  lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp
  lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
  lldb/source/Utility/StringExtractorGDBRemote.cpp
  lldb/test/API/tools/lldb-server/TestGdbRemoteSaveCore.py

Index: lldb/test/API/tools/lldb-server/TestGdbRemoteSaveCore.py
===
--- /dev/null
+++ lldb/test/API/tools/lldb-server/TestGdbRemoteSaveCore.py
@@ -0,0 +1,34 @@
+import gdbremote_testcase
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+import binascii
+
+class TestGdbSaveCore(gdbremote_testcase.GdbRemoteTestCaseBase):
+mydir = TestBase.compute_mydir(__file__)
+
+@skipUnlessPlatform(oslist=["netbsd"])
+def test_netbsd_path(self):
+self.build()
+self.set_inferior_startup_attach()
+procs = self.prep_debug_monitor_and_inferior()
+self.add_qSupported_packets()
+ret = self.expect_gdbremote_sequence()
+self.assertIn("qSaveCore", ret["qSupported_response"])
+self.reset_test_sequence()
+
+core_path = lldbutil.append_to_process_working_directory(self, "core")
+self.test_sequence.add_log_lines([
+"read packet: $qSaveCore;path-hint:{}#00".format(
+binascii.b2a_hex(core_path.encode()).decode()),
+{"direction": "send", "regex": "[$]core-path:([0-9a-f]+)#.*",
+ "capture": {1: "path"}},
+], True)
+ret = self.expect_gdbremote_sequence()
+out_path = binascii.a2b_hex(ret["path"].encode()).decode()
+
+target = self.dbg.CreateTarget(None)
+process = target.LoadCore(out_path)
+self.assertTrue(process, PROCESS_IS_VALID)
+self.assertEqual(process.GetProcessID(), procs["inferior"].pid)
Index: lldb/source/Utility/StringExtractorGDBRemote.cpp
===
--- lldb/source/Utility/StringExtractorGDBRemote.cpp
+++ lldb/source/Utility/StringExtractorGDBRemote.cpp
@@ -253,6 +253,8 @@
   break;
 
 case 'S':
+  if (PACKET_STARTS_WITH("qSaveCore"))
+return eServerPacketType_qLLDBSaveCore;
   if (PACKET_STARTS_WITH("qSpeedTest:"))
 return eServerPacketType_qSpeedTest;
   if (PACKET_MATCHES("qShlibInfoAddr"))
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
@@ -214,6 +214,8 @@
 
   PacketResult Handle_QPassSignals(StringExtractorGDBRemote );
 
+  PacketResult Handle_qSaveCore(StringExtractorGDBRemote );
+
   PacketResult Handle_g(StringExtractorGDBRemote );
 
   void SetCurrentThreadID(lldb::tid_t tid);
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -217,6 +217,10 @@
   quit = true;
   return this->Handle_k(packet);
 });
+
+  RegisterMemberFunctionHandler(
+  StringExtractorGDBRemote::eServerPacketType_qLLDBSaveCore,
+  ::Handle_qSaveCore);
 }
 
 void GDBRemoteCommunicationServerLLGS::SetLaunchInfo(const ProcessLaunchInfo ) {
@@ -3414,6 +3418,44 @@
   return SendOKResponse();
 }
 
+GDBRemoteCommunication::PacketResult
+GDBRemoteCommunicationServerLLGS::Handle_qSaveCore(
+StringExtractorGDBRemote ) {
+  // Fail if we don't have a current process.
+  if (!m_current_process ||
+  (m_current_process->GetID() == LLDB_INVALID_PROCESS_ID))
+return SendErrorResponse(Status("Process not running."));
+
+  std::string path_hint;
+
+  StringRef 

[Lldb-commits] [PATCH] D100965: [lldb] Refactor argument group by SourceLocationSpec (NFCI)

2021-04-26 Thread Raphael Isemann via Phabricator via lldb-commits
teemperor added a comment.

I am wondering how SourceLocationSpec is related to lldb's Declaration class 
(which is FileSpec + line + column and describes a location in source code)? It 
seems like the current SourceLocationSpec is just a Declaration with the two 
additional search variables (and the first iteration was exactly the same as 
Declaration).

Could we maybe turn SourceLocationSpec to store a lldb::Declaration instead of 
file/line/column? I'm aware that the Declaration class first needs some cleanup 
(and a removal of that #ifdef around column), but right now we are already 
using Declarations in a bunch of places to describe source locations.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100965

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


[Lldb-commits] [PATCH] D101153: [lldb][NFC] Specify guidelines for API tests

2021-04-26 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added inline comments.



Comment at: lldb/docs/resources/test.rst:222
+``gmodules`` need to recompile external headers when they encounter
+test-specific flags (including defines) which can be very expensive.
+

Does this mean only use the flags you really need, or to put those flags 
somewhere other than the makefile? (I guess the former since they'd have to be 
in the makefile for your test to work anyway)

Would be good to make it clear so the reader doesn't think that there's some 
other place to put compiler flags, which isn't specified.


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

https://reviews.llvm.org/D101153

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