[Lldb-commits] [PATCH] D130307: [LLDB][Reliability] Fix register value unpacking

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

So I looked into what *should* be testing this and if it was done 
comprehensively it would have been testing it. However the Arm emulation tests:

- Don't check D registers directly, only S (which is why we didn't miss the 
upper 32 bits).
- Don't verify that memory expected matches memory got (memory is mostly used 
as an input to registers).
- Don't specify the final state of memory anyway.

I got some way into fixing that and found ~50 tests that use memory but don't 
verify the content.

Which is to say, yes this can and should have been tested :)

I will put up some patches shortly to build on this and maybe make the memory 
checking optional. Then this can be tested, and I'll work towards adding memory 
checks to the other tests over time.

So hold off on landing while I come up with that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130307

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

[Lldb-commits] [PATCH] D130462: [LLDB][ARM] Generalise adding register state in emulation tests

[David Spickett via Phabricator via lldb-commits]

DavidSpickett created this revision.
Herald added a subscriber: kristof.beyls.
Herald added a project: All.
DavidSpickett requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Since some s and d registers overlap we will error if we find both.
This prevents you overwriting one with the other in a test case.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D130462

Files:
  lldb/source/Plugins/Instruction/ARM/EmulationStateARM.cpp
  lldb/source/Plugins/Instruction/ARM/EmulationStateARM.h


Index: lldb/source/Plugins/Instruction/ARM/EmulationStateARM.h
===
--- lldb/source/Plugins/Instruction/ARM/EmulationStateARM.h
+++ lldb/source/Plugins/Instruction/ARM/EmulationStateARM.h
@@ -61,6 +61,10 @@
   const lldb_private::RegisterValue ®_value);
 
 private:
+  bool LoadRegistersStateFromDictionary(
+  lldb_private::OptionValueDictionary *reg_dict, char kind, int first_reg,
+  int num);
+
   uint32_t m_gpr[17] = {0};
   struct _sd_regs {
 uint32_t s_regs[32]; // sregs 0 - 31 & dregs 0 - 15
Index: lldb/source/Plugins/Instruction/ARM/EmulationStateARM.cpp
===
--- lldb/source/Plugins/Instruction/ARM/EmulationStateARM.cpp
+++ lldb/source/Plugins/Instruction/ARM/EmulationStateARM.cpp
@@ -272,6 +272,23 @@
   return match;
 }
 
+bool EmulationStateARM::LoadRegistersStateFromDictionary(
+OptionValueDictionary *reg_dict, char kind, int first_reg, int num) {
+  StreamString sstr;
+  for (int i = 0; i < num; ++i) {
+sstr.Clear();
+sstr.Printf("%c%d", kind, i);
+OptionValueSP value_sp =
+reg_dict->GetValueForKey(ConstString(sstr.GetString()));
+if (value_sp.get() == nullptr)
+  return false;
+uint64_t reg_value = value_sp->GetUInt64Value();
+StorePseudoRegisterValue(first_reg + i, reg_value);
+  }
+
+  return true;
+}
+
 bool EmulationStateARM::LoadStateFromDictionary(
 OptionValueDictionary *test_data) {
   static ConstString memory_key("memory");
@@ -321,18 +338,8 @@
   // Load General Registers
 
   OptionValueDictionary *reg_dict = value_sp->GetAsDictionary();
-
-  StreamString sstr;
-  for (int i = 0; i < 16; ++i) {
-sstr.Clear();
-sstr.Printf("r%d", i);
-ConstString reg_name(sstr.GetString());
-value_sp = reg_dict->GetValueForKey(reg_name);
-if (value_sp.get() == nullptr)
-  return false;
-uint64_t reg_value = value_sp->GetUInt64Value();
-StorePseudoRegisterValue(dwarf_r0 + i, reg_value);
-  }
+  if (!LoadRegistersStateFromDictionary(reg_dict, 'r', dwarf_r0, 16))
+return false;
 
   static ConstString cpsr_name("cpsr");
   value_sp = reg_dict->GetValueForKey(cpsr_name);
@@ -341,16 +348,13 @@
   StorePseudoRegisterValue(dwarf_cpsr, value_sp->GetUInt64Value());
 
   // Load s/d Registers
-  for (int i = 0; i < 32; ++i) {
-sstr.Clear();
-sstr.Printf("s%d", i);
-ConstString reg_name(sstr.GetString());
-value_sp = reg_dict->GetValueForKey(reg_name);
-if (value_sp.get() == nullptr)
-  return false;
-uint64_t reg_value = value_sp->GetUInt64Value();
-StorePseudoRegisterValue(dwarf_s0 + i, reg_value);
-  }
-
-  return true;
+  // To prevent you giving both types in a state and overwriting
+  // one or the other, we'll expect to get either all S registers,
+  // or all D registers. Not a mix of the two.
+  bool found_s_registers =
+  LoadRegistersStateFromDictionary(reg_dict, 's', dwarf_s0, 32);
+  bool found_d_registers =
+  LoadRegistersStateFromDictionary(reg_dict, 'd', dwarf_d0, 32);
+
+  return found_s_registers != found_d_registers;
 }


Index: lldb/source/Plugins/Instruction/ARM/EmulationStateARM.h
===
--- lldb/source/Plugins/Instruction/ARM/EmulationStateARM.h
+++ lldb/source/Plugins/Instruction/ARM/EmulationStateARM.h
@@ -61,6 +61,10 @@
   const lldb_private::RegisterValue ®_value);
 
 private:
+  bool LoadRegistersStateFromDictionary(
+  lldb_private::OptionValueDictionary *reg_dict, char kind, int first_reg,
+  int num);
+
   uint32_t m_gpr[17] = {0};
   struct _sd_regs {
 uint32_t s_regs[32]; // sregs 0 - 31 & dregs 0 - 15
Index: lldb/source/Plugins/Instruction/ARM/EmulationStateARM.cpp
===
--- lldb/source/Plugins/Instruction/ARM/EmulationStateARM.cpp
+++ lldb/source/Plugins/Instruction/ARM/EmulationStateARM.cpp
@@ -272,6 +272,23 @@
   return match;
 }
 
+bool EmulationStateARM::LoadRegistersStateFromDictionary(
+OptionValueDictionary *reg_dict, char kind, int first_reg, int num) {
+  StreamString sstr;
+  for (int i = 0; i < num; ++i) {
+sstr.Clear();
+sstr.Printf("%c%d", kind, i);
+OptionValueSP value_sp =
+reg_dict->GetValueForKey(ConstString(sstr.GetString()));
+if (value_sp.get() == null

[Lldb-commits] [PATCH] D130464: [lldb][ARM] Print mismatched registers in emulation tests

[David Spickett via Phabricator via lldb-commits]

DavidSpickett created this revision.
Herald added a subscriber: kristof.beyls.
Herald added a project: All.
DavidSpickett requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Also correct the test failed message. It implies that what
it's done is compare the 'before' and 'ater' states from the
test input.

Except that that's the whole point of the test, that the state changes.
It should tell you that it compared the result of the emulation to the
'after'.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D130464

Files:
  lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.cpp
  lldb/source/Plugins/Instruction/ARM/EmulationStateARM.cpp
  lldb/source/Plugins/Instruction/ARM/EmulationStateARM.h


Index: lldb/source/Plugins/Instruction/ARM/EmulationStateARM.h
===
--- lldb/source/Plugins/Instruction/ARM/EmulationStateARM.h
+++ lldb/source/Plugins/Instruction/ARM/EmulationStateARM.h
@@ -36,7 +36,8 @@
 
   bool LoadStateFromDictionary(lldb_private::OptionValueDictionary *test_data);
 
-  bool CompareState(EmulationStateARM &other_state);
+  bool CompareState(EmulationStateARM &other_state,
+lldb_private::Stream *out_stream);
 
   static size_t
   ReadPseudoMemory(lldb_private::EmulateInstruction *instruction, void *baton,
Index: lldb/source/Plugins/Instruction/ARM/EmulationStateARM.cpp
===
--- lldb/source/Plugins/Instruction/ARM/EmulationStateARM.cpp
+++ lldb/source/Plugins/Instruction/ARM/EmulationStateARM.cpp
@@ -251,22 +251,32 @@
 reg_value.GetAsUInt64());
 }
 
-bool EmulationStateARM::CompareState(EmulationStateARM &other_state) {
+bool EmulationStateARM::CompareState(EmulationStateARM &other_state,
+ Stream *out_stream) {
   bool match = true;
 
   for (int i = 0; match && i < 17; ++i) {
-if (m_gpr[i] != other_state.m_gpr[i])
+if (m_gpr[i] != other_state.m_gpr[i]) {
   match = false;
+  out_stream->Printf("r%d: 0x%x != 0x%x\n", i, m_gpr[i],
+ other_state.m_gpr[i]);
+}
   }
 
   for (int i = 0; match && i < 32; ++i) {
-if (m_vfp_regs.s_regs[i] != other_state.m_vfp_regs.s_regs[i])
+if (m_vfp_regs.s_regs[i] != other_state.m_vfp_regs.s_regs[i]) {
   match = false;
+  out_stream->Printf("s%d: 0x%x != 0x%x\n", i, m_vfp_regs.s_regs[i],
+ other_state.m_vfp_regs.s_regs[i]);
+}
   }
 
   for (int i = 0; match && i < 16; ++i) {
-if (m_vfp_regs.d_regs[i] != other_state.m_vfp_regs.d_regs[i])
+if (m_vfp_regs.d_regs[i] != other_state.m_vfp_regs.d_regs[i]) {
   match = false;
+  out_stream->Printf("d%d: 0x%lx != 0x%lx\n", i + 16, m_vfp_regs.d_regs[i],
+ other_state.m_vfp_regs.d_regs[i]);
+}
   }
 
   return match;
Index: lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.cpp
===
--- lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.cpp
+++ lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.cpp
@@ -14453,10 +14453,10 @@
 return false;
   }
 
-  success = before_state.CompareState(after_state);
+  success = before_state.CompareState(after_state, out_stream);
   if (!success)
 out_stream->Printf(
-"TestEmulation:  'before' and 'after' states do not match.\n");
+"TestEmulation:  State after emulation does not match 'after' 
state.\n");
 
   return success;
 }


Index: lldb/source/Plugins/Instruction/ARM/EmulationStateARM.h
===
--- lldb/source/Plugins/Instruction/ARM/EmulationStateARM.h
+++ lldb/source/Plugins/Instruction/ARM/EmulationStateARM.h
@@ -36,7 +36,8 @@
 
   bool LoadStateFromDictionary(lldb_private::OptionValueDictionary *test_data);
 
-  bool CompareState(EmulationStateARM &other_state);
+  bool CompareState(EmulationStateARM &other_state,
+lldb_private::Stream *out_stream);
 
   static size_t
   ReadPseudoMemory(lldb_private::EmulateInstruction *instruction, void *baton,
Index: lldb/source/Plugins/Instruction/ARM/EmulationStateARM.cpp
===
--- lldb/source/Plugins/Instruction/ARM/EmulationStateARM.cpp
+++ lldb/source/Plugins/Instruction/ARM/EmulationStateARM.cpp
@@ -251,22 +251,32 @@
 reg_value.GetAsUInt64());
 }
 
-bool EmulationStateARM::CompareState(EmulationStateARM &other_state) {
+bool EmulationStateARM::CompareState(EmulationStateARM &other_state,
+ Stream *out_stream) {
   bool match = true;
 
   for (int i = 0; match && i < 17; ++i) {
-if (m_gpr[i] != other_state.m_gpr[i])
+if (m_gpr[i] != other_state.m_gpr[i]) {
   match = false

[Lldb-commits] [PATCH] D130467: [lldb][ARM] Misc improvements to TestEmulations

[David Spickett via Phabricator via lldb-commits]

DavidSpickett created this revision.
Herald added a subscriber: kristof.beyls.
Herald added a project: All.
DavidSpickett requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

- Look for files that end width arm/thumb.dat, meaning we don't try to run, for 
example, vim swap files.
- Print the name of the test that failed.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D130467

Files:
  lldb/test/API/arm/emulation/TestEmulations.py


Index: lldb/test/API/arm/emulation/TestEmulations.py
===
--- lldb/test/API/arm/emulation/TestEmulations.py
+++ lldb/test/API/arm/emulation/TestEmulations.py
@@ -20,7 +20,7 @@
 files = os.listdir(test_dir)
 thumb_files = list()
 for f in files:
-if '-thumb.dat' in f:
+if f.endswith('-thumb.dat'):
 thumb_files.append(f)
 
 for f in thumb_files:
@@ -33,7 +33,7 @@
 files = os.listdir(test_dir)
 arm_files = list()
 for f in files:
-if '-arm.dat' in f:
+if f.endswith('-arm.dat'):
 arm_files.append(f)
 
 for f in arm_files:
@@ -49,4 +49,5 @@
 print('\nRunning test ' + os.path.basename(filename))
 print(output)
 
-self.assertTrue(success, 'Emulation test failed.')
+self.assertTrue(success, 'Emulation test {} failed.'.format(
+filename))


Index: lldb/test/API/arm/emulation/TestEmulations.py
===
--- lldb/test/API/arm/emulation/TestEmulations.py
+++ lldb/test/API/arm/emulation/TestEmulations.py
@@ -20,7 +20,7 @@
 files = os.listdir(test_dir)
 thumb_files = list()
 for f in files:
-if '-thumb.dat' in f:
+if f.endswith('-thumb.dat'):
 thumb_files.append(f)
 
 for f in thumb_files:
@@ -33,7 +33,7 @@
 files = os.listdir(test_dir)
 arm_files = list()
 for f in files:
-if '-arm.dat' in f:
+if f.endswith('-arm.dat'):
 arm_files.append(f)
 
 for f in arm_files:
@@ -49,4 +49,5 @@
 print('\nRunning test ' + os.path.basename(filename))
 print(output)
 
-self.assertTrue(success, 'Emulation test failed.')
+self.assertTrue(success, 'Emulation test {} failed.'.format(
+filename))
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D130468: [lldb][ARM] Add tests for vpush/vpop D registers

[David Spickett via Phabricator via lldb-commits]

DavidSpickett created this revision.
Herald added a subscriber: kristof.beyls.
Herald added a project: All.
DavidSpickett requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Previously we just checked via S regs and were not checking
memory content after pushes.

The vpush test confirms that the fix in https://reviews.llvm.org/D130307
is working.

Memory will only be checked if an "after" state is provided.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D130468

Files:
  lldb/source/Plugins/Instruction/ARM/EmulationStateARM.cpp
  lldb/test/API/arm/emulation/new-test-files/test-vpop-1-dregs-thumb.dat
  lldb/test/API/arm/emulation/new-test-files/test-vpush-1-dregs-thumb.dat

Index: lldb/test/API/arm/emulation/new-test-files/test-vpush-1-dregs-thumb.dat
===
--- /dev/null
+++ lldb/test/API/arm/emulation/new-test-files/test-vpush-1-dregs-thumb.dat
@@ -0,0 +1,125 @@
+InstructionEmulationState={
+assembly_string="vpush{d11, d12, d13, d14}"
+triple=thumb-apple-ios
+opcode=0xed2dbb08
+before_state={
+registers={
+r0=0x
+r1=0x0001
+r2=0x0002
+r3=0x0003
+r4=0x0004
+r5=0x0005
+r6=0x0006
+r7=0x2fdffe60
+r8=0x0008
+r9=0x0009
+r10=0x000a
+r11=0x000b
+r12=0x000c
+r13=0x2fdffe80
+r14=0x2f80
+r15=0x2ff8
+cpsr=0x6030
+d0=0x
+d1=0x
+d2=0x
+d3=0x
+d4=0x
+d5=0x
+d6=0x
+d7=0x
+d8=0x
+d9=0x
+d10=0x
+d11=0x
+d12=0x
+d13=0x
+d14=0x
+d15=0x
+d16=0x
+d17=0x
+d18=0x
+d19=0x
+d20=0x
+d21=0x
+d22=0x
+d23=0x
+d24=0x
+d25=0x
+d26=0x
+d27=0x
+d28=0x
+d29=0x
+d30=0x
+d31=0x
+}
+}
+after_state={
+memory={
+address=0x2fdffe60
+data_encoding=uint32_t
+data=[
+0x
+0x
+0x
+0x
+0x
+0x
+0x
+0x
+]
+}
+registers={
+r0=0x
+r1=0x0001
+r2=0x0002
+r3=0x0003
+r4=0x0004
+r5=0x0005
+r6=0x0006
+r7=0x2fdffe60
+r8=0x0008
+r9=0x0009
+r10=0x000a
+r11=0x000b
+r12=0x000c
+r13=0x2fdffe60
+r14=0x2f80
+r15=0x2ffc
+cpsr=0x6030
+d0=0x
+d1=0x
+d2=0x
+d3=0x
+d4=0x
+d5=0x
+d6=0x
+d7=0x
+d8=0x
+d9=0x
+d10=0x
+d11=0x
+d12=0x
+d13=0x
+d14=0x
+d15=0x
+d16=0x
+d17=0x
+d18=0x
+d19=0x
+d20=0x
+d21=0x
+d22=0x
+d23=0x
+d24=0x
+d25=0x
+d26=0x
+d27=0x
+d28=0x
+d29=0x
+d30=0x
+d31=0x
+}
+}
+}
Index: lldb/test/API/arm/emulation/new-test-files/test-vpop-1-dregs-thumb.dat
===
--- /dev/null
+++ lldb/test/API/arm/emulation/new-test-files/test-vpop-1-dregs-thumb.dat
@@ -0,0 +1,125 @@
+InstructionEmulationState={
+assembly_string="vpop{d11, d12, d13, d14}"
+triple=thumb-apple-ios
+opcode=0xecbdbb08
+before_state={
+memory={
+address=0x2fdffe60
+data_encoding=uint32_t
+data=[
+0x
+0x
+0x
+0x
+0x
+0x
+0x
+0x
+]
+}
+registers={
+r0=0x
+r1=0x0001
+r2=0x0002
+r3=0x0003
+r4=0x0004
+r5=0x0005
+r6=0x0006
+r7=0x2fdffe60
+r8=0x0008
+r9=0x0009
+r10=0x000a
+r11=0x000b
+r12=0x000c
+r13=0x2fdffe60
+r14=0x2f80
+r15=0x2ff8
+cpsr=0x6030
+d0=0x0
+d1=0x0
+d2=0x0
+d3=0x0
+d4=0x0
+d5=0x0
+d6=0x0
+d7=0x0
+d8=0x0
+d9=0x0
+d10=0x0
+d11=0x0
+d12=0x0
+d13=0x0
+d14=0x0
+d15=0x0
+d16=0x0
+d17=0x0
+d18=0x0
+d19=0x0
+d20=0x0
+d21=0x0
+d22=0x0
+d23=0x0
+d24=0x0
+d25=0x0
+d26=0x0
+d27=0x0
+d28=0x0
+d29=0x0
+d30=0x0
+d31=0x0
+}
+}
+after_state={
+registers={
+r0=0x
+r1=0x0001
+r2=0x0002
+r3=0x0003
+r4=0x0004
+r5=0x0005
+r6=0x0006
+r7=0x2fdffe60
+r8=0x0008
+r9=0x0009
+r10=0x000a
+r11=0x000b
+r12=0x000c
+r13=0x2fdffe80
+r14=0x2f80
+r15=0x2ffc
+cpsr=0x6030
+d0=0x
+d1=0x
+d2=0x
+d3=0x
+d4=0x
+d5=0x
+d6=0x
+d7=0x
+

[Lldb-commits] [PATCH] D130468: [lldb][ARM] Add tests for vpush/vpop D registers

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a reviewer: clayborg.
DavidSpickett added inline comments.



Comment at: 
lldb/test/API/arm/emulation/new-test-files/test-vpush-1-dregs-thumb.dat:70
+0x
+0x
+]

This is the part that fails without the fix from 
https://reviews.llvm.org/D130307. Every other word will be zero.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130468

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

[Lldb-commits] [PATCH] D130307: [LLDB][Reliability] Fix register value unpacking

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

I've stacked some reviews onto this that end in a new test that covers this 
change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130307

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

[Lldb-commits] [PATCH] D130342: [LLDB][RISCV] Add Register Info and Context

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added inline comments.



Comment at: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:1347
   case llvm::COFF::IMAGE_FILE_MACHINE_ARM64:
+  case llvm::COFF::IMAGE_FILE_MACHINE_RISCV64:
 ArchSpec arch;

Why is this only needed for PECOFF? (maybe it is the only one that lists them 
like this)



Comment at: 
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_riscv64.cpp:24
+// they define some macros which collide with variable names in other modules
+#include 
+// NT_PRSTATUS and NT_FPREGSET definition

This can be `sys/uio.h` I'm pretty sure.

I know the other NativeRegisterContexts include socket.h but I'm looking at 
changing those.


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

https://reviews.llvm.org/D130342

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

[Lldb-commits] [PATCH] D130045: Implement better path matching in FileSpecList::FindFileIndex(...).

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D130045#310 , @JDevlieghere 
wrote:

> I'm slightly worried about the change to make the new "fuzzy" matching the 
> default. While it makes sense for the breakpoints, I wouldn't generally 
> expect `./a/b/c/main.cpp` to match `/build/a/b/c/main.cpp`,

Would you expect that `main.cpp` "generally" matches `/build/a/b/c/main.cpp`?

(I'm not arguing for/against anything (yet, at least), but I would like to hear 
your reasoning if the answer to the question is "yes".)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130045

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

[Lldb-commits] [PATCH] D130401: Implement better path matching in FileSpecList::FindCompatibleIndex(...).

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: lldb/include/lldb/Core/FileSpecList.h:140-141
+  ///
+  /// \param[in] full
+  /// Should FileSpec::Equal be called with "full" true or false.
+  ///

Is this ever being called with full=false? If not can we drop it?



Comment at: lldb/include/lldb/Utility/FileSpec.h:219-224
+  /// Get the path separator for the current path style.
+  ///
+  /// \return
+  /// A path separator character for this path.
+  char GetPathSeparator() const;
+

Are you sure about this part? It is my impression that we're already storing 
the windows paths in the "normalized" form (using `/` as separator), and so 
there shouldn't be a need to do anything special (at least regarding directory 
separators) when working with windows paths.



Comment at: lldb/source/Core/FileSpecList.cpp:93
+  const bool file_spec_case_sensitive = file_spec.IsCaseSensitive();
+  // When looking for files, we will compare only the filename if the FILE_SPEC
+  // argument is empty

use lower case?



Comment at: lldb/source/Core/FileSpecList.cpp:121-141
+  if (file_spec_case_sensitive && curr_file.IsCaseSensitive()) {
+if (file_spec_dir.consume_back(curr_file_dir)) {
+  if (file_spec_dir.empty() ||
+  file_spec_dir.back() == file_spec.GetPathSeparator())
+return idx;
+} else if (curr_file_dir.consume_back(file_spec_dir)) {
+  if (curr_file_dir.empty() ||

This could be a lot less repetitive. Perhaps something like:
```
const bool comparison_case_sensitive = file_spec_case_sensitive || 
curr_file.IsCaseSensitive(); // I changed this from && to || as that's the 
logic used elsewhere
auto &is_suffix = [&](StringRef a, StringRef b) {
  return a.consume_back(b) && (a.empty() || a.endswith("/"));
};
if (is_suffix(curr_file_dir, file_spec_dir) || is_suffix(file_spec_dir, 
curr_file_dir))
  return idx;
```



Comment at: lldb/unittests/Core/FileSpecListTest.cpp:19-21
+// static FileSpec WindowsSpec(llvm::StringRef path) {
+//   return FileSpec(path, FileSpec::Style::windows);
+// }

It would be nice to have a quick windows test as well, to confirm the separator 
story.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130401

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

[Lldb-commits] [lldb] ae8a660 - [lldb][ARM/AArch64] Use sys/uio.h instead of socket.h in native register context

[David Spickett via lldb-commits]

Author: David Spickett
Date: 2022-07-25T12:35:57Z
New Revision: ae8a6602fb7260cc4b6d07689c8ac80f0fb4d86a

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

LOG: [lldb][ARM/AArch64] Use sys/uio.h instead of socket.h in native register 
context

We only want iovec and uio.h is just that without a lot
of other stuff. Saves me wondering why this code might
want to open sockets.

https://pubs.opengroup.org/onlinepubs/007904975/basedefs/sys/uio.h.html

Added: 


Modified: 
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp

Removed: 




diff  --git 
a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp 
b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
index 10ffe49f6b4e8..07a37514e7ede 100644
--- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
@@ -20,7 +20,7 @@
 #include "lldb/Utility/Status.h"
 
 #include 
-#include 
+#include 
 
 #define REG_CONTEXT_SIZE (GetGPRSize() + sizeof(m_fpr))
 

diff  --git 
a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp 
b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
index cee727877a13d..6022a6a373605 100644
--- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
@@ -27,7 +27,7 @@
 
 // System includes - They have to be included after framework includes because
 // they define some macros which collide with variable names in other modules
-#include 
+#include 
 // NT_PRSTATUS and NT_FPREGSET definition
 #include 
 



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

Re: [Lldb-commits] [lldb] 2622c5e - [lldb] Adapt lldb tests to changes in 71cdb8c6f144

[Pavel Labath via lldb-commits]

On 24/07/2022 00:45, Augusto Noronha via lldb-commits wrote:


Author: Augusto Noronha
Date: 2022-07-23T15:37:26-07:00
New Revision: 2622c5e212646d1c8d6a43444d7c5b551f0221ad

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

LOG: [lldb] Adapt lldb tests to changes in 71cdb8c6f144



Are you sure you have the right commit there?

Somehow I doubt that "[ADT] Use default member initialization (NFC)" 
(https://github.com/llvm/llvm-project/commit/71cdb8c6f144b8f78fed1f39dc6b9c153f9023c1) 
could cause this to break.


pl


Added:
 


Modified:
 lldb/test/API/functionalities/dlopen_other_executable/main.c
 lldb/test/API/macosx/ignore_exceptions/main.c

Removed:
 




diff  --git a/lldb/test/API/functionalities/dlopen_other_executable/main.c 
b/lldb/test/API/functionalities/dlopen_other_executable/main.c
index 8f21e862a2b58..f48f6177e7db0 100644
--- a/lldb/test/API/functionalities/dlopen_other_executable/main.c
+++ b/lldb/test/API/functionalities/dlopen_other_executable/main.c
@@ -4,7 +4,7 @@
  int main() {
int i = 0; // break here
// dlopen the 'other' test executable.
-  int h = dlopen("other", RTLD_LAZY);
+  int h = (int) dlopen("other", RTLD_LAZY);
assert(h && "dlopen failed?");
return i; // break after dlopen
  }

diff  --git a/lldb/test/API/macosx/ignore_exceptions/main.c 
b/lldb/test/API/macosx/ignore_exceptions/main.c
index 682c5f23627e0..b9513aa35b3ed 100644
--- a/lldb/test/API/macosx/ignore_exceptions/main.c
+++ b/lldb/test/API/macosx/ignore_exceptions/main.c
@@ -13,7 +13,7 @@ void
  saction_handler(int signo, siginfo_t info, void *baton) {
printf("Got into handler.\n");   // stop here in the signal handler
kern_return_t success
-  = mach_vm_protect(mach_task_self(), g_int_ptr,
+  = mach_vm_protect(mach_task_self(), (mach_vm_address_t) g_int_ptr,
  g_size, 0, VM_PROT_READ|VM_PROT_WRITE);
g_int_ptr[1] = 20;
  }
@@ -24,7 +24,7 @@ main()
for (int i = 0; i < 10; i++)
  g_int_ptr[i] = i * 10;

-  vm_result = mach_vm_protect(mach_task_self(), g_int_ptr, g_size, 0, VM_PROT_NONE);

+  vm_result = mach_vm_protect(mach_task_self(), (mach_vm_address_t) g_int_ptr, 
g_size, 0, VM_PROT_NONE);
struct sigaction my_action;
sigemptyset(&my_action.sa_mask);
my_action.sa_handler = (void (*)(int)) saction_handler;


 
___

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


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

[Lldb-commits] [PATCH] D130342: [LLDB][RISCV] Add Register Info and Context

[Emmmer S via Phabricator via lldb-commits]

Emmmer updated this revision to Diff 447322.
Emmmer added a comment.

> Why is this only needed for PECOFF? (maybe it is the only one that lists them 
> like this)

At first, I tracked a bug from `LLDBServerTests` and thought that 
`GetArchitecture()` returned a wrong match, so I added `case 
llvm::COFF::IMAGE_FILE_MACHINE_RISCV64` to the function (there is indeed an 
`IMAGE_FILE_MACHINE_RISCV64` in `COFF.h`, so I regarded it was a global 
architecture judgment ).

Later, I found out that `HostInfoBase.cpp` did not add a match to `riscv64`, 
and it ran as expected after adding it.

I forgot to delete it in my local repo, sorry.


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

https://reviews.llvm.org/D130342

Files:
  lldb/source/Host/common/HostInfoBase.cpp
  lldb/source/Plugins/Process/Linux/CMakeLists.txt
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_riscv64.cpp
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_riscv64.h
  lldb/source/Plugins/Process/Utility/CMakeLists.txt
  lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_riscv64.cpp
  lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_riscv64.h
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_riscv64.cpp
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_riscv64.h
  lldb/source/Plugins/Process/Utility/RegisterInfos_riscv64.h
  lldb/source/Utility/ArchSpec.cpp

Index: lldb/source/Utility/ArchSpec.cpp
===
--- lldb/source/Utility/ArchSpec.cpp
+++ lldb/source/Utility/ArchSpec.cpp
@@ -724,6 +724,7 @@
   case llvm::Triple::systemz:
   case llvm::Triple::xcore:
   case llvm::Triple::arc:
+  case llvm::Triple::riscv64:
 return false;
   }
 }
Index: lldb/source/Plugins/Process/Utility/RegisterInfos_riscv64.h
===
--- /dev/null
+++ lldb/source/Plugins/Process/Utility/RegisterInfos_riscv64.h
@@ -0,0 +1,238 @@
+//===-- RegisterInfos_riscv64.h -*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifdef DECLARE_REGISTER_INFOS_RISCV64_STRUCT
+
+#include 
+
+#include "lldb/lldb-defines.h"
+#include "lldb/lldb-enumerations.h"
+#include "lldb/lldb-private.h"
+
+#ifndef GPR_OFFSET
+#error GPR_OFFSET must be defined before including this header file
+#endif
+
+#ifndef GPR_OFFSET_NAME
+#error GPR_OFFSET_NAME must be defined before including this header file
+#endif
+
+#ifndef FPR_OFFSET
+#error FPR_OFFSET must be defined before including this header file
+#endif
+
+#ifndef FPR_OFFSET_NAME
+#error FPR_OFFSET_NAME must be defined before including this header file
+#endif
+
+enum {
+  gpr_x0 = 0,
+  gpr_x1,
+  gpr_x2,
+  gpr_x3,
+  gpr_x4,
+  gpr_x5,
+  gpr_x6,
+  gpr_x7,
+  gpr_x8,
+  gpr_x9,
+  gpr_x10,
+  gpr_x11,
+  gpr_x12,
+  gpr_x13,
+  gpr_x14,
+  gpr_x15,
+  gpr_x16,
+  gpr_x17,
+  gpr_x18,
+  gpr_x19,
+  gpr_x20,
+  gpr_x21,
+  gpr_x22,
+  gpr_x23,
+  gpr_x24,
+  gpr_x25,
+  gpr_x26,
+  gpr_x27,
+  gpr_x28,
+  gpr_x29,
+  gpr_x30,
+  gpr_x31,
+  gpr_ra = gpr_x1,
+  gpr_sp = gpr_x2,
+  gpr_fp = gpr_x8,
+
+  gpr_pc = 32,
+
+  fpr_f0 = 33,
+  fpr_f1,
+  fpr_f2,
+  fpr_f3,
+  fpr_f4,
+  fpr_f5,
+  fpr_f6,
+  fpr_f7,
+  fpr_f8,
+  fpr_f9,
+  fpr_f10,
+  fpr_f11,
+  fpr_f12,
+  fpr_f13,
+  fpr_f14,
+  fpr_f15,
+  fpr_f16,
+  fpr_f17,
+  fpr_f18,
+  fpr_f19,
+  fpr_f20,
+  fpr_f21,
+  fpr_f22,
+  fpr_f23,
+  fpr_f24,
+  fpr_f25,
+  fpr_f26,
+  fpr_f27,
+  fpr_f28,
+  fpr_f29,
+  fpr_f30,
+  fpr_f31,
+
+  fpr_fflags,
+  fpr_frm,
+  fpr_fcsr,
+
+  k_num_registers
+};
+
+// Generates register kinds array with DWARF, EH frame and generic kind
+#define MISC_KIND(reg, type, generic_kind) \
+  { reg, reg, generic_kind, LLDB_INVALID_REGNUM, reg }
+
+// Generates register kinds array for registers with only lldb kind
+#define LLDB_KIND(lldb_kind)   \
+  {\
+LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, \
+LLDB_INVALID_REGNUM, lldb_kind \
+  }
+
+// Generates register kinds array for vector registers
+#define GPR64_KIND(reg, generic_kind) MISC_KIND(reg, gpr, generic_kind)
+
+// FPR register kinds array for vector registers
+#define FPR64_KIND(reg, generic_kind) MISC_KIND(reg, fpr, generic_kind)
+
+#define MISC_FPR_KIND(lldb_kind) LLDB_KIND(lldb_kind)
+
+// Defines a 64-bit general purpose register
+#define DEFINE_GPR64(reg, generic_kind)\
+  {  

[Lldb-commits] [lldb] 883b0d5 - [lldb][AArch64] Add UnpackTagsFromCoreFileSegment to MemoryTagManager

[David Spickett via lldb-commits]

Author: David Spickett
Date: 2022-07-25T15:51:36+01:00
New Revision: 883b0d5b7f873a7d6f8c8ee13c7f6174a2a79a50

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

LOG: [lldb][AArch64] Add UnpackTagsFromCoreFileSegment to MemoryTagManager

This is the first part of support for reading MTE tags from Linux
core files. The format is documented here:
https://www.kernel.org/doc/html/latest/arm64/memory-tagging-extension.html#core-dump-support

This patch adds a method to unpack from the format the core
file uses, which is different to the one chosen for GDB packets.

MemoryTagManagerAArch64MTE is not tied one OS so another OS
might choose a different format in future. However, infrastructure
to handle that would go untested until then so I've chosen not to
attempt to handle that.

Reviewed By: omjavaid

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

Added: 


Modified: 
lldb/include/lldb/Target/MemoryTagManager.h
lldb/source/Plugins/Process/Utility/MemoryTagManagerAArch64MTE.cpp
lldb/source/Plugins/Process/Utility/MemoryTagManagerAArch64MTE.h
lldb/unittests/Process/Utility/MemoryTagManagerAArch64MTETest.cpp

Removed: 




diff  --git a/lldb/include/lldb/Target/MemoryTagManager.h 
b/lldb/include/lldb/Target/MemoryTagManager.h
index 28a8acc34632c..b082224c38edb 100644
--- a/lldb/include/lldb/Target/MemoryTagManager.h
+++ b/lldb/include/lldb/Target/MemoryTagManager.h
@@ -113,6 +113,21 @@ class MemoryTagManager {
   UnpackTagsData(const std::vector &tags,
  size_t granules = 0) const = 0;
 
+  // Unpack tags from a corefile segment containing compressed tags
+  // (compression that may be 
diff erent from the one used for GDB transport).
+  //
+  // This method asumes that:
+  // * addr and len have been granule aligned by a tag manager
+  // * addr >= tag_segment_virtual_address
+  //
+  // 'reader' will always be a wrapper around a CoreFile in real use
+  // but allows testing without having to mock a CoreFile.
+  typedef std::function CoreReaderFn;
+  std::vector virtual UnpackTagsFromCoreFileSegment(
+  CoreReaderFn reader, lldb::addr_t tag_segment_virtual_address,
+  lldb::addr_t tag_segment_data_address, lldb::addr_t addr,
+  size_t len) const = 0;
+
   // Pack uncompressed tags into their storage format (e.g. for gdb QMemTags).
   // Checks that each tag is within the expected value range.
   // We do not check the number of tags or range they apply to because

diff  --git 
a/lldb/source/Plugins/Process/Utility/MemoryTagManagerAArch64MTE.cpp 
b/lldb/source/Plugins/Process/Utility/MemoryTagManagerAArch64MTE.cpp
index b71de4cadb185..e0126d840971e 100644
--- a/lldb/source/Plugins/Process/Utility/MemoryTagManagerAArch64MTE.cpp
+++ b/lldb/source/Plugins/Process/Utility/MemoryTagManagerAArch64MTE.cpp
@@ -247,6 +247,70 @@ MemoryTagManagerAArch64MTE::UnpackTagsData(const 
std::vector &tags,
   return unpacked;
 }
 
+std::vector
+MemoryTagManagerAArch64MTE::UnpackTagsFromCoreFileSegment(
+CoreReaderFn reader, lldb::addr_t tag_segment_virtual_address,
+lldb::addr_t tag_segment_data_address, lldb::addr_t addr,
+size_t len) const {
+  // We can assume by now that addr and len have been granule aligned by a tag
+  // manager. However because we have 2 tags per byte we need to round the 
range
+  // up again to align to 2 granule boundaries.
+  const size_t granule = GetGranuleSize();
+  const size_t two_granules = granule * 2;
+  lldb::addr_t aligned_addr = addr;
+  size_t aligned_len = len;
+
+  // First align the start address down.
+  if (aligned_addr % two_granules) {
+assert(aligned_addr % two_granules == granule);
+aligned_addr -= granule;
+aligned_len += granule;
+  }
+
+  // Then align the length up.
+  bool aligned_length_up = false;
+  if (aligned_len % two_granules) {
+assert(aligned_len % two_granules == granule);
+aligned_len += granule;
+aligned_length_up = true;
+  }
+
+  // ProcessElfCore should have validated this when it found the segment.
+  assert(aligned_addr >= tag_segment_virtual_address);
+
+  // By now we know that aligned_addr is aligned to a 2 granule boundary.
+  const size_t offset_granules =
+  (aligned_addr - tag_segment_virtual_address) / granule;
+  // 2 tags per byte.
+  const size_t file_offset_in_bytes = offset_granules / 2;
+
+  // By now we know that aligned_len is at least 2 granules.
+  const size_t tag_bytes_to_read = aligned_len / granule / 2;
+  std::vector tag_data(tag_bytes_to_read);
+  const size_t bytes_copied =
+  reader(tag_segment_data_address + file_offset_in_bytes, 
tag_bytes_to_read,
+ tag_data.data());
+  assert(bytes_copied == tag_bytes_to_read);
+
+  std::vector tags;
+  tags.reserve(2 * tag_data.size());
+  // No need to check the range

[Lldb-commits] [PATCH] D129487: [lldb][AArch64] Add UnpackTagsFromCoreFileSegment to MemoryTagManager

[David Spickett via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG883b0d5b7f87: [lldb][AArch64] Add 
UnpackTagsFromCoreFileSegment to MemoryTagManager (authored by DavidSpickett).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129487

Files:
  lldb/include/lldb/Target/MemoryTagManager.h
  lldb/source/Plugins/Process/Utility/MemoryTagManagerAArch64MTE.cpp
  lldb/source/Plugins/Process/Utility/MemoryTagManagerAArch64MTE.h
  lldb/unittests/Process/Utility/MemoryTagManagerAArch64MTETest.cpp

Index: lldb/unittests/Process/Utility/MemoryTagManagerAArch64MTETest.cpp
===
--- lldb/unittests/Process/Utility/MemoryTagManagerAArch64MTETest.cpp
+++ lldb/unittests/Process/Utility/MemoryTagManagerAArch64MTETest.cpp
@@ -80,6 +80,72 @@
   ASSERT_THAT(expected, testing::ContainerEq(*packed));
 }
 
+TEST(MemoryTagManagerAArch64MTETest, UnpackTagsFromCoreFileSegment) {
+  MemoryTagManagerAArch64MTE manager;
+  // This is our fake segment data where tags are compressed as 2 4 bit tags
+  // per byte.
+  std::vector tags_data;
+  MemoryTagManager::CoreReaderFn reader =
+  [&tags_data](lldb::offset_t offset, size_t length, void *dst) {
+std::memcpy(dst, tags_data.data() + offset, length);
+return length;
+  };
+
+  // Zero length is ok.
+  std::vector tags =
+  manager.UnpackTagsFromCoreFileSegment(reader, 0, 0, 0, 0);
+  ASSERT_EQ(tags.size(), (size_t)0);
+
+  // In the simplest case we read 2 tags which are in the same byte.
+  tags_data.push_back(0x21);
+  // The least significant bits are the first tag in memory.
+  std::vector expected{1, 2};
+  tags = manager.UnpackTagsFromCoreFileSegment(reader, 0, 0, 0, 32);
+  ASSERT_THAT(expected, testing::ContainerEq(tags));
+
+  // If we read just one then it will have to trim off the second one.
+  expected = std::vector{1};
+  tags = manager.UnpackTagsFromCoreFileSegment(reader, 0, 0, 0, 16);
+  ASSERT_THAT(expected, testing::ContainerEq(tags));
+
+  // If we read the second tag only then the first one must be trimmed.
+  expected = std::vector{2};
+  tags = manager.UnpackTagsFromCoreFileSegment(reader, 0, 0, 16, 16);
+  ASSERT_THAT(expected, testing::ContainerEq(tags));
+
+  // This trimming logic applies if you read a larger set of tags.
+  tags_data = std::vector{0x21, 0x43, 0x65, 0x87};
+
+  // Trailing tag should be trimmed.
+  expected = std::vector{1, 2, 3};
+  tags = manager.UnpackTagsFromCoreFileSegment(reader, 0, 0, 0, 48);
+  ASSERT_THAT(expected, testing::ContainerEq(tags));
+
+  // Leading tag should be trimmed.
+  expected = std::vector{2, 3, 4};
+  tags = manager.UnpackTagsFromCoreFileSegment(reader, 0, 0, 16, 48);
+  ASSERT_THAT(expected, testing::ContainerEq(tags));
+
+  // Leading and trailing trimmmed.
+  expected = std::vector{2, 3, 4, 5};
+  tags = manager.UnpackTagsFromCoreFileSegment(reader, 0, 0, 16, 64);
+  ASSERT_THAT(expected, testing::ContainerEq(tags));
+
+  // The address given is an offset into the whole file so the address requested
+  // from the reader should be beyond that.
+  tags_data = std::vector{0xFF, 0xFF, 0x21, 0x43, 0x65, 0x87};
+  expected = std::vector{1, 2};
+  tags = manager.UnpackTagsFromCoreFileSegment(reader, 0, 2, 0, 32);
+  ASSERT_THAT(expected, testing::ContainerEq(tags));
+
+  // addr is a virtual address that we expect to be >= the tag segment's
+  // starting virtual address. So again an offset must be made from the
+  // difference.
+  expected = std::vector{3, 4};
+  tags = manager.UnpackTagsFromCoreFileSegment(reader, 32, 2, 64, 32);
+  ASSERT_THAT(expected, testing::ContainerEq(tags));
+}
+
 TEST(MemoryTagManagerAArch64MTETest, GetLogicalTag) {
   MemoryTagManagerAArch64MTE manager;
 
Index: lldb/source/Plugins/Process/Utility/MemoryTagManagerAArch64MTE.h
===
--- lldb/source/Plugins/Process/Utility/MemoryTagManagerAArch64MTE.h
+++ lldb/source/Plugins/Process/Utility/MemoryTagManagerAArch64MTE.h
@@ -44,6 +44,12 @@
   UnpackTagsData(const std::vector &tags,
  size_t granules = 0) const override;
 
+  std::vector
+  UnpackTagsFromCoreFileSegment(CoreReaderFn reader,
+lldb::addr_t tag_segment_virtual_address,
+lldb::addr_t tag_segment_data_address,
+lldb::addr_t addr, size_t len) const override;
+
   llvm::Expected>
   PackTags(const std::vector &tags) const override;
 
Index: lldb/source/Plugins/Process/Utility/MemoryTagManagerAArch64MTE.cpp
===
--- lldb/source/Plugins/Process/Utility/MemoryTagManagerAArch64MTE.cpp
+++ lldb/source/Plugins/Process/Utility/MemoryTagManagerAArch64MTE.cpp
@@ -247,6 +247,70 @@
   return unpacked;
 }
 
+std::vector
+MemoryTagManagerAArch64MTE::UnpackTagsFromCore

[Lldb-commits] [PATCH] D128250: [LLDB][RISCV]Add initial support for lldb-server.

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added inline comments.



Comment at: lldb/source/Plugins/Architecture/RISCV64/ArchitectureRISCV64.cpp:23
+void lldb_private::ArchitectureRISCV64::Initialize() {
+  PluginManager::RegisterPlugin(GetPluginNameStatic(), "RISC-V64",
+&ArchitectureRISCV64::Create);

Just noticed this in passing, should this and the 32 bit one be named like 
"RISCV-64"/"RISCV-32" instead?


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

https://reviews.llvm.org/D128250

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

[Lldb-commits] [lldb] 82d4f39 - [lldb][AArch64] Fix an unused variable warning in release builds. NFC

[Dmitri Gribenko via lldb-commits]

Author: Dmitri Gribenko
Date: 2022-07-25T16:58:03+02:00
New Revision: 82d4f39f342165a92eaa1fe74488158942cf27b4

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

LOG: [lldb][AArch64] Fix an unused variable warning in release builds. NFC

Added: 


Modified: 
lldb/source/Plugins/Process/Utility/MemoryTagManagerAArch64MTE.cpp

Removed: 




diff  --git 
a/lldb/source/Plugins/Process/Utility/MemoryTagManagerAArch64MTE.cpp 
b/lldb/source/Plugins/Process/Utility/MemoryTagManagerAArch64MTE.cpp
index e0126d840971..dbd36fbcb212 100644
--- a/lldb/source/Plugins/Process/Utility/MemoryTagManagerAArch64MTE.cpp
+++ b/lldb/source/Plugins/Process/Utility/MemoryTagManagerAArch64MTE.cpp
@@ -290,6 +290,7 @@ MemoryTagManagerAArch64MTE::UnpackTagsFromCoreFileSegment(
   const size_t bytes_copied =
   reader(tag_segment_data_address + file_offset_in_bytes, 
tag_bytes_to_read,
  tag_data.data());
+  (void)bytes_copied;
   assert(bytes_copied == tag_bytes_to_read);
 
   std::vector tags;



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

[Lldb-commits] [PATCH] D130342: [LLDB][RISCV] Add Register Info and Context

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

Please note in the commit title/description that this is adding riscv64 only.

Does this build if you don't have the rest of the changes from 
https://reviews.llvm.org/D128250? Or do you plan to split out more from that 
and have this depend on those changes.

(thanks for splitting them out though, it is easier to get through!)


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

https://reviews.llvm.org/D130342

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

[Lldb-commits] [PATCH] D130342: [LLDB][RISCV] Add Register Info and Context

[Emmmer S via Phabricator via lldb-commits]

Emmmer updated this revision to Diff 447344.
Emmmer added a comment.

`case llvm::Triple::riscv64:` in `ArchSpec::CharIsSignedByDefault()` is 
unnecessary as well.


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

https://reviews.llvm.org/D130342

Files:
  lldb/source/Host/common/HostInfoBase.cpp
  lldb/source/Plugins/Process/Linux/CMakeLists.txt
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_riscv64.cpp
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_riscv64.h
  lldb/source/Plugins/Process/Utility/CMakeLists.txt
  lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_riscv64.cpp
  lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_riscv64.h
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_riscv64.cpp
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_riscv64.h
  lldb/source/Plugins/Process/Utility/RegisterInfos_riscv64.h

Index: lldb/source/Plugins/Process/Utility/RegisterInfos_riscv64.h
===
--- /dev/null
+++ lldb/source/Plugins/Process/Utility/RegisterInfos_riscv64.h
@@ -0,0 +1,238 @@
+//===-- RegisterInfos_riscv64.h -*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifdef DECLARE_REGISTER_INFOS_RISCV64_STRUCT
+
+#include 
+
+#include "lldb/lldb-defines.h"
+#include "lldb/lldb-enumerations.h"
+#include "lldb/lldb-private.h"
+
+#ifndef GPR_OFFSET
+#error GPR_OFFSET must be defined before including this header file
+#endif
+
+#ifndef GPR_OFFSET_NAME
+#error GPR_OFFSET_NAME must be defined before including this header file
+#endif
+
+#ifndef FPR_OFFSET
+#error FPR_OFFSET must be defined before including this header file
+#endif
+
+#ifndef FPR_OFFSET_NAME
+#error FPR_OFFSET_NAME must be defined before including this header file
+#endif
+
+enum {
+  gpr_x0 = 0,
+  gpr_x1,
+  gpr_x2,
+  gpr_x3,
+  gpr_x4,
+  gpr_x5,
+  gpr_x6,
+  gpr_x7,
+  gpr_x8,
+  gpr_x9,
+  gpr_x10,
+  gpr_x11,
+  gpr_x12,
+  gpr_x13,
+  gpr_x14,
+  gpr_x15,
+  gpr_x16,
+  gpr_x17,
+  gpr_x18,
+  gpr_x19,
+  gpr_x20,
+  gpr_x21,
+  gpr_x22,
+  gpr_x23,
+  gpr_x24,
+  gpr_x25,
+  gpr_x26,
+  gpr_x27,
+  gpr_x28,
+  gpr_x29,
+  gpr_x30,
+  gpr_x31,
+  gpr_ra = gpr_x1,
+  gpr_sp = gpr_x2,
+  gpr_fp = gpr_x8,
+
+  gpr_pc = 32,
+
+  fpr_f0 = 33,
+  fpr_f1,
+  fpr_f2,
+  fpr_f3,
+  fpr_f4,
+  fpr_f5,
+  fpr_f6,
+  fpr_f7,
+  fpr_f8,
+  fpr_f9,
+  fpr_f10,
+  fpr_f11,
+  fpr_f12,
+  fpr_f13,
+  fpr_f14,
+  fpr_f15,
+  fpr_f16,
+  fpr_f17,
+  fpr_f18,
+  fpr_f19,
+  fpr_f20,
+  fpr_f21,
+  fpr_f22,
+  fpr_f23,
+  fpr_f24,
+  fpr_f25,
+  fpr_f26,
+  fpr_f27,
+  fpr_f28,
+  fpr_f29,
+  fpr_f30,
+  fpr_f31,
+
+  fpr_fflags,
+  fpr_frm,
+  fpr_fcsr,
+
+  k_num_registers
+};
+
+// Generates register kinds array with DWARF, EH frame and generic kind
+#define MISC_KIND(reg, type, generic_kind) \
+  { reg, reg, generic_kind, LLDB_INVALID_REGNUM, reg }
+
+// Generates register kinds array for registers with only lldb kind
+#define LLDB_KIND(lldb_kind)   \
+  {\
+LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, \
+LLDB_INVALID_REGNUM, lldb_kind \
+  }
+
+// Generates register kinds array for vector registers
+#define GPR64_KIND(reg, generic_kind) MISC_KIND(reg, gpr, generic_kind)
+
+// FPR register kinds array for vector registers
+#define FPR64_KIND(reg, generic_kind) MISC_KIND(reg, fpr, generic_kind)
+
+#define MISC_FPR_KIND(lldb_kind) LLDB_KIND(lldb_kind)
+
+// Defines a 64-bit general purpose register
+#define DEFINE_GPR64(reg, generic_kind)\
+  {\
+#reg, nullptr, 8, GPR_OFFSET(gpr_##reg), lldb::eEncodingUint,  \
+lldb::eFormatHex, GPR64_KIND(gpr_##reg, generic_kind), nullptr,\
+nullptr\
+  }
+
+// Defines a 64-bit general purpose register
+#define DEFINE_GPR64_ALT(reg, alt, generic_kind)   \
+  {\
+#reg, #alt, 8, GPR_OFFSET(gpr_##reg), lldb::eEncodingUint, \
+lldb::eFormatHex, GPR64_KIND(gpr_##reg, generic_kind), nullptr,\
+nullptr\
+  }
+
+#define DEFINE_FPR64(reg, generic_kind)\
+  {

[Lldb-commits] [PATCH] D130342: [LLDB][RISCV] Add Register Info and Context

[Emmmer S via Phabricator via lldb-commits]

Emmmer added a comment.

In D130342#3676318 , @DavidSpickett 
wrote:

> Please note in the commit title/description that this is adding riscv64 only.
>
> Does this build if you don't have the rest of the changes from 
> https://reviews.llvm.org/D128250? Or do you plan to split out more from that 
> and have this depend on those changes.
>
> (thanks for splitting them out though, it is easier to get through!)

I try to split unrelated code as much as possible so that they are easier to 
check and look cleaner.


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

https://reviews.llvm.org/D130342

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

[Lldb-commits] [lldb] 91098fe - [lldb][AArch64] Use macro for unused var without asserts enabled

[David Spickett via lldb-commits]

Author: David Spickett
Date: 2022-07-25T15:14:49Z
New Revision: 91098fec960f501c95c6ecdede604484750367e4

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

LOG: [lldb][AArch64] Use macro for unused var without asserts enabled

82d4f39f342165a92eaa1fe74488158942cf27b4 marked an unused
var of mine (thanks Dmitri Gribenko!). Which reminded me lldb has
a macro just for that purpose.

Added: 


Modified: 
lldb/source/Plugins/Process/Utility/MemoryTagManagerAArch64MTE.cpp

Removed: 




diff  --git 
a/lldb/source/Plugins/Process/Utility/MemoryTagManagerAArch64MTE.cpp 
b/lldb/source/Plugins/Process/Utility/MemoryTagManagerAArch64MTE.cpp
index dbd36fbcb212..7e25bc4ea2a2 100644
--- a/lldb/source/Plugins/Process/Utility/MemoryTagManagerAArch64MTE.cpp
+++ b/lldb/source/Plugins/Process/Utility/MemoryTagManagerAArch64MTE.cpp
@@ -290,7 +290,7 @@ MemoryTagManagerAArch64MTE::UnpackTagsFromCoreFileSegment(
   const size_t bytes_copied =
   reader(tag_segment_data_address + file_offset_in_bytes, 
tag_bytes_to_read,
  tag_data.data());
-  (void)bytes_copied;
+  UNUSED_IF_ASSERT_DISABLED(bytes_copied);
   assert(bytes_copied == tag_bytes_to_read);
 
   std::vector tags;



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

[Lldb-commits] [lldb] 52465dc - [lldb] Make compiler-rt an optional LLDB test dependency

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2022-07-25T09:28:56-07:00
New Revision: 52465dc17877d742aad737622e0da04aea2d72cb

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

LOG: [lldb] Make compiler-rt an optional LLDB test dependency

Make compiler-rt an LLDB test dependency if the corresponding target
exists. Similarly we already have `asan` and `tsan` as optional test
dependencies, but we need the `compiler-rt` target when enabling
compiler-rt trough LLVM_ENABLE_RUNTIMES.

Added: 


Modified: 
lldb/test/CMakeLists.txt

Removed: 




diff  --git a/lldb/test/CMakeLists.txt b/lldb/test/CMakeLists.txt
index 49626ce13a7ce..938420aea18ea 100644
--- a/lldb/test/CMakeLists.txt
+++ b/lldb/test/CMakeLists.txt
@@ -103,6 +103,10 @@ if(TARGET clang)
 add_lldb_test_dependency(tsan)
   endif()
 
+  if (TARGET compiler-rt)
+add_lldb_test_dependency(compiler-rt)
+  endif()
+
   if(APPLE AND NOT LLVM_TARGET_IS_CROSSCOMPILE_HOST)
 # FIXME: Standalone builds should import the cxx target as well.
 if(LLDB_BUILT_STANDALONE)



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

[Lldb-commits] [PATCH] D128250: [LLDB][RISCV]Add initial support for lldb-server.

[Tiancheng Zhang via Phabricator via lldb-commits]

tzb99 added a comment.

In D128250#3640942 , @Emmmer wrote:

> This patch change:
>
> - Add the recognition of architecture riscv64 in `HostInfoBase.cpp`
> - Add the recognition of architecture riscv64 and riscv32 in 
> `ObjectFilePECOFF.cpp`
> - Add riscv's `ebreak` command to `Platform.cpp`
>
> Now lldb can debug with simple executables on `qemu-system-riscv64` and be 
> able to attach to a `gdbserver`.
>
> ---
>
> TODO: some unittest failed
>
>   bash
>   [ RUN  ] TestBase.LaunchModePreservesEnvironment
>   
> /home/emmmer/git/llvm-project/lldb/unittests/tools/lldb-server/tests/LLGSTest.cpp:29:
>  Failure
>   Value of: llvm::detail::TakeExpected(ClientOr)
>   Expected: succeeded
> Actual: failed  (Unable to parse qRegisterInfo: generic)
>   [  FAILED  ] TestBase.LaunchModePreservesEnvironment (662 ms)
>   
>   
>   [ RUN  ] TestBase.vAttachRichError
>   Connection established.
>   Launched 
> '/home/emmmer/git/llvm-project/build-cross/tools/lldb/unittests/tools/lldb-server/./environment_check'
>  as process 1553...
>   lldb-server-local_build
>   Connection established.
>   Launched 
> '/home/emmmer/git/llvm-project/build-cross/tools/lldb/unittests/tools/lldb-server/./environment_check'
>  as process 1556...
>   lldb-server-local_build
>   
> /home/emmmer/git/llvm-project/lldb/unittests/tools/lldb-server/tests/LLGSTest.cpp:60:
>  Failure
>   Value of: llvm::detail::TakeExpected(ClientOr)
>   Expected: succeeded
> Actual: failed  (Unable to parse qRegisterInfo: generic)
>   [  FAILED  ] TestBase.vAttachRichError (364 ms)
>
> In riscv, the user-mode process cannot directly take the `pc` register but 
> must obtain the pc state through `auipc`, while the `pc` register is required 
> to exist in the test, so it failed.
>
> ---
>
>   bash
>   [ RUN  ] DumpDataExtractorTest.Formats
>   
> /home/emmmer/git/llvm-project/lldb/unittests/Core/DumpDataExtractorTest.cpp:90:
>  Failure
>   Expected equality of these values:
> expected
>   Which is: "{-nan -nan nan nan}"
> result.GetString()
>   Which is: "{nan nan nan nan}"
>   [  FAILED  ] DumpDataExtractorTest.Formats (25 ms)
>
> The reason is currently unknown, and further verification is required
>
> ---
>
> About buildbot: Unfortunately, as an individual developer, I may not have the 
> ability to maintain a 7*24-hour compile server or even a cluster, but I will 
> do my best to provide some test reports.

Hello:

I implemented the diff into my local llvm project and cross-compiled the 
project using in-tree build with enable projects as:
-DLLVM_ENABLE_PROJECTS="clang;lld;lldb"

The project can be compiled using the "Release" mode. The lldb-server can be 
initiated and connected to the host machine from the qemu environment. It can 
ran the riscv binary using process continue / thread continue, but the compiled 
lldb-server cannot get any thread information and cannot perform thread step-in 
functionality. Then I performed the Debug mode to build the project. Error 
occurred so the build command cannot be finished. The error shows like:

[3759/4081] Linking CXX shared library lib/libclang-cpp.so.15git
FAILED: lib/libclang-cpp.so.15git 
: && riscv-gnu-toolchain/bin/riscv64-unknown-linux-gnu-g++ -

Can your lldb-server work properly on the riscv qemu environment? My question 
might be, what is the proper recipe for cross-building the lldb-server? Or, 
should the diff be changed to enable getting the thread instruction info of the 
lldb-server?

Thank you very much!


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

https://reviews.llvm.org/D128250

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

[Lldb-commits] [lldb] 8068751 - [lldb] [gdb-remote] Refactor killing process and move it to client

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2022-07-25T18:43:32+02:00
New Revision: 8068751189af3099d9abef8953a9639d6798535c

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

LOG: [lldb] [gdb-remote] Refactor killing process and move it to client

Refactor the code responsible for sending the "k" packet and move it
into GDBRemoteCommunicationClient::KillProcess() method.  This is part
of refactoring to enable multiprocess support in the client,
and to support using the vKill packet instead.

As part of the refactoring, the following functional changes apply:

- Some redundant logging has been removed, as any failures are returned
  via exit_string anyway.

- SetLastStopPacket() is no longer called.  It is used only to populate
  the thread list, and since the process has just exited and we're
  terminating the process instance, there's really no reason to set it.

- On successful kill, exit_string is set to "killed", to clearly
  indicate that the process has terminated on our request rather than
  on its own.

Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.llvm.org/D130340

Added: 


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

Removed: 




diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
index c44ace96dd55c..580cdde57d80f 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -4265,3 +4265,21 @@ bool GDBRemoteCommunicationClient::UsesNativeSignals() {
   // check whether it is an old version of lldb-server.
   return GetThreadSuffixSupported();
 }
+
+llvm::Expected GDBRemoteCommunicationClient::KillProcess(lldb::pid_t pid) 
{
+  StringExtractorGDBRemote response;
+  GDBRemoteCommunication::ScopedTimeout(*this, seconds(3));
+
+  if (SendPacketAndWaitForResponse("k", response, GetPacketTimeout()) !=
+  PacketResult::Success)
+return llvm::createStringError(llvm::inconvertibleErrorCode(),
+   "failed to send k packet");
+
+  char packet_cmd = response.GetChar(0);
+  if (packet_cmd == 'W' || packet_cmd == 'X')
+return response.GetHexU8();
+
+  return llvm::createStringError(llvm::inconvertibleErrorCode(),
+ "unexpected response to k packet: %s",
+ response.GetStringRef().str().c_str());
+}

diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
index d367f75cee0e9..3d838d6d80747 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
@@ -521,6 +521,8 @@ class GDBRemoteCommunicationClient : public 
GDBRemoteClientBase {
 
   bool GetSaveCoreSupported() const;
 
+  llvm::Expected KillProcess(lldb::pid_t pid);
+
 protected:
   LazyBool m_supports_not_sending_acks = eLazyBoolCalculate;
   LazyBool m_supports_thread_suffix = eLazyBoolCalculate;

diff  --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp 
b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index 5f18706f67e5d..3e1a6fb6620a7 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -2394,7 +2394,6 @@ Status ProcessGDBRemote::DoDetach(bool keep_stopped) {
 }
 
 Status ProcessGDBRemote::DoDestroy() {
-  Status error;
   Log *log = GetLog(GDBRLog::Process);
   LLDB_LOGF(log, "ProcessGDBRemote::DoDestroy()");
 
@@ -2404,54 +2403,35 @@ Status ProcessGDBRemote::DoDestroy() {
 
   if (m_gdb_comm.IsConnected()) {
 if (m_public_state.GetValue() != eStateAttaching) {
-  StringExtractorGDBRemote response;
-  GDBRemoteCommunication::ScopedTimeout(m_gdb_comm,
-std::chrono::seconds(3));
-
-  if (m_gdb_comm.SendPacketAndWaitForResponse("k", response,
-  GetInterruptTimeout()) ==
-  GDBRemoteCommunication::PacketResult::Success) {
-char packet_cmd = response.GetChar(0);
+  llvm::Expected kill_res = m_gdb_comm.KillProcess(GetID());
 
-if (packet_cmd == 'W' || packet_cmd == 'X') {
+  if (kill_res) {
+exit_status = kill_res.get();
 #if defined(__APPLE__)
-  // For Native processes on Mac OS X, we launch through the Host
-  // Platform, then hand the process

[Lldb-commits] [PATCH] D130340: [lldb] [gdb-remote] Refactor killing process and move it to client

[Michał Górny 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 rG8068751189af: [lldb] [gdb-remote] Refactor killing process 
and move it to client (authored by mgorny).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D130340?vs=446760&id=447379#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130340

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

Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -2394,7 +2394,6 @@
 }
 
 Status ProcessGDBRemote::DoDestroy() {
-  Status error;
   Log *log = GetLog(GDBRLog::Process);
   LLDB_LOGF(log, "ProcessGDBRemote::DoDestroy()");
 
@@ -2404,54 +2403,35 @@
 
   if (m_gdb_comm.IsConnected()) {
 if (m_public_state.GetValue() != eStateAttaching) {
-  StringExtractorGDBRemote response;
-  GDBRemoteCommunication::ScopedTimeout(m_gdb_comm,
-std::chrono::seconds(3));
-
-  if (m_gdb_comm.SendPacketAndWaitForResponse("k", response,
-  GetInterruptTimeout()) ==
-  GDBRemoteCommunication::PacketResult::Success) {
-char packet_cmd = response.GetChar(0);
+  llvm::Expected kill_res = m_gdb_comm.KillProcess(GetID());
 
-if (packet_cmd == 'W' || packet_cmd == 'X') {
+  if (kill_res) {
+exit_status = kill_res.get();
 #if defined(__APPLE__)
-  // For Native processes on Mac OS X, we launch through the Host
-  // Platform, then hand the process off to debugserver, which becomes
-  // the parent process through "PT_ATTACH".  Then when we go to kill
-  // the process on Mac OS X we call ptrace(PT_KILL) to kill it, then
-  // we call waitpid which returns with no error and the correct
-  // status.  But amusingly enough that doesn't seem to actually reap
-  // the process, but instead it is left around as a Zombie.  Probably
-  // the kernel is in the process of switching ownership back to lldb
-  // which was the original parent, and gets confused in the handoff.
-  // Anyway, so call waitpid here to finally reap it.
-  PlatformSP platform_sp(GetTarget().GetPlatform());
-  if (platform_sp && platform_sp->IsHost()) {
-int status;
-::pid_t reap_pid;
-reap_pid = waitpid(GetID(), &status, WNOHANG);
-LLDB_LOGF(log, "Reaped pid: %d, status: %d.\n", reap_pid, status);
-  }
-#endif
-  SetLastStopPacket(response);
-  ClearThreadIDList();
-  exit_status = response.GetHexU8();
-} else {
-  LLDB_LOGF(log,
-"ProcessGDBRemote::DoDestroy - got unexpected response "
-"to k packet: %s",
-response.GetStringRef().data());
-  exit_string.assign("got unexpected response to k packet: ");
-  exit_string.append(std::string(response.GetStringRef()));
+// For Native processes on Mac OS X, we launch through the Host
+// Platform, then hand the process off to debugserver, which becomes
+// the parent process through "PT_ATTACH".  Then when we go to kill
+// the process on Mac OS X we call ptrace(PT_KILL) to kill it, then
+// we call waitpid which returns with no error and the correct
+// status.  But amusingly enough that doesn't seem to actually reap
+// the process, but instead it is left around as a Zombie.  Probably
+// the kernel is in the process of switching ownership back to lldb
+// which was the original parent, and gets confused in the handoff.
+// Anyway, so call waitpid here to finally reap it.
+PlatformSP platform_sp(GetTarget().GetPlatform());
+if (platform_sp && platform_sp->IsHost()) {
+  int status;
+  ::pid_t reap_pid;
+  reap_pid = waitpid(GetID(), &status, WNOHANG);
+  LLDB_LOGF(log, "Reaped pid: %d, status: %d.\n", reap_pid, status);
 }
+#endif
+ClearThreadIDList();
+exit_string.assign("killed");
   } else {
-LLDB_LOGF(log, "ProcessGDBRemote::DoDestroy - failed to send k packet");
-exit_string.assign("failed to send the k packet");
+exit_string.assign(llvm::toString(kill_res.takeError()));
   }
 } else {
-  LLDB_LOGF(log,
-"ProcessGDBRemote::DoDestroy - killed or interrupted while "
-"attac

[Lldb-commits] [PATCH] D130504: [lldb][NFC] Pass ParsedDWARFTypeAttributes as const reference into ParseArrayType()

[Arthur Eubanks via Phabricator via lldb-commits]

aeubanks created this revision.
aeubanks added a reviewer: dblaikie.
Herald added a reviewer: shafik.
Herald added a project: All.
aeubanks requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Fixes a FIXME


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D130504

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h


Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
@@ -239,9 +239,8 @@
  const DWARFDIE &die, ParsedDWARFTypeAttributes 
&attrs);
   lldb::TypeSP ParseSubroutine(const DWARFDIE &die,
ParsedDWARFTypeAttributes &attrs);
-  // FIXME: attrs should be passed as a const reference.
   lldb::TypeSP ParseArrayType(const DWARFDIE &die,
-  ParsedDWARFTypeAttributes &attrs);
+  const ParsedDWARFTypeAttributes &attrs);
   lldb::TypeSP ParsePointerToMemberType(const DWARFDIE &die,
 const ParsedDWARFTypeAttributes 
&attrs);
 
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1278,8 +1278,9 @@
   Type::eEncodingIsUID, &attrs.decl, clang_type, Type::ResolveState::Full);
 }
 
-TypeSP DWARFASTParserClang::ParseArrayType(const DWARFDIE &die,
-   ParsedDWARFTypeAttributes &attrs) {
+TypeSP
+DWARFASTParserClang::ParseArrayType(const DWARFDIE &die,
+const ParsedDWARFTypeAttributes &attrs) {
   SymbolFileDWARF *dwarf = die.GetDWARF();
 
   DEBUG_PRINTF("0x%8.8" PRIx64 ": %s (\"%s\")\n", die.GetID(),
@@ -1292,17 +1293,18 @@
 return nullptr;
 
   llvm::Optional array_info = ParseChildArrayInfo(die);
+  uint32_t byte_stride = attrs.byte_stride;
+  uint32_t bit_stride = attrs.bit_stride;
   if (array_info) {
-attrs.byte_stride = array_info->byte_stride;
-attrs.bit_stride = array_info->bit_stride;
+byte_stride = array_info->byte_stride;
+bit_stride = array_info->bit_stride;
   }
-  if (attrs.byte_stride == 0 && attrs.bit_stride == 0)
-attrs.byte_stride = element_type->GetByteSize(nullptr).value_or(0);
+  if (byte_stride == 0 && bit_stride == 0)
+byte_stride = element_type->GetByteSize(nullptr).value_or(0);
   CompilerType array_element_type = element_type->GetForwardCompilerType();
   RequireCompleteType(array_element_type);
 
-  uint64_t array_element_bit_stride =
-  attrs.byte_stride * 8 + attrs.bit_stride;
+  uint64_t array_element_bit_stride = byte_stride * 8 + bit_stride;
   CompilerType clang_type;
   if (array_info && array_info->element_orders.size() > 0) {
 uint64_t num_elements = 0;


Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
@@ -239,9 +239,8 @@
  const DWARFDIE &die, ParsedDWARFTypeAttributes &attrs);
   lldb::TypeSP ParseSubroutine(const DWARFDIE &die,
ParsedDWARFTypeAttributes &attrs);
-  // FIXME: attrs should be passed as a const reference.
   lldb::TypeSP ParseArrayType(const DWARFDIE &die,
-  ParsedDWARFTypeAttributes &attrs);
+  const ParsedDWARFTypeAttributes &attrs);
   lldb::TypeSP ParsePointerToMemberType(const DWARFDIE &die,
 const ParsedDWARFTypeAttributes &attrs);
 
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1278,8 +1278,9 @@
   Type::eEncodingIsUID, &attrs.decl, clang_type, Type::ResolveState::Full);
 }
 
-TypeSP DWARFASTParserClang::ParseArrayType(const DWARFDIE &die,
-   ParsedDWARFTypeAttributes &attrs) {
+TypeSP
+DWARFASTParserClang::ParseArrayType(const DWARFDIE &die,
+const ParsedDWARFTypeAttributes &attrs) {
   SymbolFileDWARF *dwarf = die.GetDWARF();
 
   DEBUG_PRINTF("0x%8.8" PRIx64 ": %s (\"%s\")\n", die.GetID(),
@@ -1292,17 +1293,18 @@
 return nullptr;
 
   llvm::Optional array_info = ParseChildArrayInfo(die);
+  uint32_t byte_stride = attrs.byte_stride;
+  uint32_t bit_stride = attrs.bi

[Lldb-commits] [PATCH] D130320: Move GetControlFlowKind's logic to DisassemblerLLVMC.cpp

[Sujin Park via Phabricator via lldb-commits]

persona0220 updated this revision to Diff 447437.
persona0220 added a comment.

Add default implementation for GetControlFlowKind()


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

https://reviews.llvm.org/D130320

Files:
  lldb/include/lldb/Core/Disassembler.h
  lldb/source/Core/Disassembler.cpp
  lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
  lldb/unittests/Disassembler/x86/TestGetControlFlowKindx86.cpp

Index: lldb/unittests/Disassembler/x86/TestGetControlFlowKindx86.cpp
===
--- lldb/unittests/Disassembler/x86/TestGetControlFlowKindx86.cpp
+++ lldb/unittests/Disassembler/x86/TestGetControlFlowKindx86.cpp
@@ -137,7 +137,8 @@
 for (size_t i = 0; i < num_of_instructions; ++i) {
   InstructionSP inst_sp;
   inst_sp = inst_list.GetInstructionAtIndex(i);
-  InstructionControlFlowKind kind = inst_sp->GetControlFlowKind(arch);
+  ExecutionContext exe_ctx (nullptr, nullptr, nullptr);
+  InstructionControlFlowKind kind = inst_sp->GetControlFlowKind(&exe_ctx);
   EXPECT_EQ(kind, result[i]);
 }
   }
Index: lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
===
--- lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
+++ lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
@@ -85,6 +85,324 @@
   std::unique_ptr m_instr_printer_up;
 };
 
+namespace x86 {
+
+/// These are the three values deciding instruction control flow kind.
+/// InstructionLengthDecode function decodes an instruction and get this struct.
+///
+/// primary_opcode
+///Primary opcode of the instruction.
+///For one-byte opcode instruction, it's the first byte after prefix.
+///For two- and three-byte opcodes, it's the second byte.
+///
+/// opcode_len
+///The length of opcode in bytes. Valid opcode lengths are 1, 2, or 3.
+///
+/// modrm
+///ModR/M byte of the instruction.
+///Bits[7:6] indicate MOD. Bits[5:3] specify a register and R/M bits[2:0]
+///may contain a register or specify an addressing mode, depending on MOD.
+struct InstructionOpcodeAndModrm {
+  uint8_t primary_opcode;
+  uint8_t opcode_len;
+  uint8_t modrm;
+};
+
+/// Determine the InstructionControlFlowKind based on opcode and modrm bytes.
+/// Refer to http://ref.x86asm.net/coder.html for the full list of opcode and
+/// instruction set.
+///
+/// \param[in] opcode_and_modrm
+///Contains primary_opcode byte, its length, and ModR/M byte.
+///Refer to the struct InstructionOpcodeAndModrm for details.
+///
+/// \return
+///   The control flow kind of the instruction or
+///   eInstructionControlFlowKindOther if the instruction doesn't affect
+///   the control flow of the program.
+lldb::InstructionControlFlowKind
+MapOpcodeIntoControlFlowKind(InstructionOpcodeAndModrm opcode_and_modrm) {
+  uint8_t opcode = opcode_and_modrm.primary_opcode;
+  uint8_t opcode_len = opcode_and_modrm.opcode_len;
+  uint8_t modrm = opcode_and_modrm.modrm;
+
+  if (opcode_len > 2)
+return lldb::eInstructionControlFlowKindOther;
+
+  if (opcode >= 0x70 && opcode <= 0x7F) {
+if (opcode_len == 1)
+  return lldb::eInstructionControlFlowKindCondJump;
+else
+  return lldb::eInstructionControlFlowKindOther;
+  }
+
+  if (opcode >= 0x80 && opcode <= 0x8F) {
+if (opcode_len == 2)
+  return lldb::eInstructionControlFlowKindCondJump;
+else
+  return lldb::eInstructionControlFlowKindOther;
+  }
+
+  switch (opcode) {
+  case 0x9A:
+if (opcode_len == 1)
+  return lldb::eInstructionControlFlowKindFarCall;
+break;
+  case 0xFF:
+if (opcode_len == 1) {
+  uint8_t modrm_reg = (modrm >> 3) & 7;
+  if (modrm_reg == 2)
+return lldb::eInstructionControlFlowKindCall;
+  else if (modrm_reg == 3)
+return lldb::eInstructionControlFlowKindFarCall;
+  else if (modrm_reg == 4)
+return lldb::eInstructionControlFlowKindJump;
+  else if (modrm_reg == 5)
+return lldb::eInstructionControlFlowKindFarJump;
+}
+break;
+  case 0xE8:
+if (opcode_len == 1)
+  return lldb::eInstructionControlFlowKindCall;
+break;
+  case 0xCD:
+  case 0xCC:
+  case 0xCE:
+  case 0xF1:
+if (opcode_len == 1)
+  return lldb::eInstructionControlFlowKindFarCall;
+break;
+  case 0xCF:
+if (opcode_len == 1)
+  return lldb::eInstructionControlFlowKindFarReturn;
+break;
+  case 0xE9:
+  case 0xEB:
+if (opcode_len == 1)
+  return lldb::eInstructionControlFlowKindJump;
+break;
+  case 0xEA:
+if (opcode_len == 1)
+  return lldb::eInstructionControlFlowKindFarJump;
+break;
+  case 0xE3:
+  case 0xE0:
+  case 0xE1:
+  case 0xE2:
+if (opcode_len == 1)
+  return lldb::eInstructionControlFlowKindCondJump;
+break;
+  case 0xC3:
+  case 0xC2:
+if (opcode_len == 1)
+  return lldb::eInstructionControlFlowKindReturn;
+break;
+  case 0xCB:

[Lldb-commits] [PATCH] D129682: [lldb] Filter DIEs based on qualified name when possible

[Alex Langford via Phabricator via lldb-commits]

bulbazord added a comment.

Friendly ping :)


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

https://reviews.llvm.org/D129682

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

[Lldb-commits] [PATCH] D128250: [LLDB][RISCV]Add initial support for lldb-server.

[Tiancheng Zhang via Phabricator via lldb-commits]

tzb99 added a comment.

In D128250#3676635 , @tzb99 wrote:

> In D128250#3640942 , @Emmmer wrote:
>
>> This patch change:
>>
>> - Add the recognition of architecture riscv64 in `HostInfoBase.cpp`
>> - Add the recognition of architecture riscv64 and riscv32 in 
>> `ObjectFilePECOFF.cpp`
>> - Add riscv's `ebreak` command to `Platform.cpp`
>>
>> Now lldb can debug with simple executables on `qemu-system-riscv64` and be 
>> able to attach to a `gdbserver`.
>>
>> ---
>>
>> TODO: some unittest failed
>>
>>   bash
>>   [ RUN  ] TestBase.LaunchModePreservesEnvironment
>>   
>> /home/emmmer/git/llvm-project/lldb/unittests/tools/lldb-server/tests/LLGSTest.cpp:29:
>>  Failure
>>   Value of: llvm::detail::TakeExpected(ClientOr)
>>   Expected: succeeded
>> Actual: failed  (Unable to parse qRegisterInfo: generic)
>>   [  FAILED  ] TestBase.LaunchModePreservesEnvironment (662 ms)
>>   
>>   
>>   [ RUN  ] TestBase.vAttachRichError
>>   Connection established.
>>   Launched 
>> '/home/emmmer/git/llvm-project/build-cross/tools/lldb/unittests/tools/lldb-server/./environment_check'
>>  as process 1553...
>>   lldb-server-local_build
>>   Connection established.
>>   Launched 
>> '/home/emmmer/git/llvm-project/build-cross/tools/lldb/unittests/tools/lldb-server/./environment_check'
>>  as process 1556...
>>   lldb-server-local_build
>>   
>> /home/emmmer/git/llvm-project/lldb/unittests/tools/lldb-server/tests/LLGSTest.cpp:60:
>>  Failure
>>   Value of: llvm::detail::TakeExpected(ClientOr)
>>   Expected: succeeded
>> Actual: failed  (Unable to parse qRegisterInfo: generic)
>>   [  FAILED  ] TestBase.vAttachRichError (364 ms)
>>
>> In riscv, the user-mode process cannot directly take the `pc` register but 
>> must obtain the pc state through `auipc`, while the `pc` register is 
>> required to exist in the test, so it failed.
>>
>> ---
>>
>>   bash
>>   [ RUN  ] DumpDataExtractorTest.Formats
>>   
>> /home/emmmer/git/llvm-project/lldb/unittests/Core/DumpDataExtractorTest.cpp:90:
>>  Failure
>>   Expected equality of these values:
>> expected
>>   Which is: "{-nan -nan nan nan}"
>> result.GetString()
>>   Which is: "{nan nan nan nan}"
>>   [  FAILED  ] DumpDataExtractorTest.Formats (25 ms)
>>
>> The reason is currently unknown, and further verification is required
>>
>> ---
>>
>> About buildbot: Unfortunately, as an individual developer, I may not have 
>> the ability to maintain a 7*24-hour compile server or even a cluster, but I 
>> will do my best to provide some test reports.
>
> Hello:
>
> I implemented the diff into my local llvm project and cross-compiled the 
> project using in-tree build with enable projects as:
> -DLLVM_ENABLE_PROJECTS="clang;lld;lldb"
>
> The project can be compiled using the "Release" mode. The lldb-server can be 
> initiated and connected to the host machine from the qemu environment. It can 
> ran the riscv binary using process continue / thread continue, but the 
> compiled lldb-server cannot get any thread information and cannot perform 
> thread step-in functionality. Then I performed the Debug mode to build the 
> project. Error occurred so the build command cannot be finished. The error 
> shows like:
>
> [3759/4081] Linking CXX shared library lib/libclang-cpp.so.15git
> FAILED: lib/libclang-cpp.so.15git 
> : && riscv-gnu-toolchain/bin/riscv64-unknown-linux-gnu-g++ -
>
> Can your lldb-server work properly on the riscv qemu environment? My question 
> might be, what is the proper recipe for cross-building the lldb-server? Or, 
> should the diff be changed to enable getting the thread instruction info of 
> the lldb-server?
>
> Thank you very much!

One more detail about the ThreadInfo: This is the strace of my lldb-server 
runing on the qemu:

recvfrom(9, "$jThreadsInfo#c1", 8192, 0, NULL, NULL) = 16
gettid()= 353
openat(AT_FDCWD, "/proc/354/task/354/comm", O_RDONLY|O_CLOEXEC) = 8
read(8, "riscvv\n", 16384)  = 7
read(8, "", 16384)  = 0
rt_sigprocmask(SIG_SETMASK, ~[RTMIN RT_1], [HUP CHLD], 8) = 0
close(8)= 0
rt_sigprocmask(SIG_SETMASK, [HUP CHLD], NULL, 8) = 0
sendto(9, "$[{\"name\":\"riscvv\",\"reason\":\"sig"..., 64, 0, NULL, 0) = 64
gettid()= 353
pselect6(10, [6 9], NULL, NULL, {tv_sec=0, tv_nsec=0}, NULL) = 1 (in [9], left 
{tv_sec=0, tv_nsec=0})
recvfrom(9, "$jThreadExtendedInfo:#b9", 8192, 0, NULL, NULL) = 24
gettid()= 353


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

https://reviews.llvm.org/D128250

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

[Lldb-commits] [PATCH] D130524: Don't hold the StackFrame mutex while getting a ValueObject Dynamic value

[Jim Ingham via Phabricator via lldb-commits]

jingham created this revision.
jingham added reviewers: JDevlieghere, clayborg, augusto2112.
Herald added a project: All.
jingham requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

We've noticed occasional hangs in the test TestGuiExpandThreadsTree.py.  They 
are caused by a lock inversion between the StackFrameList and StackFrame 
mutexes.

One thread has the StackFrame mutex, and is trying to acquire the 
StackFrameList mutex for that thread:

- thread #1, queue = 'com.apple.main-thread', stop reason = signal SIGSTOP
  - frame #0: 0x00018fffaafc libsystem_kernel.dylib`__psynch_mutexwait + 8 
frame #1: 0x000190034358 
libsystem_pthread.dylib`_pthread_mutex_firstfit_lock_wait + 84 frame #2: 
0x000190031cbc libsystem_pthread.dylib`_pthread_mutex_firstfit_lock_slow + 
248 frame #3: 0x00018ff86b7c 
libc++.1.dylib`std::__1::recursive_mutex::lock() + 16 frame #4: 
0x00010dc85c0c 
liblldb.15.0.0git.dylib`lldb_private::StackFrameList::GetFrameWithStackID(lldb_private::StackID
 const&) + 72 frame #5: 0x00010dcbad44 
liblldb.15.0.0git.dylib`lldb_private::Thread::GetFrameWithStackID(lldb_private::StackID
 const&) + 64 frame #6: 0x00010dc3cd94 
liblldb.15.0.0git.dylib`lldb_private::ExecutionContextRef::GetFrameSP() const + 
84 frame #7: 0x00010dc3ca3c 
liblldb.15.0.0git.dylib`lldb_private::ExecutionContext::ExecutionContext(lldb_private::ExecutionContextRef
 const&) + 244 frame #8: 0x00010db314a8 
liblldb.15.0.0git.dylib`lldb_private::ValueObject::CalculateDynamicValue(lldb::DynamicValueType)
 + 84 frame #9: 0x00010db31564 
liblldb.15.0.0git.dylib`lldb_private::ValueObject::GetDynamicValue(lldb::DynamicValueType)
 + 108 frame #10: 0x00010dc7f4ac 
liblldb.15.0.0git.dylib`lldb_private::StackFrame::GetValueObjectForFrameVariable(std::__1::shared_ptr
 const&, lldb::DynamicValueType) + 204

^^  frame #10 is the frame that acquires the StackFrame mutex  
^^

frame #11: 0x00010daffdc0 
liblldb.15.0.0git.dylib`FrameVariablesWindowDelegate::WindowDelegateDraw(curses::Window&,
 bool) + 216 frame #12: 0x00010daeb208 
liblldb.15.0.0git.dylib`curses::Window::Draw(bool) + 52 frame #13: 
0x00010daeb23c liblldb.15.0.0git.dylib`curses::Window::Draw(bool) + 104 
frame #14: 0x00010daea3f8 
liblldb.15.0.0git.dylib`curses::Application::Run(lldb_private::Debugger&) + 172 
frame #15: 0x00010daea338 
liblldb.15.0.0git.dylib`lldb_private::IOHandlerCursesGUI::Run() + 28 frame #16: 
0x00010dac8abc 
liblldb.15.0.0git.dylib`lldb_private::Debugger::RunIOHandlers() + 140 frame 
#17: 0x00010dbbe210 
liblldb.15.0.0git.dylib`lldb_private::CommandInterpreter::RunCommandInterpreter(lldb_private::CommandInterpreterRunOptions&)
 + 156 frame #18: 0x00010d90f780 
liblldb.15.0.0git.dylib`lldb::SBDebugger::RunCommandInterpreter(bool, bool) + 
168 frame #19: 0x000104058938 lldb`Driver::MainLoop() + 2776 frame #20: 
0x00010405959c lldb`main + 2456 frame #21: 0x00021b293c14 dyld`start + 
2372

The other has the StackFrameList mutex and is trying to get the StackFrame 
mutex held above.

  thread #2, name = 'lldb.debugger.event-handler'
frame #0: 0x00018fffaafc libsystem_kernel.dylib`__psynch_mutexwait + 8
frame #1: 0x000190034358 
libsystem_pthread.dylib`_pthread_mutex_firstfit_lock_wait + 84
frame #2: 0x000190031cbc 
libsystem_pthread.dylib`_pthread_mutex_firstfit_lock_slow + 248
frame #3: 0x00018ff86b7c 
libc++.1.dylib`std::__1::recursive_mutex::lock() + 16
frame #4: 0x00010dc7c630 
liblldb.15.0.0git.dylib`lldb_private::StackFrame::GetStackID() + 32

 frame #4 is trying to get the StackFrame mutex held by thread 1

  frame #5: 0x00010dc85c34 
liblldb.15.0.0git.dylib`lldb_private::StackFrameList::GetFrameWithStackID(lldb_private::StackID
 const&) + 112

 frame #5 is the frame that acquires the StackFrameList mutex  

  frame #6: 0x00010dcbad44 
liblldb.15.0.0git.dylib`lldb_private::Thread::GetFrameWithStackID(lldb_private::StackID
 const&) + 64
  frame #7: 0x00010dc3cd94 
liblldb.15.0.0git.dylib`lldb_private::ExecutionContextRef::GetFrameSP() const + 
84
  frame #8: 0x00010dc3d04c 
liblldb.15.0.0git.dylib`lldb_private::ExecutionContext::ExecutionContext(lldb_private::ExecutionContextRef
 const*, bool) + 528
  frame #9: 0x00010db3435c 
liblldb.15.0.0git.dylib`lldb_private::ValueObject::EvaluationPoint::SyncWithProcessState(bool)
 + 52
  frame #10: 0x00010db2ae28 
liblldb.15.0.0git.dylib`lldb_private::ValueObject::UpdateValueIfNeeded(bool) + 
120
  frame #11: 0x00010db2e678 
liblldb.15.0.0git.dylib`lldb_private::ValueObject::GetValueAsCString(lldb_private::TypeFormatImpl
 const&, std::__1::basic_string, 
std::__1::allocator >&) + 36
  frame #12: 0x00010db2e840 
liblldb.15.0.0git.dylib`lldb_private::ValueObject::GetValueAsCString() + 300
  frame #13: 0x00010db2f1b8 
liblldb.15.0.0git.dylib`lldb_private::ValueObj

[Lldb-commits] [PATCH] D130528: [LLDB][NFC][Reliability] Fix uninitialized variables from Coverity scan. Part 2

[Slava Gurevich via Phabricator via lldb-commits]

fixathon created this revision.
fixathon added reviewers: clayborg, aadsm, yinghuitan, jingham, JDevlieghere.
Herald added a project: All.
fixathon requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Improve LLDB reliability by fixing the following "uninitialized variables" 
static code inspection warnings from
scan.coverity.com:

1476275, 1274012, 1455035, 1364789, 1454282
1467483, 1406152, 1406255, 1454837, 1454416
1467446, 1462022, 1461909, 1420566, 1327228
1367767, 1431254, 1467299, 1312678, 1431780
1454731, 1490403


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D130528

Files:
  lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
  lldb/source/Plugins/Process/Utility/RegisterContextDarwin_arm64.cpp
  lldb/source/Plugins/Process/Utility/ThreadMemory.cpp
  lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
  lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/PdbUtil.cpp
  lldb/source/Plugins/SystemRuntime/MacOSX/SystemRuntimeMacOSX.cpp
  lldb/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp
  lldb/source/Symbol/Type.cpp
  lldb/source/Target/Process.cpp
  lldb/source/Target/RegisterContextUnwind.cpp
  lldb/source/Target/ThreadPlanCallFunction.cpp
  lldb/source/Target/ThreadPlanTracer.cpp
  lldb/source/Target/TraceDumper.cpp

Index: lldb/source/Target/TraceDumper.cpp
===
--- lldb/source/Target/TraceDumper.cpp
+++ lldb/source/Target/TraceDumper.cpp
@@ -286,7 +286,7 @@
 }
 
 TraceDumper::TraceItem TraceDumper::CreatRawTraceItem() {
-  TraceItem item;
+  TraceItem item = {};
   item.id = m_cursor_up->GetId();
 
   if (m_options.show_tsc)
Index: lldb/source/Target/ThreadPlanTracer.cpp
===
--- lldb/source/Target/ThreadPlanTracer.cpp
+++ lldb/source/Target/ThreadPlanTracer.cpp
@@ -36,11 +36,11 @@
 
 ThreadPlanTracer::ThreadPlanTracer(Thread &thread, lldb::StreamSP &stream_sp)
 : m_process(*thread.GetProcess().get()), m_tid(thread.GetID()),
-  m_enabled(false), m_stream_sp(stream_sp) {}
+  m_enabled(false), m_stream_sp(stream_sp), m_thread(nullptr) {}
 
 ThreadPlanTracer::ThreadPlanTracer(Thread &thread)
 : m_process(*thread.GetProcess().get()), m_tid(thread.GetID()),
-  m_enabled(false), m_stream_sp() {}
+  m_enabled(false), m_stream_sp(), m_thread(nullptr) {}
 
 Stream *ThreadPlanTracer::GetLogStream() {
   if (m_stream_sp)
Index: lldb/source/Target/ThreadPlanCallFunction.cpp
===
--- lldb/source/Target/ThreadPlanCallFunction.cpp
+++ lldb/source/Target/ThreadPlanCallFunction.cpp
@@ -104,7 +104,10 @@
   m_ignore_breakpoints(options.DoesIgnoreBreakpoints()),
   m_debug_execution(options.GetDebug()),
   m_trap_exceptions(options.GetTrapExceptions()), m_function_addr(function),
-  m_function_sp(0), m_takedown_done(false),
+  m_start_addr(), m_function_sp(0), m_subplan_sp(),
+  m_cxx_language_runtime(nullptr), m_objc_language_runtime(nullptr),
+  m_stored_thread_state(), m_real_stop_info_sp(), m_constructor_errors(),
+  m_return_valobj_sp(), m_takedown_done(false),
   m_should_clear_objc_exception_bp(false),
   m_should_clear_cxx_exception_bp(false),
   m_stop_address(LLDB_INVALID_ADDRESS), m_return_type(return_type) {
@@ -134,8 +137,11 @@
   m_ignore_breakpoints(options.DoesIgnoreBreakpoints()),
   m_debug_execution(options.GetDebug()),
   m_trap_exceptions(options.GetTrapExceptions()), m_function_addr(function),
-  m_function_sp(0), m_takedown_done(false),
-  m_should_clear_objc_exception_bp(false),
+  m_start_addr(), m_function_sp(0), m_subplan_sp(),
+  m_cxx_language_runtime(nullptr), m_objc_language_runtime(nullptr),
+  m_stored_thread_state(), m_real_stop_info_sp(), m_constructor_errors(),
+  m_return_valobj_sp(), m_takedown_done(false), m_function_sp(0),
+  m_takedown_done(false), m_should_clear_objc_exception_bp(false),
   m_should_clear_cxx_exception_bp(false),
   m_stop_address(LLDB_INVALID_ADDRESS), m_return_type(CompilerType()) {}
 
Index: lldb/source/Target/RegisterContextUnwind.cpp
===
--- lldb/source/Target/RegisterContextUnwind.cpp
+++ lldb/source/Target/RegisterContextUnwind.cpp
@@ -1525,7 +1525,7 @@
 
   // unwindplan_regloc has valid contents about where to retrieve the register
   if (unwindplan_regloc.IsUnspecified()) {
-lldb_private::UnwindLLDB::RegisterLocation new_regloc;
+lldb_private::UnwindLLDB::RegisterLocation new_regloc = {};
 new_regloc.type = UnwindLLDB::RegisterLocation::eRegisterNotSaved;
 m_registers[regnum.GetAsKind(eRegisterKindLLDB)] = new_regloc;
 UnwindLogMsg("save location for %s

[Lldb-commits] [PATCH] D130401: Implement better path matching in FileSpecList::FindCompatibleIndex(...).

[Greg Clayton via Phabricator via lldb-commits]

clayborg added inline comments.



Comment at: lldb/include/lldb/Core/FileSpecList.h:140-141
+  ///
+  /// \param[in] full
+  /// Should FileSpec::Equal be called with "full" true or false.
+  ///

labath wrote:
> Is this ever being called with full=false? If not can we drop it?
Probably yes!



Comment at: lldb/include/lldb/Utility/FileSpec.h:219-224
+  /// Get the path separator for the current path style.
+  ///
+  /// \return
+  /// A path separator character for this path.
+  char GetPathSeparator() const;
+

labath wrote:
> Are you sure about this part? It is my impression that we're already storing 
> the windows paths in the "normalized" form (using `/` as separator), and so 
> there shouldn't be a need to do anything special (at least regarding 
> directory separators) when working with windows paths.
I can check to make sure by adding some unit tests and if so, I can drop this



Comment at: lldb/source/Core/FileSpecList.cpp:93
+  const bool file_spec_case_sensitive = file_spec.IsCaseSensitive();
+  // When looking for files, we will compare only the filename if the FILE_SPEC
+  // argument is empty

labath wrote:
> use lower case?
I will reword



Comment at: lldb/source/Core/FileSpecList.cpp:121-141
+  if (file_spec_case_sensitive && curr_file.IsCaseSensitive()) {
+if (file_spec_dir.consume_back(curr_file_dir)) {
+  if (file_spec_dir.empty() ||
+  file_spec_dir.back() == file_spec.GetPathSeparator())
+return idx;
+} else if (curr_file_dir.consume_back(file_spec_dir)) {
+  if (curr_file_dir.empty() ||

labath wrote:
> This could be a lot less repetitive. Perhaps something like:
> ```
> const bool comparison_case_sensitive = file_spec_case_sensitive || 
> curr_file.IsCaseSensitive(); // I changed this from && to || as that's the 
> logic used elsewhere
> auto &is_suffix = [&](StringRef a, StringRef b) {
>   return a.consume_back(b) && (a.empty() || a.endswith("/"));
> };
> if (is_suffix(curr_file_dir, file_spec_dir) || is_suffix(file_spec_dir, 
> curr_file_dir))
>   return idx;
> ```
Good idea



Comment at: lldb/unittests/Core/FileSpecListTest.cpp:19-21
+// static FileSpec WindowsSpec(llvm::StringRef path) {
+//   return FileSpec(path, FileSpec::Style::windows);
+// }

labath wrote:
> It would be nice to have a quick windows test as well, to confirm the 
> separator story.
will do!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130401

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

[Lldb-commits] [lldb] b9aedd9 - [LLDB][NFC][Reliability] Fix uninitialized variables from Coverity scan. Part 2

[Slava Gurevich via lldb-commits]

Author: Slava Gurevich
Date: 2022-07-25T16:40:57-07:00
New Revision: b9aedd94e6796e4b4866ab4c091b736b3db58cb7

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

LOG: [LLDB][NFC][Reliability] Fix uninitialized variables from Coverity scan. 
Part 2

Improve LLDB reliability by fixing the following "uninitialized variables" 
static code inspection warnings from
scan.coverity.com:

1476275, 1274012, 1455035, 1364789, 1454282
1467483, 1406152, 1406255, 1454837, 1454416
1467446, 1462022, 1461909, 1420566, 1327228
1367767, 1431254, 1467299, 1312678, 1431780
1454731, 1490403

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

Added: 


Modified: 
lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
lldb/source/Plugins/Process/Utility/RegisterContextDarwin_arm64.cpp
lldb/source/Plugins/Process/Utility/ThreadMemory.cpp
lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
lldb/source/Plugins/SymbolFile/NativePDB/PdbUtil.cpp
lldb/source/Plugins/SystemRuntime/MacOSX/SystemRuntimeMacOSX.cpp
lldb/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp
lldb/source/Symbol/Type.cpp
lldb/source/Target/Process.cpp
lldb/source/Target/RegisterContextUnwind.cpp
lldb/source/Target/ThreadPlanCallFunction.cpp
lldb/source/Target/ThreadPlanTracer.cpp
lldb/source/Target/TraceDumper.cpp

Removed: 




diff  --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp 
b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
index acb131b8a775a..c396cb061c017 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
@@ -348,7 +348,7 @@ llvm::support::ulittle64_t 
read_register_u64(RegisterContext *reg_ctx,
 
 lldb_private::minidump::MinidumpContext_x86_64
 GetThreadContext_64(RegisterContext *reg_ctx) {
-  lldb_private::minidump::MinidumpContext_x86_64 thread_context;
+  lldb_private::minidump::MinidumpContext_x86_64 thread_context = {};
   thread_context.p1_home = {};
   thread_context.context_flags = static_cast(
   lldb_private::minidump::MinidumpContext_x86_64_Flags::x86_64_Flag |
@@ -534,7 +534,7 @@ Status MinidumpFileBuilder::AddException(const 
lldb::ProcessSP &process_sp) {
   helper_data.AppendData(
   &thread_context, sizeof(lldb_private::minidump::MinidumpContext_x86_64));
 
-  Exception exp_record;
+  Exception exp_record = {};
   exp_record.ExceptionCode =
   static_cast(stop_info_sp->GetValue());
   exp_record.ExceptionFlags = static_cast(0);

diff  --git a/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp 
b/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
index 90118f9386da3..f7f52deb173fb 100644
--- a/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
@@ -234,7 +234,7 @@ NativeProcessLinux::Factory::Launch(ProcessLaunchInfo 
&launch_info,
   }
 
   // Wait for the child process to trap on its call to execve.
-  int wstatus;
+  int wstatus = 0;
   ::pid_t wpid = llvm::sys::RetryAfterSignal(-1, ::waitpid, pid, &wstatus, 0);
   assert(wpid == pid);
   (void)wpid;

diff  --git 
a/lldb/source/Plugins/Process/Utility/RegisterContextDarwin_arm64.cpp 
b/lldb/source/Plugins/Process/Utility/RegisterContextDarwin_arm64.cpp
index 11b300bc44fbb..691e7db3fc79e 100644
--- a/lldb/source/Plugins/Process/Utility/RegisterContextDarwin_arm64.cpp
+++ b/lldb/source/Plugins/Process/Utility/RegisterContextDarwin_arm64.cpp
@@ -95,7 +95,7 @@ static size_t k_num_register_infos =
 
 RegisterContextDarwin_arm64::RegisterContextDarwin_arm64(
 Thread &thread, uint32_t concrete_frame_idx)
-: RegisterContext(thread, concrete_frame_idx), gpr(), fpu(), exc() {
+: RegisterContext(thread, concrete_frame_idx), gpr(), fpu(), exc(), dbg() {
   uint32_t i;
   for (i = 0; i < kNumErrors; i++) {
 gpr_errs[i] = -1;

diff  --git a/lldb/source/Plugins/Process/Utility/ThreadMemory.cpp 
b/lldb/source/Plugins/Process/Utility/ThreadMemory.cpp
index 7469e7633e71d..89ecc757a68f5 100644
--- a/lldb/source/Plugins/Process/Utility/ThreadMemory.cpp
+++ b/lldb/source/Plugins/Process/Utility/ThreadMemory.cpp
@@ -23,7 +23,8 @@ using namespace lldb_private;
 ThreadMemory::ThreadMemory(Process &process, tid_t tid,
const ValueObjectSP &thread_info_valobj_sp)
 : Thread(process, tid), m_backing_thread_sp(),
-  m_thread_info_valobj_sp(thread_info_valobj_sp), m_name(), m_queue() {}
+  m_thread_info_valobj_sp(thread_info_valobj_sp), m_name(), m_queue(),
+  m_register_data_addr(LLDB_INVALID_ADDRESS) {}
 

[Lldb-commits] [PATCH] D130528: [LLDB][NFC][Reliability] Fix uninitialized variables from Coverity scan. Part 2

[Slava Gurevich via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGb9aedd94e679: [LLDB][NFC][Reliability] Fix uninitialized 
variables from Coverity scan. Part 2 (authored by fixathon).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130528

Files:
  lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
  lldb/source/Plugins/Process/Utility/RegisterContextDarwin_arm64.cpp
  lldb/source/Plugins/Process/Utility/ThreadMemory.cpp
  lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
  lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/PdbUtil.cpp
  lldb/source/Plugins/SystemRuntime/MacOSX/SystemRuntimeMacOSX.cpp
  lldb/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp
  lldb/source/Symbol/Type.cpp
  lldb/source/Target/Process.cpp
  lldb/source/Target/RegisterContextUnwind.cpp
  lldb/source/Target/ThreadPlanCallFunction.cpp
  lldb/source/Target/ThreadPlanTracer.cpp
  lldb/source/Target/TraceDumper.cpp

Index: lldb/source/Target/TraceDumper.cpp
===
--- lldb/source/Target/TraceDumper.cpp
+++ lldb/source/Target/TraceDumper.cpp
@@ -286,7 +286,7 @@
 }
 
 TraceDumper::TraceItem TraceDumper::CreatRawTraceItem() {
-  TraceItem item;
+  TraceItem item = {};
   item.id = m_cursor_up->GetId();
 
   if (m_options.show_tsc)
Index: lldb/source/Target/ThreadPlanTracer.cpp
===
--- lldb/source/Target/ThreadPlanTracer.cpp
+++ lldb/source/Target/ThreadPlanTracer.cpp
@@ -36,11 +36,11 @@
 
 ThreadPlanTracer::ThreadPlanTracer(Thread &thread, lldb::StreamSP &stream_sp)
 : m_process(*thread.GetProcess().get()), m_tid(thread.GetID()),
-  m_enabled(false), m_stream_sp(stream_sp) {}
+  m_enabled(false), m_stream_sp(stream_sp), m_thread(nullptr) {}
 
 ThreadPlanTracer::ThreadPlanTracer(Thread &thread)
 : m_process(*thread.GetProcess().get()), m_tid(thread.GetID()),
-  m_enabled(false), m_stream_sp() {}
+  m_enabled(false), m_stream_sp(), m_thread(nullptr) {}
 
 Stream *ThreadPlanTracer::GetLogStream() {
   if (m_stream_sp)
Index: lldb/source/Target/ThreadPlanCallFunction.cpp
===
--- lldb/source/Target/ThreadPlanCallFunction.cpp
+++ lldb/source/Target/ThreadPlanCallFunction.cpp
@@ -104,7 +104,10 @@
   m_ignore_breakpoints(options.DoesIgnoreBreakpoints()),
   m_debug_execution(options.GetDebug()),
   m_trap_exceptions(options.GetTrapExceptions()), m_function_addr(function),
-  m_function_sp(0), m_takedown_done(false),
+  m_start_addr(), m_function_sp(0), m_subplan_sp(),
+  m_cxx_language_runtime(nullptr), m_objc_language_runtime(nullptr),
+  m_stored_thread_state(), m_real_stop_info_sp(), m_constructor_errors(),
+  m_return_valobj_sp(), m_takedown_done(false),
   m_should_clear_objc_exception_bp(false),
   m_should_clear_cxx_exception_bp(false),
   m_stop_address(LLDB_INVALID_ADDRESS), m_return_type(return_type) {
@@ -134,8 +137,11 @@
   m_ignore_breakpoints(options.DoesIgnoreBreakpoints()),
   m_debug_execution(options.GetDebug()),
   m_trap_exceptions(options.GetTrapExceptions()), m_function_addr(function),
-  m_function_sp(0), m_takedown_done(false),
-  m_should_clear_objc_exception_bp(false),
+  m_start_addr(), m_function_sp(0), m_subplan_sp(),
+  m_cxx_language_runtime(nullptr), m_objc_language_runtime(nullptr),
+  m_stored_thread_state(), m_real_stop_info_sp(), m_constructor_errors(),
+  m_return_valobj_sp(), m_takedown_done(false), m_function_sp(0),
+  m_takedown_done(false), m_should_clear_objc_exception_bp(false),
   m_should_clear_cxx_exception_bp(false),
   m_stop_address(LLDB_INVALID_ADDRESS), m_return_type(CompilerType()) {}
 
Index: lldb/source/Target/RegisterContextUnwind.cpp
===
--- lldb/source/Target/RegisterContextUnwind.cpp
+++ lldb/source/Target/RegisterContextUnwind.cpp
@@ -1525,7 +1525,7 @@
 
   // unwindplan_regloc has valid contents about where to retrieve the register
   if (unwindplan_regloc.IsUnspecified()) {
-lldb_private::UnwindLLDB::RegisterLocation new_regloc;
+lldb_private::UnwindLLDB::RegisterLocation new_regloc = {};
 new_regloc.type = UnwindLLDB::RegisterLocation::eRegisterNotSaved;
 m_registers[regnum.GetAsKind(eRegisterKindLLDB)] = new_regloc;
 UnwindLogMsg("save location for %s (%d) is unspecified, continue searching",
@@ -1731,7 +1731,7 @@
 
   addr_t old_caller_pc_value = LLDB_INVALID_ADDRESS;
   addr_t new_caller_pc_value = LLDB_INVALID_ADDRESS;
-  UnwindLLDB::RegisterLocation regloc;
+  UnwindLLDB::RegisterLocation regloc = {};
   if (SavedLocationForRegister(pc_regnum.G

[Lldb-commits] [PATCH] D130401: Implement better path matching in FileSpecList::FindCompatibleIndex(...).

[Greg Clayton via Phabricator via lldb-commits]

clayborg updated this revision to Diff 447523.
clayborg added a comment.

- Added windows tests to make sure FileSpecList::FindCompatibleIndex() works on 
windows
- Use lambdas to simplify code in FileSpecList::FindCompatibleIndex()
- Remove the "full" argument from FindCompatibleIndex
- Reword the comment that explains when we will use a filename match only.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130401

Files:
  lldb/include/lldb/Breakpoint/BreakpointResolverFileLine.h
  lldb/include/lldb/Core/FileSpecList.h
  lldb/include/lldb/Utility/FileSpec.h
  lldb/source/Breakpoint/BreakpointResolverFileLine.cpp
  lldb/source/Core/FileSpecList.cpp
  lldb/source/Symbol/CompileUnit.cpp
  lldb/source/Utility/FileSpec.cpp
  
lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
  lldb/test/API/functionalities/breakpoint/breakpoint_command/relative.yaml
  lldb/unittests/Core/CMakeLists.txt
  lldb/unittests/Core/FileSpecListTest.cpp

Index: lldb/unittests/Core/FileSpecListTest.cpp
===
--- /dev/null
+++ lldb/unittests/Core/FileSpecListTest.cpp
@@ -0,0 +1,125 @@
+//===-- FileSpecListTest.cpp --===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "gtest/gtest.h"
+
+#include "lldb/Core/FileSpecList.h"
+
+using namespace lldb_private;
+
+static FileSpec PosixSpec(llvm::StringRef path) {
+  return FileSpec(path, FileSpec::Style::posix);
+}
+
+static FileSpec WindowsSpec(llvm::StringRef path) {
+  return FileSpec(path, FileSpec::Style::windows);
+}
+
+TEST(FileSpecListTest, RelativePathMatchesPosix) {
+
+  const FileSpec fullpath = PosixSpec("/build/src/main.cpp");
+  const FileSpec relative = PosixSpec("./src/main.cpp");
+  const FileSpec basename = PosixSpec("./main.cpp");
+  const FileSpec full_wrong = PosixSpec("/other/wrong/main.cpp");
+  const FileSpec rel_wrong = PosixSpec("./wrong/main.cpp");
+  // Make sure these don't match "src/main.cpp" as we want to match full
+  // directories only
+  const FileSpec rel2_wrong = PosixSpec("asrc/main.cpp");
+  const FileSpec rel3_wrong = PosixSpec("rc/main.cpp");
+
+  FileSpecList files;
+  files.Append(fullpath);
+  files.Append(relative);
+  files.Append(basename);
+  files.Append(full_wrong);
+  files.Append(rel_wrong);
+  files.Append(rel2_wrong);
+  files.Append(rel3_wrong);
+
+  // Make sure the full path only matches the first entry
+  EXPECT_EQ((size_t)0, files.FindCompatibleIndex(0, fullpath));
+  EXPECT_EQ((size_t)1, files.FindCompatibleIndex(1, fullpath));
+  EXPECT_EQ((size_t)2, files.FindCompatibleIndex(2, fullpath));
+  EXPECT_EQ((size_t)UINT32_MAX, files.FindCompatibleIndex(3, fullpath));
+  // Make sure the relative path matches the all of the entries that contain
+  // the relative path
+  EXPECT_EQ((size_t)0, files.FindCompatibleIndex(0, relative));
+  EXPECT_EQ((size_t)1, files.FindCompatibleIndex(1, relative));
+  EXPECT_EQ((size_t)2, files.FindCompatibleIndex(2, relative));
+  EXPECT_EQ((size_t)UINT32_MAX, files.FindCompatibleIndex(3, relative));
+
+  // Make sure looking file a file using the basename matches all entries
+  EXPECT_EQ((size_t)0, files.FindCompatibleIndex(0, basename));
+  EXPECT_EQ((size_t)1, files.FindCompatibleIndex(1, basename));
+  EXPECT_EQ((size_t)2, files.FindCompatibleIndex(2, basename));
+  EXPECT_EQ((size_t)3, files.FindCompatibleIndex(3, basename));
+  EXPECT_EQ((size_t)4, files.FindCompatibleIndex(4, basename));
+  EXPECT_EQ((size_t)5, files.FindCompatibleIndex(5, basename));
+  EXPECT_EQ((size_t)6, files.FindCompatibleIndex(6, basename));
+
+  // Make sure that paths that have a common suffix don't return values that
+  // don't match on directory delimiters.
+  EXPECT_EQ((size_t)2, files.FindCompatibleIndex(0, rel2_wrong));
+  EXPECT_EQ((size_t)5, files.FindCompatibleIndex(3, rel2_wrong));
+  EXPECT_EQ((size_t)UINT32_MAX, files.FindCompatibleIndex(6, rel2_wrong));
+
+  EXPECT_EQ((size_t)2, files.FindCompatibleIndex(0, rel3_wrong));
+  EXPECT_EQ((size_t)6, files.FindCompatibleIndex(3, rel3_wrong));
+}
+
+TEST(FileSpecListTest, RelativePathMatchesWindows) {
+
+  const FileSpec fullpath = WindowsSpec(R"(C:\build\src\main.cpp)");
+  const FileSpec relative = WindowsSpec(R"(.\src\main.cpp)");
+  const FileSpec basename = WindowsSpec(R"(.\main.cpp)");
+  const FileSpec full_wrong = WindowsSpec(R"(\other\wrong\main.cpp)");
+  const FileSpec rel_wrong = WindowsSpec(R"(.\wrong\main.cpp)");
+  // Make sure these don't match "src\main.cpp" as we want to match full
+  // directories only
+  const FileSpec rel2_wrong = WindowsSpec(R"(asrc\main.cpp)");
+  const

[Lldb-commits] [PATCH] D130401: Implement better path matching in FileSpecList::FindCompatibleIndex(...).

[Greg Clayton via Phabricator via lldb-commits]

clayborg marked 4 inline comments as done.
clayborg added a comment.

Marking things as done


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130401

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

[Lldb-commits] [PATCH] D130534: loading a binary at a slide multiple times leaves old entries in the SectionLoadList

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda created this revision.
jasonmolenda added a reviewer: jingham.
jasonmolenda added a project: LLDB.
Herald added a subscriber: JDevlieghere.
Herald added a project: All.
jasonmolenda requested review of this revision.
Herald added a subscriber: lldb-commits.

If you add a binary to lldb and do `target modules load -s slide -f name` 
multiple times, the old entries are not cleared from the SectionLoadList's 
addr->section map, leading to confusing lldb behavior.

SectionLoadList has a section-to-address map (m_sect_to_addr) and an address to 
section map (m_addr_to_sect) that are registered with the Target.  
SectionLoadList::SetSectionLoadAddress updates the entry in the m_sect_to_addr 
map if it exists, or adds it.  And it adds a new entry to m_addr_to_sect as 
long as there isn't a section at that address already.  But it never removes 
the old entry from m_addr_to_sect.

This results in each section having multiple entries, with each the load 
addresses that had been set previously, so load address -> file address 
translation behaves very oddly.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D130534

Files:
  lldb/source/Target/SectionLoadList.cpp
  lldb/test/API/functionalities/multiple-slides/Makefile
  lldb/test/API/functionalities/multiple-slides/TestMultipleSlides.py
  lldb/test/API/functionalities/multiple-slides/main.c


Index: lldb/test/API/functionalities/multiple-slides/main.c
===
--- /dev/null
+++ lldb/test/API/functionalities/multiple-slides/main.c
@@ -0,0 +1,5 @@
+int first[2048] = { 5 };
+int second[2048] = { 6 };
+int main()  {
+  return first[0] + second[0];
+}
Index: lldb/test/API/functionalities/multiple-slides/TestMultipleSlides.py
===
--- /dev/null
+++ lldb/test/API/functionalities/multiple-slides/TestMultipleSlides.py
@@ -0,0 +1,44 @@
+"""
+Test that a binary can be slid to different load addresses correctly
+"""
+
+
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class MultipleSlidesTestCase(TestBase):
+
+NO_DEBUG_INFO_TESTCASE = True
+def test_mulitple_slides(self):
+"""Test that a binary can be slid multiple times correctly."""
+self.build()
+exe = self.getBuildArtifact("a.out")
+err = lldb.SBError()
+load_dependent_modules = False
+target = self.dbg.CreateTarget(exe, '', '', load_dependent_modules, 
err)
+self.assertTrue(target.IsValid())
+module = target.GetModuleAtIndex(0)
+self.assertTrue(module.IsValid())
+
+# View the first element of `first` and `second` while
+# they have no load address set.
+self.expect("p/d ((int*)&first)[0]", substrs=['= 5'])
+self.expect("p/d ((int*)&second)[0]", substrs=['= 6'])
+
+target.SetModuleLoadAddress(module, 1990)
+
+# View the first element of `first` and `second` with
+# a load address .
+self.expect("p/d ((int*)&first)[0]", substrs=['= 5'])
+self.expect("p/d ((int*)&second)[0]", substrs=['= 6'])
+
+target.SetModuleLoadAddress(module, 4)
+
+# View the first element of `first` and `second` with
+# a new load address.
+self.expect("p/d ((int*)&first)[0]", substrs=['= 5'])
+self.expect("p/d ((int*)&second)[0]", substrs=['= 6'])
Index: lldb/test/API/functionalities/multiple-slides/Makefile
===
--- /dev/null
+++ lldb/test/API/functionalities/multiple-slides/Makefile
@@ -0,0 +1,12 @@
+C_SOURCES := main.c
+MAKE_DSYM := NO
+
+include Makefile.rules
+
+# lldb has a separate bug where this test case
+# does not work if we have debug info - after
+# sliding the binary, the address of `first` and
+# `second` are not slid for some reason on Darwin.
+main.o: main.c
+   $(CC) $(CFLAGS_NO_DEBUG) -c $< -o $@
+
Index: lldb/source/Target/SectionLoadList.cpp
===
--- lldb/source/Target/SectionLoadList.cpp
+++ lldb/source/Target/SectionLoadList.cpp
@@ -116,8 +116,18 @@
 }
   }
   ats_pos->second = section;
-} else
+} else {
+  // Remove the old address->section entry, if
+  // there is one.
+  for (const auto &it : m_addr_to_sect) {
+if (it.second == section) {
+  const auto &it_pos = m_addr_to_sect.find(it.first);
+  m_addr_to_sect.erase(it_pos);
+  break;
+}
+  }
   m_addr_to_sect[load_addr] = section;
+}
 return true; // Changed
 
   } else {


Index: lldb/test/API/functionalities/multiple-slides/main.c
===
--- /dev/null
+++ lldb/test/API/functionalities/multiple-slides/main.c
@@ -0,0 +1,5 @@
+int first[2048] = { 5 };
+int second[2048] = { 6 

[Lldb-commits] [PATCH] D130534: loading a binary at a slide multiple times leaves old entries in the SectionLoadList

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda added a comment.

I should have added, this bug has been in lldb probably forever - at least for 
a few years.

Also, this test case explicitly builds without debug information because debug 
information (on Darwin) results in a different bug being hit, where these data 
symbols are not updated when `target modules load -s slide` is done, somehow.  
The symbol table from the debug info must not be updated, I don't know, I filed 
a little bug on myself to look into that.  This has also behaved this way for 
years as well, not a recent regression.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130534

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

[Lldb-commits] [PATCH] D130045: Implement better path matching in FileSpecList::FindFileIndex(...).

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In D130045#3675738 , @labath wrote:

> In D130045#310 , @JDevlieghere 
> wrote:
>
>> I'm slightly worried about the change to make the new "fuzzy" matching the 
>> default. While it makes sense for the breakpoints, I wouldn't generally 
>> expect `./a/b/c/main.cpp` to match `/build/a/b/c/main.cpp`,
>
> Would you expect that `main.cpp` "generally" matches `/build/a/b/c/main.cpp`?
>
> (I'm not arguing for/against anything (yet, at least), but I would like to 
> hear your reasoning if the answer to the question is "yes".)

I would say it should match. If FindFileIndex is currently called with a 
FileSpec that only has "main.cpp" as the m_filename, then it will fall back to 
only matching by filename even if "full = true;". I would expect it to work the 
other way around too if we have any files in the file list that are base name 
only. Does that make sense?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130045

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

[Lldb-commits] [PATCH] D130528: [LLDB][NFC][Reliability] Fix uninitialized variables from Coverity scan. Part 2

[Slava Gurevich via Phabricator via lldb-commits]

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

Reopen to fix up a couple of typomatic bugs


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130528

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

[Lldb-commits] [lldb] 9877159 - Revert "[LLDB][NFC][Reliability] Fix uninitialized variables from Coverity scan. Part 2"

[Slava Gurevich via lldb-commits]

Author: Slava Gurevich
Date: 2022-07-25T18:23:19-07:00
New Revision: 9877159dd65ae6d2c4afc4c459d2eefe2473e616

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

LOG: Revert "[LLDB][NFC][Reliability] Fix uninitialized variables from Coverity 
scan. Part 2"

This reverts commit b9aedd94e6796e4b4866ab4c091b736b3db58cb7.

Added: 


Modified: 
lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
lldb/source/Plugins/Process/Utility/RegisterContextDarwin_arm64.cpp
lldb/source/Plugins/Process/Utility/ThreadMemory.cpp
lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
lldb/source/Plugins/SymbolFile/NativePDB/PdbUtil.cpp
lldb/source/Plugins/SystemRuntime/MacOSX/SystemRuntimeMacOSX.cpp
lldb/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp
lldb/source/Symbol/Type.cpp
lldb/source/Target/Process.cpp
lldb/source/Target/RegisterContextUnwind.cpp
lldb/source/Target/ThreadPlanCallFunction.cpp
lldb/source/Target/ThreadPlanTracer.cpp
lldb/source/Target/TraceDumper.cpp

Removed: 




diff  --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp 
b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
index c396cb061c017..acb131b8a775a 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
@@ -348,7 +348,7 @@ llvm::support::ulittle64_t 
read_register_u64(RegisterContext *reg_ctx,
 
 lldb_private::minidump::MinidumpContext_x86_64
 GetThreadContext_64(RegisterContext *reg_ctx) {
-  lldb_private::minidump::MinidumpContext_x86_64 thread_context = {};
+  lldb_private::minidump::MinidumpContext_x86_64 thread_context;
   thread_context.p1_home = {};
   thread_context.context_flags = static_cast(
   lldb_private::minidump::MinidumpContext_x86_64_Flags::x86_64_Flag |
@@ -534,7 +534,7 @@ Status MinidumpFileBuilder::AddException(const 
lldb::ProcessSP &process_sp) {
   helper_data.AppendData(
   &thread_context, sizeof(lldb_private::minidump::MinidumpContext_x86_64));
 
-  Exception exp_record = {};
+  Exception exp_record;
   exp_record.ExceptionCode =
   static_cast(stop_info_sp->GetValue());
   exp_record.ExceptionFlags = static_cast(0);

diff  --git a/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp 
b/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
index f7f52deb173fb..90118f9386da3 100644
--- a/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
@@ -234,7 +234,7 @@ NativeProcessLinux::Factory::Launch(ProcessLaunchInfo 
&launch_info,
   }
 
   // Wait for the child process to trap on its call to execve.
-  int wstatus = 0;
+  int wstatus;
   ::pid_t wpid = llvm::sys::RetryAfterSignal(-1, ::waitpid, pid, &wstatus, 0);
   assert(wpid == pid);
   (void)wpid;

diff  --git 
a/lldb/source/Plugins/Process/Utility/RegisterContextDarwin_arm64.cpp 
b/lldb/source/Plugins/Process/Utility/RegisterContextDarwin_arm64.cpp
index 691e7db3fc79e..11b300bc44fbb 100644
--- a/lldb/source/Plugins/Process/Utility/RegisterContextDarwin_arm64.cpp
+++ b/lldb/source/Plugins/Process/Utility/RegisterContextDarwin_arm64.cpp
@@ -95,7 +95,7 @@ static size_t k_num_register_infos =
 
 RegisterContextDarwin_arm64::RegisterContextDarwin_arm64(
 Thread &thread, uint32_t concrete_frame_idx)
-: RegisterContext(thread, concrete_frame_idx), gpr(), fpu(), exc(), dbg() {
+: RegisterContext(thread, concrete_frame_idx), gpr(), fpu(), exc() {
   uint32_t i;
   for (i = 0; i < kNumErrors; i++) {
 gpr_errs[i] = -1;

diff  --git a/lldb/source/Plugins/Process/Utility/ThreadMemory.cpp 
b/lldb/source/Plugins/Process/Utility/ThreadMemory.cpp
index 89ecc757a68f5..7469e7633e71d 100644
--- a/lldb/source/Plugins/Process/Utility/ThreadMemory.cpp
+++ b/lldb/source/Plugins/Process/Utility/ThreadMemory.cpp
@@ -23,8 +23,7 @@ using namespace lldb_private;
 ThreadMemory::ThreadMemory(Process &process, tid_t tid,
const ValueObjectSP &thread_info_valobj_sp)
 : Thread(process, tid), m_backing_thread_sp(),
-  m_thread_info_valobj_sp(thread_info_valobj_sp), m_name(), m_queue(),
-  m_register_data_addr(LLDB_INVALID_ADDRESS) {}
+  m_thread_info_valobj_sp(thread_info_valobj_sp), m_name(), m_queue() {}
 
 ThreadMemory::ThreadMemory(Process &process, lldb::tid_t tid,
llvm::StringRef name, llvm::StringRef queue,

diff  --git a/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp 
b/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
index 24c942f1d290a..58b4fe3add1b3 100644
---

[Lldb-commits] [PATCH] D130528: [LLDB][NFC][Reliability] Fix uninitialized variables from Coverity scan. Part 2

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added a comment.

I believe this broke the lldb build bot: 
https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/45608/console


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130528

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

[Lldb-commits] [PATCH] D130540: [lldb] Read from the Rosetta shared cache with Xcode 14

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added reviewers: aprantl, jasonmolenda, mib.
Herald added subscribers: jsji, ctetreau, pengfei.
Herald added a project: All.
JDevlieghere requested review of this revision.

Xcode 14 no longer puts the Rosetta expanded shared cache under `16.0`. 
Instead, it includes the real version number (e.g. 13.0), the build string  and 
the architecture, similar to the device support directory names for iOS, tvOS 
and watchOS.

Currently, when there are multiple directories, we end up picking the wrong one 
in `GetSDKDirectoryForCurrentOSVersion`. The problem is that without the build 
string we have no way to differentiate between directories with the same 
version number.

  13.0 (22A111) x86_64/
  13.0 (22A222) x86_64/
  13.0 (22A333) x86_64/

This patch fixes the problem by using `GetOSBuildString` which, as the name 
implies, returns the build string if it's known. For the host platform this 
returns the host build string while for a remote platform this returns the 
remote build string.


https://reviews.llvm.org/D130540

Files:
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp
  lldb/test/API/macosx/rosetta/Makefile
  lldb/test/API/macosx/rosetta/TestRosetta.py
  lldb/test/API/macosx/rosetta/main.c

Index: lldb/test/API/macosx/rosetta/main.c
===
--- /dev/null
+++ lldb/test/API/macosx/rosetta/main.c
@@ -0,0 +1,6 @@
+#include 
+
+int main() {
+  int i = 0; // Set a breakpoint here
+  return i;
+}
Index: lldb/test/API/macosx/rosetta/TestRosetta.py
===
--- /dev/null
+++ lldb/test/API/macosx/rosetta/TestRosetta.py
@@ -0,0 +1,36 @@
+import lldb
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+
+
+def apple_silicon(test):
+return platform.system() == 'Darwin' and test.getArchitecture() in [
+'arm64', 'arm64e'
+]
+
+
+class TestRosetta(TestBase):
+
+NO_DEBUG_INFO_TESTCASE = True
+
+def no_apple_silicon(self):
+if not apple_silicon(self):
+return "Rosetta requires Apple Silicon"
+return None
+
+@skipTestIfFn(no_apple_silicon)
+def test_rosetta(self):
+"""There can be many tests in a test case - describe this test here."""
+self.build()
+self.main_source_file = lldb.SBFileSpec("main.c")
+
+broadcaster = self.dbg.GetBroadcaster()
+listener = lldbutil.start_listening_from(
+broadcaster, lldb.SBDebugger.eBroadcastBitWarning)
+
+target, process, thread, bkpt = lldbutil.run_to_source_breakpoint(
+self, "Set a breakpoint here", self.main_source_file)
+
+event = lldb.SBEvent()
+self.assertFalse(listener.GetNextEvent(event))
Index: lldb/test/API/macosx/rosetta/Makefile
===
--- /dev/null
+++ lldb/test/API/macosx/rosetta/Makefile
@@ -0,0 +1,4 @@
+C_SOURCES := main.c
+override ARCH = x86_64
+
+include Makefile.rules
Index: lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp
===
--- lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp
+++ lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp
@@ -148,11 +148,20 @@
   uint32_t i;
   if (UpdateSDKDirectoryInfosIfNeeded()) {
 const uint32_t num_sdk_infos = m_sdk_directory_infos.size();
-
-// Check to see if the user specified a build string. If they did, then be
-// sure to match it.
 std::vector check_sdk_info(num_sdk_infos, true);
-ConstString build(m_sdk_build);
+
+// Prefer the user SDK build string.
+ConstString build = GetSDKBuild();
+
+// Fall back to the platform's build string.
+if (!build) {
+  if (llvm::Optional os_build_str = GetOSBuildString()) {
+build = ConstString(*os_build_str);
+  }
+}
+
+// If we have a build string, only check platforms for which the build
+// string matches.
 if (build) {
   for (i = 0; i < num_sdk_infos; ++i)
 check_sdk_info[i] = m_sdk_directory_infos[i].build == build;
@@ -163,14 +172,14 @@
 llvm::VersionTuple version = GetOSVersion();
 if (!version.empty()) {
   if (UpdateSDKDirectoryInfosIfNeeded()) {
-// First try for an exact match of major, minor and update
+// First try for an exact match of major, minor and update.
 for (i = 0; i < num_sdk_infos; ++i) {
   if (check_sdk_info[i]) {
 if (m_sdk_directory_infos[i].version == version)
   return &m_sdk_directory_infos[i];
   }
 }
-// First try for an exact match of major and minor
+// Try for an exact match of major and minor.
 for (i = 0; i < num_sdk_infos; ++i) {
   if (check_sdk_info[i]) {
 if (m_sdk_directory_infos[i].version.getMaj

[Lldb-commits] [PATCH] D130528: [LLDB][NFC][Reliability] Fix uninitialized variables from Coverity scan. Part 2

[Slava Gurevich via Phabricator via lldb-commits]

fixathon updated this revision to Diff 447545.
fixathon added a comment.

Fix the build


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130528

Files:
  lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
  lldb/source/Plugins/Process/Utility/RegisterContextDarwin_arm64.cpp
  lldb/source/Plugins/Process/Utility/ThreadMemory.cpp
  lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
  lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/PdbUtil.cpp
  lldb/source/Plugins/SystemRuntime/MacOSX/SystemRuntimeMacOSX.cpp
  lldb/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp
  lldb/source/Symbol/Type.cpp
  lldb/source/Target/Process.cpp
  lldb/source/Target/RegisterContextUnwind.cpp
  lldb/source/Target/ThreadPlanCallFunction.cpp
  lldb/source/Target/ThreadPlanTracer.cpp
  lldb/source/Target/TraceDumper.cpp

Index: lldb/source/Target/TraceDumper.cpp
===
--- lldb/source/Target/TraceDumper.cpp
+++ lldb/source/Target/TraceDumper.cpp
@@ -286,7 +286,7 @@
 }
 
 TraceDumper::TraceItem TraceDumper::CreatRawTraceItem() {
-  TraceItem item;
+  TraceItem item = {};
   item.id = m_cursor_up->GetId();
 
   if (m_options.show_tsc)
Index: lldb/source/Target/ThreadPlanTracer.cpp
===
--- lldb/source/Target/ThreadPlanTracer.cpp
+++ lldb/source/Target/ThreadPlanTracer.cpp
@@ -36,11 +36,11 @@
 
 ThreadPlanTracer::ThreadPlanTracer(Thread &thread, lldb::StreamSP &stream_sp)
 : m_process(*thread.GetProcess().get()), m_tid(thread.GetID()),
-  m_enabled(false), m_stream_sp(stream_sp) {}
+  m_enabled(false), m_stream_sp(stream_sp), m_thread(nullptr) {}
 
 ThreadPlanTracer::ThreadPlanTracer(Thread &thread)
 : m_process(*thread.GetProcess().get()), m_tid(thread.GetID()),
-  m_enabled(false), m_stream_sp() {}
+  m_enabled(false), m_stream_sp(), m_thread(nullptr) {}
 
 Stream *ThreadPlanTracer::GetLogStream() {
   if (m_stream_sp)
Index: lldb/source/Target/ThreadPlanCallFunction.cpp
===
--- lldb/source/Target/ThreadPlanCallFunction.cpp
+++ lldb/source/Target/ThreadPlanCallFunction.cpp
@@ -104,7 +104,10 @@
   m_ignore_breakpoints(options.DoesIgnoreBreakpoints()),
   m_debug_execution(options.GetDebug()),
   m_trap_exceptions(options.GetTrapExceptions()), m_function_addr(function),
-  m_function_sp(0), m_takedown_done(false),
+  m_start_addr(), m_function_sp(0), m_subplan_sp(),
+  m_cxx_language_runtime(nullptr), m_objc_language_runtime(nullptr),
+  m_stored_thread_state(), m_real_stop_info_sp(), m_constructor_errors(),
+  m_return_valobj_sp(), m_takedown_done(false),
   m_should_clear_objc_exception_bp(false),
   m_should_clear_cxx_exception_bp(false),
   m_stop_address(LLDB_INVALID_ADDRESS), m_return_type(return_type) {
@@ -134,7 +137,10 @@
   m_ignore_breakpoints(options.DoesIgnoreBreakpoints()),
   m_debug_execution(options.GetDebug()),
   m_trap_exceptions(options.GetTrapExceptions()), m_function_addr(function),
-  m_function_sp(0), m_takedown_done(false),
+  m_start_addr(), m_function_sp(0), m_subplan_sp(),
+  m_cxx_language_runtime(nullptr), m_objc_language_runtime(nullptr),
+  m_stored_thread_state(), m_real_stop_info_sp(), m_constructor_errors(),
+  m_return_valobj_sp(), m_takedown_done(false),
   m_should_clear_objc_exception_bp(false),
   m_should_clear_cxx_exception_bp(false),
   m_stop_address(LLDB_INVALID_ADDRESS), m_return_type(CompilerType()) {}
Index: lldb/source/Target/RegisterContextUnwind.cpp
===
--- lldb/source/Target/RegisterContextUnwind.cpp
+++ lldb/source/Target/RegisterContextUnwind.cpp
@@ -1525,7 +1525,7 @@
 
   // unwindplan_regloc has valid contents about where to retrieve the register
   if (unwindplan_regloc.IsUnspecified()) {
-lldb_private::UnwindLLDB::RegisterLocation new_regloc;
+lldb_private::UnwindLLDB::RegisterLocation new_regloc = {};
 new_regloc.type = UnwindLLDB::RegisterLocation::eRegisterNotSaved;
 m_registers[regnum.GetAsKind(eRegisterKindLLDB)] = new_regloc;
 UnwindLogMsg("save location for %s (%d) is unspecified, continue searching",
@@ -1731,7 +1731,7 @@
 
   addr_t old_caller_pc_value = LLDB_INVALID_ADDRESS;
   addr_t new_caller_pc_value = LLDB_INVALID_ADDRESS;
-  UnwindLLDB::RegisterLocation regloc;
+  UnwindLLDB::RegisterLocation regloc = {};
   if (SavedLocationForRegister(pc_regnum.GetAsKind(eRegisterKindLLDB),
regloc) ==
   UnwindLLDB::RegisterSearchResult::eRegisterFound) {
Index: lldb/source/Target/Process.cpp
==

[Lldb-commits] [lldb] 4871dfc - [LLDB][NFC][Reliability] Fix uninitialized variables from Coverity scan. Part 2

[Slava Gurevich via lldb-commits]

Author: Slava Gurevich
Date: 2022-07-25T20:52:45-07:00
New Revision: 4871dfc64e35e9cf07008c56299125694c81720a

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

LOG: [LLDB][NFC][Reliability] Fix uninitialized variables from Coverity scan. 
Part 2

Improve LLDB reliability by fixing the following "uninitialized variables" 
static code inspection warnings from
scan.coverity.com:

1476275, 1274012, 1455035, 1364789, 1454282
1467483, 1406152, 1406255, 1454837, 1454416
1467446, 1462022, 1461909, 1420566, 1327228
1367767, 1431254, 1467299, 1312678, 1431780
1454731, 1490403

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

Added: 


Modified: 
lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
lldb/source/Plugins/Process/Utility/RegisterContextDarwin_arm64.cpp
lldb/source/Plugins/Process/Utility/ThreadMemory.cpp
lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
lldb/source/Plugins/SymbolFile/NativePDB/PdbUtil.cpp
lldb/source/Plugins/SystemRuntime/MacOSX/SystemRuntimeMacOSX.cpp
lldb/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp
lldb/source/Symbol/Type.cpp
lldb/source/Target/Process.cpp
lldb/source/Target/RegisterContextUnwind.cpp
lldb/source/Target/ThreadPlanCallFunction.cpp
lldb/source/Target/ThreadPlanTracer.cpp
lldb/source/Target/TraceDumper.cpp

Removed: 




diff  --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp 
b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
index acb131b8a775a..c396cb061c017 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
@@ -348,7 +348,7 @@ llvm::support::ulittle64_t 
read_register_u64(RegisterContext *reg_ctx,
 
 lldb_private::minidump::MinidumpContext_x86_64
 GetThreadContext_64(RegisterContext *reg_ctx) {
-  lldb_private::minidump::MinidumpContext_x86_64 thread_context;
+  lldb_private::minidump::MinidumpContext_x86_64 thread_context = {};
   thread_context.p1_home = {};
   thread_context.context_flags = static_cast(
   lldb_private::minidump::MinidumpContext_x86_64_Flags::x86_64_Flag |
@@ -534,7 +534,7 @@ Status MinidumpFileBuilder::AddException(const 
lldb::ProcessSP &process_sp) {
   helper_data.AppendData(
   &thread_context, sizeof(lldb_private::minidump::MinidumpContext_x86_64));
 
-  Exception exp_record;
+  Exception exp_record = {};
   exp_record.ExceptionCode =
   static_cast(stop_info_sp->GetValue());
   exp_record.ExceptionFlags = static_cast(0);

diff  --git a/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp 
b/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
index 90118f9386da3..f7f52deb173fb 100644
--- a/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
@@ -234,7 +234,7 @@ NativeProcessLinux::Factory::Launch(ProcessLaunchInfo 
&launch_info,
   }
 
   // Wait for the child process to trap on its call to execve.
-  int wstatus;
+  int wstatus = 0;
   ::pid_t wpid = llvm::sys::RetryAfterSignal(-1, ::waitpid, pid, &wstatus, 0);
   assert(wpid == pid);
   (void)wpid;

diff  --git 
a/lldb/source/Plugins/Process/Utility/RegisterContextDarwin_arm64.cpp 
b/lldb/source/Plugins/Process/Utility/RegisterContextDarwin_arm64.cpp
index 11b300bc44fbb..691e7db3fc79e 100644
--- a/lldb/source/Plugins/Process/Utility/RegisterContextDarwin_arm64.cpp
+++ b/lldb/source/Plugins/Process/Utility/RegisterContextDarwin_arm64.cpp
@@ -95,7 +95,7 @@ static size_t k_num_register_infos =
 
 RegisterContextDarwin_arm64::RegisterContextDarwin_arm64(
 Thread &thread, uint32_t concrete_frame_idx)
-: RegisterContext(thread, concrete_frame_idx), gpr(), fpu(), exc() {
+: RegisterContext(thread, concrete_frame_idx), gpr(), fpu(), exc(), dbg() {
   uint32_t i;
   for (i = 0; i < kNumErrors; i++) {
 gpr_errs[i] = -1;

diff  --git a/lldb/source/Plugins/Process/Utility/ThreadMemory.cpp 
b/lldb/source/Plugins/Process/Utility/ThreadMemory.cpp
index 7469e7633e71d..89ecc757a68f5 100644
--- a/lldb/source/Plugins/Process/Utility/ThreadMemory.cpp
+++ b/lldb/source/Plugins/Process/Utility/ThreadMemory.cpp
@@ -23,7 +23,8 @@ using namespace lldb_private;
 ThreadMemory::ThreadMemory(Process &process, tid_t tid,
const ValueObjectSP &thread_info_valobj_sp)
 : Thread(process, tid), m_backing_thread_sp(),
-  m_thread_info_valobj_sp(thread_info_valobj_sp), m_name(), m_queue() {}
+  m_thread_info_valobj_sp(thread_info_valobj_sp), m_name(), m_queue(),
+  m_register_data_addr(LLDB_INVALID_ADDRESS) {}
 

[Lldb-commits] [PATCH] D130528: [LLDB][NFC][Reliability] Fix uninitialized variables from Coverity scan. Part 2

[Slava Gurevich via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG4871dfc64e35: [LLDB][NFC][Reliability] Fix uninitialized 
variables from Coverity scan. Part 2 (authored by fixathon).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130528

Files:
  lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
  lldb/source/Plugins/Process/Utility/RegisterContextDarwin_arm64.cpp
  lldb/source/Plugins/Process/Utility/ThreadMemory.cpp
  lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
  lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/PdbUtil.cpp
  lldb/source/Plugins/SystemRuntime/MacOSX/SystemRuntimeMacOSX.cpp
  lldb/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp
  lldb/source/Symbol/Type.cpp
  lldb/source/Target/Process.cpp
  lldb/source/Target/RegisterContextUnwind.cpp
  lldb/source/Target/ThreadPlanCallFunction.cpp
  lldb/source/Target/ThreadPlanTracer.cpp
  lldb/source/Target/TraceDumper.cpp

Index: lldb/source/Target/TraceDumper.cpp
===
--- lldb/source/Target/TraceDumper.cpp
+++ lldb/source/Target/TraceDumper.cpp
@@ -286,7 +286,7 @@
 }
 
 TraceDumper::TraceItem TraceDumper::CreatRawTraceItem() {
-  TraceItem item;
+  TraceItem item = {};
   item.id = m_cursor_up->GetId();
 
   if (m_options.show_tsc)
Index: lldb/source/Target/ThreadPlanTracer.cpp
===
--- lldb/source/Target/ThreadPlanTracer.cpp
+++ lldb/source/Target/ThreadPlanTracer.cpp
@@ -36,11 +36,11 @@
 
 ThreadPlanTracer::ThreadPlanTracer(Thread &thread, lldb::StreamSP &stream_sp)
 : m_process(*thread.GetProcess().get()), m_tid(thread.GetID()),
-  m_enabled(false), m_stream_sp(stream_sp) {}
+  m_enabled(false), m_stream_sp(stream_sp), m_thread(nullptr) {}
 
 ThreadPlanTracer::ThreadPlanTracer(Thread &thread)
 : m_process(*thread.GetProcess().get()), m_tid(thread.GetID()),
-  m_enabled(false), m_stream_sp() {}
+  m_enabled(false), m_stream_sp(), m_thread(nullptr) {}
 
 Stream *ThreadPlanTracer::GetLogStream() {
   if (m_stream_sp)
Index: lldb/source/Target/ThreadPlanCallFunction.cpp
===
--- lldb/source/Target/ThreadPlanCallFunction.cpp
+++ lldb/source/Target/ThreadPlanCallFunction.cpp
@@ -104,7 +104,10 @@
   m_ignore_breakpoints(options.DoesIgnoreBreakpoints()),
   m_debug_execution(options.GetDebug()),
   m_trap_exceptions(options.GetTrapExceptions()), m_function_addr(function),
-  m_function_sp(0), m_takedown_done(false),
+  m_start_addr(), m_function_sp(0), m_subplan_sp(),
+  m_cxx_language_runtime(nullptr), m_objc_language_runtime(nullptr),
+  m_stored_thread_state(), m_real_stop_info_sp(), m_constructor_errors(),
+  m_return_valobj_sp(), m_takedown_done(false),
   m_should_clear_objc_exception_bp(false),
   m_should_clear_cxx_exception_bp(false),
   m_stop_address(LLDB_INVALID_ADDRESS), m_return_type(return_type) {
@@ -134,7 +137,10 @@
   m_ignore_breakpoints(options.DoesIgnoreBreakpoints()),
   m_debug_execution(options.GetDebug()),
   m_trap_exceptions(options.GetTrapExceptions()), m_function_addr(function),
-  m_function_sp(0), m_takedown_done(false),
+  m_start_addr(), m_function_sp(0), m_subplan_sp(),
+  m_cxx_language_runtime(nullptr), m_objc_language_runtime(nullptr),
+  m_stored_thread_state(), m_real_stop_info_sp(), m_constructor_errors(),
+  m_return_valobj_sp(), m_takedown_done(false),
   m_should_clear_objc_exception_bp(false),
   m_should_clear_cxx_exception_bp(false),
   m_stop_address(LLDB_INVALID_ADDRESS), m_return_type(CompilerType()) {}
Index: lldb/source/Target/RegisterContextUnwind.cpp
===
--- lldb/source/Target/RegisterContextUnwind.cpp
+++ lldb/source/Target/RegisterContextUnwind.cpp
@@ -1525,7 +1525,7 @@
 
   // unwindplan_regloc has valid contents about where to retrieve the register
   if (unwindplan_regloc.IsUnspecified()) {
-lldb_private::UnwindLLDB::RegisterLocation new_regloc;
+lldb_private::UnwindLLDB::RegisterLocation new_regloc = {};
 new_regloc.type = UnwindLLDB::RegisterLocation::eRegisterNotSaved;
 m_registers[regnum.GetAsKind(eRegisterKindLLDB)] = new_regloc;
 UnwindLogMsg("save location for %s (%d) is unspecified, continue searching",
@@ -1731,7 +1731,7 @@
 
   addr_t old_caller_pc_value = LLDB_INVALID_ADDRESS;
   addr_t new_caller_pc_value = LLDB_INVALID_ADDRESS;
-  UnwindLLDB::RegisterLocation regloc;
+  UnwindLLDB::RegisterLocation regloc = {};
   if (SavedLocationForRegister(pc_regnum.GetAsKind(eRegisterKindLLDB),
regloc) ==
   UnwindLLDB::R

[Lldb-commits] [PATCH] D130549: [NFC] Improve FileSpec internal APIs and usage in preparation for adding caching of resolved/absolute.

[Greg Clayton via Phabricator via lldb-commits]

clayborg created this revision.
clayborg added reviewers: JDevlieghere, thakis, smeenai.
Herald added a project: All.
clayborg requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added subscribers: lldb-commits, sstefan1.
Herald added a project: LLDB.

Resubmission of https://reviews.llvm.org/D130309 with the 2 patches that fixed 
the linux buildbot, and new windows fixes.

The FileSpec APIs allow users to modify instance variables directly by getting 
a non const reference to the directory and filename instance variables. This 
makes it impossible to control all of the times the FileSpec object is modified 
so we can clear cached member variables like m_resolved and with an upcoming 
patch caching if the file is relative or absolute. This patch modifies the APIs 
of FileSpec so no one can modify the directory or filename instance variables 
directly by adding set accessors and by removing the get accessors that are non 
const.

Many clients were using FileSpec::GetCString(...) which returned a unique C 
string from a ConstString'ified version of the result of GetPath() which 
returned a std::string. This caused many locations to use this convenient 
function incorrectly and could cause many strings to be added to the constant 
string pool that didn't need to. Most clients were converted to using 
FileSpec::GetPath().c_str() when possible. Other clients were modified to use 
the newly renamed version of this function which returns an actualy ConstString:

ConstString FileSpec::GetPathAsConstString(bool denormalize = true) const;

This avoids the issue where people were getting an already uniqued "const char 
*" that came from a ConstString only to put the "const char *" back into a 
"ConstString" object. By returning the ConstString instead of a "const char *" 
clients can be more efficient with the result.

The patch:

- Removes the non const GetDirectory() and GetFilename() get accessors
- Adds set accessors to replace the above functions: SetDirectory() and 
SetFilename().
- Adds ClearDirectory() and ClearFilename() to replace usage of the 
FileSpec::GetDirectory().Clear()/FileSpec::GetFilename().Clear() call sites
- Fixed all incorrect usage of FileSpec::GetCString() to use 
FileSpec::GetPath().c_str() where appropriate, and updated other call sites 
that wanted a ConstString to use the newly returned ConstString appropriately 
and efficiently.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D130549

Files:
  lldb/include/lldb/Utility/FileSpec.h
  lldb/source/API/SBFileSpec.cpp
  lldb/source/API/SBLaunchInfo.cpp
  lldb/source/API/SBPlatform.cpp
  lldb/source/API/SBReproducer.cpp
  lldb/source/Breakpoint/BreakpointResolverFileLine.cpp
  lldb/source/Commands/CommandObjectLog.cpp
  lldb/source/Commands/CommandObjectTarget.cpp
  lldb/source/Core/Debugger.cpp
  lldb/source/Core/IOHandlerCursesGUI.cpp
  lldb/source/Expression/FunctionCaller.cpp
  lldb/source/Expression/REPL.cpp
  lldb/source/Host/common/FileAction.cpp
  lldb/source/Host/common/FileSystem.cpp
  lldb/source/Host/common/HostInfoBase.cpp
  lldb/source/Host/linux/HostInfoLinux.cpp
  lldb/source/Host/macosx/objcxx/Host.mm
  lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
  lldb/source/Host/posix/FileSystemPosix.cpp
  lldb/source/Host/posix/HostInfoPosix.cpp
  lldb/source/Host/windows/FileSystem.cpp
  lldb/source/Host/windows/ProcessLauncherWindows.cpp
  lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangHost.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangUtilityFunction.cpp
  
lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
  lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
  lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
  lldb/source/Plugins/ObjectFile/PECOFF/WindowsMiniDump.cpp
  lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp
  lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
  lldb/source/Plugins/Process/Windows/Common/NativeProcessWindows.cpp
  lldb/source/Plugins/Process/Windows/Common/ProcessDebugger.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp