[Lldb-commits] [PATCH] D154907: [LLDB] Fix buffer overflow problem in DWARFExpression::Evaluate (2nd attempt)

[Caroline Tice via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG3885ceafa934: [LLDB] Fix buffer overflow problem in 
DWARFExpression::Evaluate (authored by cmtice).
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154907

Files:
  lldb/source/Expression/DWARFExpression.cpp


Index: lldb/source/Expression/DWARFExpression.cpp
===
--- lldb/source/Expression/DWARFExpression.cpp
+++ lldb/source/Expression/DWARFExpression.cpp
@@ -1069,6 +1069,13 @@
 return false;
   }
   uint8_t size = opcodes.GetU8();
+  if (size > 8) {
+if (error_ptr)
+  error_ptr->SetErrorStringWithFormat(
+  "Invalid address size for DW_OP_deref_size: %d\n",
+  size);
+return false;
+  }
   Value::ValueType value_type = stack.back().GetValueType();
   switch (value_type) {
   case Value::ValueType::HostAddress: {
@@ -1141,7 +1148,7 @@
   } else {
 if (error_ptr)
   error_ptr->SetErrorStringWithFormat(
-  "Failed to dereference pointer for for DW_OP_deref_size: "
+  "Failed to dereference pointer for DW_OP_deref_size: "
   "%s\n",
   error.AsCString());
 return false;


Index: lldb/source/Expression/DWARFExpression.cpp
===
--- lldb/source/Expression/DWARFExpression.cpp
+++ lldb/source/Expression/DWARFExpression.cpp
@@ -1069,6 +1069,13 @@
 return false;
   }
   uint8_t size = opcodes.GetU8();
+  if (size > 8) {
+if (error_ptr)
+  error_ptr->SetErrorStringWithFormat(
+  "Invalid address size for DW_OP_deref_size: %d\n",
+  size);
+return false;
+  }
   Value::ValueType value_type = stack.back().GetValueType();
   switch (value_type) {
   case Value::ValueType::HostAddress: {
@@ -1141,7 +1148,7 @@
   } else {
 if (error_ptr)
   error_ptr->SetErrorStringWithFormat(
-  "Failed to dereference pointer for for DW_OP_deref_size: "
+  "Failed to dereference pointer for DW_OP_deref_size: "
   "%s\n",
   error.AsCString());
 return false;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D153840: [LLDB] Fix buffer overflow problem in DWARFExpression::Evaluate.

[Caroline Tice via Phabricator via lldb-commits]

cmtice marked an inline comment as done.
cmtice added a comment.

Hi Jason,

I had been talking more with David, and yes, I had come to the conclusion that 
you are both right and that this was not the right fix.  I am planning on 
reverting this, but I am trying to figure out the right fix to replace it with. 
 I can't share the source that was causing the bug to manifest, because it's in 
proprietary code, but David is looking at it and I believe he has come to the 
conclusion that there is a bug in the DWARF code generation -- we were getting 
a size of 16, which is absolutely not right.  The question is, in the case of 
bad DWARF being generated, what (if anything) should the LLDB code here be 
doing? Should we check the size as soon as we read it in, and assert that  it 
must be <= 8?  Or something else?  Or just leave the LLDB code entirely alone?

What do you (and other reviewers) think is the right thing to do here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153840

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

[Lldb-commits] [PATCH] D153840: [LLDB] Fix buffer overflow problem in DWARFExpression::Evaluate.

[Caroline Tice via Phabricator via lldb-commits]

cmtice added a comment.
Herald added subscribers: Michael137, JDevlieghere.

I updated the version that I committed to use 'sizeof' as recommended.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153840

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

[Lldb-commits] [PATCH] D153840: [LLDB] Fix buffer overflow problem in DWARFExpression::Evaluate.

[Caroline Tice via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGee476996bec7: [LLDB] Fix buffer overflow problem in 
DWARFExpression::Evaluate. (authored by cmtice).
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Changed prior to commit:
  https://reviews.llvm.org/D153840?vs=534856=535482#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153840

Files:
  lldb/source/Expression/DWARFExpression.cpp


Index: lldb/source/Expression/DWARFExpression.cpp
===
--- lldb/source/Expression/DWARFExpression.cpp
+++ lldb/source/Expression/DWARFExpression.cpp
@@ -1138,15 +1138,16 @@
 
 if (load_addr == LLDB_INVALID_ADDRESS && so_addr.IsSectionOffset()) {
   uint8_t addr_bytes[8];
+  size_t buf_size = sizeof(addr_bytes);
   Status error;
 
   if (target &&
-  target->ReadMemory(so_addr, _bytes, size, error,
- /*force_live_memory=*/false) == size) {
+  target->ReadMemory(so_addr, _bytes, buf_size, error,
+ /*force_live_memory=*/false) == buf_size) {
 ObjectFile *objfile = module_sp->GetObjectFile();
 
 stack.back().GetScalar() = DerefSizeExtractDataHelper(
-addr_bytes, size, objfile->GetByteOrder(), size);
+addr_bytes, size, objfile->GetByteOrder(), buf_size);
 stack.back().ClearContext();
 break;
   } else {
@@ -1170,13 +1171,13 @@
 lldb::addr_t pointer_addr =
 stack.back().GetScalar().ULongLong(LLDB_INVALID_ADDRESS);
 uint8_t addr_bytes[sizeof(lldb::addr_t)];
+size_t buf_size = sizeof(addr_bytes);
 Status error;
-if (process->ReadMemory(pointer_addr, _bytes, size, error) ==
-size) {
-
+if (process->ReadMemory(pointer_addr, _bytes, buf_size, error)
+== buf_size) {
   stack.back().GetScalar() =
   DerefSizeExtractDataHelper(addr_bytes, sizeof(addr_bytes),
- process->GetByteOrder(), size);
+ process->GetByteOrder(), 
buf_size);
   stack.back().ClearContext();
 } else {
   if (error_ptr)


Index: lldb/source/Expression/DWARFExpression.cpp
===
--- lldb/source/Expression/DWARFExpression.cpp
+++ lldb/source/Expression/DWARFExpression.cpp
@@ -1138,15 +1138,16 @@
 
 if (load_addr == LLDB_INVALID_ADDRESS && so_addr.IsSectionOffset()) {
   uint8_t addr_bytes[8];
+  size_t buf_size = sizeof(addr_bytes);
   Status error;
 
   if (target &&
-  target->ReadMemory(so_addr, _bytes, size, error,
- /*force_live_memory=*/false) == size) {
+  target->ReadMemory(so_addr, _bytes, buf_size, error,
+ /*force_live_memory=*/false) == buf_size) {
 ObjectFile *objfile = module_sp->GetObjectFile();
 
 stack.back().GetScalar() = DerefSizeExtractDataHelper(
-addr_bytes, size, objfile->GetByteOrder(), size);
+addr_bytes, size, objfile->GetByteOrder(), buf_size);
 stack.back().ClearContext();
 break;
   } else {
@@ -1170,13 +1171,13 @@
 lldb::addr_t pointer_addr =
 stack.back().GetScalar().ULongLong(LLDB_INVALID_ADDRESS);
 uint8_t addr_bytes[sizeof(lldb::addr_t)];
+size_t buf_size = sizeof(addr_bytes);
 Status error;
-if (process->ReadMemory(pointer_addr, _bytes, size, error) ==
-size) {
-
+if (process->ReadMemory(pointer_addr, _bytes, buf_size, error)
+== buf_size) {
   stack.back().GetScalar() =
   DerefSizeExtractDataHelper(addr_bytes, sizeof(addr_bytes),
- process->GetByteOrder(), size);
+ process->GetByteOrder(), buf_size);
   stack.back().ClearContext();
 } else {
   if (error_ptr)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D150019: [lldb] Fix language label in ObjC Language unittest

[Caroline Tice via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG123f939c14f7: [lldb] Fix language label in ObjC Language 
unittest (authored by cmtice).
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150019

Files:
  lldb/unittests/Language/ObjC/ObjCLanguageTest.cpp


Index: lldb/unittests/Language/ObjC/ObjCLanguageTest.cpp
===
--- lldb/unittests/Language/ObjC/ObjCLanguageTest.cpp
+++ lldb/unittests/Language/ObjC/ObjCLanguageTest.cpp
@@ -92,7 +92,7 @@
   }
 }
 
-TEST(CPlusPlusLanguage, InvalidMethodNameParsing) {
+TEST(ObjCLanguage, InvalidMethodNameParsing) {
   // Tests that we correctly reject malformed function names
 
   llvm::StringRef test_cases[] = {"+[Uh oh!",


Index: lldb/unittests/Language/ObjC/ObjCLanguageTest.cpp
===
--- lldb/unittests/Language/ObjC/ObjCLanguageTest.cpp
+++ lldb/unittests/Language/ObjC/ObjCLanguageTest.cpp
@@ -92,7 +92,7 @@
   }
 }
 
-TEST(CPlusPlusLanguage, InvalidMethodNameParsing) {
+TEST(ObjCLanguage, InvalidMethodNameParsing) {
   // Tests that we correctly reject malformed function names
 
   llvm::StringRef test_cases[] = {"+[Uh oh!",
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D149397: Host: generalise `GetXcodeSDKPath`

[Caroline Tice via Phabricator via lldb-commits]

cmtice added a comment.

I'm seeing the same build failures reported by mgorny. Would you consider 
reverting this patch until you can fix it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149397

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

[Lldb-commits] [PATCH] D148151: [lldb] Add 'CHECK' to class-type-nullptr-deref.s test.

[Caroline Tice via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGbe58b42a7510: [lldb] Add CHECK to 
class-type-nullptr-deref.s test. (authored by cmtice).
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148151

Files:
  lldb/test/Shell/SymbolFile/DWARF/x86/class-type-nullptr-deref.s


Index: lldb/test/Shell/SymbolFile/DWARF/x86/class-type-nullptr-deref.s
===
--- lldb/test/Shell/SymbolFile/DWARF/x86/class-type-nullptr-deref.s
+++ lldb/test/Shell/SymbolFile/DWARF/x86/class-type-nullptr-deref.s
@@ -2,7 +2,9 @@
 # null), LLDB does not try to dereference the null pointer.
 
 # RUN: llvm-mc --triple x86_64-pc-linux %s --filetype=obj -o %t
-# RUN: %lldb %t -o "target variable x" -o exit 2>&1
+# RUN: %lldb %t -o "target variable x" -o exit 2>&1 | FileCheck %s
+
+# CHECK: 'Unable to determine byte size.'
 
 # This tests a fix for a crash. If things are working we don't get a segfault.
 


Index: lldb/test/Shell/SymbolFile/DWARF/x86/class-type-nullptr-deref.s
===
--- lldb/test/Shell/SymbolFile/DWARF/x86/class-type-nullptr-deref.s
+++ lldb/test/Shell/SymbolFile/DWARF/x86/class-type-nullptr-deref.s
@@ -2,7 +2,9 @@
 # null), LLDB does not try to dereference the null pointer.
 
 # RUN: llvm-mc --triple x86_64-pc-linux %s --filetype=obj -o %t
-# RUN: %lldb %t -o "target variable x" -o exit 2>&1
+# RUN: %lldb %t -o "target variable x" -o exit 2>&1 | FileCheck %s
+
+# CHECK: 'Unable to determine byte size.'
 
 # This tests a fix for a crash. If things are working we don't get a segfault.
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D128152: [lldb] [llgs] Support multiprocess in qfThreadInfo

[Caroline Tice via Phabricator via lldb-commits]

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

Just FYI, this commit appears to cause 
lldb//test/API/commands/target/auto-install-main-executable/TestAutoInstallMainExecutable.py
 to fail. The error I'm seeing is:

Traceback (most recent call last):

File 
"../lldb/test/API/commands/target/auto-install-main-executable/TestAutoInstallMainExecutable.py",
 line 72, in test_target_auto_install_main_executable

  self.expect("process launch", substrs=["exited with status = 74"])

File ".../lldb/packages/Python/lldbsuite/test/lldbtest.py", line 2295, in expect

  self.runCmd(

File ".../lldb/packages/Python/lldbsuite/test/lldbtest.py", line 1998, in runCmd

  self.assertTrue(self.res.Succeeded(),

AssertionError: False is not True : Command 'process launch
Error output:
error: failed to get reply to handshake packet within timeout of 0.0 seconds
' did not return successfully


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128152

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

[Lldb-commits] [PATCH] D127702: Support logpoints in lldb-vscode

[Caroline Tice via Phabricator via lldb-commits]

cmtice added a comment.

I apologize; I was looking at several commits, and I accidentally added my 
comment to the wrong one. No, this commit is not the one that caused my problem 
(I am very sorry about the confusion).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127702

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

[Lldb-commits] [PATCH] D127702: Support logpoints in lldb-vscode

[Caroline Tice via Phabricator via lldb-commits]

cmtice added a comment.

Just FYI, this commit appears to cause 
lldb//test/API/commands/target/auto-install-main-executable/TestAutoInstallMainExecutable.py
 to fail.  The error I'm seeing is:

Traceback (most recent call last):

  File 
"../lldb/test/API/commands/target/auto-install-main-executable/TestAutoInstallMainExecutable.py",
 line 72, in test_target_auto_install_main_executable
self.expect("process launch", substrs=["exited with status = 74"])
  File ".../lldb/packages/Python/lldbsuite/test/lldbtest.py", line 2295, in 
expect
self.runCmd(
  File ".../lldb/packages/Python/lldbsuite/test/lldbtest.py", line 1998, in 
runCmd
self.assertTrue(self.res.Succeeded(),

AssertionError: False is not True : Command 'process launch
Error output:
error: failed to get reply to handshake packet within timeout of 0.0 seconds
' did not return successfully


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127702

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

[Lldb-commits] [PATCH] D126513: Add -b (--continue-to-breakpoint) option to the "process continue" command

[Caroline Tice via Phabricator via lldb-commits]

cmtice added a comment.

Ignore my revert request; Your later commit seems to have fixed most of my 
issues.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126513

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

[Lldb-commits] [PATCH] D126513: Add -b (--continue-to-breakpoint) option to the "process continue" command

[Caroline Tice via Phabricator via lldb-commits]

cmtice added a comment.

This appears to have broken a lot of LLDB tests in our bootstrap as well. Is 
there any chance this might get reverted?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126513

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

[Lldb-commits] [PATCH] D104826: Replace SVE_PT* macros in NativeRegisterContextLinux_arm64.{cpp, h} with their equivalent defintions in LinuxPTraceDefines_arm64sve.h

[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 rG05915400b7f9: [lldb] Replace SVE_PT* macros in 
NativeRegisterContextLinux_arm64.{cpp,h} with… (authored by cmtice).
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Changed prior to commit:
  https://reviews.llvm.org/D104826?vs=354109=355592#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104826

Files:
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h

Index: lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
===
--- lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
+++ lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
@@ -12,6 +12,7 @@
 #define lldb_NativeRegisterContextLinux_arm64_h
 
 #include "Plugins/Process/Linux/NativeRegisterContextLinux.h"
+#include "Plugins/Process/Utility/LinuxPTraceDefines_arm64sve.h"
 #include "Plugins/Process/Utility/NativeRegisterContextDBReg_arm64.h"
 #include "Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h"
 
@@ -90,7 +91,7 @@
   m_fpr; // floating-point registers including extended register sets.
 
   SVEState m_sve_state;
-  struct user_sve_header m_sve_header;
+  struct sve::user_sve_header m_sve_header;
   std::vector m_sve_ptrace_payload;
 
   bool m_refresh_hwdebug_info;
Index: lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
===
--- lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
+++ lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
@@ -58,7 +58,7 @@
   switch (target_arch.GetMachine()) {
   case llvm::Triple::arm:
 return std::make_unique(target_arch,
- native_thread);
+native_thread);
   case llvm::Triple::aarch64: {
 // Configure register sets supported by this AArch64 target.
 // Read SVE header to check for SVE support.
@@ -207,15 +207,15 @@
   if (reg == GetRegisterInfo().GetRegNumFPSR()) {
 sve_reg_num = reg;
 if (m_sve_state == SVEState::Full)
-  offset = SVE_PT_SVE_FPSR_OFFSET(sve_vq_from_vl(m_sve_header.vl));
+  offset = sve::PTraceFPSROffset(sve::vq_from_vl(m_sve_header.vl));
 else if (m_sve_state == SVEState::FPSIMD)
-  offset = SVE_PT_FPSIMD_OFFSET + (32 * 16);
+  offset = sve::ptrace_fpsimd_offset + (32 * 16);
   } else if (reg == GetRegisterInfo().GetRegNumFPCR()) {
 sve_reg_num = reg;
 if (m_sve_state == SVEState::Full)
-  offset = SVE_PT_SVE_FPCR_OFFSET(sve_vq_from_vl(m_sve_header.vl));
+  offset = sve::PTraceFPCROffset(sve::vq_from_vl(m_sve_header.vl));
 else if (m_sve_state == SVEState::FPSIMD)
-  offset = SVE_PT_FPSIMD_OFFSET + (32 * 16) + 4;
+  offset = sve::ptrace_fpsimd_offset + (32 * 16) + 4;
   } else {
 // Extract SVE Z register value register number for this reg_info
 if (reg_info->value_regs &&
@@ -341,15 +341,15 @@
   if (reg == GetRegisterInfo().GetRegNumFPSR()) {
 sve_reg_num = reg;
 if (m_sve_state == SVEState::Full)
-  offset = SVE_PT_SVE_FPSR_OFFSET(sve_vq_from_vl(m_sve_header.vl));
+  offset = sve::PTraceFPSROffset(sve::vq_from_vl(m_sve_header.vl));
 else if (m_sve_state == SVEState::FPSIMD)
-  offset = SVE_PT_FPSIMD_OFFSET + (32 * 16);
+  offset = sve::ptrace_fpsimd_offset + (32 * 16);
   } else if (reg == GetRegisterInfo().GetRegNumFPCR()) {
 sve_reg_num = reg;
 if (m_sve_state == SVEState::Full)
-  offset = SVE_PT_SVE_FPCR_OFFSET(sve_vq_from_vl(m_sve_header.vl));
+  offset = sve::PTraceFPCROffset(sve::vq_from_vl(m_sve_header.vl));
 else if (m_sve_state == SVEState::FPSIMD)
-  offset = SVE_PT_FPSIMD_OFFSET + (32 * 16) + 4;
+  offset = sve::ptrace_fpsimd_offset + (32 * 16) + 4;
   } else {
 // Extract SVE Z register value register number for this reg_info
 if (reg_info->value_regs &&
@@ -824,19 +824,21 @@
 if (error.Success()) {
   // If SVE is enabled thread can switch between SVEState::FPSIMD and
   // SVEState::Full on every stop.
-  if ((m_sve_header.flags & SVE_PT_REGS_MASK) == SVE_PT_REGS_FPSIMD)
+  if ((m_sve_header.flags & sve::ptrace_regs_mask) ==
+  sve::ptrace_regs_fpsimd)
 m_sve_state = SVEState::FPSIMD;
-  else if ((m_sve_header.flags & SVE_PT_REGS_MASK) == SVE_PT_REGS_SVE)
+  else if ((m_sve_header.flags & sve::ptrace_regs_mask) ==
+   

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

[Caroline Tice via Phabricator via lldb-commits]

cmtice added a comment.

Pavel's test case passed on my local machine (x86_64 linux workstation).


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.

[Caroline Tice via Phabricator via lldb-commits]

cmtice added a comment.

Hi Pavel,

Thank you for both the detailed explanation and the updated test case (and 
thank you Jim for your recommendations as well!).  I'm testing the updated test 
case now.  Assuming the tests pass, is it ok for me to re-land this CL using 
the updated test case?  and if so, do I need to upload the updated patch to 
this code review?


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.

[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] [PATCH] D97786: LLDB: Use path relative to binary, not relative to debugger CWD, for finding .dwo files.

[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] [PATCH] D97786: LLDB: Use path relative to binary, not relative to debugger CWD, for finding .dwo files.

[Caroline Tice via Phabricator via lldb-commits]

cmtice added a comment.

I did leave 'main' in the .s file, but it's not very big.  Is this ok now?


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.

[Caroline Tice via Phabricator via lldb-commits]

cmtice updated this revision to Diff 337622.
cmtice added a comment.

Update test case to use lldb on .o file and not run the inferior.


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   # DW_AT_high_pc
+	.long	.Laddr_table_base0  # DW_AT_addr_base
+.Ldebug_info_end0:
+	.section	.debug_str_offsets,"",@progbits
+	.long	

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

[Caroline Tice via Phabricator via lldb-commits]

cmtice added a comment.

Pavel, your last comment lost me completely.  How can I test the code I added 
to lldb if I don't run lldb?  I am a complete newbie at writing test cases so 
there's probably something basic I'm missing?  How would I observe the value of 
the variable without running lldb?  Also, if the program doesn't contain 
'main', won't the compiler complain?  Is there an existing test that does what 
you're suggesting that I could look at?


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.

[Caroline Tice via Phabricator via lldb-commits]

cmtice updated this revision to Diff 336276.
cmtice added a comment.
Herald added a reviewer: alexshap.

Add a test case.


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,390 @@
+# 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: ld.lld %t.o -o %T/dwo-relative-path
+
+# RUN: cd ../..
+
+# RUN: %lldb %T/dwo-relative-path -o "br set -n main" -o run -o step -o "frame var x" -b 2>&1 | FileCheck %s
+# CHECK: stop reason = breakpoint
+# CHECK: x = 10
+
+	.text
+	.file	"dwo-relative-path.cpp"
+	.file	0 "." "dwo-relative-path.cpp" md5 0x9dfc8a64128702e53fdbd698fad5ffa8
+	.globl	main# -- Begin function main
+	.p2align	4, 0x90
+	.type	main,@function
+main:   # @main
+.Lfunc_begin0:
+	.loc	0 12 0  # dwo-relative-path.cpp:12: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 13 7 prologue_end # dwo-relative-path.cpp:13:7
+	movl	$10, -20(%rbp)
+	.loc	0 14 8  # dwo-relative-path.cpp:14:8
+	movw	.L__const.main.y, %ax
+	movw	%ax, -23(%rbp)
+	movb	.L__const.main.y+2, %al
+	movb	%al, -21(%rbp)
+	.loc	0 16 14 # dwo-relative-path.cpp:16:14
+	movsbl	-23(%rbp), %eax
+	.loc	0 16 27 is_stmt 0   # dwo-relative-path.cpp:16:27
+	movsbl	-22(%rbp), %ecx
+	.loc	0 16 19 # dwo-relative-path.cpp:16:19
+	addl	%ecx, %eax
+	.loc	0 16 5  # dwo-relative-path.cpp:16:5
+	addl	-20(%rbp), %eax
+	movl	%eax, -20(%rbp)
+	.loc	0 18 3 is_stmt 1# dwo-relative-path.cpp:18: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	.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	6502600918997875500
+	.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	0   # DW_AT_low_pc
+	.long	.Lfunc_end0-.Lfunc_begin0   # DW_AT_high_pc
+	.long	.Laddr_table_base0  # 

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

[Caroline Tice via Phabricator via lldb-commits]

cmtice added a comment.

It has taken a bit of time to get through the GDB reviews, but the change to 
GDB was accepted and committed:  
https://sourceware.org/pipermail/gdb-cvs/2021-April/050267.html

May I commit this change to LLDB now?


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.

[Caroline Tice via Phabricator via lldb-commits]

cmtice added a comment.

In D97786#2607275 , @labath wrote:

> In D97786#2603656 , @cmtice wrote:
>
>> I'm not sure about using target.debug-file-search-paths, and what the 
>> changes Pavel is suggesting would entail.
>
> I imagine it would involve calling Symbols::LocateExecutableSymbolFile to 
> locate the dwo file, similar to how we do that for dwp files (see 
> `SymbolFileDWARF::GetDwpSymbolFile`). (Disclaimer: I have not tried doing 
> this, so I don't know if that function would work out-of-the-box.)
> Note that I myself am not convinced that this is the right solution (that 
> function is rather heavy), but it does solve the problem of "how do we let 
> the user specify/choose the location of the dwo files" (answer: put the path 
> in the target.debug-file-search-paths setting), and it would search the cwd 
> and the module directory automatically. And the "heavy-ness" of the function 
> is moderated by the fact that it is a no-op for absolute paths to existing 
> files.
>
>> But the real question I am trying to resolve here is not "what are all the 
>> places we should be searching for the .dwo files?" but "When we're given a 
>> *relative path* in the DWARF for finding the files, what should it be 
>> relative TO?".
>
> I'm sorry, but what's the difference between those two questions? Is it about 
> the fact that the second question sort of implies that there should only be 
> one correct location where we should be searching?

I think I'm saying this poorly (I apologize).  Let me try a bit differently.  
It's a matter of defaults.  If the user builds with split debug and relative 
paths, and does not ever move the built binary:  1). it seems wrong to me that 
the debugger will sometimes find the .dwo files and sometimes will not, 
depending on where the debugger was launched from.  It ought, at the very 
least, to behave consistently no matter where the debugger was launched from.  
2).It is not possible that the compiler could ever have intended that paths to 
be relative to the location from where the debugger was launched because the 
compiler could have absolutely no idea where that would be.  So when the 
compiler generated the relative paths, whatever it was truly intended to be 
relative to, the location from which the debugger was launched could not be it. 
 3).  Given statements 1 & 2, it seems reasonable that the *default* behavior 
of the debugger is to search relative to the location of the binary.

Yes, the user could always use debugger settings to add or update search paths, 
but I don't think that should be required as part of the default behavior for 
finding the .dwo files.

>> I still think the most correct answer to that question is "relative to the 
>> location of the binary" (I know, this is assuming that the binary has not 
>> been moved since it was built, or all bets are off).  If you like, I can 
>> update this patch so that it continues to search relative to the cwd (where 
>> the debugger was launched), and IN ADDITION, will search relative to where 
>> the binary is.
>
> I don't really have a strong opinion either way -- the thing I'm mostly 
> interested in is some sort of consistency between lldb & gdb. Have you 
> already discussed this with the gdb folks? If they want to move to the 
> binary-relative mechanism, then I don't see a problem with us doing the same. 
> If they want to keep the existing behavior, then I think it would be good to 
> preserve that in lldb as well (but I don't have a problem with searching in 
> other places too; and `LocateExecutableSymbolFile` is one way to achieve 
> that).

I have submitted a patch for this to GDB; it is currently under review.  I am 
ok with waiting for this (LLDB) patch to be resolved until the GDB one is.


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.

[Caroline Tice via Phabricator via lldb-commits]

cmtice updated this revision to Diff 328191.
cmtice added a comment.

Update to incorporate Pavel's suggested simplification.


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

https://reviews.llvm.org/D97786

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


Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -1639,6 +1639,13 @@
   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);
   }


Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -1639,6 +1639,13 @@
   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);
   }
___
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.

[Caroline Tice via Phabricator via lldb-commits]

cmtice added a comment.

I'm not sure about using target.debug-file-search-paths, and what the changes 
Pavel is suggesting would entail.  But the real question I am trying to resolve 
here is not "what are all the places we should be searching for the .dwo 
files?" but "When we're given a *relative path* in the DWARF for finding the 
files, what should it be relative TO?".

I still think the most correct answer to that question is "relative to the 
location of the binary" (I know, this is assuming that the binary has not been 
moved since it was built, or all bets are off).  If you like, I can update this 
patch so that it continues to search relative to the cwd (where the debugger 
was launched), and IN ADDITION, will search relative to where the binary is.


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.

[Caroline Tice via Phabricator via lldb-commits]

cmtice added a comment.

It is true that the compiler will not know where the final executable will be 
placed, and if the executable gets moved then debugging with .dwo files will 
probably not work anyway,  However as it is currently written, LLDB will fail 
to find the .dwo files,  *even when the binary has not been moved*, if the 
debugger is launched from any directory other than the one containing the 
binary.

E.g. we have a team where the standard workflow is  'cd src'; build binary in 
src/out/Debug/.; from src, do 'lldb out/Debug/binary'.  lldb  cannot find the 
.dwo files.

re "GDB does it this way" -- I am also writing/submitting a patch to get GDB to 
change this behavior.


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.

[Caroline Tice via Phabricator via lldb-commits]

cmtice updated this revision to Diff 327523.
cmtice added a comment.

Upload the correct patch this time. (sorry!)


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

https://reviews.llvm.org/D97786

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


Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -1639,6 +1639,18 @@
   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.
+  ModuleSpec module_spec;
+  module_spec.GetFileSpec() = m_objfile_sp->GetFileSpec();
+  module_spec.GetSymbolFileSpec() =
+  FileSpec(m_objfile_sp->GetModule()->GetFileSpec().GetPath());
+  llvm::StringRef exe_dir =
+  module_spec.GetFileSpec().GetDirectory().GetStringRef();
+  dwo_file.PrependPathComponent(exe_dir);
+}
 FileSystem::Instance().Resolve(dwo_file);
 dwo_file.AppendPathComponent(dwo_name);
   }


Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -1639,6 +1639,18 @@
   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.
+  ModuleSpec module_spec;
+  module_spec.GetFileSpec() = m_objfile_sp->GetFileSpec();
+  module_spec.GetSymbolFileSpec() =
+  FileSpec(m_objfile_sp->GetModule()->GetFileSpec().GetPath());
+  llvm::StringRef exe_dir =
+  module_spec.GetFileSpec().GetDirectory().GetStringRef();
+  dwo_file.PrependPathComponent(exe_dir);
+}
 FileSystem::Instance().Resolve(dwo_file);
 dwo_file.AppendPathComponent(dwo_name);
   }
___
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.

[Caroline Tice via Phabricator via lldb-commits]

cmtice created this revision.
cmtice added a reviewer: jingham.
Herald added subscribers: cishida, hiraditya.
Herald added a reviewer: int3.
Herald added a project: lld-macho.
Herald added a reviewer: lld-macho.
cmtice requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added subscribers: llvm-commits, lldb-commits, sstefan1.
Herald added projects: LLDB, LLVM.

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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97786

Files:
  lld/MachO/Driver.h
  lld/MachO/InputFiles.cpp
  lld/test/MachO/Inputs/iPhoneSimulator.sdk/usr/lib/libReexportSystem.tbd
  lld/test/MachO/Inputs/iPhoneSimulator.sdk/usr/lib/libSystem.tbd
  lld/test/MachO/reexport-nested-lib.s
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  llvm/include/llvm/TextAPI/MachO/InterfaceFile.h
  llvm/lib/TextAPI/MachO/InterfaceFile.cpp

Index: llvm/lib/TextAPI/MachO/InterfaceFile.cpp
===
--- llvm/lib/TextAPI/MachO/InterfaceFile.cpp
+++ llvm/lib/TextAPI/MachO/InterfaceFile.cpp
@@ -124,6 +124,7 @@
   const std::shared_ptr ) {
  return LHS->InstallName < RHS->InstallName;
});
+  Document->Parent = this;
   Documents.insert(Pos, Document);
 }
 
Index: llvm/include/llvm/TextAPI/MachO/InterfaceFile.h
===
--- llvm/include/llvm/TextAPI/MachO/InterfaceFile.h
+++ llvm/include/llvm/TextAPI/MachO/InterfaceFile.h
@@ -338,6 +338,9 @@
   ///\param Document The library to inline with top level library.
   void addDocument(std::shared_ptr &);
 
+  /// Returns the pointer to parent document if exists or nullptr otherwise.
+  InterfaceFile *getParent() const { return Parent; }
+
   /// Get the list of inlined libraries.
   ///
   /// \return Returns a list of the inlined frameworks.
@@ -431,6 +434,7 @@
   std::vector> Documents;
   std::vector> UUIDs;
   SymbolMapType Symbols;
+  InterfaceFile *Parent = nullptr;
 };
 
 template 
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -1639,6 +1639,18 @@
   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.
+  ModuleSpec module_spec;
+  module_spec.GetFileSpec() = m_objfile_sp->GetFileSpec();
+  module_spec.GetSymbolFileSpec() =
+  FileSpec(m_objfile_sp->GetModule()->GetFileSpec().GetPath());
+  llvm::StringRef exe_dir =
+  module_spec.GetFileSpec().GetDirectory().GetStringRef();
+  dwo_file.PrependPathComponent(exe_dir);
+}
 FileSystem::Instance().Resolve(dwo_file);
 dwo_file.AppendPathComponent(dwo_name);
   }
Index: lld/test/MachO/reexport-nested-lib.s
===
--- /dev/null
+++ lld/test/MachO/reexport-nested-lib.s
@@ -0,0 +1,28 @@
+# REQUIRES: x86
+#
+# This tests that we can reference symbols from a dylib,
+# re-exported by a top-level tapi document, which itself is
+# re-exported by another top-level tapi document.
+#
+# RUN: rm -rf %t; mkdir -p %t
+# RUN: llvm-mc -filetype obj -triple x86_64-apple-darwin %s -o %t/test.o
+# RUN: %lld -o %t/test -syslibroot %S/Inputs/iPhoneSimulator.sdk -lReexportSystem %t/test.o
+# RUN: llvm-objdump %t/test --macho --bind %t/test | FileCheck %s
+
+# CHECK: segment  section   addresstypeaddend  dylib   symbol
+# CHECK: __DATA   __data0x{{[0-9a-f]*}}pointer 0   libReexportSystem __crashreporter_info__
+# CHECK: __DATA   __data0x{{[0-9a-f]*}}pointer 0   libReexportSystem _cache_create
+
+.text
+.globl _main
+
+_main:
+  ret
+
+.data
+// This symbol is from libSystem, which is re-exported by libReexportSystem.
+// Reference it here to verify that it is visible.
+.quad __crashreporter_info__
+
+// This symbol is from /usr/lib/system/libcache.dylib, which is re-exported in libSystem.
+.quad _cache_create