[Lldb-commits] [PATCH] D100521: [lldb] Fix up code addresses in RegisterContextUnwind

2021-04-15 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere updated this revision to Diff 337991.
JDevlieghere added a comment.

Save a line or two


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

https://reviews.llvm.org/D100521

Files:
  lldb/include/lldb/Target/ABI.h
  lldb/source/Plugins/ABI/AArch64/ABIAArch64.cpp
  lldb/source/Plugins/ABI/AArch64/ABIAArch64.h
  lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp
  lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.h
  lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp
  lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.h
  lldb/source/Target/RegisterContextUnwind.cpp

Index: lldb/source/Target/RegisterContextUnwind.cpp
===
--- lldb/source/Target/RegisterContextUnwind.cpp
+++ lldb/source/Target/RegisterContextUnwind.cpp
@@ -383,7 +383,7 @@
   // symbol/function information - just stick in some reasonable defaults and
   // hope we can unwind past this frame.  If we're above a trap handler,
   // we may be at a bogus address because we jumped through a bogus function
-  // pointer and trapped, so don't force the arch default unwind plan in that 
+  // pointer and trapped, so don't force the arch default unwind plan in that
   // case.
   ModuleSP pc_module_sp(m_current_pc.GetModule());
   if ((!m_current_pc.IsValid() || !pc_module_sp) &&
@@ -1277,7 +1277,7 @@
 // arch default unwind plan is used as the Fast Unwind Plan, we
 // need to recognize this & switch over to the Full Unwind Plan
 // to see what unwind rule that (more knoweldgeable, probably)
-// UnwindPlan has.  If the full UnwindPlan says the register 
+// UnwindPlan has.  If the full UnwindPlan says the register
 // location is Undefined, then it really is.
 if (active_row->GetRegisterInfo(regnum.GetAsKind(unwindplan_registerkind),
 unwindplan_regloc) &&
@@ -1325,14 +1325,14 @@
   m_full_unwind_plan_sp->GetReturnAddressRegister() !=
   LLDB_INVALID_REGNUM) {
 // If this is a trap handler frame, we should have access to
-// the complete register context when the interrupt/async 
+// the complete register context when the interrupt/async
 // signal was received, we should fetch the actual saved $pc
 // value instead of the Return Address register.
 // If $pc is not available, fall back to the RA reg.
 UnwindPlan::Row::RegisterLocation scratch;
 if (m_frame_type == eTrapHandlerFrame &&
-active_row->GetRegisterInfo 
-  (pc_regnum.GetAsKind (unwindplan_registerkind), scratch)) {
+active_row->GetRegisterInfo(
+pc_regnum.GetAsKind(unwindplan_registerkind), scratch)) {
   UnwindLogMsg("Providing pc register instead of rewriting to "
"RA reg because this is a trap handler and there is "
"a location for the saved pc register value.");
@@ -1730,6 +1730,12 @@
   RegisterValue reg_value;
   if (ReadRegisterValueFromRegisterLocation(regloc, reg_info, reg_value)) {
 old_caller_pc_value = reg_value.GetAsUInt64();
+ProcessSP process_sp(m_thread.GetProcess());
+if (process_sp) {
+  ABI *abi = process_sp->GetABI().get();
+  if (abi)
+old_caller_pc_value = abi->FixCodeAddress(old_caller_pc_value);
+}
   }
 }
   }
@@ -1785,6 +1791,12 @@
 if (ReadRegisterValueFromRegisterLocation(regloc, reg_info,
   reg_value)) {
   new_caller_pc_value = reg_value.GetAsUInt64();
+  ProcessSP process_sp(m_thread.GetProcess());
+  if (process_sp) {
+ABI *abi = process_sp->GetABI().get();
+if (abi)
+  new_caller_pc_value = abi->FixCodeAddress(new_caller_pc_value);
+  }
 }
   }
 }
@@ -2121,6 +2133,14 @@
   }
   if (ReadRegisterValueFromRegisterLocation(regloc, reg_info, reg_value)) {
 value = reg_value.GetAsUInt64();
+if (pc_register) {
+  ProcessSP process_sp(m_thread.GetProcess());
+  if (process_sp) {
+ABI *abi = process_sp->GetABI().get();
+if (abi)
+  value = abi->FixCodeAddress(value);
+  }
+}
 return true;
   }
   return false;
@@ -2162,7 +2182,19 @@
   lldb_regnum, regloc, m_frame_number - 1, is_pc_regnum))
 return false;
 
-  return ReadRegisterValueFromRegisterLocation(regloc, reg_info, value);
+  bool result = ReadRegisterValueFromRegisterLocation(regloc, reg_info, value);
+  if (result) {
+if (is_pc_regnum && value.GetType() == RegisterValue::eTypeUInt64) {
+  ProcessSP process_sp(m_thread.GetProcess());
+  addr_t reg_value = value.GetAsUInt64(LLDB_INVALID_ADDRESS);
+  if (process_sp && reg_value != LLDB_INVALID_ADDRESS) {
+ABI *abi = process_sp->GetABI().get();
+if (abi)
+  value = abi->FixCodeAddress(reg_value);
+  }
+

[Lldb-commits] [PATCH] D100521: [lldb] Fix up code addresses in RegisterContextUnwind

2021-04-15 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere added a comment.

In D100521#2693669 , @jasonmolenda 
wrote:

> This looks good.  Jonas, what do you think about having FixDataAddress() 
> methods in the ABI's.  We're going to be quickly sprinkling FixCodeAddress 
> calls throughout lldb at places where Linux/Darwin ABI need them, and it'd be 
> nice if we have the FixDataAddress method available (even if they're 
> identical right now) so someone doesn't need to audit all the calls in the 
> future and adjust them as appropriate.
>
> Or we can go with FixCodeAddress and leave this for someone to do when it 
> actually matters -- some people will probably pick the wrong call as we add 
> them, so it's not going to work bug-free when such a system exists.  I'm 
> honestly fine with just landing this and leaving that work for when it would 
> actually be exercised.
>
> (the two cases I can think of, where you'd call FixDataAddress, on Darwin 
> might be where we get the vtable pointer, and when a ptrauth qualifier has 
> been added to a type by the user in their program.)

Yep, that all makes sense. I added the method and implemented both in terms of 
`FixAddress` (which takes a pc and a mask) allowing me to avoid the some of the 
code duplication.


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

https://reviews.llvm.org/D100521

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


[Lldb-commits] [PATCH] D100521: [lldb] Fix up code addresses in RegisterContextUnwind

2021-04-15 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere updated this revision to Diff 337989.
JDevlieghere added a comment.

Implement both `FixCodeAddress` and `FixDataAddress`.


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

https://reviews.llvm.org/D100521

Files:
  lldb/include/lldb/Target/ABI.h
  lldb/source/Plugins/ABI/AArch64/ABIAArch64.cpp
  lldb/source/Plugins/ABI/AArch64/ABIAArch64.h
  lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp
  lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.h
  lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp
  lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.h
  lldb/source/Target/RegisterContextUnwind.cpp

Index: lldb/source/Target/RegisterContextUnwind.cpp
===
--- lldb/source/Target/RegisterContextUnwind.cpp
+++ lldb/source/Target/RegisterContextUnwind.cpp
@@ -383,7 +383,7 @@
   // symbol/function information - just stick in some reasonable defaults and
   // hope we can unwind past this frame.  If we're above a trap handler,
   // we may be at a bogus address because we jumped through a bogus function
-  // pointer and trapped, so don't force the arch default unwind plan in that 
+  // pointer and trapped, so don't force the arch default unwind plan in that
   // case.
   ModuleSP pc_module_sp(m_current_pc.GetModule());
   if ((!m_current_pc.IsValid() || !pc_module_sp) &&
@@ -1277,7 +1277,7 @@
 // arch default unwind plan is used as the Fast Unwind Plan, we
 // need to recognize this & switch over to the Full Unwind Plan
 // to see what unwind rule that (more knoweldgeable, probably)
-// UnwindPlan has.  If the full UnwindPlan says the register 
+// UnwindPlan has.  If the full UnwindPlan says the register
 // location is Undefined, then it really is.
 if (active_row->GetRegisterInfo(regnum.GetAsKind(unwindplan_registerkind),
 unwindplan_regloc) &&
@@ -1325,14 +1325,14 @@
   m_full_unwind_plan_sp->GetReturnAddressRegister() !=
   LLDB_INVALID_REGNUM) {
 // If this is a trap handler frame, we should have access to
-// the complete register context when the interrupt/async 
+// the complete register context when the interrupt/async
 // signal was received, we should fetch the actual saved $pc
 // value instead of the Return Address register.
 // If $pc is not available, fall back to the RA reg.
 UnwindPlan::Row::RegisterLocation scratch;
 if (m_frame_type == eTrapHandlerFrame &&
-active_row->GetRegisterInfo 
-  (pc_regnum.GetAsKind (unwindplan_registerkind), scratch)) {
+active_row->GetRegisterInfo(
+pc_regnum.GetAsKind(unwindplan_registerkind), scratch)) {
   UnwindLogMsg("Providing pc register instead of rewriting to "
"RA reg because this is a trap handler and there is "
"a location for the saved pc register value.");
@@ -1730,6 +1730,12 @@
   RegisterValue reg_value;
   if (ReadRegisterValueFromRegisterLocation(regloc, reg_info, reg_value)) {
 old_caller_pc_value = reg_value.GetAsUInt64();
+ProcessSP process_sp(m_thread.GetProcess());
+if (process_sp) {
+  ABI *abi = process_sp->GetABI().get();
+  if (abi)
+old_caller_pc_value = abi->FixCodeAddress(old_caller_pc_value);
+}
   }
 }
   }
@@ -1785,6 +1791,12 @@
 if (ReadRegisterValueFromRegisterLocation(regloc, reg_info,
   reg_value)) {
   new_caller_pc_value = reg_value.GetAsUInt64();
+  ProcessSP process_sp(m_thread.GetProcess());
+  if (process_sp) {
+ABI *abi = process_sp->GetABI().get();
+if (abi)
+  new_caller_pc_value = abi->FixCodeAddress(new_caller_pc_value);
+  }
 }
   }
 }
@@ -2121,6 +2133,14 @@
   }
   if (ReadRegisterValueFromRegisterLocation(regloc, reg_info, reg_value)) {
 value = reg_value.GetAsUInt64();
+if (pc_register) {
+  ProcessSP process_sp(m_thread.GetProcess());
+  if (process_sp) {
+ABI *abi = process_sp->GetABI().get();
+if (abi)
+  value = abi->FixCodeAddress(value);
+  }
+}
 return true;
   }
   return false;
@@ -2162,7 +2182,19 @@
   lldb_regnum, regloc, m_frame_number - 1, is_pc_regnum))
 return false;
 
-  return ReadRegisterValueFromRegisterLocation(regloc, reg_info, value);
+  bool result = ReadRegisterValueFromRegisterLocation(regloc, reg_info, value);
+  if (result) {
+if (is_pc_regnum && value.GetType() == RegisterValue::eTypeUInt64) {
+  ProcessSP process_sp(m_thread.GetProcess());
+  addr_t reg_value = value.GetAsUInt64(LLDB_INVALID_ADDRESS);
+  if (process_sp && reg_value != LLDB_INVALID_ADDRESS) {
+ABI *abi = process_sp->GetABI().get();
+if (abi)
+  value = 

[Lldb-commits] [PATCH] D100521: [lldb] Fix up code addresses in RegisterContextUnwind

2021-04-15 Thread Jason Molenda via Phabricator via lldb-commits
jasonmolenda accepted this revision.
jasonmolenda added a comment.
This revision is now accepted and ready to land.

This looks good.  Jonas, what do you think about having FixDataAddress() 
methods in the ABI's.  We're going to be quickly sprinkling FixCodeAddress 
calls throughout lldb at places where Linux/Darwin ABI need them, and it'd be 
nice if we have the FixDataAddress method available (even if they're identical 
right now) so someone doesn't need to audit all the calls in the future and 
adjust them as appropriate.

Or we can go with FixCodeAddress and leave this for someone to do when it 
actually matters -- some people will probably pick the wrong call as we add 
them, so it's not going to work bug-free when such a system exists.  I'm 
honestly fine with just landing this and leaving that work for when it would 
actually be exercised.

(the two cases I can think of, where you'd call FixDataAddress, on Darwin might 
be where we get the vtable pointer, and when a ptrauth qualifier has been added 
to a type by the user in their program.)


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

https://reviews.llvm.org/D100521

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


[Lldb-commits] [PATCH] D100515: [lldb] Add GetCodeAddressMask and GetDataAddressMask to Process

2021-04-15 Thread Jason Molenda via Phabricator via lldb-commits
jasonmolenda accepted this revision.
jasonmolenda added a comment.

I think we've heard from everyone involved with this & it's good to land.  
Thanks for bringing this home.


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

https://reviews.llvm.org/D100515

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


[Lldb-commits] [lldb] 9d4415d - Don't refer to allocation map entry after deallocating it

2021-04-15 Thread Jason Molenda via lldb-commits

Author: Jason Molenda
Date: 2021-04-15T20:16:38-07:00
New Revision: 9d4415d01d234efb5dd46bdcbb280c03e12a8101

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

LOG: Don't refer to allocation map entry after deallocating it

debugserver's MachTask::DeallocateMemory when removing an
allocate entry from our map (in resposne to an '_m' packet),
copy the size from the entry before removing it from the
map and then using the iterator to fix an ASAN error on
the bots when running TestGdbRemoteMemoryAllocation.py

rdar://76595998

Added: 


Modified: 
lldb/tools/debugserver/source/MacOSX/MachTask.mm

Removed: 




diff  --git a/lldb/tools/debugserver/source/MacOSX/MachTask.mm 
b/lldb/tools/debugserver/source/MacOSX/MachTask.mm
index 7c46ec8d5ecd9..767dc59b67808 100644
--- a/lldb/tools/debugserver/source/MacOSX/MachTask.mm
+++ b/lldb/tools/debugserver/source/MacOSX/MachTask.mm
@@ -984,15 +984,15 @@ static void 
get_threads_profile_data(DNBProfileDataScanType scanType,
   allocation_collection::iterator pos, end = m_allocations.end();
   for (pos = m_allocations.begin(); pos != end; pos++) {
 if ((*pos).first == addr) {
+  size_t size = (*pos).second;
   m_allocations.erase(pos);
 #define ALWAYS_ZOMBIE_ALLOCATIONS 0
   if (ALWAYS_ZOMBIE_ALLOCATIONS ||
   getenv("DEBUGSERVER_ZOMBIE_ALLOCATIONS")) {
-::mach_vm_protect(task, (*pos).first, (*pos).second, 0, VM_PROT_NONE);
+::mach_vm_protect(task, addr, size, 0, VM_PROT_NONE);
 return true;
   } else
-return ::mach_vm_deallocate(task, (*pos).first, (*pos).second) ==
-   KERN_SUCCESS;
+return ::mach_vm_deallocate(task, addr, size) == KERN_SUCCESS;
 }
   }
   return false;



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


[Lldb-commits] [PATCH] D97786: LLDB: Use path relative to binary, not relative to debugger CWD, for finding .dwo files.

2021-04-15 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment.

In D97786#2693381 , @cmtice wrote:

> I had to revert this change because the test case broke the windows builder.  
> What's the right way to update/mark the test case so that it is only run on 
> appropriate architectures & operating systems?

You add the python skipIfWindows "decorator" before each test case definition.  
Just grep around in the test/API directory and you'll see plenty of instances 
of its use.

This does seem like it should work on Windows, however.  Maybe some path 
handling problem?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97786

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


[Lldb-commits] [PATCH] D97786: LLDB: Use path relative to binary, not relative to debugger CWD, for finding .dwo files.

2021-04-15 Thread Caroline Tice via Phabricator via lldb-commits
cmtice added a comment.

I had to revert this change because the test case broke the windows builder.  
What's the right way to update/mark the test case so that it is only run on 
appropriate architectures & operating systems?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97786

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


[Lldb-commits] [lldb] 042668d - Revert "[LLDB] Use path relative to binary for finding .dwo files."

2021-04-15 Thread Caroline Tice via lldb-commits

Author: Caroline Tice
Date: 2021-04-15T17:17:44-07:00
New Revision: 042668d092bb585e526023027faee8d28f968a76

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

LOG: Revert "[LLDB] Use path relative to binary for finding .dwo files."

This reverts commit b241f3cb292d0ba1ad5a33b3bbd4a8a3a9c909dc.

Test case is breaking windows builder.

Added: 


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

Removed: 
lldb/test/Shell/SymbolFile/DWARF/dwo-relative-path.s



diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index da170383405f..3a04f429c7c7 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -1651,13 +1651,6 @@ SymbolFileDWARF::GetDwoSymbolFileForCompileUnit(
   return nullptr;
 
 dwo_file.SetFile(comp_dir, FileSpec::Style::native);
-if (dwo_file.IsRelative()) {
-  // if DW_AT_comp_dir is relative, it should be relative to the location
-  // of the executable, not to the location from which the debugger was
-  // launched.
-  dwo_file.PrependPathComponent(
-  m_objfile_sp->GetFileSpec().GetDirectory().GetStringRef());
-}
 FileSystem::Instance().Resolve(dwo_file);
 dwo_file.AppendPathComponent(dwo_name);
   }

diff  --git a/lldb/test/Shell/SymbolFile/DWARF/dwo-relative-path.s 
b/lldb/test/Shell/SymbolFile/DWARF/dwo-relative-path.s
deleted file mode 100644
index cc05434261e4..
--- a/lldb/test/Shell/SymbolFile/DWARF/dwo-relative-path.s
+++ /dev/null
@@ -1,417 +0,0 @@
-# Test to verify LLDB searches for dwos with relative paths relative to the
-# binary location, not relative to LLDB's launch location.
-
-# RUN: llvm-mc --filetype=obj --triple x86_64-pc-linux %s -o %t.o
-# RUN: llvm-objcopy --split-dwo=%T/dwo-relative-path.dwo %t.o
-
-# RUN: cd ../..
-
-# RUN: %lldb %t.o -o "target var x" -b 2>&1 | FileCheck %s
-
-# CHECK: x = 0
-
-   .text
-   .file   "dwo-relative-path.cpp"
-   .file   0 "." "dwo-relative-path.cpp" md5 
0xadc61d242247514c5d402d62db34b825
-   .globl  main# -- Begin function main
-   .p2align4, 0x90
-   .type   main,@function
-main:   # @main
-.Lfunc_begin0:
-   .loc0 14 0  # dwo-relative-path.cpp:14:0
-   .cfi_startproc
-# %bb.0:# %entry
-   pushq   %rbp
-   .cfi_def_cfa_offset 16
-   .cfi_offset %rbp, -16
-   movq%rsp, %rbp
-   .cfi_def_cfa_register %rbp
-   movl$0, -4(%rbp)
-   movl%edi, -8(%rbp)
-   movq%rsi, -16(%rbp)
-.Ltmp0:
-   .loc0 15 8 prologue_end # dwo-relative-path.cpp:15:8
-   movw.L__const.main.y, %ax
-   movw%ax, -19(%rbp)
-   movb.L__const.main.y+2, %al
-   movb%al, -17(%rbp)
-   .loc0 17 14 # dwo-relative-path.cpp:17:14
-   movsbl  -19(%rbp), %eax
-   .loc0 17 27 is_stmt 0   # dwo-relative-path.cpp:17:27
-   movsbl  -18(%rbp), %ecx
-   .loc0 17 19 # dwo-relative-path.cpp:17:19
-   addl%ecx, %eax
-   .loc0 17 5  # dwo-relative-path.cpp:17:5
-   addlx, %eax
-   movl%eax, x
-   .loc0 19 3 is_stmt 1# dwo-relative-path.cpp:19:3
-   xorl%eax, %eax
-   popq%rbp
-   .cfi_def_cfa %rsp, 8
-   retq
-.Ltmp1:
-.Lfunc_end0:
-   .size   main, .Lfunc_end0-main
-   .cfi_endproc
-# -- End function
-   .type   x,@object   # @x
-   .data
-   .globl  x
-   .p2align2
-x:
-   .long   10  # 0xa
-   .size   x, 4
-
-   .type   .L__const.main.y,@object# @__const.main.y
-   .section.rodata,"a",@progbits
-.L__const.main.y:
-   .ascii  "abc"
-   .size   .L__const.main.y, 3
-
-   .section.debug_abbrev,"",@progbits
-   .byte   1   # Abbreviation Code
-   .byte   74  # DW_TAG_skeleton_unit
-   .byte   0   # DW_CHILDREN_no
-   .byte   16  # DW_AT_stmt_list
-   .byte   23  # DW_FORM_sec_offset
-   .byte   114 # DW_AT_str_offsets_base
-   .byte   23  # DW_FORM_sec_offset
-   .byte   27  # DW_AT_comp_dir
-   .byte   37  # DW_FORM_strx1
-   

[Lldb-commits] [PATCH] D97786: LLDB: Use path relative to binary, not relative to debugger CWD, for finding .dwo files.

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

This broke the windows LLDB bot. Please fix it soon or revert.

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97786

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


[Lldb-commits] [lldb] 8639e2a - [lldb] Raise a CrashLogParseException when failing to parse JSON crashlog

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

Author: Jonas Devlieghere
Date: 2021-04-15T15:28:23-07:00
New Revision: 8639e2aaaffe1c58fbab238b25d49620ca86a76d

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

LOG: [lldb] Raise a CrashLogParseException when failing to parse JSON crashlog

Throw an exception with an actually helpful message when we fail to
parse a JSON crashlog.

Added: 


Modified: 
lldb/examples/python/crashlog.py

Removed: 




diff  --git a/lldb/examples/python/crashlog.py 
b/lldb/examples/python/crashlog.py
index 88692271bdcec..afae31f4a3620 100755
--- a/lldb/examples/python/crashlog.py
+++ b/lldb/examples/python/crashlog.py
@@ -383,6 +383,10 @@ class CrashLogFormatException(Exception):
 pass
 
 
+class CrashLogParseException(Exception):
+   pass
+
+
 class CrashLogParser:
 def parse(self, debugger, path, verbose):
 try:
@@ -409,12 +413,16 @@ def parse(self):
 except ValueError:
 raise CrashLogFormatException()
 
-self.parse_process_info(self.data)
-self.parse_images(self.data['usedImages'])
-self.parse_threads(self.data['threads'])
-
-thread = self.crashlog.threads[self.crashlog.crashed_thread_idx]
-thread.reason = self.parse_crash_reason(self.data['exception'])
+try:
+self.parse_process_info(self.data)
+self.parse_images(self.data['usedImages'])
+self.parse_threads(self.data['threads'])
+thread = self.crashlog.threads[self.crashlog.crashed_thread_idx]
+thread.reason = self.parse_crash_reason(self.data['exception'])
+except (KeyError, ValueError, TypeError) as e:
+   raise CrashLogParseException(
+   'Failed to parse JSON crashlog: {}: {}'.format(
+   type(e).__name__, e))
 
 return self.crashlog
 



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


[Lldb-commits] [PATCH] D97786: LLDB: Use path relative to binary, not relative to debugger CWD, for finding .dwo files.

2021-04-15 Thread Caroline Tice 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 rGb241f3cb292d: [LLDB] Use path relative to binary for finding 
.dwo files. (authored by cmtice).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97786

Files:
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/test/Shell/SymbolFile/DWARF/dwo-relative-path.s

Index: lldb/test/Shell/SymbolFile/DWARF/dwo-relative-path.s
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/DWARF/dwo-relative-path.s
@@ -0,0 +1,417 @@
+# Test to verify LLDB searches for dwos with relative paths relative to the
+# binary location, not relative to LLDB's launch location.
+
+# RUN: llvm-mc --filetype=obj --triple x86_64-pc-linux %s -o %t.o
+# RUN: llvm-objcopy --split-dwo=%T/dwo-relative-path.dwo %t.o
+
+# RUN: cd ../..
+
+# RUN: %lldb %t.o -o "target var x" -b 2>&1 | FileCheck %s
+
+# CHECK: x = 0
+
+	.text
+	.file	"dwo-relative-path.cpp"
+	.file	0 "." "dwo-relative-path.cpp" md5 0xadc61d242247514c5d402d62db34b825
+	.globl	main# -- Begin function main
+	.p2align	4, 0x90
+	.type	main,@function
+main:   # @main
+.Lfunc_begin0:
+	.loc	0 14 0  # dwo-relative-path.cpp:14:0
+	.cfi_startproc
+# %bb.0:# %entry
+	pushq	%rbp
+	.cfi_def_cfa_offset 16
+	.cfi_offset %rbp, -16
+	movq	%rsp, %rbp
+	.cfi_def_cfa_register %rbp
+	movl	$0, -4(%rbp)
+	movl	%edi, -8(%rbp)
+	movq	%rsi, -16(%rbp)
+.Ltmp0:
+	.loc	0 15 8 prologue_end # dwo-relative-path.cpp:15:8
+	movw	.L__const.main.y, %ax
+	movw	%ax, -19(%rbp)
+	movb	.L__const.main.y+2, %al
+	movb	%al, -17(%rbp)
+	.loc	0 17 14 # dwo-relative-path.cpp:17:14
+	movsbl	-19(%rbp), %eax
+	.loc	0 17 27 is_stmt 0   # dwo-relative-path.cpp:17:27
+	movsbl	-18(%rbp), %ecx
+	.loc	0 17 19 # dwo-relative-path.cpp:17:19
+	addl	%ecx, %eax
+	.loc	0 17 5  # dwo-relative-path.cpp:17:5
+	addl	x, %eax
+	movl	%eax, x
+	.loc	0 19 3 is_stmt 1# dwo-relative-path.cpp:19:3
+	xorl	%eax, %eax
+	popq	%rbp
+	.cfi_def_cfa %rsp, 8
+	retq
+.Ltmp1:
+.Lfunc_end0:
+	.size	main, .Lfunc_end0-main
+	.cfi_endproc
+# -- End function
+	.type	x,@object   # @x
+	.data
+	.globl	x
+	.p2align	2
+x:
+	.long	10  # 0xa
+	.size	x, 4
+
+	.type	.L__const.main.y,@object# @__const.main.y
+	.section	.rodata,"a",@progbits
+.L__const.main.y:
+	.ascii	"abc"
+	.size	.L__const.main.y, 3
+
+	.section	.debug_abbrev,"",@progbits
+	.byte	1   # Abbreviation Code
+	.byte	74  # DW_TAG_skeleton_unit
+	.byte	0   # DW_CHILDREN_no
+	.byte	16  # DW_AT_stmt_list
+	.byte	23  # DW_FORM_sec_offset
+	.byte	114 # DW_AT_str_offsets_base
+	.byte	23  # DW_FORM_sec_offset
+	.byte	27  # DW_AT_comp_dir
+	.byte	37  # DW_FORM_strx1
+	.ascii	"\264B" # DW_AT_GNU_pubnames
+	.byte	25  # DW_FORM_flag_present
+	.byte	118 # DW_AT_dwo_name
+	.byte	37  # DW_FORM_strx1
+	.byte	17  # DW_AT_low_pc
+	.byte	27  # DW_FORM_addrx
+	.byte	18  # DW_AT_high_pc
+	.byte	6   # DW_FORM_data4
+	.byte	115 # DW_AT_addr_base
+	.byte	23  # DW_FORM_sec_offset
+	.byte	0   # EOM(1)
+	.byte	0   # EOM(2)
+	.byte	0   # EOM(3)
+	.section	.debug_info,"",@progbits
+.Lcu_begin0:
+	.long	.Ldebug_info_end0-.Ldebug_info_start0 # Length of Unit
+.Ldebug_info_start0:
+	.short	5   # DWARF version number
+	.byte	4   # DWARF Unit Type
+	.byte	8   # Address Size (in bytes)
+	.long	.debug_abbrev   # Offset Into Abbrev. Section
+	.quad	3752513468363206953
+	.byte	1   # Abbrev [1] 0x14:0x14 DW_TAG_skeleton_unit
+	.long	.Lline_table_start0 # DW_AT_stmt_list
+	.long	.Lstr_offsets_base0 # DW_AT_str_offsets_base
+	.byte	0   # DW_AT_comp_dir
+# DW_AT_GNU_pubnames
+	.byte	1   # DW_AT_dwo_name
+	.byte	1   # DW_AT_low_pc
+	.long	.Lfunc_end0-.Lfunc_begin0 

[Lldb-commits] [lldb] b241f3c - [LLDB] Use path relative to binary for finding .dwo files.

2021-04-15 Thread Caroline Tice via lldb-commits

Author: Caroline Tice
Date: 2021-04-15T14:43:47-07:00
New Revision: b241f3cb292d0ba1ad5a33b3bbd4a8a3a9c909dc

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

LOG: [LLDB] Use path relative to binary for finding .dwo files.

DWARF allows .dwo file paths to be relative rather than absolute. When
they are relative, DWARF uses DW_AT_comp_dir to find the .dwo
file. DW_AT_comp_dir can also be relative, making the entire search
patch for the .dwo file relative. In this case, LLDB currently
searches relative to its current working directory, i.e. the directory
from which the debugger was launched. This is not right, as the
compiler, which generated the relative paths, can have no idea where
the debugger will be launched. The correct thing is to search relative
to the location of the executable binary. That is what this patch
does.

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

Added: 
lldb/test/Shell/SymbolFile/DWARF/dwo-relative-path.s

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

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index 3a04f429c7c75..da170383405fb 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -1651,6 +1651,13 @@ SymbolFileDWARF::GetDwoSymbolFileForCompileUnit(
   return nullptr;
 
 dwo_file.SetFile(comp_dir, FileSpec::Style::native);
+if (dwo_file.IsRelative()) {
+  // if DW_AT_comp_dir is relative, it should be relative to the location
+  // of the executable, not to the location from which the debugger was
+  // launched.
+  dwo_file.PrependPathComponent(
+  m_objfile_sp->GetFileSpec().GetDirectory().GetStringRef());
+}
 FileSystem::Instance().Resolve(dwo_file);
 dwo_file.AppendPathComponent(dwo_name);
   }

diff  --git a/lldb/test/Shell/SymbolFile/DWARF/dwo-relative-path.s 
b/lldb/test/Shell/SymbolFile/DWARF/dwo-relative-path.s
new file mode 100644
index 0..cc05434261e4e
--- /dev/null
+++ b/lldb/test/Shell/SymbolFile/DWARF/dwo-relative-path.s
@@ -0,0 +1,417 @@
+# Test to verify LLDB searches for dwos with relative paths relative to the
+# binary location, not relative to LLDB's launch location.
+
+# RUN: llvm-mc --filetype=obj --triple x86_64-pc-linux %s -o %t.o
+# RUN: llvm-objcopy --split-dwo=%T/dwo-relative-path.dwo %t.o
+
+# RUN: cd ../..
+
+# RUN: %lldb %t.o -o "target var x" -b 2>&1 | FileCheck %s
+
+# CHECK: x = 0
+
+   .text
+   .file   "dwo-relative-path.cpp"
+   .file   0 "." "dwo-relative-path.cpp" md5 
0xadc61d242247514c5d402d62db34b825
+   .globl  main# -- Begin function main
+   .p2align4, 0x90
+   .type   main,@function
+main:   # @main
+.Lfunc_begin0:
+   .loc0 14 0  # dwo-relative-path.cpp:14:0
+   .cfi_startproc
+# %bb.0:# %entry
+   pushq   %rbp
+   .cfi_def_cfa_offset 16
+   .cfi_offset %rbp, -16
+   movq%rsp, %rbp
+   .cfi_def_cfa_register %rbp
+   movl$0, -4(%rbp)
+   movl%edi, -8(%rbp)
+   movq%rsi, -16(%rbp)
+.Ltmp0:
+   .loc0 15 8 prologue_end # dwo-relative-path.cpp:15:8
+   movw.L__const.main.y, %ax
+   movw%ax, -19(%rbp)
+   movb.L__const.main.y+2, %al
+   movb%al, -17(%rbp)
+   .loc0 17 14 # dwo-relative-path.cpp:17:14
+   movsbl  -19(%rbp), %eax
+   .loc0 17 27 is_stmt 0   # dwo-relative-path.cpp:17:27
+   movsbl  -18(%rbp), %ecx
+   .loc0 17 19 # dwo-relative-path.cpp:17:19
+   addl%ecx, %eax
+   .loc0 17 5  # dwo-relative-path.cpp:17:5
+   addlx, %eax
+   movl%eax, x
+   .loc0 19 3 is_stmt 1# dwo-relative-path.cpp:19:3
+   xorl%eax, %eax
+   popq%rbp
+   .cfi_def_cfa %rsp, 8
+   retq
+.Ltmp1:
+.Lfunc_end0:
+   .size   main, .Lfunc_end0-main
+   .cfi_endproc
+# -- End function
+   .type   x,@object   # @x
+   .data
+   .globl  x
+   .p2align2
+x:
+   .long   10  # 0xa
+   .size   x, 4
+
+   .type   .L__const.main.y,@object# @__const.main.y
+   .section.rodata,"a",@progbits
+.L__const.main.y:
+   .ascii  "abc"
+   .size   .L__const.main.y, 3
+
+   .section.debug_abbrev,"",@progbits
+   .byte   1   # Abbreviation 

[Lldb-commits] [PATCH] D100191: [lldb] [llgs] Support owning and detaching extra processes

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

In D100191#2689489 , @labath wrote:

> We should also start thinking about tests. I suppose the smallest piece of 
> functionality that could be usefully tested (with a lldb-server test) is 
> debugging a process that forks, stopping after the fork, and detaching from 
> the child. Shall we try making that work first?

How about using a `MockProcess` to unittest server's behavior wrt getting 
`NewSubprocess()` and `Detach()` calls?


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

https://reviews.llvm.org/D100191

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


[Lldb-commits] [PATCH] D100515: [lldb] Add GetCodeAddressMask and GetDataAddressMask to Process

2021-04-15 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere added inline comments.



Comment at: lldb/source/Target/Process.cpp:5569-5570
+
+  if (m_code_address_mask == 0)
+return -1; // All bits are used for addressing.
+

jasonmolenda wrote:
> pcc wrote:
> > Is this part correct? (Same below.) In D100521 you have
> > ```
> >   if (pc & pac_sign_extension)
> > return pc | mask;
> >   return pc & (~mask);
> > ```
> > So it looks like this would cause the pc to be set to 0 (or -1)?
> I get confused so I like to do this by hand quickly to make sure I understand.
> 
> given mask of 111 and addr of xxx1011 where 'x' are PAC bits,
> 
> b55 == 1: m | a == 011
> b55 == 0: ~m & a == 0001011
> 
> given mask of 111, low address 0001011 and high address 011,
> 
> b55 == 1: m & ha == ha
> b55 == 0: ~m | la == la
> 
> am I  not thinking of something that could unify these?  I can confuse myself 
> so easily with these things.
> 
> We could also detect a mask of -1 and just return the original address in 
> FixCodeAddress/FixDataAddress, right.  That would be very simple.
I've added checking for -1 in D100521


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

https://reviews.llvm.org/D100515

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


[Lldb-commits] [PATCH] D100521: [lldb] Fix up code addresses in RegisterContextUnwind

2021-04-15 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere updated this revision to Diff 337892.
JDevlieghere added a comment.

Check for mask == -1


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

https://reviews.llvm.org/D100521

Files:
  lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp
  lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.h
  lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp
  lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.h
  lldb/source/Target/RegisterContextUnwind.cpp

Index: lldb/source/Target/RegisterContextUnwind.cpp
===
--- lldb/source/Target/RegisterContextUnwind.cpp
+++ lldb/source/Target/RegisterContextUnwind.cpp
@@ -383,7 +383,7 @@
   // symbol/function information - just stick in some reasonable defaults and
   // hope we can unwind past this frame.  If we're above a trap handler,
   // we may be at a bogus address because we jumped through a bogus function
-  // pointer and trapped, so don't force the arch default unwind plan in that 
+  // pointer and trapped, so don't force the arch default unwind plan in that
   // case.
   ModuleSP pc_module_sp(m_current_pc.GetModule());
   if ((!m_current_pc.IsValid() || !pc_module_sp) &&
@@ -1277,7 +1277,7 @@
 // arch default unwind plan is used as the Fast Unwind Plan, we
 // need to recognize this & switch over to the Full Unwind Plan
 // to see what unwind rule that (more knoweldgeable, probably)
-// UnwindPlan has.  If the full UnwindPlan says the register 
+// UnwindPlan has.  If the full UnwindPlan says the register
 // location is Undefined, then it really is.
 if (active_row->GetRegisterInfo(regnum.GetAsKind(unwindplan_registerkind),
 unwindplan_regloc) &&
@@ -1325,14 +1325,14 @@
   m_full_unwind_plan_sp->GetReturnAddressRegister() !=
   LLDB_INVALID_REGNUM) {
 // If this is a trap handler frame, we should have access to
-// the complete register context when the interrupt/async 
+// the complete register context when the interrupt/async
 // signal was received, we should fetch the actual saved $pc
 // value instead of the Return Address register.
 // If $pc is not available, fall back to the RA reg.
 UnwindPlan::Row::RegisterLocation scratch;
 if (m_frame_type == eTrapHandlerFrame &&
-active_row->GetRegisterInfo 
-  (pc_regnum.GetAsKind (unwindplan_registerkind), scratch)) {
+active_row->GetRegisterInfo(
+pc_regnum.GetAsKind(unwindplan_registerkind), scratch)) {
   UnwindLogMsg("Providing pc register instead of rewriting to "
"RA reg because this is a trap handler and there is "
"a location for the saved pc register value.");
@@ -1730,6 +1730,12 @@
   RegisterValue reg_value;
   if (ReadRegisterValueFromRegisterLocation(regloc, reg_info, reg_value)) {
 old_caller_pc_value = reg_value.GetAsUInt64();
+ProcessSP process_sp(m_thread.GetProcess());
+if (process_sp) {
+  ABI *abi = process_sp->GetABI().get();
+  if (abi)
+old_caller_pc_value = abi->FixCodeAddress(old_caller_pc_value);
+}
   }
 }
   }
@@ -1785,6 +1791,12 @@
 if (ReadRegisterValueFromRegisterLocation(regloc, reg_info,
   reg_value)) {
   new_caller_pc_value = reg_value.GetAsUInt64();
+  ProcessSP process_sp(m_thread.GetProcess());
+  if (process_sp) {
+ABI *abi = process_sp->GetABI().get();
+if (abi)
+  new_caller_pc_value = abi->FixCodeAddress(new_caller_pc_value);
+  }
 }
   }
 }
@@ -2121,6 +2133,14 @@
   }
   if (ReadRegisterValueFromRegisterLocation(regloc, reg_info, reg_value)) {
 value = reg_value.GetAsUInt64();
+if (pc_register) {
+  ProcessSP process_sp(m_thread.GetProcess());
+  if (process_sp) {
+ABI *abi = process_sp->GetABI().get();
+if (abi)
+  value = abi->FixCodeAddress(value);
+  }
+}
 return true;
   }
   return false;
@@ -2162,7 +2182,19 @@
   lldb_regnum, regloc, m_frame_number - 1, is_pc_regnum))
 return false;
 
-  return ReadRegisterValueFromRegisterLocation(regloc, reg_info, value);
+  bool result = ReadRegisterValueFromRegisterLocation(regloc, reg_info, value);
+  if (result) {
+if (is_pc_regnum && value.GetType() == RegisterValue::eTypeUInt64) {
+  ProcessSP process_sp(m_thread.GetProcess());
+  addr_t reg_value = value.GetAsUInt64(LLDB_INVALID_ADDRESS);
+  if (process_sp && reg_value != LLDB_INVALID_ADDRESS) {
+ABI *abi = process_sp->GetABI().get();
+if (abi)
+  value = abi->FixCodeAddress(reg_value);
+  }
+}
+  }
+  return result;
 }
 
 bool RegisterContextUnwind::WriteRegister(const RegisterInfo *reg_info,
Index: 

[Lldb-commits] [PATCH] D100418: [lldb] [MainLoop] Support multiple callbacks per signal

2021-04-15 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

Looks good, just add the test case.




Comment at: lldb/source/Host/common/MainLoop.cpp:413-415
+llvm::SmallVector callbacks_to_run;
+for (auto  : it->second.callbacks)
+  callbacks_to_run.push_back(x);

llvm::SmallVector callbacks_to_run(it->second.callbacks.begin(), 
it->second.callbacks.end());


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

https://reviews.llvm.org/D100418

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


[Lldb-commits] [PATCH] D100515: [lldb] Add GetCodeAddressMask and GetDataAddressMask to Process

2021-04-15 Thread Jason Molenda via Phabricator via lldb-commits
jasonmolenda added inline comments.



Comment at: lldb/source/Target/Process.cpp:5569-5570
+
+  if (m_code_address_mask == 0)
+return -1; // All bits are used for addressing.
+

pcc wrote:
> Is this part correct? (Same below.) In D100521 you have
> ```
>   if (pc & pac_sign_extension)
> return pc | mask;
>   return pc & (~mask);
> ```
> So it looks like this would cause the pc to be set to 0 (or -1)?
I get confused so I like to do this by hand quickly to make sure I understand.

given mask of 111 and addr of xxx1011 where 'x' are PAC bits,

b55 == 1: m | a == 011
b55 == 0: ~m & a == 0001011

given mask of 111, low address 0001011 and high address 011,

b55 == 1: m & ha == ha
b55 == 0: ~m | la == la

am I  not thinking of something that could unify these?  I can confuse myself 
so easily with these things.

We could also detect a mask of -1 and just return the original address in 
FixCodeAddress/FixDataAddress, right.  That would be very simple.


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

https://reviews.llvm.org/D100515

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


[Lldb-commits] [PATCH] D97786: LLDB: Use path relative to binary, not relative to debugger CWD, for finding .dwo files.

2021-04-15 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

Yes, thanks for bearing with me.


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

https://reviews.llvm.org/D97786

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


[Lldb-commits] [PATCH] D100384: [lldb/Plugins] Update ScriptedProcess Process Plugin

2021-04-15 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment.

A bunch of little comments.  I was also curious why you directly set the 
running private state, but use the async thread to generate the stopped events?




Comment at: lldb/source/Plugins/Process/CMakeLists.txt:15
 endif()
+if (LLDB_ENABLE_PYTHON)
+  add_subdirectory(scripted)

Why is this necessary?  The code in Plugins/Process/scripted should work for 
any script interpreter (and should handle the case when there isn't one.)



Comment at: lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp:78
+
+  if (!m_interpreter)
+return;

If making the scripted process fails because there's no script interpreter, it 
would be good to get that fact out to the user.  Maybe the ScriptedProcess 
constructor should take a Status object that it's caller can somehow bubble up?



Comment at: lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp:85
+
+  if (object_sp && object_sp->IsValid())
+m_script_object_sp = object_sp;

If these two conditions aren't true, do you actually want to continue?  Seems 
like you should error out here as well.



Comment at: lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp:170
+
+  if (m_async_thread.IsJoinable()) {
+m_async_thread.Join(nullptr);

Does it do any good to broadcast the ShouldExit bit if the Async thread isn't 
running?  If you're following the llvm style, you would do:

if (!m_async_thread.IsJoinable()) {
  // Do your logging
  return;
}

First, and then all the actual work of the function outside an if.  That would 
the odd look of sending an event that you know is going to a particular thread 
when the thread isn't running?



Comment at: lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp:261
+Status ScriptedProcess::WillLaunch(Module *module) {
+  if (m_interpreter)
+m_pid = GetInterface().GetProcessID();

I don't know if you actually need this, but it seems weird to be setting the 
PID in WillLaunch.  I don't think there's any real process plugin that would 
know the PID of the launched process in WillLaunch.  Maybe it just looks odd, 
but still is seems worthwhile to follow the behavior of "real" process plugins 
wherever you can.



Comment at: lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp:300
+
+  if (!m_interpreter)
+return Status("No interpreter.");

Maybe these should be asserts.  It really shouldn't be possible to get to 
DoResume for a ScriptedProcess with no interpreter and no script object.  Also 
messages in the generic ScriptProcess code shouldn't refer to Python directly.  
You don't know that that's the script interpreter language being used here.



Comment at: lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp:315
+
+  Status status;
+  if (resume) {

We call Status objects "error" pretty much everywhere.  If we end up changing 
to use "status" then we should do that wholesale.



Comment at: lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp:322
+if (status.Success()) {
+  SetPrivateState(eStateRunning);
+  m_async_broadcaster.BroadcastEvent(eBroadcastBitAsyncContinue);

This seems asymmetric, why do you use the async thread to generate the stopped 
event, but not the running event?



Comment at: lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp:327
+  } else {
+status.SetErrorString("thread is suspended");
+  }

Presumably the Interface knew why Resume failed if it did.  Don't erase that 
information in the error you return here.



Comment at: lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp:336
+bool ScriptedProcess::IsAlive() {
+  if (!m_interpreter)
+return false;

You checked both m_interpreter & m_script_object_sp above when checking for 
validity.  Do you need to check both here as well.



Comment at: lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp:342
+
+size_t ScriptedProcess::ReadMemory(lldb::addr_t addr, void *buf, size_t size,
+   Status ) {

The comment above Process::ReadMemory says processes plugins should not 
override the method.  Why do you need to do this here?  If there is a reason, 
it should be explained.



Comment at: lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp:388
+
+  if (!m_interpreter) {
+error.SetErrorString("No interpreter.");

You are inconsistent about what you check for "We didn't manage to make the 
scripted process at all".  Sometimes you just check for the interpreter and 
sometimes you check that and the m_script_object_sp...

Maybe you should put those checks into a ScriptedProcess method so you do it 
the same way everywhere.



Comment 

[Lldb-commits] [PATCH] D100515: [lldb] Add GetCodeAddressMask and GetDataAddressMask to Process

2021-04-15 Thread Justin Cohen via Phabricator via lldb-commits
justincohen accepted this revision.
justincohen added a comment.
This revision is now accepted and ready to land.

> On Darwin, we use the same number of bits for both code and data, but given 
> the way ptrace() behaves on Linux, I'm guessing this may not be the case 
> everywhere.  Should we store both masks, and add FixCodeAddress + 
> FixDataAddress methods in the ABI's, Justin?  What do you think?

This all LGTM!  I don't have a strong opinion on naming.  My understanding is 
both code and data will be necessary in case TBID0 is set on Linux.


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

https://reviews.llvm.org/D100515

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


[Lldb-commits] [PATCH] D100515: [lldb] Add GetCodeAddressMask and GetDataAddressMask to Process

2021-04-15 Thread Jason Molenda via Phabricator via lldb-commits
jasonmolenda added a comment.

In D100515#2690972 , @omjavaid wrote:

> 



> This looks very good and I have couple of highlights to make from 
> Linux/AArch64 perspective:
> qHostInfo is not sufficient for mask calculation on Linux because both 
> data/code ptr auth masks are read by thread register context (both masks are 
> currently same and they same for all the threads of a process).

Jonas was handling the Darwin case where debugserver returns the number of bits 
used in addressing in the qHostInfo packet here -- and we use the same value 
for both.  On Linux, we would want lldb-server to fetch the masks and send them 
up as well.  Given that they are fetched with ptrace(), it would make more 
sense to put these masks in the qProcessInfo response where we're attached to a 
process.  I think we always send that packet after we've launched/attached to 
something.

You would get masks, so those would be set in Process directly.

> IMO moving the mask calculation to ABI GetCodeAddress method may be a better 
> option.

I believe Jonas still wants to have an ABI::FixCodeAddress and 
ABI::FixDataAddress (I'm still not SUPER thrilled with the name "Fix", but 
whatever, we can refine it easily if someone has a better suggestion that isn't 
super long).  But the Process will maintain the dynamically-determined masks 
and the user-specified override setting.  The ABI will get the current mask 
from Process when running these methods.

We're going to have the mask format be the canonical one in Process, so 
FixCodeAddress and FixDataAddress methods in ABI will get it in that form from 
Process.


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

https://reviews.llvm.org/D100515

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


[Lldb-commits] [PATCH] D100500: Add setting to artifically delay communication with the debug server, in order ro simulate slow network conditions

2021-04-15 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added inline comments.



Comment at: 
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemoteProperties.td:23
+DefaultUnsignedValue<0>,
+Desc<"If greater than 0, sleep for packet-delay-multiplier * packet size 
nanoseconds for every packet sent or received.">;
 }

`To simulate slower connections for testing purposes...`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100500

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


[Lldb-commits] [PATCH] D100515: [lldb] Add GetCodeAddressMask and GetDataAddressMask to Process

2021-04-15 Thread Peter Collingbourne via Phabricator via lldb-commits
pcc added inline comments.



Comment at: lldb/source/Target/Process.cpp:5569-5570
+
+  if (m_code_address_mask == 0)
+return -1; // All bits are used for addressing.
+

Is this part correct? (Same below.) In D100521 you have
```
  if (pc & pac_sign_extension)
return pc | mask;
  return pc & (~mask);
```
So it looks like this would cause the pc to be set to 0 (or -1)?


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

https://reviews.llvm.org/D100515

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


[Lldb-commits] [PATCH] D100508: [lldb] Simplify output for skipped categories in dotest.py

2021-04-15 Thread Jordan Rupprecht via Phabricator via lldb-commits
rupprecht added a comment.

> The motivation for this change is that sometimes engineers misidentify the 
> output of these messages as the cause for a test failure

I have worked on one these checks and *still* gotten confused by the message, 
so thanks :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100508

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


[Lldb-commits] [PATCH] D100500: Add setting to artifically delay communication with the debug server, in order ro simulate slow network conditions

2021-04-15 Thread Augusto Noronha via Phabricator via lldb-commits
augusto2112 added a comment.

Hey @jasonmolenda thanks for the input, and nice empirical experiment! Do you 
think simply adding a setting that sleeps by a constant amount would be enough 
to model this problem?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100500

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


[Lldb-commits] [PATCH] D100554: [lldb] [Process/NetBSD] Report fork/vfork events to LLGS

2021-04-15 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.

https://reviews.llvm.org/D100554

Files:
  lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp
  lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.h
  lldb/source/Plugins/Process/NetBSD/NativeThreadNetBSD.cpp
  lldb/source/Plugins/Process/NetBSD/NativeThreadNetBSD.h
  lldb/test/Shell/Subprocess/clone-follow-child-softbp.test
  lldb/test/Shell/Subprocess/clone-follow-child-wp.test
  lldb/test/Shell/Subprocess/clone-follow-child.test

Index: lldb/test/Shell/Subprocess/clone-follow-child.test
===
--- lldb/test/Shell/Subprocess/clone-follow-child.test
+++ lldb/test/Shell/Subprocess/clone-follow-child.test
@@ -1,4 +1,4 @@
-# REQUIRES: native && (system-linux || system-netbsd)
+# REQUIRES: native && system-linux
 # clone() tests fails on arm64 Linux, PR #49899
 # UNSUPPORTED: system-linux && target-aarch64
 # RUN: %clangxx_host %p/Inputs/fork.cpp -DTEST_CLONE -o %t
Index: lldb/test/Shell/Subprocess/clone-follow-child-wp.test
===
--- lldb/test/Shell/Subprocess/clone-follow-child-wp.test
+++ lldb/test/Shell/Subprocess/clone-follow-child-wp.test
@@ -1,4 +1,4 @@
-# REQUIRES: native && (system-linux || system-netbsd) && dbregs-set
+# REQUIRES: native && system-linux && dbregs-set
 # clone() tests fails on arm64 Linux, PR #49899
 # UNSUPPORTED: system-linux && target-aarch64
 # RUN: %clangxx_host -g %p/Inputs/fork.cpp -DTEST_CLONE -o %t
Index: lldb/test/Shell/Subprocess/clone-follow-child-softbp.test
===
--- lldb/test/Shell/Subprocess/clone-follow-child-softbp.test
+++ lldb/test/Shell/Subprocess/clone-follow-child-softbp.test
@@ -1,4 +1,4 @@
-# REQUIRES: native && (system-linux || system-netbsd)
+# REQUIRES: native && system-linux
 # clone() tests fails on arm64 Linux, PR #49899
 # UNSUPPORTED: system-linux && target-aarch64
 # RUN: %clangxx_host %p/Inputs/fork.cpp -DTEST_CLONE -o %t
Index: lldb/source/Plugins/Process/NetBSD/NativeThreadNetBSD.h
===
--- lldb/source/Plugins/Process/NetBSD/NativeThreadNetBSD.h
+++ lldb/source/Plugins/Process/NetBSD/NativeThreadNetBSD.h
@@ -59,6 +59,9 @@
   void SetStoppedByTrace();
   void SetStoppedByExec();
   void SetStoppedByWatchpoint(uint32_t wp_index);
+  void SetStoppedByFork(lldb::pid_t child_pid, lldb::tid_t child_tid);
+  void SetStoppedByVFork(lldb::pid_t child_pid, lldb::tid_t child_tid);
+  void SetStoppedByVForkDone();
   void SetStoppedWithNoReason();
   void SetStopped();
   void SetRunning();
Index: lldb/source/Plugins/Process/NetBSD/NativeThreadNetBSD.cpp
===
--- lldb/source/Plugins/Process/NetBSD/NativeThreadNetBSD.cpp
+++ lldb/source/Plugins/Process/NetBSD/NativeThreadNetBSD.cpp
@@ -130,6 +130,30 @@
   m_stop_info.details.signal.signo = SIGTRAP;
 }
 
+void NativeThreadNetBSD::SetStoppedByFork(lldb::pid_t child_pid,
+   lldb::tid_t child_tid) {
+  SetStopped();
+
+  m_stop_info.reason = StopReason::eStopReasonFork;
+  m_stop_info.details.fork.child_pid = child_pid;
+  m_stop_info.details.fork.child_tid = child_tid;
+}
+
+void NativeThreadNetBSD::SetStoppedByVFork(lldb::pid_t child_pid,
+lldb::tid_t child_tid) {
+  SetStopped();
+
+  m_stop_info.reason = StopReason::eStopReasonVFork;
+  m_stop_info.details.fork.child_pid = child_pid;
+  m_stop_info.details.fork.child_tid = child_tid;
+}
+
+void NativeThreadNetBSD::SetStoppedByVForkDone() {
+  SetStopped();
+
+  m_stop_info.reason = StopReason::eStopReasonVForkDone;
+}
+
 void NativeThreadNetBSD::SetStoppedWithNoReason() {
   SetStopped();
 
Index: lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.h
===
--- lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.h
+++ lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.h
@@ -36,6 +36,8 @@
 llvm::Expected>
 Attach(lldb::pid_t pid, NativeDelegate _delegate,
MainLoop ) const override;
+
+Extension GetSupportedExtensions() const override;
   };
 
   // NativeProcessProtocol Interface
@@ -89,6 +91,7 @@
 private:
   MainLoop::SignalHandleUP m_sigchld_handle;
   ArchSpec m_arch;
+  MainLoop& m_main_loop;
   LazyBool m_supports_mem_region = eLazyBoolCalculate;
   std::vector> m_mem_region_cache;
 
@@ -106,7 +109,8 @@
   void MonitorSIGSTOP(lldb::pid_t pid);
   void MonitorSIGTRAP(lldb::pid_t pid);
   void MonitorSignal(lldb::pid_t pid, int signal);
-  void MonitorClone(::pid_t child_pid);
+  void MonitorClone(::pid_t child_pid, bool is_vfork,
+NativeThreadNetBSD _thread);
 
   Status PopulateMemoryRegionCache();
   void 

[Lldb-commits] [PATCH] D100338: Add a setting that enables memory to be read from the file cache instead of process when the section LLDB is reading from is read-only

2021-04-15 Thread Augusto Noronha via Phabricator via lldb-commits
augusto2112 updated this revision to Diff 337735.
augusto2112 added a comment.

Updates all calls to ReadMemory to keep the current behavior.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100338

Files:
  lldb/include/lldb/Core/Disassembler.h
  lldb/include/lldb/Symbol/Function.h
  lldb/include/lldb/Target/Target.h
  lldb/source/API/SBFunction.cpp
  lldb/source/API/SBSymbol.cpp
  lldb/source/API/SBTarget.cpp
  lldb/source/Commands/CommandObjectMemory.cpp
  lldb/source/Core/Address.cpp
  lldb/source/Core/Disassembler.cpp
  lldb/source/Core/IOHandlerCursesGUI.cpp
  lldb/source/Core/Value.cpp
  lldb/source/Core/ValueObject.cpp
  lldb/source/Expression/IRMemoryMap.cpp
  lldb/source/Plugins/Architecture/Mips/ArchitectureMips.cpp
  lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
  lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
  lldb/source/Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.cpp
  lldb/source/Plugins/Instruction/MIPS/EmulateInstructionMIPS.cpp
  lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  
lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp
  lldb/source/Plugins/UnwindAssembly/x86/UnwindAssembly-x86.cpp
  lldb/source/Symbol/Function.cpp
  lldb/source/Symbol/Symbol.cpp
  lldb/source/Target/Process.cpp
  lldb/source/Target/StackFrame.cpp
  lldb/source/Target/Target.cpp
  lldb/source/Target/ThreadPlanStepRange.cpp
  lldb/source/Target/Trace.cpp

Index: lldb/source/Target/Trace.cpp
===
--- lldb/source/Target/Trace.cpp
+++ lldb/source/Target/Trace.cpp
@@ -200,7 +200,7 @@
   // Now we try using the current function's disassembler
   if (sc.function) {
 DisassemblerSP disassembler =
-sc.function->GetInstructions(exe_ctx, nullptr, true);
+sc.function->GetInstructions(exe_ctx, nullptr);
 if (TryDumpInstructionInfo(s, disassembler, exe_ctx, address))
   return disassembler;
   }
@@ -209,9 +209,9 @@
   Target  = exe_ctx.GetTargetRef();
   const ArchSpec  = target.GetArchitecture();
   AddressRange range(address, arch.GetMaximumOpcodeByteSize() * 1);
-  DisassemblerSP disassembler = Disassembler::DisassembleRange(
-  arch, /*plugin_name*/ nullptr,
-  /*flavor*/ nullptr, target, range, /*prefer_file_cache*/ true);
+  DisassemblerSP disassembler =
+  Disassembler::DisassembleRange(arch, /*plugin_name*/ nullptr,
+ /*flavor*/ nullptr, target, range);
   if (TryDumpInstructionInfo(s, disassembler, exe_ctx, address))
 return disassembler;
   return nullptr;
Index: lldb/source/Target/ThreadPlanStepRange.cpp
===
--- lldb/source/Target/ThreadPlanStepRange.cpp
+++ lldb/source/Target/ThreadPlanStepRange.cpp
@@ -264,10 +264,9 @@
 // Disassemble the address range given:
 const char *plugin_name = nullptr;
 const char *flavor = nullptr;
-const bool prefer_file_cache = true;
 m_instruction_ranges[i] = Disassembler::DisassembleRange(
 GetTarget().GetArchitecture(), plugin_name, flavor, GetTarget(),
-m_address_ranges[i], prefer_file_cache);
+m_address_ranges[i]);
   }
   if (!m_instruction_ranges[i])
 return nullptr;
Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -1717,8 +1717,8 @@
   return 0;
 }
 
-size_t Target::ReadMemory(const Address , bool prefer_file_cache,
-  void *dst, size_t dst_len, Status ,
+size_t Target::ReadMemory(const Address , void *dst, size_t dst_len,
+  Status , bool force_live_memory,
   lldb::addr_t *load_addr_ptr) {
   error.Clear();
 
@@ -1753,10 +1753,20 @@
   if (!resolved_addr.IsValid())
 resolved_addr = addr;
 
-  if (prefer_file_cache) {
-bytes_read = ReadMemoryFromFileCache(resolved_addr, dst, dst_len, error);
-if (bytes_read > 0)
-  return bytes_read;
+  bool is_readonly = false;
+  // Read from file cache if read-only section.
+  if (!force_live_memory && resolved_addr.IsSectionOffset()) {
+SectionSP section_sp(addr.GetSection());
+if (section_sp) {
+  auto permissions = Flags(section_sp->GetPermissions());
+  is_readonly = !permissions.Test(ePermissionsWritable) &&
+permissions.Test(ePermissionsReadable);
+}
+if (is_readonly) {
+  bytes_read = ReadMemoryFromFileCache(resolved_addr, dst, dst_len, error);
+  if (bytes_read > 0)
+return bytes_read;
+}
   }
 
   if (ProcessIsValid()) {
@@ -1791,17 +1801,10 @@
   *load_addr_ptr = load_addr;
 return bytes_read;
   }
-  // If the address is not section offset we have 

[Lldb-commits] [PATCH] D100547: [lldb] [Process/FreeBSD] Report fork/vfork events to LLGS

2021-04-15 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.

https://reviews.llvm.org/D100547

Files:
  lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
  lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.h
  lldb/source/Plugins/Process/FreeBSD/NativeThreadFreeBSD.cpp
  lldb/source/Plugins/Process/FreeBSD/NativeThreadFreeBSD.h

Index: lldb/source/Plugins/Process/FreeBSD/NativeThreadFreeBSD.h
===
--- lldb/source/Plugins/Process/FreeBSD/NativeThreadFreeBSD.h
+++ lldb/source/Plugins/Process/FreeBSD/NativeThreadFreeBSD.h
@@ -59,6 +59,9 @@
   void SetStoppedByTrace();
   void SetStoppedByExec();
   void SetStoppedByWatchpoint(uint32_t wp_index);
+  void SetStoppedByFork(lldb::pid_t child_pid, lldb::tid_t child_tid);
+  void SetStoppedByVFork(lldb::pid_t child_pid, lldb::tid_t child_tid);
+  void SetStoppedByVForkDone();
   void SetStoppedWithNoReason();
   void SetStopped();
   void SetRunning();
Index: lldb/source/Plugins/Process/FreeBSD/NativeThreadFreeBSD.cpp
===
--- lldb/source/Plugins/Process/FreeBSD/NativeThreadFreeBSD.cpp
+++ lldb/source/Plugins/Process/FreeBSD/NativeThreadFreeBSD.cpp
@@ -130,6 +130,30 @@
   m_stop_info.details.signal.signo = SIGTRAP;
 }
 
+void NativeThreadFreeBSD::SetStoppedByFork(lldb::pid_t child_pid,
+   lldb::tid_t child_tid) {
+  SetStopped();
+
+  m_stop_info.reason = StopReason::eStopReasonFork;
+  m_stop_info.details.fork.child_pid = child_pid;
+  m_stop_info.details.fork.child_tid = child_tid;
+}
+
+void NativeThreadFreeBSD::SetStoppedByVFork(lldb::pid_t child_pid,
+lldb::tid_t child_tid) {
+  SetStopped();
+
+  m_stop_info.reason = StopReason::eStopReasonVFork;
+  m_stop_info.details.fork.child_pid = child_pid;
+  m_stop_info.details.fork.child_tid = child_tid;
+}
+
+void NativeThreadFreeBSD::SetStoppedByVForkDone() {
+  SetStopped();
+
+  m_stop_info.reason = StopReason::eStopReasonVForkDone;
+}
+
 void NativeThreadFreeBSD::SetStoppedWithNoReason() {
   SetStopped();
 
Index: lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.h
===
--- lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.h
+++ lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.h
@@ -39,6 +39,8 @@
 llvm::Expected>
 Attach(lldb::pid_t pid, NativeDelegate _delegate,
MainLoop ) const override;
+
+Extension GetSupportedExtensions() const override;
   };
 
   // NativeProcessProtocol Interface
@@ -96,6 +98,7 @@
 private:
   MainLoop::SignalHandleUP m_sigchld_handle;
   ArchSpec m_arch;
+  MainLoop& m_main_loop;
   LazyBool m_supports_mem_region = eLazyBoolCalculate;
   std::vector> m_mem_region_cache;
 
@@ -113,7 +116,8 @@
   void MonitorSIGSTOP(lldb::pid_t pid);
   void MonitorSIGTRAP(lldb::pid_t pid);
   void MonitorSignal(lldb::pid_t pid, int signal);
-  void MonitorClone(::pid_t child_pid);
+  void MonitorClone(::pid_t child_pid, bool is_vfork,
+NativeThreadFreeBSD _thread);
 
   Status PopulateMemoryRegionCache();
   void SigchldHandler();
Index: lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
===
--- lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
+++ lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
@@ -128,13 +128,19 @@
   return std::move(process_up);
 }
 
+NativeProcessFreeBSD::Extension
+NativeProcessFreeBSD::Factory::GetSupportedExtensions() const {
+  return Extension::fork | Extension::vfork;
+}
+
 // Public Instance Methods
 
 NativeProcessFreeBSD::NativeProcessFreeBSD(::pid_t pid, int terminal_fd,
NativeDelegate ,
const ArchSpec ,
MainLoop )
-: NativeProcessELF(pid, terminal_fd, delegate), m_arch(arch) {
+: NativeProcessELF(pid, terminal_fd, delegate), m_arch(arch),
+  m_main_loop(mainloop) {
   if (m_terminal_fd != -1) {
 Status status = EnsureFDFlags(m_terminal_fd, O_NONBLOCK);
 assert(status.Success());
@@ -247,19 +253,6 @@
 return;
   }
 
-  if (info.pl_flags & PL_FLAG_FORKED) {
-MonitorClone(info.pl_child_pid);
-return;
-  }
-
-  if (info.pl_flags & PL_FLAG_VFORK_DONE) {
-Status error =
-PtraceWrapper(PT_CONTINUE, pid, reinterpret_cast(1), 0);
-if (error.Fail())
-  SetState(StateType::eStateInvalid);
-return;
-  }
-
   if (info.pl_lwpid > 0) {
 for (const auto  : m_threads) {
   if (t->GetID() == static_cast(info.pl_lwpid))
@@ -271,6 +264,26 @@
info.pl_lwpid);
   }
 
+  if (info.pl_flags & PL_FLAG_FORKED) {
+assert(thread);
+MonitorClone(info.pl_child_pid, 

[Lldb-commits] [PATCH] D100541: [lldb] [gdb-remote client] Support detaching by PID

2021-04-15 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.

https://reviews.llvm.org/D100541

Files:
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h


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
@@ -229,7 +229,7 @@
 
   bool DeallocateMemory(lldb::addr_t addr);
 
-  Status Detach(bool keep_stopped);
+  Status Detach(bool keep_stopped, lldb::pid_t pid = LLDB_INVALID_PROCESS_ID);
 
   Status GetMemoryRegionInfo(lldb::addr_t addr, MemoryRegionInfo _info);
 
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
@@ -1412,9 +1412,12 @@
   return false;
 }
 
-Status GDBRemoteCommunicationClient::Detach(bool keep_stopped) {
+Status GDBRemoteCommunicationClient::Detach(bool keep_stopped,
+lldb::pid_t pid) {
   Status error;
+  lldb_private::StreamString packet;
 
+  packet.PutChar('D');
   if (keep_stopped) {
 if (m_supports_detach_stay_stopped == eLazyBoolCalculate) {
   char packet[64];
@@ -1436,19 +1439,25 @@
   error.SetErrorString("Stays stopped not supported by this target.");
   return error;
 } else {
-  StringExtractorGDBRemote response;
-  PacketResult packet_result =
-  SendPacketAndWaitForResponse("D1", response, false);
-  if (packet_result != PacketResult::Success)
-error.SetErrorString("Sending extended disconnect packet failed.");
+  packet.PutChar('1');
 }
-  } else {
-StringExtractorGDBRemote response;
-PacketResult packet_result =
-SendPacketAndWaitForResponse("D", response, false);
-if (packet_result != PacketResult::Success)
-  error.SetErrorString("Sending disconnect packet failed.");
   }
+
+  if (pid != LLDB_INVALID_PROCESS_ID) {
+if (!m_supports_multiprocess) {
+  error.SetErrorString(
+  "Multiprocess extension not supported by the server.");
+  return error;
+}
+packet.PutChar(';');
+packet.PutHex64(pid);
+  }
+
+  StringExtractorGDBRemote response;
+  PacketResult packet_result =
+  SendPacketAndWaitForResponse(packet.GetString(), response, false);
+  if (packet_result != PacketResult::Success)
+error.SetErrorString("Sending isconnect packet failed.");
   return error;
 }
 


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
@@ -229,7 +229,7 @@
 
   bool DeallocateMemory(lldb::addr_t addr);
 
-  Status Detach(bool keep_stopped);
+  Status Detach(bool keep_stopped, lldb::pid_t pid = LLDB_INVALID_PROCESS_ID);
 
   Status GetMemoryRegionInfo(lldb::addr_t addr, MemoryRegionInfo _info);
 
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
@@ -1412,9 +1412,12 @@
   return false;
 }
 
-Status GDBRemoteCommunicationClient::Detach(bool keep_stopped) {
+Status GDBRemoteCommunicationClient::Detach(bool keep_stopped,
+lldb::pid_t pid) {
   Status error;
+  lldb_private::StreamString packet;
 
+  packet.PutChar('D');
   if (keep_stopped) {
 if (m_supports_detach_stay_stopped == eLazyBoolCalculate) {
   char packet[64];
@@ -1436,19 +1439,25 @@
   error.SetErrorString("Stays stopped not supported by this target.");
   return error;
 } else {
-  StringExtractorGDBRemote response;
-  PacketResult packet_result =
-  SendPacketAndWaitForResponse("D1", response, false);
-  if (packet_result != PacketResult::Success)
-error.SetErrorString("Sending extended disconnect packet failed.");
+  packet.PutChar('1');
 }
-  } else {
-StringExtractorGDBRemote response;
-PacketResult packet_result =
-SendPacketAndWaitForResponse("D", response, false);
-if (packet_result != PacketResult::Success)
-  error.SetErrorString("Sending disconnect packet failed.");
   }
+
+  if (pid != LLDB_INVALID_PROCESS_ID) {
+if (!m_supports_multiprocess) {
+  error.SetErrorString(
+  "Multiprocess extension not 

[Lldb-commits] [lldb] 0f3ed7a - [lldb] Fix incorrect test data in FileSpecTest.IsRelative

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

Author: Raphael Isemann
Date: 2021-04-15T12:42:47+02:00
New Revision: 0f3ed7a48dba2eb3ad726fc8f513159baca13b71

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

LOG: [lldb] Fix incorrect test data in FileSpecTest.IsRelative

Found by clang-tidy's bugprone-suspicious-missing-comma.

Added: 


Modified: 
lldb/unittests/Utility/FileSpecTest.cpp

Removed: 




diff  --git a/lldb/unittests/Utility/FileSpecTest.cpp 
b/lldb/unittests/Utility/FileSpecTest.cpp
index ad2e328ce82fe..3dd355284ce0f 100644
--- a/lldb/unittests/Utility/FileSpecTest.cpp
+++ b/lldb/unittests/Utility/FileSpecTest.cpp
@@ -305,7 +305,7 @@ TEST(FileSpecTest, IsRelative) {
 "~/",
 "~/a",
 "~/a/",
-"~/a/b"
+"~/a/b",
 "~/a/b/",
 "/foo/.",
 "/foo/..",



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


[Lldb-commits] [PATCH] D100515: [lldb] Add GetCodeAddressMask and GetDataAddressMask to Process

2021-04-15 Thread Muhammad Omair Javaid via Phabricator via lldb-commits
omjavaid added a comment.

In D100515#2690225 , @jasonmolenda 
wrote:

> Thanks Jonas, I think this change to Process can cover
>
>   Omair's use case, where the number of bits are used hardcoded.
>   
>   The Darwin use case were ProcessGDBRemote, DynamicLoaderDarwinKernel, and 
> ProcessMachCore can all set the number of bits used in addressing 
> programmatically, detected at runtime.
>   
>   The firmware use case where people (often in a python script in a dSYM) can 
> set the number of bits manually so it is correct in their environment, when 
> the dynamic detection schemes are not available.

This looks very good and I have couple of highlights to make from Linux/AArch64 
perspective:
qHostInfo is not sufficient for mask calculation on Linux because both 
data/code ptr auth masks are read by thread register context (both masks are 
currently same and they same for all the threads of a process). 
IMO moving the mask calculation to ABI GetCodeAddress method may be a better 
option. We can pull in information of addressable bits from process and also 
read register context for code/data masks by passing threadsp to 
ABI::GetCodeAddress. But in case we dont get agreement on moving mask 
calculation to ABI we can calculate mask somewhere inside ProcessGDBRemote and 
set it from there similar to what I have done in D99947 
.

> Justin's case where ptrace(PTRACE_GETREGSET) gets us a code and data address 
> mask.
>
> Canonicalizing them as masks in Process, accepting either form, and providing 
> the value as a mask, should make it easy to use in the ABI plugins where we 
> have a FixCodeAddress and FixDataAddress.
>
> On Darwin, we use the same number of bits for both code and data, but given 
> the way ptrace() behaves on Linux, I'm guessing this may not be the case 
> everywhere.  Should we store both masks, and add FixCodeAddress + 
> FixDataAddress methods in the ABI's, Justin?  What do you think?
>
> The Darwin support can be adapted to this centralized scheme that Jonas has 
> written easily.  Everyone else OK with this?
>
> I don't feel strongly about the method name.  Justin used 
> Process::GetPointerAuthenticationAddressMask which makes it sound specific to 
> ARM v8.3 ptrauth.  The ARM v8.5 MTE tagging might be another use case (or TBI 
> as Omair has noted).




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

https://reviews.llvm.org/D100515

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


[Lldb-commits] [PATCH] D100493: [lldb][AArch64] Don't check for VmFlags in smaps files

2021-04-15 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added a comment.

Looks like buildkite had issues calling back to Phab yesterday so the builds 
didn't signal here. This test wouldn't get run in those anyway.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100493

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


[Lldb-commits] [lldb] 71a45e7 - NFC put the armv6m entry with the other Cortex-M entries

2021-04-15 Thread Jason Molenda via lldb-commits

Author: Jason Molenda
Date: 2021-04-15T02:02:26-07:00
New Revision: 71a45e7c63285614364d3d5dfcb4d48da6e7535d

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

LOG: NFC put the armv6m entry with the other Cortex-M entries

The armv6m entry in cores_match() got separated from its
friends armv7m and armv7em.  Reuniting them to make it
easier to keep them updated in all at the same time.

Added: 


Modified: 
lldb/source/Utility/ArchSpec.cpp

Removed: 




diff  --git a/lldb/source/Utility/ArchSpec.cpp 
b/lldb/source/Utility/ArchSpec.cpp
index 90c1bbb4abcc..604131cd9ec4 100644
--- a/lldb/source/Utility/ArchSpec.cpp
+++ b/lldb/source/Utility/ArchSpec.cpp
@@ -1118,20 +1118,6 @@ static bool cores_match(const ArchSpec::Core core1, 
const ArchSpec::Core core2,
   return true;
 break;
 
-  case ArchSpec::eCore_arm_armv6m:
-if (!enforce_exact_match) {
-  if (core2 == ArchSpec::eCore_arm_generic)
-return true;
-  if (core2 == ArchSpec::eCore_arm_armv7em)
-return true;
-  if (core2 == ArchSpec::eCore_arm_armv7)
-return true;
-  if (core2 == ArchSpec::eCore_arm_armv6m)
-return true;
-  try_inverse = false;
-}
-break;
-
   case ArchSpec::kCore_hexagon_any:
 if ((core2 >= ArchSpec::kCore_hexagon_first &&
  core2 <= ArchSpec::kCore_hexagon_last) ||
@@ -1140,8 +1126,9 @@ static bool cores_match(const ArchSpec::Core core1, const 
ArchSpec::Core core2,
 break;
 
   // v. https://en.wikipedia.org/wiki/ARM_Cortex-M#Silicon_customization
-  // Cortex-M0 - ARMv6-M - armv6m Cortex-M3 - ARMv7-M - armv7m Cortex-M4 -
-  // ARMv7E-M - armv7em
+  // Cortex-M0 - ARMv6-M - armv6m 
+  // Cortex-M3 - ARMv7-M - armv7m 
+  // Cortex-M4 - ARMv7E-M - armv7em
   case ArchSpec::eCore_arm_armv7em:
 if (!enforce_exact_match) {
   if (core2 == ArchSpec::eCore_arm_generic)
@@ -1157,8 +1144,9 @@ static bool cores_match(const ArchSpec::Core core1, const 
ArchSpec::Core core2,
 break;
 
   // v. https://en.wikipedia.org/wiki/ARM_Cortex-M#Silicon_customization
-  // Cortex-M0 - ARMv6-M - armv6m Cortex-M3 - ARMv7-M - armv7m Cortex-M4 -
-  // ARMv7E-M - armv7em
+  // Cortex-M0 - ARMv6-M - armv6m 
+  // Cortex-M3 - ARMv7-M - armv7m 
+  // Cortex-M4 - ARMv7E-M - armv7em
   case ArchSpec::eCore_arm_armv7m:
 if (!enforce_exact_match) {
   if (core2 == ArchSpec::eCore_arm_generic)
@@ -1173,6 +1161,24 @@ static bool cores_match(const ArchSpec::Core core1, 
const ArchSpec::Core core2,
 }
 break;
 
+  // v. https://en.wikipedia.org/wiki/ARM_Cortex-M#Silicon_customization
+  // Cortex-M0 - ARMv6-M - armv6m 
+  // Cortex-M3 - ARMv7-M - armv7m 
+  // Cortex-M4 - ARMv7E-M - armv7em
+  case ArchSpec::eCore_arm_armv6m:
+if (!enforce_exact_match) {
+  if (core2 == ArchSpec::eCore_arm_generic)
+return true;
+  if (core2 == ArchSpec::eCore_arm_armv7em)
+return true;
+  if (core2 == ArchSpec::eCore_arm_armv7)
+return true;
+  if (core2 == ArchSpec::eCore_arm_armv6m)
+return true;
+  try_inverse = false;
+}
+break;
+
   case ArchSpec::eCore_arm_armv7f:
   case ArchSpec::eCore_arm_armv7k:
   case ArchSpec::eCore_arm_armv7s:



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


[Lldb-commits] [PATCH] D100515: [lldb] Add GetCodeAddressMask and GetDataAddressMask to Process

2021-04-15 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added a comment.

> I don't feel strongly about the method name. Justin used 
> Process::GetPointerAuthenticationAddressMask which makes it sound specific to 
> ARM v8.3 ptrauth. The ARM v8.5 MTE tagging might be another use case (or TBI 
> as Omair has noted).

I'd go with the generic code/address name for the full masks (which would 
remove MTE/PAC/TBI all at once) and if we find situations where you only need 
one, add those as separate methods.
(maybe we already have a few of those situations, just my guess that most 
callers want to remove all the non address bits)


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

https://reviews.llvm.org/D100515

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


[Lldb-commits] [lldb] 01ad95f - Mark armv6m compat with armv7em; match armv7em being compat with armv6m

2021-04-15 Thread Jason Molenda via lldb-commits

Author: Jason Molenda
Date: 2021-04-15T01:30:51-07:00
New Revision: 01ad95ff2a7cc127eddf3832693bbd5c6c54f9ef

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

LOG: Mark armv6m compat with armv7em; match armv7em being compat with armv6m

armv7em and armv6m in ArchSpec cores_match() will return true.
There was a small bug where the reverse order would not return true.

rdar://76387176

Added: 


Modified: 
lldb/source/Utility/ArchSpec.cpp

Removed: 




diff  --git a/lldb/source/Utility/ArchSpec.cpp 
b/lldb/source/Utility/ArchSpec.cpp
index 52fbf3551b1a2..90c1bbb4abcc8 100644
--- a/lldb/source/Utility/ArchSpec.cpp
+++ b/lldb/source/Utility/ArchSpec.cpp
@@ -1122,11 +1122,13 @@ static bool cores_match(const ArchSpec::Core core1, 
const ArchSpec::Core core2,
 if (!enforce_exact_match) {
   if (core2 == ArchSpec::eCore_arm_generic)
 return true;
-  try_inverse = false;
+  if (core2 == ArchSpec::eCore_arm_armv7em)
+return true;
   if (core2 == ArchSpec::eCore_arm_armv7)
 return true;
   if (core2 == ArchSpec::eCore_arm_armv6m)
 return true;
+  try_inverse = false;
 }
 break;
 



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