[Lldb-commits] [PATCH] D62221: [lldb-server][LLGS] Support 'g' packets

[Pavel Labath via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL362063: [lldb-server] Support 'g' packets 
(authored by labath, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D62221?vs=201790&id=202124#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D62221

Files:
  
lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/TestGdbRemoteGPacket.py
  
lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/TestLldbGdbServer.py
  
lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
  
lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/register-reading/Makefile
  
lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/register-reading/TestGdbRemoteGPacket.py
  
lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/register-reading/main.cpp
  
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
  lldb/trunk/source/Utility/StringExtractorGDBRemote.cpp

Index: lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/TestLldbGdbServer.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/TestLldbGdbServer.py
+++ lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/TestLldbGdbServer.py
@@ -558,9 +558,7 @@
 self.assertIsNotNone(reg_infos)
 self.assertTrue(len(reg_infos) > 0)
 
-inferior_exe_path = self.getBuildArtifact("a.out")
-Target = self.dbg.CreateTarget(inferior_exe_path)
-byte_order = Target.GetByteOrder()
+byte_order = self.get_target_byte_order()
 
 # Read value for each register.
 reg_index = 0
Index: lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/register-reading/main.cpp
===
--- lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/register-reading/main.cpp
+++ lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/register-reading/main.cpp
@@ -0,0 +1,54 @@
+#include 
+
+struct alignas(16) xmm_t {
+  uint64_t a, b;
+};
+
+int main() {
+  uint64_t r8 = 0x0102030405060708;
+  uint64_t r9 = 0x1112131415161718;
+  uint64_t r10 = 0x2122232425262728;
+  uint64_t r11 = 0x3132333435363738;
+  uint64_t r12 = 0x4142434445464748;
+  uint64_t r13 = 0x5152535455565758;
+  uint64_t r14 = 0x6162636465666768;
+  uint64_t r15 = 0x7172737475767778;
+
+  xmm_t xmm8 = {0x020406080A0C0E01, 0x030507090B0D0F00};
+  xmm_t xmm9 = {0x121416181A1C1E11, 0x131517191B1D1F10};
+  xmm_t xmm10 = {0x222426282A2C2E21, 0x232527292B2D2F20};
+  xmm_t xmm11 = {0x323436383A3C3E31, 0x333537393B3D3F30};
+  xmm_t xmm12 = {0x424446484A4C4E41, 0x434547494B4D4F40};
+  xmm_t xmm13 = {0x525456585A5C5E51, 0x535557595B5D5F50};
+  xmm_t xmm14 = {0x626466686A6C6E61, 0x636567696B6D6F60};
+  xmm_t xmm15 = {0x727476787A7C7E71, 0x737577797B7D7F70};
+
+  asm volatile("movq%0, %%r8\n\t"
+   "movq%1, %%r9\n\t"
+   "movq%2, %%r10\n\t"
+   "movq%3, %%r11\n\t"
+   "movq%4, %%r12\n\t"
+   "movq%5, %%r13\n\t"
+   "movq%6, %%r14\n\t"
+   "movq%7, %%r15\n\t"
+   "\n\t"
+   "movaps  %8, %%xmm8\n\t"
+   "movaps  %9, %%xmm9\n\t"
+   "movaps  %10, %%xmm10\n\t"
+   "movaps  %11, %%xmm11\n\t"
+   "movaps  %12, %%xmm12\n\t"
+   "movaps  %13, %%xmm13\n\t"
+   "movaps  %14, %%xmm14\n\t"
+   "movaps  %15, %%xmm15\n\t"
+   "\n\t"
+   "int3"
+   :
+   : "g"(r8), "g"(r9), "g"(r10), "g"(r11), "g"(r12), "g"(r13),
+ "g"(r14), "g"(r15), "m"(xmm8), "m"(xmm9), "m"(xmm10),
+ "m"(xmm11), "m"(xmm12), "m"(xmm13), "m"(xmm14), "m"(xmm15)
+   : "%r8", "%r9", "%r10", "%r11", "%r12", "%r13", "%r14", "%r15",
+ "%xmm8", "%xmm9", "%xmm10", "%xmm11", "%xmm12", "%xmm13",
+ "%xmm14", "%xmm15");
+
+  return 0;
+}
Index: lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/register-reading/TestGdbRemoteGPacket.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/register-reading/TestGdbRemoteGPacket.py
+++ lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/register-reading/TestGdbRemoteGPacket.py
@@ -0,0 +1,153 @@
+from __future__ import print_function
+
+
+import gdbremote_testcase
+import textwrap
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+def _extract_register_value(reg_info, reg_bank, byte_o

[Lldb-commits] [lldb] r362063 - [lldb-server] Support 'g' packets

[Pavel Labath via lldb-commits]

Author: labath
Date: Thu May 30 00:25:22 2019
New Revision: 362063

URL: http://llvm.org/viewvc/llvm-project?rev=362063&view=rev
Log:
[lldb-server] Support 'g' packets

Differential Revision: https://reviews.llvm.org/D62221
Patch by Guilherme Andrade .

Added:

lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/register-reading/

lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/register-reading/Makefile

lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/register-reading/TestGdbRemoteGPacket.py

lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/register-reading/main.cpp
Removed:

lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/TestGdbRemoteGPacket.py
Modified:

lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/TestLldbGdbServer.py

lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py

lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp

lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
lldb/trunk/source/Utility/StringExtractorGDBRemote.cpp

Removed: 
lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/TestGdbRemoteGPacket.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/TestGdbRemoteGPacket.py?rev=362062&view=auto
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/TestGdbRemoteGPacket.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/TestGdbRemoteGPacket.py
 (removed)
@@ -1,41 +0,0 @@
-from __future__ import print_function
-
-
-import gdbremote_testcase
-from lldbsuite.test.decorators import *
-from lldbsuite.test.lldbtest import *
-from lldbsuite.test import lldbutil
-
-
-class TestGdbRemoteGPacket(gdbremote_testcase.GdbRemoteTestCaseBase):
-
-mydir = TestBase.compute_mydir(__file__)
-
-def run_test_g_packet(self):
-self.build()
-self.prep_debug_monitor_and_inferior()
-self.test_sequence.add_log_lines(
-["read packet: $g#67",
- {"direction": "send", "regex": r"^\$(.+)#[0-9a-fA-F]{2}$",
-  "capture": {1: "register_bank"}}],
-True)
-self.connect_to_debug_monitor()
-context = self.expect_gdbremote_sequence()
-register_bank = context.get("register_bank")
-self.assertTrue(register_bank[0] != 'E')
-
-self.test_sequence.add_log_lines(
-["read packet: $G" + register_bank + "#00",
- {"direction": "send", "regex": r"^\$(.+)#[0-9a-fA-F]{2}$",
-  "capture": {1: "G_reply"}}],
-True)
-context = self.expect_gdbremote_sequence()
-self.assertTrue(context.get("G_reply")[0] != 'E')
-
-
-@skipIfOutOfTreeDebugserver
-@debugserver_test
-@skipIfDarwinEmbedded
-def test_g_packet_debugserver(self):
-self.init_debugserver_test()
-self.run_test_g_packet()

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/TestLldbGdbServer.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/TestLldbGdbServer.py?rev=362063&r1=362062&r2=362063&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/TestLldbGdbServer.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/TestLldbGdbServer.py
 Thu May 30 00:25:22 2019
@@ -558,9 +558,7 @@ class LldbGdbServerTestCase(gdbremote_te
 self.assertIsNotNone(reg_infos)
 self.assertTrue(len(reg_infos) > 0)
 
-inferior_exe_path = self.getBuildArtifact("a.out")
-Target = self.dbg.CreateTarget(inferior_exe_path)
-byte_order = Target.GetByteOrder()
+byte_order = self.get_target_byte_order()
 
 # Read value for each register.
 reg_index = 0

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py?rev=362063&r1=362062&r2=362063&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
 Thu May 30 00:25:22 2019
@@ -378,6 +378,11 @@ class GdbRemoteTestCaseBase(TestBase):
 commandline_args += ["--named-pipe", self.named_pipe_path]
 return commandline_args
 
+def get_target_byte_order(self):
+inferior_exe_path = self.getBuildArtifact("a.out")
+target = self.dbg.CreateTarget(inferior_exe_path)
+return target.GetByteOrder()
+
 def launch_debug_monitor(self, attach_

[Lldb-commits] [PATCH] D62634: Improve DWARF parsing and accessing by 1% to 2%

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added inline comments.



Comment at: source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp:724
 uint32_t curr_depth) const {
-  const DWARFAbbreviationDeclaration *abbrevDecl = nullptr;
-  lldb::offset_t offset = 0;
-  if (cu)
-abbrevDecl = GetAbbreviationDeclarationPtr(cu, offset);
-
+  auto abbrevDecl = GetAbbreviationDeclarationPtr(cu);
   if (abbrevDecl) {

It would be better to say `auto *abbrevDecl` here. The extra char doesn't cost 
much, and it saves one from wondering whether there is anything fancy going on 
behind the scenes 



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

https://reviews.llvm.org/D62634



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

[Lldb-commits] [lldb] r362069 - Make CompileUnit::GetSupportFiles return a const list

[Pavel Labath via lldb-commits]

Author: labath
Date: Thu May 30 01:21:25 2019
New Revision: 362069

URL: http://llvm.org/viewvc/llvm-project?rev=362069&view=rev
Log:
Make CompileUnit::GetSupportFiles return a const list

There's no reason for anyone to modify a list from outside of a symbol
file (as that would break a lot of invariants that symbol files depend
on).

Make the function return a const FileSpecList and fix up a couple of
places that were needlessly binding non-const references to the result
of this function.

Modified:
lldb/trunk/include/lldb/Symbol/CompileUnit.h
lldb/trunk/source/API/SBCompileUnit.cpp
lldb/trunk/source/Symbol/CompileUnit.cpp

Modified: lldb/trunk/include/lldb/Symbol/CompileUnit.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Symbol/CompileUnit.h?rev=362069&r1=362068&r2=362069&view=diff
==
--- lldb/trunk/include/lldb/Symbol/CompileUnit.h (original)
+++ lldb/trunk/include/lldb/Symbol/CompileUnit.h Thu May 30 01:21:25 2019
@@ -232,7 +232,7 @@ public:
   ///
   /// \return
   /// A support file list object.
-  FileSpecList &GetSupportFiles();
+  const FileSpecList &GetSupportFiles();
 
   /// Get the compile unit's imported module list.
   ///

Modified: lldb/trunk/source/API/SBCompileUnit.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/API/SBCompileUnit.cpp?rev=362069&r1=362068&r2=362069&view=diff
==
--- lldb/trunk/source/API/SBCompileUnit.cpp (original)
+++ lldb/trunk/source/API/SBCompileUnit.cpp Thu May 30 01:21:25 2019
@@ -118,10 +118,9 @@ uint32_t SBCompileUnit::FindLineEntryInd
 uint32_t SBCompileUnit::GetNumSupportFiles() const {
   LLDB_RECORD_METHOD_CONST_NO_ARGS(uint32_t, SBCompileUnit, 
GetNumSupportFiles);
 
-  if (m_opaque_ptr) {
-FileSpecList &support_files = m_opaque_ptr->GetSupportFiles();
-return support_files.GetSize();
-  }
+  if (m_opaque_ptr)
+return m_opaque_ptr->GetSupportFiles().GetSize();
+
   return 0;
 }
 
@@ -155,9 +154,8 @@ SBFileSpec SBCompileUnit::GetSupportFile
 
   SBFileSpec sb_file_spec;
   if (m_opaque_ptr) {
-FileSpecList &support_files = m_opaque_ptr->GetSupportFiles();
-FileSpec file_spec = support_files.GetFileSpecAtIndex(idx);
-sb_file_spec.SetFileSpec(file_spec);
+FileSpec spec = m_opaque_ptr->GetSupportFiles().GetFileSpecAtIndex(idx);
+sb_file_spec.SetFileSpec(spec);
   }
 
 
@@ -172,7 +170,7 @@ uint32_t SBCompileUnit::FindSupportFileI
  sb_file, full);
 
   if (m_opaque_ptr) {
-FileSpecList &support_files = m_opaque_ptr->GetSupportFiles();
+const FileSpecList &support_files = m_opaque_ptr->GetSupportFiles();
 return support_files.FindFileIndex(start_idx, sb_file.ref(), full);
   }
   return 0;

Modified: lldb/trunk/source/Symbol/CompileUnit.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Symbol/CompileUnit.cpp?rev=362069&r1=362068&r2=362069&view=diff
==
--- lldb/trunk/source/Symbol/CompileUnit.cpp (original)
+++ lldb/trunk/source/Symbol/CompileUnit.cpp Thu May 30 01:21:25 2019
@@ -248,7 +248,7 @@ uint32_t CompileUnit::FindLineEntry(uint
 // All the line table entries actually point to the version of the Compile
 // Unit that is in the support files (the one at 0 was artificially added.)
 // So prefer the one further on in the support files if it exists...
-FileSpecList &support_files = GetSupportFiles();
+const FileSpecList &support_files = GetSupportFiles();
 const bool full = true;
 file_idx = support_files.FindFileIndex(
 1, support_files.GetFileSpecAtIndex(0), full);
@@ -397,7 +397,7 @@ const std::vector &Compile
   return m_imported_modules;
 }
 
-FileSpecList &CompileUnit::GetSupportFiles() {
+const FileSpecList &CompileUnit::GetSupportFiles() {
   if (m_support_files.GetSize() == 0) {
 if (m_flags.IsClear(flagsParsedSupportFiles)) {
   m_flags.Set(flagsParsedSupportFiles);


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

[Lldb-commits] [PATCH] D62570: [WIP] Use LLVM's debug line parser in LLDB

[George Rimar via Phabricator via lldb-commits]

grimar added inline comments.



Comment at: lldb/include/lldb/Core/FileSpecList.h:29
+  typedef std::vector collection;
+  typedef collection::iterator iterator;
+  typedef collection::const_iterator const_iterator;

Seems you don't use `iterator` anywhere?



Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:792
+  cu_die.GetAttributeValueAsUnsigned(DW_AT_stmt_list, DW_INVALID_OFFSET);
+  if (stmt_list == DW_INVALID_OFFSET)
+return false;

You do not use `stmt_list` anywhere else, so I would suggest to:

```
  if (cu_die.GetAttributeValueAsUnsigned(DW_AT_stmt_list, DW_INVALID_OFFSET) ==
  DW_INVALID_OFFSET)
continue;
```



Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:879
+
+  auto AddSection = [&](Section §ion) {
+DataExtractor section_data;

`AddSection`->`addSection`?



Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:929-938
+  llvm::DWARFDebugLine line;
+  llvm::Expected line_table =
+  line.getOrParseLineTable(
+  data, offset, *ctx, ctx->getUnitForOffset(dwarf_cu->GetOffset()),
+  [](llvm::Error e) { llvm::consumeError(std::move(e)); });
+
+  if (!line_table) {

clayborg wrote:
> Can we make a function out of this that just returns the "const 
> llvm::DWARFDebugLine::LineTable *"? Or make a new location variable that is 
> just a "const llvm::DWARFDebugLine::LineTable *" to avoid the 
> "(*line_table)->" stuff below?
+1 for `new location variable`.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:974
+if (!found_original)
+  continue;
+std::string remapped_file;

Seems you can avoid having `found_original` and `found_remapped`:
(assuming `table` is a new variable mentioned above)

```
  if (!table->getFileNameByIndex(
  index + 1, compile_dir,
  llvm::DILineInfoSpecifier::FileLineInfoKind::Default, original_file))
continue;
```


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D62570



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

[Lldb-commits] [PATCH] D62649: CompileUnit: Use shared_ptr for storing support file lists

[Pavel Labath via Phabricator via lldb-commits]

labath created this revision.
labath added reviewers: JDevlieghere, clayborg, aprantl.
Herald added a subscriber: jdoerfert.

This patch refactors the CompileUnit class to store the support files in
a shared_ptr, and changes the SymbolFiles to hand them out as such.

This allows the file lists to be shared between compile units, or
between other objects. The motivation for this is to enable
SymbolFileDWARF to keep a handle on the file lists it parses, so that it
can use it when processing type units (which will not be handed out as
lldb_private::CompileUnits). This will be implemented in follow-up
patch(es).

Since I was already changing the signature of this function, I made it
return an Expected and changed all the places that were
silently doing nothing to return an error. The CompileUnit class still
returns an empty file list in case the parsing failed, but it first
makes sure any errors get reported.


https://reviews.llvm.org/D62649

Files:
  include/lldb/Symbol/CompileUnit.h
  include/lldb/Symbol/SymbolFile.h
  include/lldb/Symbol/SymbolVendor.h
  source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
  source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
  source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
  source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
  source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
  source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
  source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.cpp
  source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.h
  source/Symbol/CompileUnit.cpp
  source/Symbol/SymbolVendor.cpp

Index: source/Symbol/SymbolVendor.cpp
===
--- source/Symbol/SymbolVendor.cpp
+++ source/Symbol/SymbolVendor.cpp
@@ -155,15 +155,19 @@
   }
   return false;
 }
-bool SymbolVendor::ParseSupportFiles(CompileUnit &comp_unit,
- FileSpecList &support_files) {
+
+llvm::Expected>
+SymbolVendor::ParseSupportFiles(CompileUnit &comp_unit) {
   ModuleSP module_sp(GetModule());
   if (module_sp) {
 std::lock_guard guard(module_sp->GetMutex());
 if (m_sym_file_up)
-  return m_sym_file_up->ParseSupportFiles(comp_unit, support_files);
+  return m_sym_file_up->ParseSupportFiles(comp_unit);
+return llvm::createStringError(llvm::inconvertibleErrorCode(),
+   "No symbol file.");
   }
-  return false;
+  return llvm::createStringError(llvm::inconvertibleErrorCode(),
+ "No module file.");
 }
 
 bool SymbolVendor::ParseIsOptimized(CompileUnit &comp_unit) {
Index: source/Symbol/CompileUnit.cpp
===
--- source/Symbol/CompileUnit.cpp
+++ source/Symbol/CompileUnit.cpp
@@ -22,8 +22,7 @@
  lldb_private::LazyBool is_optimized)
 : ModuleChild(module_sp), FileSpec(pathname), UserID(cu_sym_id),
   m_user_data(user_data), m_language(language), m_flags(0),
-  m_support_files(), m_line_table_up(), m_variables(),
-  m_is_optimized(is_optimized) {
+  m_line_table_up(), m_variables(), m_is_optimized(is_optimized) {
   if (language != eLanguageTypeUnknown)
 m_flags.Set(flagsParsedLanguage);
   assert(module_sp);
@@ -35,8 +34,7 @@
  lldb_private::LazyBool is_optimized)
 : ModuleChild(module_sp), FileSpec(fspec), UserID(cu_sym_id),
   m_user_data(user_data), m_language(language), m_flags(0),
-  m_support_files(), m_line_table_up(), m_variables(),
-  m_is_optimized(is_optimized) {
+  m_line_table_up(), m_variables(), m_is_optimized(is_optimized) {
   if (language != eLanguageTypeUnknown)
 m_flags.Set(flagsParsedLanguage);
   assert(module_sp);
@@ -398,16 +396,24 @@
 }
 
 const FileSpecList &CompileUnit::GetSupportFiles() {
-  if (m_support_files.GetSize() == 0) {
-if (m_flags.IsClear(flagsParsedSupportFiles)) {
-  m_flags.Set(flagsParsedSupportFiles);
-  SymbolVendor *symbol_vendor = GetModule()->GetSymbolVendor();
-  if (symbol_vendor) {
-symbol_vendor->ParseSupportFiles(*this, m_support_files);
-  }
+  if (!m_support_files_sp) {
+SymbolVendor *symbol_vendor = GetModule()->GetSymbolVendor();
+llvm::Expected> expected_list(
+symbol_vendor ? symbol_vendor->ParseSupportFiles(*this)
+  : llvm::createStringError(llvm::inconvertibleErrorCode(),
+"No symbol vendor."));
+
+if (expected_list) {
+  assert(*expected_list);
+  m_support_files_sp = *expected_list;
+} else {
+  GetModule()->ReportError(
+  "Parsing support files for compile unit `%s` failed: %s",
+  GetCString(), toString(expected_list.t

[Lldb-commits] [PATCH] D62649: CompileUnit: Use shared_ptr for storing support file lists

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

This isn't strictly necessary. I could just have SymbolFileDWARF keep one copy 
a file list for each line table. That way each list would be stored exactly 
twice. However, it seemed like it would be nice to avoid that (though I don't 
have any data about how much this saves or anything).


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

https://reviews.llvm.org/D62649



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

[Lldb-commits] [PATCH] D62634: Improve DWARF parsing and accessing by 1% to 2%

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

BTW, how do you measure these things?

When I tried to benchmark something I got a difference of at least 5% on what 
should be identical runs. I'd have to make hundreds of runs in order for a 1% 
difference in average to show up as statistically significant..


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

https://reviews.llvm.org/D62634



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

[Lldb-commits] [lldb] r362075 - DWARFASTParserClang: Move attribute parsing into a single function

[Pavel Labath via lldb-commits]

Author: labath
Date: Thu May 30 02:39:36 2019
New Revision: 362075

URL: http://llvm.org/viewvc/llvm-project?rev=362075&view=rev
Log:
DWARFASTParserClang: Move attribute parsing into a single function

Summary:
The ParseTypeFromDWARF function consists of a huge switch on the kind of
type being parsed. Each case in this switch starts with parsing the
attributes of the current DIE. A lot of these attributes are specific to
one kind of a type, but a lot of them are common too, leading to code
duplication.

This patch reduces the duplication (and the size of ParseTypeFromDWARF)
by moving the attribute parsing to a separate function. It creates a
struct (ParsedTypeAttributes), which contains a parsed form of all
attributes which are useful for parsing any kind of a type. The parsing
code for a specific type kind can then access the fields which are
relevant for that specific case.

Reviewers: JDevlieghere, clayborg, aprantl

Subscribers: jdoerfert, lldb-commits

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

Modified:
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp

Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp?rev=362075&r1=362074&r2=362075&view=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp 
(original)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp Thu May 
30 02:39:36 2019
@@ -228,13 +228,169 @@ static void CompleteExternalTagDeclType(
   }
 }
 
+namespace {
+/// Parsed form of all attributes that are relevant for type reconstruction.
+/// Some attributes are relevant for all kinds of types (declaration), while
+/// others are only meaningful to a specific type (is_virtual)
+struct ParsedTypeAttributes {
+  ParsedTypeAttributes(const DWARFDIE &die, CompileUnit &comp_unit);
+
+  AccessType accessibility = eAccessNone;
+  bool is_artificial = false;
+  bool is_complete_objc_class = false;
+  bool is_explicit = false;
+  bool is_forward_declaration = false;
+  bool is_inline = false;
+  bool is_scoped_enum = false;
+  bool is_vector = false;
+  bool is_virtual = false;
+  clang::StorageClass storage = clang::SC_None;
+  const char *mangled_name = nullptr;
+  ConstString name;
+  Declaration decl;
+  DWARFDIE object_pointer;
+  DWARFFormValue abstract_origin;
+  DWARFFormValue containing_type;
+  DWARFFormValue signature;
+  DWARFFormValue specification;
+  DWARFFormValue type;
+  LanguageType class_language = eLanguageTypeUnknown;
+  llvm::Optional byte_size;
+  size_t calling_convention = llvm::dwarf::DW_CC_normal;
+  uint32_t bit_stride = 0;
+  uint32_t byte_stride = 0;
+  uint32_t encoding = 0;
+};
+} // namespace
+
+ParsedTypeAttributes::ParsedTypeAttributes(const DWARFDIE &die,
+   CompileUnit &comp_unit) {
+  DWARFAttributes attributes;
+  size_t num_attributes = die.GetAttributes(attributes);
+  for (size_t i = 0; i < num_attributes; ++i) {
+dw_attr_t attr = attributes.AttributeAtIndex(i);
+DWARFFormValue form_value;
+if (!attributes.ExtractFormValueAtIndex(i, form_value))
+  continue;
+switch (attr) {
+case DW_AT_abstract_origin:
+  abstract_origin = form_value;
+  break;
+
+case DW_AT_accessibility:
+  accessibility = DW_ACCESS_to_AccessType(form_value.Unsigned());
+  break;
+
+case DW_AT_artificial:
+  is_artificial = form_value.Boolean();
+  break;
+
+case DW_AT_bit_stride:
+  bit_stride = form_value.Unsigned();
+  break;
+
+case DW_AT_byte_size:
+  byte_size = form_value.Unsigned();
+  break;
+
+case DW_AT_byte_stride:
+  byte_stride = form_value.Unsigned();
+  break;
+
+case DW_AT_calling_convention:
+  calling_convention = form_value.Unsigned();
+  break;
+
+case DW_AT_containing_type:
+  containing_type = form_value;
+  break;
+
+case DW_AT_decl_file:
+  decl.SetFile(comp_unit.GetSupportFiles().GetFileSpecAtIndex(
+  form_value.Unsigned()));
+  break;
+case DW_AT_decl_line:
+  decl.SetLine(form_value.Unsigned());
+  break;
+case DW_AT_decl_column:
+  decl.SetColumn(form_value.Unsigned());
+  break;
+
+case DW_AT_declaration:
+  is_forward_declaration = form_value.Boolean();
+  break;
+
+case DW_AT_encoding:
+  encoding = form_value.Unsigned();
+  break;
+
+case DW_AT_enum_class:
+  is_scoped_enum = form_value.Boolean();
+  break;
+
+case DW_AT_explicit:
+  is_explicit = form_value.Boolean();
+  break;
+
+case DW_AT_external:
+  if (form_value.Unsigned())
+storage = clang::SC_Extern;
+  break;
+
+case DW_AT_inline:
+  is_inline = form_value.Boolean();
+  break;
+
+case DW_AT_linkage_name:
+case DW_AT_MIPS_linkage_nam

[Lldb-commits] [PATCH] D62477: DWARFASTParserClang: Move attribute parsing into a single function

[Pavel Labath via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL362075: DWARFASTParserClang: Move attribute parsing into a 
single function (authored by labath, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D62477?vs=201497&id=202142#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D62477

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

Index: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -228,13 +228,169 @@
   }
 }
 
+namespace {
+/// Parsed form of all attributes that are relevant for type reconstruction.
+/// Some attributes are relevant for all kinds of types (declaration), while
+/// others are only meaningful to a specific type (is_virtual)
+struct ParsedTypeAttributes {
+  ParsedTypeAttributes(const DWARFDIE &die, CompileUnit &comp_unit);
+
+  AccessType accessibility = eAccessNone;
+  bool is_artificial = false;
+  bool is_complete_objc_class = false;
+  bool is_explicit = false;
+  bool is_forward_declaration = false;
+  bool is_inline = false;
+  bool is_scoped_enum = false;
+  bool is_vector = false;
+  bool is_virtual = false;
+  clang::StorageClass storage = clang::SC_None;
+  const char *mangled_name = nullptr;
+  ConstString name;
+  Declaration decl;
+  DWARFDIE object_pointer;
+  DWARFFormValue abstract_origin;
+  DWARFFormValue containing_type;
+  DWARFFormValue signature;
+  DWARFFormValue specification;
+  DWARFFormValue type;
+  LanguageType class_language = eLanguageTypeUnknown;
+  llvm::Optional byte_size;
+  size_t calling_convention = llvm::dwarf::DW_CC_normal;
+  uint32_t bit_stride = 0;
+  uint32_t byte_stride = 0;
+  uint32_t encoding = 0;
+};
+} // namespace
+
+ParsedTypeAttributes::ParsedTypeAttributes(const DWARFDIE &die,
+   CompileUnit &comp_unit) {
+  DWARFAttributes attributes;
+  size_t num_attributes = die.GetAttributes(attributes);
+  for (size_t i = 0; i < num_attributes; ++i) {
+dw_attr_t attr = attributes.AttributeAtIndex(i);
+DWARFFormValue form_value;
+if (!attributes.ExtractFormValueAtIndex(i, form_value))
+  continue;
+switch (attr) {
+case DW_AT_abstract_origin:
+  abstract_origin = form_value;
+  break;
+
+case DW_AT_accessibility:
+  accessibility = DW_ACCESS_to_AccessType(form_value.Unsigned());
+  break;
+
+case DW_AT_artificial:
+  is_artificial = form_value.Boolean();
+  break;
+
+case DW_AT_bit_stride:
+  bit_stride = form_value.Unsigned();
+  break;
+
+case DW_AT_byte_size:
+  byte_size = form_value.Unsigned();
+  break;
+
+case DW_AT_byte_stride:
+  byte_stride = form_value.Unsigned();
+  break;
+
+case DW_AT_calling_convention:
+  calling_convention = form_value.Unsigned();
+  break;
+
+case DW_AT_containing_type:
+  containing_type = form_value;
+  break;
+
+case DW_AT_decl_file:
+  decl.SetFile(comp_unit.GetSupportFiles().GetFileSpecAtIndex(
+  form_value.Unsigned()));
+  break;
+case DW_AT_decl_line:
+  decl.SetLine(form_value.Unsigned());
+  break;
+case DW_AT_decl_column:
+  decl.SetColumn(form_value.Unsigned());
+  break;
+
+case DW_AT_declaration:
+  is_forward_declaration = form_value.Boolean();
+  break;
+
+case DW_AT_encoding:
+  encoding = form_value.Unsigned();
+  break;
+
+case DW_AT_enum_class:
+  is_scoped_enum = form_value.Boolean();
+  break;
+
+case DW_AT_explicit:
+  is_explicit = form_value.Boolean();
+  break;
+
+case DW_AT_external:
+  if (form_value.Unsigned())
+storage = clang::SC_Extern;
+  break;
+
+case DW_AT_inline:
+  is_inline = form_value.Boolean();
+  break;
+
+case DW_AT_linkage_name:
+case DW_AT_MIPS_linkage_name:
+  mangled_name = form_value.AsCString();
+  break;
+
+case DW_AT_name:
+  name.SetCString(form_value.AsCString());
+  break;
+
+case DW_AT_object_pointer:
+  object_pointer = form_value.Reference();
+  break;
+
+case DW_AT_signature:
+  signature = form_value;
+  break;
+
+case DW_AT_specification:
+  specification = form_value;
+  break;
+
+case DW_AT_type:
+  type = form_value;
+  break;
+
+case DW_AT_virtuality:
+  is_virtual = form_value.Boolean();
+  break;
+
+case DW_AT_APPLE_objc_complete_type:
+  is_complete_objc_class = form_value.Signed();
+  break;
+
+case DW_AT_APPLE_runtime_class:
+  class_language = (LanguageType)form_value.Signed();
+  break;
+
+case DW_AT_

[Lldb-commits] [lldb] r362086 - DWARFASTParserClang: Delete dead code

[Pavel Labath via lldb-commits]

Author: labath
Date: Thu May 30 04:24:16 2019
New Revision: 362086

URL: http://llvm.org/viewvc/llvm-project?rev=362086&view=rev
Log:
DWARFASTParserClang: Delete dead code

This removes places where DW_AT_decl_file/line/column was being parsed,
but not used.

Modified:
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp

Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp?rev=362086&r1=362085&r2=362086&view=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp 
(original)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp Thu May 
30 04:24:16 2019
@@ -2481,7 +2481,6 @@ bool DWARFASTParserClang::ParseChildMemb
   DWARFAttributes attributes;
   const size_t num_attributes = die.GetAttributes(attributes);
   if (num_attributes > 0) {
-Declaration decl;
 const char *name = nullptr;
 const char *prop_name = nullptr;
 const char *prop_getter_name = nullptr;
@@ -2505,16 +2504,6 @@ bool DWARFASTParserClang::ParseChildMemb
   DWARFFormValue form_value;
   if (attributes.ExtractFormValueAtIndex(i, form_value)) {
 switch (attr) {
-case DW_AT_decl_file:
-  decl.SetFile(sc.comp_unit->GetSupportFiles().GetFileSpecAtIndex(
-  form_value.Unsigned()));
-  break;
-case DW_AT_decl_line:
-  decl.SetLine(form_value.Unsigned());
-  break;
-case DW_AT_decl_column:
-  decl.SetColumn(form_value.Unsigned());
-  break;
 case DW_AT_name:
   name = form_value.AsCString();
   break;
@@ -2960,7 +2949,6 @@ bool DWARFASTParserClang::ParseChildMemb
   DWARFAttributes attributes;
   const size_t num_attributes = die.GetAttributes(attributes);
   if (num_attributes > 0) {
-Declaration decl;
 DWARFFormValue encoding_form;
 AccessType accessibility = default_accessibility;
 bool is_virtual = false;
@@ -2972,16 +2960,6 @@ bool DWARFASTParserClang::ParseChildMemb
   DWARFFormValue form_value;
   if (attributes.ExtractFormValueAtIndex(i, form_value)) {
 switch (attr) {
-case DW_AT_decl_file:
-  decl.SetFile(sc.comp_unit->GetSupportFiles().GetFileSpecAtIndex(
-  form_value.Unsigned()));
-  break;
-case DW_AT_decl_line:
-  decl.SetLine(form_value.Unsigned());
-  break;
-case DW_AT_decl_column:
-  decl.SetColumn(form_value.Unsigned());
-  break;
 case DW_AT_type:
   encoding_form = form_value;
   break;
@@ -3105,7 +3083,6 @@ size_t DWARFASTParserClang::ParseChildPa
   const size_t num_attributes = die.GetAttributes(attributes);
   if (num_attributes > 0) {
 const char *name = nullptr;
-Declaration decl;
 DWARFFormValue param_type_die_form;
 bool is_artificial = false;
 // one of None, Auto, Register, Extern, Static, PrivateExtern
@@ -3117,16 +3094,6 @@ size_t DWARFASTParserClang::ParseChildPa
   DWARFFormValue form_value;
   if (attributes.ExtractFormValueAtIndex(i, form_value)) {
 switch (attr) {
-case DW_AT_decl_file:
-  decl.SetFile(comp_unit.GetSupportFiles().GetFileSpecAtIndex(
-  form_value.Unsigned()));
-  break;
-case DW_AT_decl_line:
-  decl.SetLine(form_value.Unsigned());
-  break;
-case DW_AT_decl_column:
-  decl.SetColumn(form_value.Unsigned());
-  break;
 case DW_AT_name:
   name = form_value.AsCString();
   break;


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

[Lldb-commits] [PATCH] D62654: [Docs] Modernize references to macOS

[J. Ryan Stinnett via Phabricator via lldb-commits]

jryans created this revision.
jryans added a reviewer: JDevlieghere.
Herald added subscribers: llvm-commits, libcxx-commits, lldb-commits, 
cfe-commits, arphaman, christof, mgorny.
Herald added projects: clang, LLDB, libc++, LLVM.

This updates all places in documentation that refer to "Mac OS X", "OS X", etc.
to instead use the modern name "macOS" when no specific version number is
mentioned.

If a specific version is mentioned, this attempts to use the OS name at the time
of that version:

- Mac OS X for 10.0 - 10.7
- OS X for 10.8 - 10.11
- macOS for 10.12 - present


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D62654

Files:
  clang/docs/AddressSanitizer.rst
  clang/docs/AutomaticReferenceCounting.rst
  clang/docs/ClangCommandLineReference.rst
  clang/docs/CommandGuide/clang.rst
  clang/docs/LeakSanitizer.rst
  clang/docs/Modules.rst
  clang/docs/SafeStack.rst
  clang/docs/UndefinedBehaviorSanitizer.rst
  clang/docs/UsersManual.rst
  clang/docs/analyzer/checkers.rst
  clang/docs/analyzer/developer-docs/DebugChecks.rst
  libcxx/docs/BuildingLibcxx.rst
  libcxx/docs/UsingLibcxx.rst
  libcxx/docs/index.rst
  libunwind/docs/index.rst
  lld/docs/sphinx_intro.rst
  lldb/docs/lldb-gdb-remote.txt
  lldb/docs/resources/build.rst
  lldb/docs/use/remote.rst
  llvm/docs/CMake.rst
  llvm/docs/CommandGuide/llvm-ar.rst
  llvm/docs/CompilerWriterInfo.rst
  llvm/docs/DebuggingJITedCode.rst
  llvm/docs/GettingStarted.rst
  llvm/docs/ProgrammersManual.rst
  llvm/docs/TestingGuide.rst
  llvm/docs/WritingAnLLVMPass.rst

Index: llvm/docs/WritingAnLLVMPass.rst
===
--- llvm/docs/WritingAnLLVMPass.rst
+++ llvm/docs/WritingAnLLVMPass.rst
@@ -77,7 +77,7 @@
 is to be compiled and linked into a shared object ``$(LEVEL)/lib/LLVMHello.so`` that
 can be dynamically loaded by the :program:`opt` tool via its :option:`-load`
 option. If your operating system uses a suffix other than ``.so`` (such as
-Windows or Mac OS X), the appropriate extension will be used.
+Windows or macOS), the appropriate extension will be used.
 
 Now that we have the build scripts set up, we just need to write the code for
 the pass itself.
Index: llvm/docs/TestingGuide.rst
===
--- llvm/docs/TestingGuide.rst
+++ llvm/docs/TestingGuide.rst
@@ -511,7 +511,7 @@
The suffix for the host platforms shared library files. This includes the
period as the first character.
 
-   Example: ``.so`` (Linux), ``.dylib`` (OS X), ``.dll`` (Windows)
+   Example: ``.so`` (Linux), ``.dylib`` (macOS), ``.dll`` (Windows)
 
 ``%exeext``
The suffix for the host platforms executable files. This includes the
Index: llvm/docs/ProgrammersManual.rst
===
--- llvm/docs/ProgrammersManual.rst
+++ llvm/docs/ProgrammersManual.rst
@@ -1372,8 +1372,8 @@
 
 Getting this to work requires a small amount of setup.  On Unix systems
 with X11, install the `graphviz `_ toolkit, and make
-sure 'dot' and 'gv' are in your path.  If you are running on Mac OS X, download
-and install the Mac OS X `Graphviz program
+sure 'dot' and 'gv' are in your path.  If you are running on macOS, download
+and install the macOS `Graphviz program
 `_ and add
 ``/Applications/Graphviz.app/Contents/MacOS/`` (or wherever you install it) to
 your path. The programs need not be present when configuring, building or
Index: llvm/docs/GettingStarted.rst
===
--- llvm/docs/GettingStarted.rst
+++ llvm/docs/GettingStarted.rst
@@ -128,8 +128,8 @@
 FreeBSDamd64 GCC, Clang
 NetBSD x86\ :sup:`1` GCC, Clang
 NetBSD amd64 GCC, Clang
-MacOS X\ :sup:`2`  PowerPC   GCC
-MacOS Xx86   GCC, Clang
+macOS\ :sup:`2`PowerPC   GCC
+macOS  x86   GCC, Clang
 Cygwin/Win32   x86\ :sup:`1, 3`  GCC
 Windowsx86\ :sup:`1` Visual Studio
 Windows x64x86-64Visual Studio
@@ -272,7 +272,7 @@
 Getting a Modern Host C++ Toolchain
 ^^^
 
-This section mostly applies to Linux and older BSDs. On Mac OS X, you should
+This section mostly applies to Linux and older BSDs. On macOS, you should
 have a sufficiently modern Xcode, or you will likely need to upgrade until you
 do. Windows does not have a "system compiler", so you must install either Visual
 Studio 2015 or a recent version of mingw64. FreeBSD 10.0 and newer have a modern
@@ -711,7 +711,7 @@
 
 The result of such a build is executables that are not runnable on the build
 host but can be executed on the target. As an example the following CMake
-invocation can generate build files targeting iOS. This will work on Mac OS X
+invocation 

[Lldb-commits] [PATCH] D62654: [Docs] Modernize references to macOS

[Marshall Clow via Phabricator via lldb-commits]

mclow.lists added a comment.

I'm fine with the libc++ changes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62654



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

[Lldb-commits] [PATCH] D62570: [WIP] Use LLVM's debug line parser in LLDB

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

BTW, how good is the llvm debug_line parser when it comes to handling invalid 
input? Looking through the asserts in the file, it looks like at least two of 
them can be tripped by feeding it bad data:

- I'm pretty sure the DW_LNCT_*** handling code can blow up if the data forms 
are not of the expected class, or if DW_LNCT_MD5 points to data of incorrect 
size 
https://github.com/llvm-mirror/llvm/blob/0d9a164240ed293701eea6af0ea338caf227c338/lib/DebugInfo/DWARF/DWARFDebugLine.cpp#L254
- possibly the header parsing code can assert if the header contains invalid 
address size (I'd have to check how exactly is the address size on the data 
extractor being set to be sure) 
https://github.com/llvm-mirror/llvm/blob/0d9a164240ed293701eea6af0ea338caf227c338/lib/DebugInfo/DWARF/DWARFDebugLine.cpp#L302


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D62570



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

[Lldb-commits] [PATCH] D62634: Improve DWARF parsing and accessing by 1% to 2%

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In D62634#1522631 , @labath wrote:

> BTW, how do you measure these things?
>
> When I tried to benchmark something I got a difference of at least 5% on what 
> should be identical runs. I'd have to make hundreds of runs in order for a 1% 
> difference in average to show up as statistically significant..


I quit all programs on my macOS machine and run the same script 5 times in a 
row. I get quite consistent results that are plus or minus 0.1 seconds usually. 
So I try to ensure nothing else is running. I have a dwarf script that loads 
100 or so files with debug info (all .so files from a directory) and then sets 
a breakpoint by name and by file and line where the file is a header file, to 
ensure all line tables get checked.

  $ ~/Documents/src/lldb/svn/lldb/build/Release/lldb -o 'command script import 
dwarfperf.py' -o 'dwarfperf -d ~/Documents/143642775' -o q

This will load all files from "~/Documents/143642775" and then set breakpoints 
and it returns a time. These are all files with no accelerator tables to ensure 
the DWARF must be manually indexed.


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

https://reviews.llvm.org/D62634



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

[Lldb-commits] [PATCH] D62634: Improve DWARF parsing and accessing by 1% to 2%

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In D62634#1522249 , @aprantl wrote:

> Out of curiosity, what was that change in the mmap call that fixed this?


We switched to mapping things private and shared. This might be been before 
open sourcing happened so there might not be a revision for it. Previously we 
weren't mapping private so we got any changes made to the file. I have never 
seen the error fire since we fixed this.


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

https://reviews.llvm.org/D62634



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

[Lldb-commits] [lldb] r362103 - Improve DWARF parsing and accessing by 1% to 2%

[Greg Clayton via lldb-commits]

Author: gclayton
Date: Thu May 30 08:21:23 2019
New Revision: 362103

URL: http://llvm.org/viewvc/llvm-project?rev=362103&view=rev
Log:
Improve DWARF parsing and accessing by 1% to 2%

When LLDB first started we didn't have our mmap of the DWARF data done 
correctly and if the backing file would change we would get live changes as the 
file changed and it would cause problems. We now mmap correctly and do not run 
into these issues. There was legacy code in 
DWARFDebugInfoEntry::GetAbbreviationDeclarationPtr(...) that would always 
extract the abbrev index each time the function was called to verify that DWARF 
data hadn't changed and a warning was emitted if it did. We no longer need this 
and the code was removed. The other thing this function did when it parsed the 
abbrev index was give us the offset of the first attribute bytes by adding the 
LEB128 size to the offset. This required an extra parameter to 
DWARFDebugInfoEntry::GetAbbreviationDeclarationPtr(...) which is now removed. I 
added "lldb::offset_t DWARFDebugInfoEntry::GetFirstAttributeOffset() const" 
which calculates this when we need it and modified all sites that need the 
offset to call it.

Now that we aren't decoding and verifying the abbrev index, it speeds up DWARF 
access by 1% to 2%.

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


Modified:
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h

Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp?rev=362103&r1=362102&r2=362103&view=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp 
(original)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp Thu May 
30 08:21:23 2019
@@ -12,6 +12,8 @@
 
 #include 
 
+#include "llvm/Support/LEB128.h"
+
 #include "lldb/Core/Module.h"
 #include "lldb/Expression/DWARFExpression.h"
 #include "lldb/Symbol/ObjectFile.h"
@@ -239,15 +241,14 @@ bool DWARFDebugInfoEntry::GetDIENamesAnd
   std::vector die_refs;
   bool set_frame_base_loclist_addr = false;
 
-  lldb::offset_t offset;
-  const DWARFAbbreviationDeclaration *abbrevDecl =
-  GetAbbreviationDeclarationPtr(cu, offset);
+  auto abbrevDecl = GetAbbreviationDeclarationPtr(cu);
 
   SymbolFileDWARF *dwarf2Data = cu->GetSymbolFileDWARF();
   lldb::ModuleSP module = dwarf2Data->GetObjectFile()->GetModule();
 
   if (abbrevDecl) {
 const DWARFDataExtractor &debug_info_data = cu->GetData();
+lldb::offset_t offset = GetFirstAttributeOffset();
 
 if (!debug_info_data.ValidOffset(offset))
   return false;
@@ -561,13 +562,10 @@ void DWARFDebugInfoEntry::DumpAttribute(
 size_t DWARFDebugInfoEntry::GetAttributes(
 const DWARFUnit *cu, DWARFAttributes &attributes,
 uint32_t curr_depth) const {
-  const DWARFAbbreviationDeclaration *abbrevDecl = nullptr;
-  lldb::offset_t offset = 0;
-  if (cu)
-abbrevDecl = GetAbbreviationDeclarationPtr(cu, offset);
-
+  auto abbrevDecl = GetAbbreviationDeclarationPtr(cu);
   if (abbrevDecl) {
 const DWARFDataExtractor &debug_info_data = cu->GetData();
+lldb::offset_t offset = GetFirstAttributeOffset();
 
 const uint32_t num_attributes = abbrevDecl->NumAttributes();
 for (uint32_t i = 0; i < num_attributes; ++i) {
@@ -631,15 +629,14 @@ dw_offset_t DWARFDebugInfoEntry::GetAttr
  form_value, end_attr_offset_ptr,
  check_specification_or_abstract_origin);
 
-  lldb::offset_t offset;
-  const DWARFAbbreviationDeclaration *abbrevDecl =
-  GetAbbreviationDeclarationPtr(cu, offset);
+  auto abbrevDecl = GetAbbreviationDeclarationPtr(cu);
 
   if (abbrevDecl) {
 uint32_t attr_idx = abbrevDecl->FindAttributeIndex(attr);
 
 if (attr_idx != DW_INVALID_INDEX) {
   const DWARFDataExtractor &debug_info_data = cu->GetData();
+  lldb::offset_t offset = GetFirstAttributeOffset();
 
   uint32_t idx = 0;
   while (idx < attr_idx)
@@ -1244,35 +1241,17 @@ bool DWARFDebugInfoEntry::LookupAddress(
   return found_address;
 }
 
+lldb::offset_t DWARFDebugInfoEntry::GetFirstAttributeOffset() const {
+  return GetOffset() + llvm::getULEB128Size(m_abbr_idx);
+}
+
 const DWARFAbbreviationDeclaration *
-DWARFDebugInfoEntry::GetAbbreviationDeclarationPtr(
-const DWARFUnit *cu, lldb::offset_t &offset) const {
+DWARFDebugInfoEntry::GetAbbreviationDeclarationPtr(const DWARFUnit *cu) const {
   if (cu) {
-offset = GetOffset();
-
 const DWARFAbbreviationDeclarationSet *abbrev_set = cu->GetAbbreviations();
-if (abbrev_set) {
-  const DWARFAbbreviationDeclaration *abbrev_decl =
-  abbrev_set->GetAbbreviationDeclaration(m_abbr_idx);
-  if (abbrev_decl) {
-// Make sure the abbreviation code still matches.

[Lldb-commits] [PATCH] D62649: CompileUnit: Use shared_ptr for storing support file lists

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Actually, I've run into a bit of a snag while trying to implement the full line 
table sharing. The thing is, before DWARF5, the line tables are not fully 
independent, and require the DW_AT_comp_dir of the compile unit in order to 
resolve the relative paths correctly. Type units do not have the DW_AT_comp_dir 
attribute, and so in order to correctly parse their line tables, I'd have to 
check their DW_AT_stmt_list attribute, find the matching compile unit, and 
fetch the compilation directory from there. This is relatively easy do to in 
regular DWARF, though it still requires some fairly elaborate dances if one 
wants to do it lazily and efficiently, etc.

However, things get a lot more complicated when split-dwarf comes into play. 
There the compiler will emit a separate line table into the dwo file for use by 
the type unit, and the compile unit will use the line table from the main file 
(located through DW_AT_stmt_list on the skeleton unit in the .o file). Here it 
is pretty much impossible to reliably connect the type unit to the relevant 
comp_dir. If the DWO file contains a single compilation unit, I might guess 
that I should look at this one, but in case of multiple compile units in a 
single dwo file we're out of luck.

So, this got me thinking that maybe this DW_AT_comp_dir business is not worth 
all the trouble it brings, and I'm now thinking of a simpler solution:

- for each type unit, parse the line tables without the DW_AT_comp_dir 
attribute. Cache them, and share them between type units.
- for each compile unit, parse the line tables *with* the DW_AT_comp_dir. Don't 
cache and don't share them (i.e., do exactly what we do now)

This means each line table will be parsed at most twice (once for its compile 
unit, and once for any type units that refer to it). It's not possible to share 
them between CUs and TUs without going through the complex matching process I 
described above, or without risking that the CU will get incomplete line tables 
if they have already been parsed via a type unit. It also means that for types 
in a TU we might not have complete declaration file names (in DWARF<=4), but I 
am not sure how many users will actually care about that (not even 
llvm-dwarfdump tries to do this matching). And this will still prevent the line 
table being parsed hundreds of times for each type unit that happens to refer 
to it...

I think that potentially parsing a line table (only it's file list, really) 
twice is worth the reduction in complexity that a full sharing would require. 
WDYT?

If you agree, then this patch is also not really necessary (though the 
llvm::Expected part is a nice refactor regardless IMO).


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

https://reviews.llvm.org/D62649



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

[Lldb-commits] [PATCH] D62649: CompileUnit: Use shared_ptr for storing support file lists

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In D62649#1523074 , @labath wrote:

> Actually, I've run into a bit of a snag while trying to implement the full 
> line table sharing. The thing is, before DWARF5, the line tables are not 
> fully independent, and require the DW_AT_comp_dir of the compile unit in 
> order to resolve the relative paths correctly. Type units do not have the 
> DW_AT_comp_dir attribute, and so in order to correctly parse their line 
> tables, I'd have to check their DW_AT_stmt_list attribute, find the matching 
> compile unit, and fetch the compilation directory from there. This is 
> relatively easy do to in regular DWARF, though it still requires some fairly 
> elaborate dances if one wants to do it lazily and efficiently, etc.
>
> However, things get a lot more complicated when split-dwarf comes into play. 
> There the compiler will emit a separate line table into the dwo file for use 
> by the type unit, and the compile unit will use the line table from the main 
> file (located through DW_AT_stmt_list on the skeleton unit in the .o file). 
> Here it is pretty much impossible to reliably connect the type unit to the 
> relevant comp_dir. If the DWO file contains a single compilation unit, I 
> might guess that I should look at this one, but in case of multiple compile 
> units in a single dwo file we're out of luck.
>
> So, this got me thinking that maybe this DW_AT_comp_dir business is not worth 
> all the trouble it brings, and I'm now thinking of a simpler solution:
>
> - for each type unit, parse the line tables without the DW_AT_comp_dir 
> attribute. Cache them, and share them between type units.
> - for each compile unit, parse the line tables *with* the DW_AT_comp_dir. 
> Don't cache and don't share them (i.e., do exactly what we do now)
>
>   This means each line table will be parsed at most twice (once for its 
> compile unit, and once for any type units that refer to it). It's not 
> possible to share them between CUs and TUs without going through the complex 
> matching process I described above, or without risking that the CU will get 
> incomplete line tables if they have already been parsed via a type unit. It 
> also means that for types in a TU we might not have complete declaration file 
> names (in DWARF<=4), but I am not sure how many users will actually care 
> about that (not even llvm-dwarfdump tries to do this matching). And this will 
> still prevent the line table being parsed hundreds of times for each type 
> unit that happens to refer to it...
>
>   I think that potentially parsing a line table (only it's file list, really) 
> twice is worth the reduction in complexity that a full sharing would require. 
> WDYT?
>
>   If you agree, then this patch is also not really necessary (though the 
> llvm::Expected part is a nice refactor regardless IMO).


That is my current solution in our internal LLDB fork, so yes, this works and 
is probably the best solution.


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

https://reviews.llvm.org/D62649



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

[Lldb-commits] [lldb] r362105 - Fix a regression in DWARF access speed caused by svn revision 356190

[Greg Clayton via lldb-commits]

Author: gclayton
Date: Thu May 30 08:32:33 2019
New Revision: 362105

URL: http://llvm.org/viewvc/llvm-project?rev=362105&view=rev
Log:
Fix a regression in DWARF access speed caused by svn revision 356190

The issue was caused by the error checking code that was added. It was 
incorrectly adding an extra abbreviation when DWARFEnumState::Complete was 
received since it would push an extra abbreviation onto the list with the 
abbreviation code of zero. This cause m_idx_offset in each 
DWARFAbbreviationDeclarationSet to be set to UINT32_MAX. This valid indicates 
we must linearly search for attributes, not access them in O(1) time. This 
caused every DWARFDebugInfoEntry that would try to get its 
DWARFAbbreviationDeclaration from the CU's DWARFAbbreviationDeclarationSet to 
always linearly search the abbreviation set for a given abbreviation code. Easy 
to see why this would cause things to be slow.

This regression was caused by: https://reviews.llvm.org/D59370. I asked to 
ensure there was no regression is parsing or access speed, but that must not 
have been done. In my test with 40 DWARF files trying to set a breakpoint by 
function name and in a header file, I see a 8% speed improvement with this fix.

There was no regression in correctness, just very inefficient access.

Added full unit testing for DWARFAbbreviationDeclarationSet parsing to ensure 
this doesn't regress.

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


Modified:
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugAbbrev.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugAbbrev.h
lldb/trunk/unittests/SymbolFile/DWARF/SymbolFileDWARFTests.cpp

Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugAbbrev.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugAbbrev.cpp?rev=362105&r1=362104&r2=362105&view=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugAbbrev.cpp (original)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugAbbrev.cpp Thu May 30 
08:32:33 2019
@@ -29,21 +29,19 @@ DWARFAbbreviationDeclarationSet::extract
   Clear();
   DWARFAbbreviationDeclaration abbrevDeclaration;
   dw_uleb128_t prev_abbr_code = 0;
-  DWARFEnumState state = DWARFEnumState::MoreItems;
-  while (state == DWARFEnumState::MoreItems) {
+  while (true) {
 llvm::Expected es =
 abbrevDeclaration.extract(data, offset_ptr);
 if (!es)
   return es.takeError();
-
-state = *es;
+if (*es == DWARFEnumState::Complete)
+  break;
 m_decls.push_back(abbrevDeclaration);
 if (m_idx_offset == 0)
   m_idx_offset = abbrevDeclaration.Code();
-else {
-  if (prev_abbr_code + 1 != abbrevDeclaration.Code())
-m_idx_offset =
-UINT32_MAX; // Out of order indexes, we can't do O(1) lookups...
+else if (prev_abbr_code + 1 != abbrevDeclaration.Code()) {
+  // Out of order indexes, we can't do O(1) lookups...
+  m_idx_offset = UINT32_MAX;
 }
 prev_abbr_code = abbrevDeclaration.Code();
   }

Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugAbbrev.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugAbbrev.h?rev=362105&r1=362104&r2=362105&view=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugAbbrev.h (original)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugAbbrev.h Thu May 30 
08:32:33 2019
@@ -45,6 +45,10 @@ public:
   const DWARFAbbreviationDeclaration *
   GetAbbreviationDeclaration(dw_uleb128_t abbrCode) const;
 
+  /// Unit test accessor functions.
+  /// @{
+  uint32_t GetIndexOffset() const { return m_idx_offset; }
+  /// @}
 private:
   dw_offset_t m_offset;
   uint32_t m_idx_offset;

Modified: lldb/trunk/unittests/SymbolFile/DWARF/SymbolFileDWARFTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/SymbolFile/DWARF/SymbolFileDWARFTests.cpp?rev=362105&r1=362104&r2=362105&view=diff
==
--- lldb/trunk/unittests/SymbolFile/DWARF/SymbolFileDWARFTests.cpp (original)
+++ lldb/trunk/unittests/SymbolFile/DWARF/SymbolFileDWARFTests.cpp Thu May 30 
08:32:33 2019
@@ -15,6 +15,9 @@
 #include "llvm/Support/Path.h"
 
 #include "Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h"
+#include "Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.h"
+#include "Plugins/SymbolFile/DWARF/DWARFDataExtractor.h"
+#include "Plugins/SymbolFile/DWARF/DWARFDebugAbbrev.h"
 #include "Plugins/SymbolFile/DWARF/SymbolFileDWARF.h"
 #include "Plugins/SymbolFile/PDB/SymbolFilePDB.h"
 #include "TestingSupport/TestUtilities.h"
@@ -28,8 +31,13 @@
 #include "lldb/Symbol/LineTable.h"
 #include "lldb/Symbol/SymbolVendor.h"
 #include "lldb/Utility/ArchSpec.h"
+#include "lldb/Utility/DataEncoder.h"
 #include "lldb/Uti

[Lldb-commits] [PATCH] D62649: CompileUnit: Use shared_ptr for storing support file lists

[Pavel Labath via Phabricator via lldb-commits]

labath abandoned this revision.
labath added a comment.

Ok, abandoning this patch in that case.


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

https://reviews.llvm.org/D62649



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

[Lldb-commits] [PATCH] D62630: Fix a regression in DWARF access speed caused by svn revision 356190

[Greg Clayton via Phabricator via lldb-commits]

clayborg closed this revision.
clayborg marked an inline comment as done.
clayborg added a comment.

r362105 | gclayton | 2019-05-30 08:32:33 -0700 (Thu, 30 May 2019) | 13 lines

Fix a regression in DWARF access speed caused by svn revision 356190

The issue was caused by the error checking code that was added. It was 
incorrectly adding an extra abbreviation when DWARFEnumState::Complete was 
received since it would push an extra abbreviation onto the list with the 
abbreviation code of zero. This cause m_idx_offset in each 
DWARFAbbreviationDeclarationSet to be set to UINT32_MAX. This valid indicates 
we must linearly search for attributes, not access them in O(1) time. This 
caused every DWARFDebugInfoEntry that would try to get its 
DWARFAbbreviationDeclaration from the CU's DWARFAbbreviationDeclarationSet to 
always linearly search the abbreviation set for a given abbreviation code. Easy 
to see why this would cause things to be slow.

This regression was caused by: https://reviews.llvm.org/D59370. I asked to 
ensure there was no regression is parsing or access speed, but that must not 
have been done. In my test with 40 DWARF files trying to set a breakpoint by 
function name and in a header file, I see a 8% speed improvement with this fix.

There was no regression in correctness, just very inefficient access.

Added full unit testing for DWARFAbbreviationDeclarationSet parsing to ensure 
this doesn't regress.

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




Comment at: source/Plugins/SymbolFile/DWARF/DWARFDebugAbbrev.cpp:32
   dw_uleb128_t prev_abbr_code = 0;
-  DWARFEnumState state = DWARFEnumState::MoreItems;
-  while (state == DWARFEnumState::MoreItems) {
+  while (true) {
 llvm::Expected es =

aprantl wrote:
> Not being too familiar with the API I still find the control flow in this 
> loop to be hard to follow. Is `offset_ptr` incremented by `extract()`?
Yes. It is passed to data.GetXXX calls and serves as the thread safe offset 
into the data.


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

https://reviews.llvm.org/D62630



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

[Lldb-commits] [PATCH] D62634: Improve DWARF parsing and accessing by 1% to 2%

[Greg Clayton via Phabricator via lldb-commits]

clayborg closed this revision.
clayborg added a comment.

r362103 | gclayton | 2019-05-30 08:21:23 -0700 (Thu, 30 May 2019) | 9 lines

Improve DWARF parsing and accessing by 1% to 2%

When LLDB first started we didn't have our mmap of the DWARF data done 
correctly and if the backing file would change we would get live changes as the 
file changed and it would cause problems. We now mmap correctly and do not run 
into these issues. There was legacy code in 
DWARFDebugInfoEntry::GetAbbreviationDeclarationPtr(...) that would always 
extract the abbrev index each time the function was called to verify that DWARF 
data hadn't changed and a warning was emitted if it did. We no longer need this 
and the code was removed. The other thing this function did when it parsed the 
abbrev index was give us the offset of the first attribute bytes by adding the 
LEB128 size to the offset. This required an extra parameter to 
DWARFDebugInfoEntry::GetAbbreviationDeclarationPtr(...) which is now removed. I 
added "lldb::offset_t DWARFDebugInfoEntry::GetFirstAttributeOffset() const" 
which calculates this when we need it and modified all sites that need the 
offset to call it.

Now that we aren't decoding and verifying the abbrev index, it speeds up DWARF 
access by 1% to 2%.

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


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

https://reviews.llvm.org/D62634



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

[Lldb-commits] [PATCH] D62654: [Docs] Modernize references to macOS

[Jonas Devlieghere via Phabricator via lldb-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62654



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

[Lldb-commits] [lldb] r362113 - [Docs] Modernize references to macOS

[J. Ryan Stinnett via lldb-commits]

Author: jryans
Date: Thu May 30 09:46:22 2019
New Revision: 362113

URL: http://llvm.org/viewvc/llvm-project?rev=362113&view=rev
Log:
[Docs] Modernize references to macOS

Summary:
This updates all places in documentation that refer to "Mac OS X", "OS X", etc.
to instead use the modern name "macOS" when no specific version number is
mentioned.

If a specific version is mentioned, this attempts to use the OS name at the time
of that version:

* Mac OS X for 10.0 - 10.7
* OS X for 10.8 - 10.11
* macOS for 10.12 - present

Reviewers: JDevlieghere

Subscribers: mgorny, christof, arphaman, cfe-commits, lldb-commits, 
libcxx-commits, llvm-commits

Tags: #clang, #lldb, #libc, #llvm

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

Modified:
lldb/trunk/docs/lldb-gdb-remote.txt
lldb/trunk/docs/resources/build.rst
lldb/trunk/docs/use/remote.rst

Modified: lldb/trunk/docs/lldb-gdb-remote.txt
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/docs/lldb-gdb-remote.txt?rev=362113&r1=362112&r2=362113&view=diff
==
--- lldb/trunk/docs/lldb-gdb-remote.txt (original)
+++ lldb/trunk/docs/lldb-gdb-remote.txt Thu May 30 09:46:22 2019
@@ -787,9 +787,9 @@ os_version: a version string that repres
 watchpoint_exceptions_received: one of "before" or "after" to specify if a 
watchpoint is triggered before or after the pc when it stops
 default_packet_timeout: an unsigned number that specifies the default timeout 
in seconds
 distribution_id: optional. For linux, specifies distribution id (e.g. ubuntu, 
fedora, etc.)
-osmajor: optional, specifies the major version number of the OS (e.g. for Mac 
OS X 10.11.2, it would be 10)
-osminor: optional, specifies the minor version number of the OS (e.g. for Mac 
OS X 10.11.2, it would be 11)
-ospatch: optional, specifies the patch level number of the OS (e.g. for Mac OS 
X 10.11.2, it would be 2)
+osmajor: optional, specifies the major version number of the OS (e.g. for 
macOS 10.12.2, it would be 10)
+osminor: optional, specifies the minor version number of the OS (e.g. for 
macOS 10.12.2, it would be 12)
+ospatch: optional, specifies the patch level number of the OS (e.g. for macOS 
10.12.2, it would be 2)
 
 //--
 // "qGDBServerVersion"
@@ -1160,7 +1160,7 @@ for this region.
 //  second form of this packet is used, otherwise the first form is 
 //  used. This packet is called prior to executing an expression, so
 //  the remote GDB server should do anything it needs to in order to 
-//  ensure the registers that are saved are correct. On MacOSX this
+//  ensure the registers that are saved are correct. On macOS this
 //  involves calling "thread_abort_safely(mach_port_t thread)" to 
 //  ensure we get the correct registers for a thread in case it is
 //  currently having code run on its behalf in the kernel.
@@ -1723,7 +1723,7 @@ for this region.
 //  There are three ways this packet can be used.  All three return a 
dictionary of
 //  binary images formatted the same way.
 //
-//  On MacOS X 10.11, iOS 9, tvOS 9, watchOS 2 and earlier, the packet is used 
like
+//  On OS X 10.11, iOS 9, tvOS 9, watchOS 2 and earlier, the packet is used 
like
 //   
jGetLoadedDynamicLibrariesInfos:{"image_count":1,"image_list_address":140734800075128}
 //  where the image_list_address is an array of {void* load_addr, void* 
mod_date, void* pathname}
 //  in the inferior process memory (and image_count is the number of elements 
in this array).
@@ -1863,9 +1863,9 @@ server to expedite memory that the clien
 stack pointer, which are needed for computing backtraces) and it reduces the 
packet
 count.
 
-On MacOSX with debugserver, we expedite the frame pointer backchain for a 
thread
+On macOS with debugserver, we expedite the frame pointer backchain for a thread
 (up to 256 entries) by reading 2 pointers worth of bytes at the frame pointer 
(for
-the previous FP and PC), and follow the backchain. Most backtraces on MacOSX 
and
+the previous FP and PC), and follow the backchain. Most backtraces on macOS and
 iOS now don't require us to read any memory!
 
 //--

Modified: lldb/trunk/docs/resources/build.rst
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/docs/resources/build.rst?rev=362113&r1=362112&r2=362113&view=diff
==
--- lldb/trunk/docs/resources/build.rst (original)
+++ lldb/trunk/docs/resources/build.rst Thu May 30 09:46:22 2019
@@ -117,8 +117,12 @@ There are two ways to build LLDB on macO
 
 **Preliminaries**
 
-* Xcode 4.3 or newer requires the "Command Line Tools" component 
(XCode->Preferences->Downloads->Components).
-* Mac OS X Lion or newer requires installing `Swig `_.
+In addition to any dependencies required by LLVM and Clang, LLDB needs a few
+development packages

[Lldb-commits] [lldb] r362116 - Code and comment cleanups [NFC]

[Greg Clayton via lldb-commits]

Author: gclayton
Date: Thu May 30 10:03:35 2019
New Revision: 362116

URL: http://llvm.org/viewvc/llvm-project?rev=362116&view=rev
Log:
Code and comment cleanups [NFC]

Changes:
- update comments to detail the info can come from .debug_info or .debug_types
- Rename "debug_info_data" to "data" now that we can get data from .debug_info 
or .debug_types.
- Also call DWARFDebugInfoEntry::GetAbbreviationDeclarationPtr(...) instead of 
manually grabbing abbreviation.



Modified:
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h

Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp?rev=362116&r1=362115&r2=362116&view=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp 
(original)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp Thu May 
30 10:03:35 2019
@@ -33,16 +33,15 @@ using namespace lldb_private;
 using namespace std;
 extern int g_verbose;
 
-// Extract a debug info entry for a given compile unit from the .debug_info and
-// .debug_abbrev data within the SymbolFileDWARF class starting at the given
-// offset
-bool DWARFDebugInfoEntry::Extract(const DWARFDataExtractor &debug_info_data,
+// Extract a debug info entry for a given DWARFUnit from the data
+// starting at the offset in offset_ptr
+bool DWARFDebugInfoEntry::Extract(const DWARFDataExtractor &data,
   const DWARFUnit *cu,
   lldb::offset_t *offset_ptr) {
   m_offset = *offset_ptr;
   m_parent_idx = 0;
   m_sibling_idx = 0;
-  const uint64_t abbr_idx = debug_info_data.GetULEB128(offset_ptr);
+  const uint64_t abbr_idx = data.GetULEB128(offset_ptr);
   lldbassert(abbr_idx <= UINT16_MAX);
   m_abbr_idx = abbr_idx;
 
@@ -51,10 +50,7 @@ bool DWARFDebugInfoEntry::Extract(const
 
   if (m_abbr_idx) {
 lldb::offset_t offset = *offset_ptr;
-
-const DWARFAbbreviationDeclaration *abbrevDecl =
-cu->GetAbbreviations()->GetAbbreviationDeclaration(m_abbr_idx);
-
+auto *abbrevDecl = GetAbbreviationDeclarationPtr(cu);
 if (abbrevDecl == nullptr) {
   cu->GetSymbolFileDWARF()->GetObjectFile()->GetModule()->ReportError(
   "{0x%8.8x}: invalid abbreviation code %u, please file a bug and "
@@ -66,14 +62,14 @@ bool DWARFDebugInfoEntry::Extract(const
 }
 m_tag = abbrevDecl->Tag();
 m_has_children = abbrevDecl->HasChildren();
-// Skip all data in the .debug_info for the attributes
+// Skip all data in the .debug_info or .debug_types for the attributes
 const uint32_t numAttributes = abbrevDecl->NumAttributes();
 uint32_t i;
 dw_form_t form;
 for (i = 0; i < numAttributes; ++i) {
   form = abbrevDecl->GetFormByIndexUnchecked(i);
-
-  llvm::Optional fixed_skip_size = 
DWARFFormValue::GetFixedSize(form, cu);
+  llvm::Optional fixed_skip_size =
+  DWARFFormValue::GetFixedSize(form, cu);
   if (fixed_skip_size)
 offset += *fixed_skip_size;
   else {
@@ -83,24 +79,24 @@ bool DWARFDebugInfoEntry::Extract(const
   uint32_t form_size = 0;
   switch (form) {
   // Blocks if inlined data that have a length field and the data bytes
-  // inlined in the .debug_info
+  // inlined in the .debug_info/.debug_types
   case DW_FORM_exprloc:
   case DW_FORM_block:
-form_size = debug_info_data.GetULEB128(&offset);
+form_size = data.GetULEB128(&offset);
 break;
   case DW_FORM_block1:
-form_size = debug_info_data.GetU8_unchecked(&offset);
+form_size = data.GetU8_unchecked(&offset);
 break;
   case DW_FORM_block2:
-form_size = debug_info_data.GetU16_unchecked(&offset);
+form_size = data.GetU16_unchecked(&offset);
 break;
   case DW_FORM_block4:
-form_size = debug_info_data.GetU32_unchecked(&offset);
+form_size = data.GetU32_unchecked(&offset);
 break;
 
   // Inlined NULL terminated C-strings
   case DW_FORM_string:
-debug_info_data.GetCStr(&offset);
+data.GetCStr(&offset);
 break;
 
   // Compile unit address sized values
@@ -166,17 +162,17 @@ bool DWARFDebugInfoEntry::Extract(const
   case DW_FORM_GNU_addr_index:
   case DW_FORM_GNU_str_index:
   case DW_FORM_strx:
-debug_info_data.Skip_LEB128(&offset);
+data.Skip_LEB128(&offset);
 break;
 
   case DW_FORM_indirect:
 form_is_indirect = true;
-form = debug_info_data.GetULEB128(&offset);
+form = data.GetULEB128(&offset);
 break;
 
  

[Lldb-commits] [PATCH] D62626: Remove length modifier when using assignment suppression in TimerTest

[António Afonso via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL362107: Remove length modifier when using assignment 
suppression in TimerTest (authored by aadsm, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D62626?vs=202046&id=202242#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D62626

Files:
  lldb/trunk/unittests/Utility/TimerTest.cpp


Index: lldb/trunk/unittests/Utility/TimerTest.cpp
===
--- lldb/trunk/unittests/Utility/TimerTest.cpp
+++ lldb/trunk/unittests/Utility/TimerTest.cpp
@@ -62,7 +62,7 @@
   Timer::DumpCategoryTimes(&ss);
   double seconds1, seconds2;
   ASSERT_EQ(2, sscanf(ss.GetData(),
-  "%lf sec (total: %*lfs; child: %*lfs; count: %*d) for "
+  "%lf sec (total: %*fs; child: %*fs; count: %*d) for "
   "CAT1%*[\n ]%lf sec for CAT2",
   &seconds1, &seconds2))
   << "String: " << ss.GetData();
@@ -98,7 +98,7 @@
   ASSERT_EQ(
   6, sscanf(ss.GetData(),
 "%lf sec (total: %lfs; child: %lfs; count: %d) for CAT1%*[\n ]"
-"%lf sec (total: %*lfs; child: %*lfs; count: %d) for CAT2",
+"%lf sec (total: %*fs; child: %*fs; count: %d) for CAT2",
 &seconds1, &total1, &child1, &count1, &seconds2, &count2))
   << "String: " << ss.GetData();
   EXPECT_NEAR(total1 - child1, seconds1, 0.002);


Index: lldb/trunk/unittests/Utility/TimerTest.cpp
===
--- lldb/trunk/unittests/Utility/TimerTest.cpp
+++ lldb/trunk/unittests/Utility/TimerTest.cpp
@@ -62,7 +62,7 @@
   Timer::DumpCategoryTimes(&ss);
   double seconds1, seconds2;
   ASSERT_EQ(2, sscanf(ss.GetData(),
-  "%lf sec (total: %*lfs; child: %*lfs; count: %*d) for "
+  "%lf sec (total: %*fs; child: %*fs; count: %*d) for "
   "CAT1%*[\n ]%lf sec for CAT2",
   &seconds1, &seconds2))
   << "String: " << ss.GetData();
@@ -98,7 +98,7 @@
   ASSERT_EQ(
   6, sscanf(ss.GetData(),
 "%lf sec (total: %lfs; child: %lfs; count: %d) for CAT1%*[\n ]"
-"%lf sec (total: %*lfs; child: %*lfs; count: %d) for CAT2",
+"%lf sec (total: %*fs; child: %*fs; count: %d) for CAT2",
 &seconds1, &total1, &child1, &count1, &seconds2, &count2))
   << "String: " << ss.GetData();
   EXPECT_NEAR(total1 - child1, seconds1, 0.002);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D62654: [Docs] Modernize references to macOS

[J. Ryan Stinnett via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL362113: [Docs] Modernize references to macOS (authored by 
jryans, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D62654?vs=202161&id=202247#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D62654

Files:
  cfe/trunk/docs/AddressSanitizer.rst
  cfe/trunk/docs/AutomaticReferenceCounting.rst
  cfe/trunk/docs/ClangCommandLineReference.rst
  cfe/trunk/docs/CommandGuide/clang.rst
  cfe/trunk/docs/LeakSanitizer.rst
  cfe/trunk/docs/Modules.rst
  cfe/trunk/docs/SafeStack.rst
  cfe/trunk/docs/UndefinedBehaviorSanitizer.rst
  cfe/trunk/docs/UsersManual.rst
  cfe/trunk/docs/analyzer/checkers.rst
  cfe/trunk/docs/analyzer/developer-docs/DebugChecks.rst
  libcxx/trunk/docs/BuildingLibcxx.rst
  libcxx/trunk/docs/UsingLibcxx.rst
  libcxx/trunk/docs/index.rst
  libunwind/trunk/docs/index.rst
  lld/trunk/docs/sphinx_intro.rst
  lldb/trunk/docs/lldb-gdb-remote.txt
  lldb/trunk/docs/resources/build.rst
  lldb/trunk/docs/use/remote.rst
  llvm/trunk/docs/CMake.rst
  llvm/trunk/docs/CommandGuide/llvm-ar.rst
  llvm/trunk/docs/CompilerWriterInfo.rst
  llvm/trunk/docs/DebuggingJITedCode.rst
  llvm/trunk/docs/GettingStarted.rst
  llvm/trunk/docs/ProgrammersManual.rst
  llvm/trunk/docs/TestingGuide.rst
  llvm/trunk/docs/WritingAnLLVMPass.rst

Index: libcxx/trunk/docs/index.rst
===
--- libcxx/trunk/docs/index.rst
+++ libcxx/trunk/docs/index.rst
@@ -93,7 +93,7 @@
    
 OS   Arch CompilersABI Library
    
-Mac OS X i386, x86_64 Clang, GCC   libc++abi
+macOSi386, x86_64 Clang, GCC   libc++abi
 FreeBSD 10+  i386, x86_64, ARMClang, GCC   libcxxrt, libc++abi
 Linuxi386, x86_64 Clang, GCC   libc++abi
    
Index: libcxx/trunk/docs/BuildingLibcxx.rst
===
--- libcxx/trunk/docs/BuildingLibcxx.rst
+++ libcxx/trunk/docs/BuildingLibcxx.rst
@@ -41,7 +41,7 @@
 
.. warning::
  * Replacing your systems libc++ installation could render the system non-functional.
- * Mac OS X will not boot without a valid copy of ``libc++.1.dylib`` in ``/usr/lib``.
+ * macOS will not boot without a valid copy of ``libc++.1.dylib`` in ``/usr/lib``.
 
 
 The instructions are for building libc++ on
Index: libcxx/trunk/docs/UsingLibcxx.rst
===
--- libcxx/trunk/docs/UsingLibcxx.rst
+++ libcxx/trunk/docs/UsingLibcxx.rst
@@ -15,7 +15,7 @@
 $ clang++ -stdlib=libc++ test.cpp
 $ clang++ -std=c++11 -stdlib=libc++ test.cpp
 
-On OS X and FreeBSD libc++ is the default standard library
+On macOS and FreeBSD libc++ is the default standard library
 and the ``-stdlib=libc++`` is not required.
 
 .. _alternate libcxx:
@@ -34,7 +34,7 @@
 The option ``-Wl,-rpath,/lib`` adds a runtime library
 search path. Meaning that the systems dynamic linker will look for libc++ in
 ``/lib`` whenever the program is run. Alternatively the
-environment variable ``LD_LIBRARY_PATH`` (``DYLD_LIBRARY_PATH`` on OS X) can
+environment variable ``LD_LIBRARY_PATH`` (``DYLD_LIBRARY_PATH`` on macOS) can
 be used to change the dynamic linkers search paths after a program is compiled.
 
 An example of using ``LD_LIBRARY_PATH``:
Index: lld/trunk/docs/sphinx_intro.rst
===
--- lld/trunk/docs/sphinx_intro.rst
+++ lld/trunk/docs/sphinx_intro.rst
@@ -43,8 +43,8 @@
 Use your distribution's standard package management tool to install it,
 i.e., ``apt-get install easy_install`` or ``yum install easy_install``.
 
-  Mac OS X
-All modern Mac OS X systems come with ``easy_install`` as part of the base
+  macOS
+All modern macOS systems come with ``easy_install`` as part of the base
 system.
 
   Windows
Index: llvm/trunk/docs/CMake.rst
===
--- llvm/trunk/docs/CMake.rst
+++ llvm/trunk/docs/CMake.rst
@@ -533,7 +533,7 @@
   `share/doc/llvm/ocaml-html`.
 
 **LLVM_CREATE_XCODE_TOOLCHAIN**:BOOL
-  OS X Only: If enabled CMake will generate a target named
+  macOS Only: If enabled CMake will generate a target named
   'install-xcode-toolchain'. This target will create a directory at
   $CMAKE_INSTALL_PREFIX/Toolchains containing an xctoolchain directory which can
   be used to override the default system tools.
Index: llvm/trunk/docs/DebuggingJITedCode.rst
===
--- llvm/trunk/docs/DebuggingJITedCode.rst
+++ llvm/trunk/docs/DebuggingJITedCode.rst
@@ -29,7 +29,7 

[Lldb-commits] [PATCH] D61921: [Target] Generalize language-specific behavior in ThreadPlanStepThrough

[Jim Ingham via Phabricator via lldb-commits]

jingham accepted this revision.
jingham added a comment.

LGTM


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

https://reviews.llvm.org/D61921



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

[Lldb-commits] [PATCH] D62702: [ABI] Fix SystemV ABI to handle nested aggregate type returned in register

[Wanyi Ye via Phabricator via lldb-commits]

kusmour created this revision.
kusmour added reviewers: xiaobai, compnerd.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Add a function to flatten the nested aggregate type


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D62702

Files:
  lldb/source/Plugins/ABI/SysV-x86_64/ABISysV_x86_64.cpp

Index: lldb/source/Plugins/ABI/SysV-x86_64/ABISysV_x86_64.cpp
===
--- lldb/source/Plugins/ABI/SysV-x86_64/ABISysV_x86_64.cpp
+++ lldb/source/Plugins/ABI/SysV-x86_64/ABISysV_x86_64.cpp
@@ -30,6 +30,8 @@
 #include "lldb/Utility/RegisterValue.h"
 #include "lldb/Utility/Status.h"
 
+#include 
+
 using namespace lldb;
 using namespace lldb_private;
 
@@ -1558,6 +1560,62 @@
   return return_valobj_sp;
 }
 
+// The compiler will faltten the nested aggregate type into single
+// layer and push the value to stack
+// This helper function will flatten an aggregate type
+// and return true if it can be returned in register(s) by value
+// return false if the aggregate is in memory
+static bool FlattenAggregateType(
+ExecutionContext &exe_ctx,
+CompilerType &return_compiler_type,
+uint32_t data_byte_offset,
+std::vector &aggregate_field_offsets,
+std::vector &aggregate_compiler_types) {
+  const uint32_t num_children = return_compiler_type.GetNumFields();
+  for (uint32_t idx = 0; idx < num_children; ++idx) {
+std::string name;
+bool is_signed;
+uint32_t count;
+bool is_complex;
+
+const bool transparent_pointers = false;
+const bool omit_empty_base_classes = true;
+const bool ignore_array_bounds = false;
+uint32_t child_byte_size = 0;
+int32_t child_byte_offset = 0;
+uint32_t child_bitfield_bit_size = 0;
+uint32_t child_bitfield_bit_offset = 0;
+bool child_is_base_class = false;
+bool child_is_deref_of_parent = false;
+uint64_t language_flags;
+CompilerType field_compiler_type =
+return_compiler_type.GetChildCompilerTypeAtIndex(
+&exe_ctx, idx, transparent_pointers, omit_empty_base_classes,
+ignore_array_bounds, name, child_byte_size, child_byte_offset,
+child_bitfield_bit_size, child_bitfield_bit_offset,
+child_is_base_class, child_is_deref_of_parent, nullptr,
+language_flags);
+
+const uint64_t field_bit_offset = child_byte_offset * 8;
+uint32_t field_byte_offset = field_bit_offset / 8 + data_byte_offset;
+
+const uint32_t field_type_flags = field_compiler_type.GetTypeInfo();
+if (field_compiler_type.IsIntegerOrEnumerationType(is_signed) ||
+field_compiler_type.IsPointerType() ||
+field_compiler_type.IsFloatingPointType(count, is_complex)) {
+  aggregate_field_offsets.push_back(field_byte_offset);
+  aggregate_compiler_types.push_back(field_compiler_type);
+} else if (field_type_flags & eTypeHasChildren) {
+  if (!FlattenAggregateType(exe_ctx, field_compiler_type,
+field_byte_offset, aggregate_field_offsets,
+aggregate_compiler_types)) {
+return false;
+  }
+}
+  }
+  return true;
+}
+
 ValueObjectSP ABISysV_x86_64::GetReturnValueObjectImpl(
 Thread &thread, CompilerType &return_compiler_type) const {
   ValueObjectSP return_valobj_sp;
@@ -1580,10 +1638,14 @@
   if (return_compiler_type.IsAggregateType()) {
 Target *target = exe_ctx.GetTargetPtr();
 bool is_memory = true;
-if (*bit_width <= 128) {
-  ByteOrder target_byte_order = target->GetArchitecture().GetByteOrder();
+std::vector aggregate_field_offsets;
+std::vector aggregate_compiler_types;
+if (*bit_width <= 128 && FlattenAggregateType(exe_ctx, return_compiler_type,
+  0, aggregate_field_offsets,
+  aggregate_compiler_types)) {
+  ByteOrder byte_order = target->GetArchitecture().GetByteOrder();
   DataBufferSP data_sp(new DataBufferHeap(16, 0));
-  DataExtractor return_ext(data_sp, target_byte_order,
+  DataExtractor return_ext(data_sp, byte_order,
target->GetArchitecture().GetAddressByteSize());
 
   const RegisterInfo *rax_info =
@@ -1613,40 +1675,27 @@
   uint32_t integer_bytes =
   0; // Tracks how much of the rax/rds registers we've consumed so far
 
-  const uint32_t num_children = return_compiler_type.GetNumFields();
+  // const uint32_t num_children = return_compiler_type.GetNumFields();
+  const uint32_t num_children = aggregate_compiler_types.size();
 
   // Since we are in the small struct regime, assume we are not in memory.
   is_memory = false;
-
   for (uint32_t idx = 0; idx < num_children; idx++) {
-std::string name;
-uint64_t field_bit_offset = 0;
 bool is_signed;
-bool is_complex;
 uint32_t count;
+bool is_complex;
 
-   

[Lldb-commits] [lldb] r362154 - [Target] Generalize Process::IsPossibleDynamicValue

[Alex Langford via lldb-commits]

Author: xiaobai
Date: Thu May 30 14:03:53 2019
New Revision: 362154

URL: http://llvm.org/viewvc/llvm-project?rev=362154&view=rev
Log:
[Target] Generalize Process::IsPossibleDynamicValue

Modified:
lldb/trunk/source/Target/Process.cpp

Modified: lldb/trunk/source/Target/Process.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/Process.cpp?rev=362154&r1=362153&r2=362154&view=diff
==
--- lldb/trunk/source/Target/Process.cpp (original)
+++ lldb/trunk/source/Target/Process.cpp Thu May 30 14:03:53 2019
@@ -1631,12 +1631,12 @@ bool Process::IsPossibleDynamicValue(Val
 return runtime ? runtime->CouldHaveDynamicValue(in_value) : false;
   }
 
-  LanguageRuntime *cpp_runtime = GetLanguageRuntime(eLanguageTypeC_plus_plus);
-  if (cpp_runtime && cpp_runtime->CouldHaveDynamicValue(in_value))
-return true;
+  for (LanguageRuntime *runtime : GetLanguageRuntimes()) {
+if (runtime->CouldHaveDynamicValue(in_value))
+  return true;
+  }
 
-  LanguageRuntime *objc_runtime = GetLanguageRuntime(eLanguageTypeObjC);
-  return objc_runtime ? objc_runtime->CouldHaveDynamicValue(in_value) : false;
+  return false;
 }
 
 void Process::SetDynamicCheckers(DynamicCheckerFunctions *dynamic_checkers) {


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

[Lldb-commits] [PATCH] D62702: [ABI] Fix SystemV ABI to handle nested aggregate type returned in register

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

There should be a test to go along with this.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D62702



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

[Lldb-commits] [PATCH] D62702: [ABI] Fix SystemV ABI to handle nested aggregate type returned in register

[Wanyi Ye via Phabricator via lldb-commits]

kusmour added a comment.

@jingham working on the unit test rn. will upload soon


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D62702



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

[Lldb-commits] [lldb] r362164 - [Target] Generalize language-specific behavior in ThreadPlanStepThrough

[Alex Langford via lldb-commits]

Author: xiaobai
Date: Thu May 30 15:00:18 2019
New Revision: 362164

URL: http://llvm.org/viewvc/llvm-project?rev=362164&view=rev
Log:
[Target] Generalize language-specific behavior in ThreadPlanStepThrough

Summary:
When creating a ThreadPlan to step through a trampoline, we ask the
ObjC language runtime and the CPP language runtime to come up with such a thread
plan if the dynamic loader fails to give us one. I don't see why this behavior
can't be language agnostic.

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

Modified:
lldb/trunk/include/lldb/Target/CPPLanguageRuntime.h
lldb/trunk/include/lldb/Target/LanguageRuntime.h
lldb/trunk/include/lldb/Target/ObjCLanguageRuntime.h
lldb/trunk/source/Target/ThreadPlanStepThrough.cpp

Modified: lldb/trunk/include/lldb/Target/CPPLanguageRuntime.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Target/CPPLanguageRuntime.h?rev=362164&r1=362163&r2=362164&view=diff
==
--- lldb/trunk/include/lldb/Target/CPPLanguageRuntime.h (original)
+++ lldb/trunk/include/lldb/Target/CPPLanguageRuntime.h Thu May 30 15:00:18 2019
@@ -61,7 +61,7 @@ public:
   /// \return
   ///  A ThreadPlan Shared pointer
   lldb::ThreadPlanSP GetStepThroughTrampolinePlan(Thread &thread,
-  bool stop_others);
+  bool stop_others) override;
 
   bool IsRuntimeSupportValue(ValueObject &valobj) override;
 protected:

Modified: lldb/trunk/include/lldb/Target/LanguageRuntime.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Target/LanguageRuntime.h?rev=362164&r1=362163&r2=362164&view=diff
==
--- lldb/trunk/include/lldb/Target/LanguageRuntime.h (original)
+++ lldb/trunk/include/lldb/Target/LanguageRuntime.h Thu May 30 15:00:18 2019
@@ -143,6 +143,9 @@ public:
 return false;
   }
 
+  virtual lldb::ThreadPlanSP GetStepThroughTrampolinePlan(Thread &thread,
+  bool stop_others) = 
0;
+
   /// Identify whether a value is a language implementation detaul
   /// that should be hidden from the user interface by default.
   virtual bool IsRuntimeSupportValue(ValueObject &valobj) { return false; }

Modified: lldb/trunk/include/lldb/Target/ObjCLanguageRuntime.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Target/ObjCLanguageRuntime.h?rev=362164&r1=362163&r2=362164&view=diff
==
--- lldb/trunk/include/lldb/Target/ObjCLanguageRuntime.h (original)
+++ lldb/trunk/include/lldb/Target/ObjCLanguageRuntime.h Thu May 30 15:00:18 
2019
@@ -216,9 +216,6 @@ public:
 
   virtual bool HasReadObjCLibrary() = 0;
 
-  virtual lldb::ThreadPlanSP GetStepThroughTrampolinePlan(Thread &thread,
-  bool stop_others) = 
0;
-
   lldb::addr_t LookupInMethodCache(lldb::addr_t class_addr, lldb::addr_t sel);
 
   void AddToMethodCache(lldb::addr_t class_addr, lldb::addr_t sel,

Modified: lldb/trunk/source/Target/ThreadPlanStepThrough.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/ThreadPlanStepThrough.cpp?rev=362164&r1=362163&r2=362164&view=diff
==
--- lldb/trunk/source/Target/ThreadPlanStepThrough.cpp (original)
+++ lldb/trunk/source/Target/ThreadPlanStepThrough.cpp Thu May 30 15:00:18 2019
@@ -8,9 +8,8 @@
 
 #include "lldb/Target/ThreadPlanStepThrough.h"
 #include "lldb/Breakpoint/Breakpoint.h"
-#include "lldb/Target/CPPLanguageRuntime.h"
 #include "lldb/Target/DynamicLoader.h"
-#include "lldb/Target/ObjCLanguageRuntime.h"
+#include "lldb/Target/LanguageRuntime.h"
 #include "lldb/Target/Process.h"
 #include "lldb/Target/RegisterContext.h"
 #include "lldb/Target/Target.h"
@@ -85,22 +84,17 @@ void ThreadPlanStepThrough::LookForPlanT
 m_sub_plan_sp =
 loader->GetStepThroughTrampolinePlan(m_thread, m_stop_others);
 
-  // If that didn't come up with anything, try the ObjC runtime plugin:
-  if (!m_sub_plan_sp.get()) {
-ObjCLanguageRuntime *objc_runtime =
-m_thread.GetProcess()->GetObjCLanguageRuntime();
-if (objc_runtime)
+  // If the DynamicLoader was unable to provide us with a ThreadPlan, then we
+  // try the LanguageRuntimes.
+  if (!m_sub_plan_sp) {
+for (LanguageRuntime *runtime :
+ m_thread.GetProcess()->GetLanguageRuntimes()) {
   m_sub_plan_sp =
-  objc_runtime->GetStepThroughTrampolinePlan(m_thread, m_stop_others);
+  runtime->GetStepThroughTrampolinePlan(m_thread, m_stop_others);
 
-CPPLanguageRuntime *cpp_runtime =
-m_thread.GetProcess()->GetCPPLanguageRuntime();
-
-// If the ObjC runtime did not provide us with a step though plan then if 
we
-/

[Lldb-commits] [PATCH] D61921: [Target] Generalize language-specific behavior in ThreadPlanStepThrough

[Alex Langford via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL362164: [Target] Generalize language-specific behavior in 
ThreadPlanStepThrough (authored by xiaobai, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D61921

Files:
  lldb/trunk/include/lldb/Target/CPPLanguageRuntime.h
  lldb/trunk/include/lldb/Target/LanguageRuntime.h
  lldb/trunk/include/lldb/Target/ObjCLanguageRuntime.h
  lldb/trunk/source/Target/ThreadPlanStepThrough.cpp


Index: lldb/trunk/source/Target/ThreadPlanStepThrough.cpp
===
--- lldb/trunk/source/Target/ThreadPlanStepThrough.cpp
+++ lldb/trunk/source/Target/ThreadPlanStepThrough.cpp
@@ -8,9 +8,8 @@
 
 #include "lldb/Target/ThreadPlanStepThrough.h"
 #include "lldb/Breakpoint/Breakpoint.h"
-#include "lldb/Target/CPPLanguageRuntime.h"
 #include "lldb/Target/DynamicLoader.h"
-#include "lldb/Target/ObjCLanguageRuntime.h"
+#include "lldb/Target/LanguageRuntime.h"
 #include "lldb/Target/Process.h"
 #include "lldb/Target/RegisterContext.h"
 #include "lldb/Target/Target.h"
@@ -85,22 +84,17 @@
 m_sub_plan_sp =
 loader->GetStepThroughTrampolinePlan(m_thread, m_stop_others);
 
-  // If that didn't come up with anything, try the ObjC runtime plugin:
-  if (!m_sub_plan_sp.get()) {
-ObjCLanguageRuntime *objc_runtime =
-m_thread.GetProcess()->GetObjCLanguageRuntime();
-if (objc_runtime)
+  // If the DynamicLoader was unable to provide us with a ThreadPlan, then we
+  // try the LanguageRuntimes.
+  if (!m_sub_plan_sp) {
+for (LanguageRuntime *runtime :
+ m_thread.GetProcess()->GetLanguageRuntimes()) {
   m_sub_plan_sp =
-  objc_runtime->GetStepThroughTrampolinePlan(m_thread, m_stop_others);
-
-CPPLanguageRuntime *cpp_runtime =
-m_thread.GetProcess()->GetCPPLanguageRuntime();
+  runtime->GetStepThroughTrampolinePlan(m_thread, m_stop_others);
 
-// If the ObjC runtime did not provide us with a step though plan then if 
we
-// have it check the C++ runtime for a step though plan.
-if (!m_sub_plan_sp.get() && cpp_runtime)
-  m_sub_plan_sp =
-  cpp_runtime->GetStepThroughTrampolinePlan(m_thread, m_stop_others);
+  if (m_sub_plan_sp)
+break;
+}
   }
 
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_STEP));
Index: lldb/trunk/include/lldb/Target/CPPLanguageRuntime.h
===
--- lldb/trunk/include/lldb/Target/CPPLanguageRuntime.h
+++ lldb/trunk/include/lldb/Target/CPPLanguageRuntime.h
@@ -61,7 +61,7 @@
   /// \return
   ///  A ThreadPlan Shared pointer
   lldb::ThreadPlanSP GetStepThroughTrampolinePlan(Thread &thread,
-  bool stop_others);
+  bool stop_others) override;
 
   bool IsRuntimeSupportValue(ValueObject &valobj) override;
 protected:
Index: lldb/trunk/include/lldb/Target/LanguageRuntime.h
===
--- lldb/trunk/include/lldb/Target/LanguageRuntime.h
+++ lldb/trunk/include/lldb/Target/LanguageRuntime.h
@@ -143,6 +143,9 @@
 return false;
   }
 
+  virtual lldb::ThreadPlanSP GetStepThroughTrampolinePlan(Thread &thread,
+  bool stop_others) = 
0;
+
   /// Identify whether a value is a language implementation detaul
   /// that should be hidden from the user interface by default.
   virtual bool IsRuntimeSupportValue(ValueObject &valobj) { return false; }
Index: lldb/trunk/include/lldb/Target/ObjCLanguageRuntime.h
===
--- lldb/trunk/include/lldb/Target/ObjCLanguageRuntime.h
+++ lldb/trunk/include/lldb/Target/ObjCLanguageRuntime.h
@@ -216,9 +216,6 @@
 
   virtual bool HasReadObjCLibrary() = 0;
 
-  virtual lldb::ThreadPlanSP GetStepThroughTrampolinePlan(Thread &thread,
-  bool stop_others) = 
0;
-
   lldb::addr_t LookupInMethodCache(lldb::addr_t class_addr, lldb::addr_t sel);
 
   void AddToMethodCache(lldb::addr_t class_addr, lldb::addr_t sel,


Index: lldb/trunk/source/Target/ThreadPlanStepThrough.cpp
===
--- lldb/trunk/source/Target/ThreadPlanStepThrough.cpp
+++ lldb/trunk/source/Target/ThreadPlanStepThrough.cpp
@@ -8,9 +8,8 @@
 
 #include "lldb/Target/ThreadPlanStepThrough.h"
 #include "lldb/Breakpoint/Breakpoint.h"
-#include "lldb/Target/CPPLanguageRuntime.h"
 #include "lldb/Target/DynamicLoader.h"
-#include "lldb/Target/ObjCLanguageRuntime.h"
+#include "lldb/Target/LanguageRuntime.h"
 #include "lldb/Target/Process.h"
 #include "lldb/Target/RegisterContext.h"
 #include "lldb

Re: [Lldb-commits] [PATCH] D62505: Fix multiple module loaded with the same UUID

[Jim Ingham via lldb-commits]

> On May 28, 2019, at 7:59 AM, Pavel Labath via Phabricator 
>  wrote:
> 
> labath added a comment.
> 
> In D62505#1519166 , @aadsm wrote:
> 
>> Interesting, I did miss that comment when I checked that class. This is 
>> something @clayborg was concerned with when I discussed this solution with 
>> him. The main reason I was not keen in having multiple load addresses for a 
>> section is because that would change the GetLoadAddress to return an array 
>> instead of an addr_t or some kind of mechanism to be able to correctly 
>> resolve the load address of an Address (felt like too big of a change for 
>> what it looks like an edge case?).
> 
> 
> I wouldn't say this is specific to some class of samsung phones. It is easy 
> to use dlmopen(3) to open the same library multiple times. Just try calling 
> `dlmopen(LM_ID_NEWLM, "/lib64/libc.so.6", RTLD_NOW)` in a loop a couple of 
> times and see what happens in /proc//maps. (I believe this is the same 
> function that android uses to implement vndk separation.)
> 
> In D62505#1519178 , @clayborg wrote:
> 
>> So if we end up going with one module, how do we propose to get around the 
>> fact that there are multiple platform paths ("/system/lib/libcutils.so" and 
>> "/system/lib/vndk-sp/libcutils.so") for this module? Just make platform path 
>> be an array instead of a single path?
> 
> 
> TBH, I'm not really convinced that the "platform path" should be a property 
> of the Module.
> 
> Imagine a situation where I have the same module loaded in multiple targets 
> (maybe on different remote platforms, or maybe not). It makes sense that each 
> target can have the module located at a different path. So maybe the 
> "platform path" should be a property of the target,  saying "where did this 
> target load the module from"? Of course, this won't help when you have the 
> same module loaded multiple times in the same target, so maybe this does need 
> to be a list of paths? I'm not sure, I'm just thinking aloud here...
> 
> Right now, I'm also lean towards making a Section be loadable multiple times, 
> however this definitely needs more discussion. I'm interested to hear what 
> @jingham thinks of this.

I agree that the platform path has to be held by the target, not by the module. 
 In your example lldb would love to have a single local copy of the executable 
so it can do reads locally, which makes it clear that there are real cases 
where it would be obvious two targets should share the same Module, but would 
need to have different remote names for the it.  

I don't see how extending the Platform Path in the module to be a vector can 
handle this situation.  We have tried to keep Modules from knowing about 
Targets, but the only way to make sense of all the platform paths associated 
with the Module is to know which one is used by a given Target.  So you'd 
really have to have a pair of Name & Target.  That argues that the Module was 
the wrong place for this information to begin with.

You could imagine doing this by having a map of platform path -> module in the 
target, but then there are all sorts of places you'd have to be careful to 
consult this, and that seems fallible.

It really sounds like the Target should be dealing with TargetModules (a pair 
of Module & PlatformPath) and not straight Modules.  Probably for the sake of 
reducing changes we could have "CacheModules" which are what Modules are today, 
and Modules that are the pair of PlatformPath and Module.  I think this is what 
Greg was talking about with his BaseModule class.

Then if you want to support the same CacheModule being reused in a given 
target, the SectionLoadList  would have to hold a pair of Section/TargetModule 
- a TargetSection? - so it would know which instance of the module was meant.  
I think you also have to do the same thing with Address.  It currently holds a 
Section, but that isn't fully specified in the case where the Module can appear 
twice.  It would have to have a TargetSection instead.  Except that you can 
pull Address's out of Modules w/o going through a Target.  So again you might 
have to make a distinction between Address and TargetAddress, which might get 
messy.

And again, if we do any of this we probably want to leave the bare names 
(Address, Section, Module) be the things you get from a Target - which is how 
most of the upper layers of lldb get these entities, and have some other name 
for the equivalents as they live in the Cache separate from Targets.

This looks like it could be fairly intrusive, but OTOH seems to accurately 
represent the more complex situation you are trying to describe, so will 
probably cause less pain in the long run.

Jim


> 
> 
> Repository:
>  rG LLVM Github Monorepo
> 
> CHANGES SINCE LAST ACTION
>  https://reviews.llvm.org/D62505/new/
> 
> https://reviews.llvm.org/D62505
> 
> 
> 

___
lldb-co

[Lldb-commits] [PATCH] D62499: Create a generic handler for Xfer packets

[António Afonso via Phabricator via lldb-commits]

aadsm updated this revision to Diff 202323.
aadsm added a comment.

- Introduce better error handling by creating 2 new error classes, one for 
generic packet errors that contain a number and another for unimplemented 
features.
- The logic for reading the xfer object was moved into its own function.
- Removes the define for linux || netbsd for the auxv xfer object.
- Switched from shared pointer to a unique pointer to store the memory buffers 
in the map


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62499

Files:
  lldb/include/lldb/Utility/StringExtractorGDBRemote.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
  lldb/source/Utility/StringExtractorGDBRemote.cpp

Index: lldb/source/Utility/StringExtractorGDBRemote.cpp
===
--- lldb/source/Utility/StringExtractorGDBRemote.cpp
+++ lldb/source/Utility/StringExtractorGDBRemote.cpp
@@ -285,8 +285,8 @@
   break;
 
 case 'X':
-  if (PACKET_STARTS_WITH("qXfer:auxv:read::"))
-return eServerPacketType_qXfer_auxv_read;
+  if (PACKET_STARTS_WITH("qXfer:"))
+return eServerPacketType_qXfer;
   break;
 }
 break;
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
@@ -82,7 +82,8 @@
   MainLoop::ReadHandleUP m_stdio_handle_up;
 
   lldb::StateType m_inferior_prev_state = lldb::StateType::eStateInvalid;
-  std::unique_ptr m_active_auxv_buffer_up;
+  std::unordered_map>
+  m_xfer_buffer_map;
   std::mutex m_saved_registers_mutex;
   std::unordered_map m_saved_registers_map;
   uint32_t m_next_saved_registers_id = 1;
@@ -150,7 +151,7 @@
 
   PacketResult Handle_s(StringExtractorGDBRemote &packet);
 
-  PacketResult Handle_qXfer_auxv_read(StringExtractorGDBRemote &packet);
+  PacketResult Handle_qXfer(StringExtractorGDBRemote &packet);
 
   PacketResult Handle_QSaveRegisterState(StringExtractorGDBRemote &packet);
 
@@ -193,6 +194,9 @@
   FileSpec FindModuleFile(const std::string &module_path,
   const ArchSpec &arch) override;
 
+  llvm::Expected>
+  ReadXferObject(llvm::StringRef object, llvm::StringRef annex);
+
 private:
   void HandleInferiorState_Exited(NativeProcessProtocol *process);
 
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -144,8 +144,8 @@
   StringExtractorGDBRemote::eServerPacketType_qWatchpointSupportInfo,
   &GDBRemoteCommunicationServerLLGS::Handle_qWatchpointSupportInfo);
   RegisterMemberFunctionHandler(
-  StringExtractorGDBRemote::eServerPacketType_qXfer_auxv_read,
-  &GDBRemoteCommunicationServerLLGS::Handle_qXfer_auxv_read);
+  StringExtractorGDBRemote::eServerPacketType_qXfer,
+  &GDBRemoteCommunicationServerLLGS::Handle_qXfer);
   RegisterMemberFunctionHandler(StringExtractorGDBRemote::eServerPacketType_s,
 &GDBRemoteCommunicationServerLLGS::Handle_s);
   RegisterMemberFunctionHandler(
@@ -2747,47 +2747,19 @@
   return PacketResult::Success;
 }
 
-GDBRemoteCommunication::PacketResult
-GDBRemoteCommunicationServerLLGS::Handle_qXfer_auxv_read(
-StringExtractorGDBRemote &packet) {
-// *BSD impls should be able to do this too.
-#if defined(__linux__) || defined(__NetBSD__)
-  Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PROCESS));
-
-  // Parse out the offset.
-  packet.SetFilePos(strlen("qXfer:auxv:read::"));
-  if (packet.GetBytesLeft() < 1)
-return SendIllFormedResponse(packet,
- "qXfer:auxv:read:: packet missing offset");
-
-  const uint64_t auxv_offset =
-  packet.GetHexMaxU64(false, std::numeric_limits::max());
-  if (auxv_offset == std::numeric_limits::max())
-return SendIllFormedResponse(packet,
- "qXfer:auxv:read:: packet missing offset");
-
-  // Parse out comma.
-  if (packet.GetBytesLeft() < 1 || packet.GetChar() != ',')
-return SendIllFormedResponse(
-packet, "qXfer:auxv:read:: packet missing comma after offset");
-
-  // Parse out the length.
-  const uint64_t auxv_length =
-  packet.GetHexMaxU64(false, std::numeric_limits::max());
-  if (auxv_length == std::numeric_limits::max())
-return SendIllFormedRespo

[Lldb-commits] [PATCH] D62501: Implement GetSharedLibraryInfoAddress

[António Afonso via Phabricator via lldb-commits]

aadsm updated this revision to Diff 202325.
aadsm added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62501

Files:
  lldb/include/lldb/Host/common/NativeProcessProtocol.h
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.h

Index: lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
===
--- lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
+++ lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
@@ -130,11 +130,14 @@
 protected:
   llvm::Expected>
   GetSoftwareBreakpointTrapOpcode(size_t size_hint) override;
+  template 
+  lldb::addr_t GetELFImageInfoAddress();
 
 private:
   MainLoop::SignalHandleUP m_sigchld_handle;
   ArchSpec m_arch;
   std::unique_ptr m_aux_vector;
+  lldb::addr_t m_elf_image_info_addr = LLDB_INVALID_ADDRESS;
 
   LazyBool m_supports_mem_region = eLazyBoolCalculate;
   std::vector> m_mem_region_cache;
Index: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
===
--- lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
+++ lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
@@ -38,6 +38,7 @@
 #include "lldb/Utility/State.h"
 #include "lldb/Utility/Status.h"
 #include "lldb/Utility/StringExtractor.h"
+#include "llvm/BinaryFormat/ELF.h"
 #include "llvm/Support/Errno.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Threading.h"
@@ -1390,8 +1391,12 @@
 }
 
 lldb::addr_t NativeProcessLinux::GetSharedLibraryInfoAddress() {
-  // punt on this for now
-  return LLDB_INVALID_ADDRESS;
+  if (GetAddressByteSize() == 8)
+return GetELFImageInfoAddress();
+  else
+return GetELFImageInfoAddress();
 }
 
 size_t NativeProcessLinux::UpdateThreads() {
@@ -2096,3 +2101,71 @@
 
   return m_aux_vector->GetAuxValue(type);
 }
+
+template 
+lldb::addr_t NativeProcessLinux::GetELFImageInfoAddress() {
+  if (m_elf_image_info_addr != LLDB_INVALID_ADDRESS)
+return m_elf_image_info_addr;
+
+  lldb::addr_t phdr_addr = GetAuxValue(AuxVector::AUXV_AT_PHDR);
+  size_t phdr_entry_size = GetAuxValue(AuxVector::AUXV_AT_PHENT);
+  size_t phdr_num_entries = GetAuxValue(AuxVector::AUXV_AT_PHNUM);
+  lldb::addr_t load_base = phdr_addr - sizeof(ELF_EHDR);
+  if (phdr_addr == 0 || phdr_entry_size == 0 || phdr_num_entries == 0)
+return LLDB_INVALID_ADDRESS;
+
+  // Find the PT_DYNAMIC segment (.dynamic section) in the program header and
+  // what the ELF believes to be the load base.
+  lldb::addr_t elf_load_base = 0;
+  bool found_elf_load_base = false;
+  lldb::addr_t dynamic_section_addr = 0;
+  uint64_t dynamic_section_size = 0;
+  bool found_dynamic_section = false;
+  ELF_PHDR phdr_entry;
+  for (size_t i = 0; i < phdr_num_entries; i++) {
+size_t bytes_read;
+auto error = ReadMemory(phdr_addr + i * phdr_entry_size, &phdr_entry,
+sizeof(phdr_entry), bytes_read);
+if (!error.Success())
+  return LLDB_INVALID_ADDRESS;
+
+if ((!found_elf_load_base || phdr_entry.p_vaddr < elf_load_base) &&
+phdr_entry.p_type == llvm::ELF::PT_LOAD) {
+  elf_load_base = phdr_entry.p_vaddr;
+  found_elf_load_base = true;
+}
+
+if (phdr_entry.p_type == llvm::ELF::PT_DYNAMIC) {
+  dynamic_section_addr = phdr_entry.p_vaddr;
+  dynamic_section_size = phdr_entry.p_memsz;
+  found_dynamic_section = true;
+}
+  }
+
+  if (!found_elf_load_base || !found_dynamic_section)
+return LLDB_INVALID_ADDRESS;
+
+  // Find the DT_DEBUG entry in the .dynamic section
+  // The load base we got from the auxiliary vector is the source of truth. If
+  // we got a different load base from the ELF then we need to correct the
+  // .dynamic section address with the difference we found.
+  dynamic_section_addr += (load_base - elf_load_base);
+  ELF_DYN dynamic_entry;
+  size_t dynamic_num_entries = dynamic_section_size / sizeof(dynamic_entry);
+  for (size_t i = 0; i < dynamic_num_entries; i++) {
+size_t bytes_read;
+auto error = ReadMemory(dynamic_section_addr + i * sizeof(dynamic_entry),
+&dynamic_entry, sizeof(dynamic_entry), bytes_read);
+if (!error.Success())
+  return LLDB_INVALID_ADDRESS;
+// Return the &DT_DEBUG->d_ptr which points to r_debug which contains the
+// link_map.
+if (dynamic_entry.d_tag == llvm::ELF::DT_DEBUG) {
+  m_elf_image_info_addr = dynamic_section_addr + i * sizeof(dynamic_entry) +
+  sizeof(dynamic_entry.d_tag);
+  return m_elf_image_info_addr;
+}
+  }
+
+  return LLDB_INVALID_ADDRESS;
+}
Index: lldb/include/lldb/Host/common/NativeProcessProtocol.h
===
--- lldb/include/lldb/Host/common/NativeProcessProtocol.h
+++ lldb/include/lldb/Host/common/Nativ

[Lldb-commits] [PATCH] D62500: Add support to read aux vector values

[António Afonso via Phabricator via lldb-commits]

aadsm updated this revision to Diff 202324.
aadsm added a comment.

Rnamed ELFAuxVector to AuxVector and got rid of the one that existed in the 
POSIX-DYLD plugin.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62500

Files:
  lldb/include/lldb/Host/common/NativeProcessProtocol.h
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/AuxVector.cpp
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/AuxVector.h
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/CMakeLists.txt
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
  lldb/source/Plugins/Process/Utility/AuxVector.cpp
  lldb/source/Plugins/Process/Utility/AuxVector.h
  lldb/source/Plugins/Process/Utility/CMakeLists.txt

Index: lldb/source/Plugins/Process/Utility/CMakeLists.txt
===
--- lldb/source/Plugins/Process/Utility/CMakeLists.txt
+++ lldb/source/Plugins/Process/Utility/CMakeLists.txt
@@ -1,4 +1,5 @@
 add_lldb_library(lldbPluginProcessUtility PLUGIN
+  AuxVector.cpp
   DynamicRegisterInfo.cpp
   FreeBSDSignals.cpp
   GDBRemoteSignals.cpp
Index: lldb/source/Plugins/Process/Utility/AuxVector.h
===
--- /dev/null
+++ lldb/source/Plugins/Process/Utility/AuxVector.h
@@ -0,0 +1,73 @@
+//===-- AuxVector.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
+//
+//===--===//
+
+#ifndef liblldb_AuxVector_H_
+#define liblldb_AuxVector_H_
+
+#include "lldb/Utility/DataExtractor.h"
+#include "lldb/Utility/Log.h"
+#include 
+
+class AuxVector {
+
+public:
+  AuxVector(lldb_private::DataExtractor &data);
+
+  /// Constants describing the type of entry.
+  /// On Linux, running "LD_SHOW_AUXV=1 ./executable" will spew AUX
+  /// information. Added AUXV prefix to avoid potential conflicts with system-
+  /// defined macros
+  enum EntryType {
+AUXV_AT_NULL = 0,  ///< End of auxv.
+AUXV_AT_IGNORE = 1,///< Ignore entry.
+AUXV_AT_EXECFD = 2,///< File descriptor of program.
+AUXV_AT_PHDR = 3,  ///< Program headers.
+AUXV_AT_PHENT = 4, ///< Size of program header.
+AUXV_AT_PHNUM = 5, ///< Number of program headers.
+AUXV_AT_PAGESZ = 6,///< Page size.
+AUXV_AT_BASE = 7,  ///< Interpreter base address.
+AUXV_AT_FLAGS = 8, ///< Flags.
+AUXV_AT_ENTRY = 9, ///< Program entry point.
+AUXV_AT_NOTELF = 10,   ///< Set if program is not an ELF.
+AUXV_AT_UID = 11,  ///< UID.
+AUXV_AT_EUID = 12, ///< Effective UID.
+AUXV_AT_GID = 13,  ///< GID.
+AUXV_AT_EGID = 14, ///< Effective GID.
+AUXV_AT_CLKTCK = 17,   ///< Clock frequency (e.g. times(2)).
+AUXV_AT_PLATFORM = 15, ///< String identifying platform.
+AUXV_AT_HWCAP =
+16, ///< Machine dependent hints about processor capabilities.
+AUXV_AT_FPUCW = 18, ///< Used FPU control word.
+AUXV_AT_DCACHEBSIZE = 19,   ///< Data cache block size.
+AUXV_AT_ICACHEBSIZE = 20,   ///< Instruction cache block size.
+AUXV_AT_UCACHEBSIZE = 21,   ///< Unified cache block size.
+AUXV_AT_IGNOREPPC = 22, ///< Entry should be ignored.
+AUXV_AT_SECURE = 23,///< Boolean, was exec setuid-like?
+AUXV_AT_BASE_PLATFORM = 24, ///< String identifying real platforms.
+AUXV_AT_RANDOM = 25,///< Address of 16 random bytes.
+AUXV_AT_EXECFN = 31,///< Filename of executable.
+AUXV_AT_SYSINFO = 32, ///< Pointer to the global system page used for system
+  /// calls and other nice things.
+AUXV_AT_SYSINFO_EHDR = 33,
+AUXV_AT_L1I_CACHESHAPE = 34, ///< Shapes of the caches.
+AUXV_AT_L1D_CACHESHAPE = 35,
+AUXV_AT_L2_CACHESHAPE = 36,
+AUXV_AT_L3_CACHESHAPE = 37,
+  };
+
+  uint64_t GetAuxValue(enum EntryType entry_type) const;
+  void DumpToLog(lldb_private::Log *log) const;
+  const char *GetEntryName(EntryType type) const;
+
+private:
+  void ParseAuxv(lldb_private::DataExtractor &data);
+
+  std::unordered_map m_auxv_entries;
+};
+
+#endif
Index: lldb/source/Plugins/Process/Utility/AuxVector.cpp
===
--- lldb/source/Plugins/Process/Utility/AuxVector.cpp
+++ lldb/source/Plugins/Process/Utility/AuxVector.cpp
@@ -7,98 +7,58 @@
 //===--===//
 
 #include "AuxVector.h"
-#include "lldb/Target/Process.h"
-#include "lldb

[Lldb-commits] [PATCH] D62502: Implement xfer:libraries-svr4:read packet

[António Afonso via Phabricator via lldb-commits]

aadsm updated this revision to Diff 202326.
aadsm added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62502

Files:
  lldb/include/lldb/Host/common/NativeProcessProtocol.h
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp

Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -2772,6 +2772,28 @@
 return std::move(*buffer_or_error);
   }
 
+#if defined(__linux__)
+  if (object == "libraries-svr4") {
+std::vector library_list;
+auto status =
+m_debugged_process_up->GetLoadedSharedLibraries(library_list);
+if (!status.Success())
+  return status.ToError();
+
+StreamString response;
+response.Printf("");
+for (auto const &library : library_list) {
+  response.Printf("", library.ld_addr);
+}
+response.Printf("");
+std::unique_ptr memory_buffer_up = std::move(
+MemoryBuffer::getMemBufferCopy(response.GetString(), __FUNCTION__));
+  }
+#endif
+
   return llvm::make_error(
   "Xfer object not supported");
 }
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
@@ -826,6 +826,9 @@
   response.PutCString(";QPassSignals+");
   response.PutCString(";qXfer:auxv:read+");
 #endif
+#if defined(__linux__)
+  response.PutCString(";qXfer:libraries-svr4:read+");
+#endif
 
   return SendPacketNoLock(response.GetString());
 }
Index: lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
===
--- lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
+++ lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
@@ -79,6 +79,9 @@
 
   lldb::addr_t GetSharedLibraryInfoAddress() override;
 
+  Status GetLoadedSharedLibraries(
+  std::vector &library_list) override;
+
   size_t UpdateThreads() override;
 
   const ArchSpec &GetArchitecture() const override { return m_arch; }
@@ -133,6 +136,17 @@
   template 
   lldb::addr_t GetELFImageInfoAddress();
 
+  template  struct ELFLinkMap {
+T l_addr;
+T l_name;
+T l_ld;
+T l_next;
+T l_prev;
+  };
+  template 
+  Status ReadSharedLibraryInfo(lldb::addr_t link_map_addr,
+   SharedLibraryInfo &info);
+
 private:
   MainLoop::SignalHandleUP m_sigchld_handle;
   ArchSpec m_arch;
Index: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
===
--- lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
+++ lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
@@ -2169,3 +2169,76 @@
 
   return LLDB_INVALID_ADDRESS;
 }
+
+template 
+Status NativeProcessLinux::ReadSharedLibraryInfo(lldb::addr_t link_map_addr,
+ SharedLibraryInfo &info) {
+  ELFLinkMap link_map;
+  size_t bytes_read;
+  auto error =
+  ReadMemory(link_map_addr, &link_map, sizeof(link_map), bytes_read);
+  if (!error.Success())
+return error;
+
+  char name_buffer[PATH_MAX];
+  error = ReadMemory(link_map.l_name, &name_buffer, sizeof(name_buffer),
+ bytes_read);
+  if (!error.Success())
+return error;
+
+  info.name = std::string(name_buffer);
+  info.link_map = link_map_addr;
+  info.base_addr = link_map.l_addr;
+  info.ld_addr = link_map.l_ld;
+  info.next = link_map.l_next;
+#if defined(__linux__) && !defined(__ANDROID__)
+  // On non-android linux systems, main executable has an empty path.
+  info.main = info.main.empty();
+#elif defined(__linux__) && defined(__ANDROID__)
+  // On android, the main executable has a load address of 0.
+  info.main = info.ld_addr == 0;
+#else
+  info.main = false;
+#endif
+
+  return Status();
+}
+
+Status NativeProcessLinux::GetLoadedSharedLibraries(
+std::vector &library_list) {
+  // Address of DT_DEBUG.d_ptr which points to r_debug
+  lldb::addr_t info_address = GetSharedLibraryInfoAddress();
+  if (info_address == LLDB_INVALID_ADDRESS)
+return Status("Invalid shared library info address");
+  // Address of r_debug
+  lldb::addr_t address = 0;
+  size_t bytes_read;
+  auto error =
+  ReadMemory(info_address, &address, GetAddressByteSize(), bytes_read);
+  if (!error.Success())
+return error;
+  if (address == 0)

[Lldb-commits] [PATCH] D62502: Implement xfer:libraries-svr4:read packet

[António Afonso via Phabricator via lldb-commits]

aadsm updated this revision to Diff 202328.
aadsm added a comment.

Update with the correct commit...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62502

Files:
  lldb/include/lldb/Host/common/NativeProcessProtocol.h
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp

Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -2772,6 +2772,27 @@
 return std::move(*buffer_or_error);
   }
 
+#if defined(__linux__)
+  if (object == "libraries-svr4") {
+std::vector library_list;
+auto status = m_debugged_process_up->GetLoadedSharedLibraries(library_list);
+if (!status.Success())
+  return status.ToError();
+
+StreamString response;
+response.Printf("");
+for (auto const &library : library_list) {
+  response.Printf("", library.ld_addr);
+}
+response.Printf("");
+return std::move(
+MemoryBuffer::getMemBufferCopy(response.GetString(), __FUNCTION__));
+  }
+#endif
+
   return llvm::make_error(
   "Xfer object not supported");
 }
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
@@ -826,6 +826,9 @@
   response.PutCString(";QPassSignals+");
   response.PutCString(";qXfer:auxv:read+");
 #endif
+#if defined(__linux__)
+  response.PutCString(";qXfer:libraries-svr4:read+");
+#endif
 
   return SendPacketNoLock(response.GetString());
 }
Index: lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
===
--- lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
+++ lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
@@ -79,6 +79,9 @@
 
   lldb::addr_t GetSharedLibraryInfoAddress() override;
 
+  Status GetLoadedSharedLibraries(
+  std::vector &library_list) override;
+
   size_t UpdateThreads() override;
 
   const ArchSpec &GetArchitecture() const override { return m_arch; }
@@ -133,6 +136,17 @@
   template 
   lldb::addr_t GetELFImageInfoAddress();
 
+  template  struct ELFLinkMap {
+T l_addr;
+T l_name;
+T l_ld;
+T l_next;
+T l_prev;
+  };
+  template 
+  Status ReadSharedLibraryInfo(lldb::addr_t link_map_addr,
+   SharedLibraryInfo &info);
+
 private:
   MainLoop::SignalHandleUP m_sigchld_handle;
   ArchSpec m_arch;
Index: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
===
--- lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
+++ lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
@@ -2169,3 +2169,76 @@
 
   return LLDB_INVALID_ADDRESS;
 }
+
+template 
+Status NativeProcessLinux::ReadSharedLibraryInfo(lldb::addr_t link_map_addr,
+ SharedLibraryInfo &info) {
+  ELFLinkMap link_map;
+  size_t bytes_read;
+  auto error =
+  ReadMemory(link_map_addr, &link_map, sizeof(link_map), bytes_read);
+  if (!error.Success())
+return error;
+
+  char name_buffer[PATH_MAX];
+  error = ReadMemory(link_map.l_name, &name_buffer, sizeof(name_buffer),
+ bytes_read);
+  if (!error.Success())
+return error;
+
+  info.name = std::string(name_buffer);
+  info.link_map = link_map_addr;
+  info.base_addr = link_map.l_addr;
+  info.ld_addr = link_map.l_ld;
+  info.next = link_map.l_next;
+#if defined(__linux__) && !defined(__ANDROID__)
+  // On non-android linux systems, main executable has an empty path.
+  info.main = info.main.empty();
+#elif defined(__linux__) && defined(__ANDROID__)
+  // On android, the main executable has a load address of 0.
+  info.main = info.ld_addr == 0;
+#else
+  info.main = false;
+#endif
+
+  return Status();
+}
+
+Status NativeProcessLinux::GetLoadedSharedLibraries(
+std::vector &library_list) {
+  // Address of DT_DEBUG.d_ptr which points to r_debug
+  lldb::addr_t info_address = GetSharedLibraryInfoAddress();
+  if (info_address == LLDB_INVALID_ADDRESS)
+return Status("Invalid shared library info address");
+  // Address of r_debug
+  lldb::addr_t address = 0;
+  size_t bytes_read;
+  auto error =
+  ReadMemory(info_address, &address, GetAddressByteSize(), bytes_read);
+  if (!error.Success())
+return error;
+  if (address == 0)
+retur

[Lldb-commits] [PATCH] D62503: Add ReadCStringFromMemory for faster string reads

[António Afonso via Phabricator via lldb-commits]

aadsm updated this revision to Diff 202329.
aadsm added a comment.

Moved ReadCStringFromMemory to NativeProcessProtocol and address some other 
comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62503

Files:
  lldb/include/lldb/Host/common/NativeProcessProtocol.h
  lldb/source/Host/common/NativeProcessProtocol.cpp
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp


Index: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
===
--- lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
+++ lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
@@ -2181,8 +2181,9 @@
 return error;
 
   char name_buffer[PATH_MAX];
-  error = ReadMemory(link_map.l_name, &name_buffer, sizeof(name_buffer),
- bytes_read);
+  error = ReadCStringFromMemory(link_map.l_name,
+reinterpret_cast(&name_buffer),
+sizeof(name_buffer), bytes_read);
   if (!error.Success())
 return error;
 
Index: lldb/source/Host/common/NativeProcessProtocol.cpp
===
--- lldb/source/Host/common/NativeProcessProtocol.cpp
+++ lldb/source/Host/common/NativeProcessProtocol.cpp
@@ -16,6 +16,8 @@
 #include "lldb/Utility/State.h"
 #include "lldb/lldb-enumerations.h"
 
+#include "llvm/Support/Process.h"
+
 using namespace lldb;
 using namespace lldb_private;
 
@@ -659,6 +661,45 @@
   return Status();
 }
 
+Status NativeProcessProtocol::ReadCStringFromMemory(lldb::addr_t addr,
+char *buffer,
+size_t max_size,
+size_t &total_bytes_read) {
+  const size_t cache_line_size = llvm::sys::Process::getPageSizeEstimate();
+  size_t bytes_read = 0;
+  size_t bytes_left = max_size;
+  addr_t curr_addr = addr;
+  char *curr_buffer = buffer;
+  total_bytes_read = 0;
+  Status error;
+
+  while (bytes_left > 0 && error.Success()) {
+addr_t cache_line_bytes_left =
+cache_line_size - (curr_addr % cache_line_size);
+addr_t bytes_to_read = std::min(bytes_left, cache_line_bytes_left);
+error = ReadMemory(curr_addr, reinterpret_cast(curr_buffer),
+   bytes_to_read, bytes_read);
+
+if (bytes_read == 0)
+  break;
+
+auto str_end = std::memchr(curr_buffer, '\0', bytes_read);
+if (str_end != NULL) {
+  total_bytes_read = (size_t)(reinterpret_cast(str_end) - buffer);
+  error.Clear();
+  break;
+}
+
+total_bytes_read += bytes_read;
+curr_buffer += bytes_read;
+curr_addr = reinterpret_cast(reinterpret_cast(curr_addr) +
+ bytes_read);
+bytes_left -= bytes_read;
+  }
+
+  return error;
+}
+
 lldb::StateType NativeProcessProtocol::GetState() const {
   std::lock_guard guard(m_state_mutex);
   return m_state;
Index: lldb/include/lldb/Host/common/NativeProcessProtocol.h
===
--- lldb/include/lldb/Host/common/NativeProcessProtocol.h
+++ lldb/include/lldb/Host/common/NativeProcessProtocol.h
@@ -85,6 +85,9 @@
   Status ReadMemoryWithoutTrap(lldb::addr_t addr, void *buf, size_t size,
size_t &bytes_read);
 
+  Status ReadCStringFromMemory(lldb::addr_t addr, char *buffer, size_t 
max_size,
+   size_t &total_bytes_read);
+
   virtual Status WriteMemory(lldb::addr_t addr, const void *buf, size_t size,
  size_t &bytes_written) = 0;
 


Index: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
===
--- lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
+++ lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
@@ -2181,8 +2181,9 @@
 return error;
 
   char name_buffer[PATH_MAX];
-  error = ReadMemory(link_map.l_name, &name_buffer, sizeof(name_buffer),
- bytes_read);
+  error = ReadCStringFromMemory(link_map.l_name,
+reinterpret_cast(&name_buffer),
+sizeof(name_buffer), bytes_read);
   if (!error.Success())
 return error;
 
Index: lldb/source/Host/common/NativeProcessProtocol.cpp
===
--- lldb/source/Host/common/NativeProcessProtocol.cpp
+++ lldb/source/Host/common/NativeProcessProtocol.cpp
@@ -16,6 +16,8 @@
 #include "lldb/Utility/State.h"
 #include "lldb/lldb-enumerations.h"
 
+#include "llvm/Support/Process.h"
+
 using namespace lldb;
 using namespace lldb_private;
 
@@ -659,6 +661,45 @@
   return Status();
 }
 
+Status NativeProcessProtocol::ReadCStringFromMemory(lldb::addr_t addr,
+  

[Lldb-commits] [PATCH] D62089: Make ConnectionFileDescription work with all sockets

[António Afonso via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL362173: Make ConnectionFileDescription work with all sockets 
(authored by aadsm, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D62089?vs=200800&id=202331#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D62089

Files:
  lldb/trunk/include/lldb/Host/Socket.h
  lldb/trunk/include/lldb/Host/common/TCPSocket.h
  lldb/trunk/include/lldb/Host/common/UDPSocket.h
  lldb/trunk/include/lldb/Host/posix/DomainSocket.h
  lldb/trunk/source/Host/common/TCPSocket.cpp
  lldb/trunk/source/Host/common/UDPSocket.cpp
  lldb/trunk/source/Host/posix/ConnectionFileDescriptorPosix.cpp
  lldb/trunk/source/Host/posix/DomainSocket.cpp
  
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
  lldb/trunk/unittests/Host/SocketTest.cpp

Index: lldb/trunk/unittests/Host/SocketTest.cpp
===
--- lldb/trunk/unittests/Host/SocketTest.cpp
+++ lldb/trunk/unittests/Host/SocketTest.cpp
@@ -7,6 +7,7 @@
 //===--===//
 
 #include "SocketTestUtilities.h"
+#include "lldb/Utility/UriParser.h"
 #include "gtest/gtest.h"
 
 using namespace lldb_private;
@@ -147,3 +148,64 @@
   EXPECT_TRUE(socket_up->IsValid());
   EXPECT_NE(socket_up->GetLocalPortNumber(), 0);
 }
+
+TEST_F(SocketTest, TCPGetConnectURI) {
+  std::unique_ptr socket_a_up;
+  std::unique_ptr socket_b_up;
+  if (!IsAddressFamilySupported("127.0.0.1")) {
+GTEST_LOG_(WARNING) << "Skipping test due to missing IPv4 support.";
+return;
+  }
+  CreateTCPConnectedSockets("127.0.0.1", &socket_a_up, &socket_b_up);
+
+  llvm::StringRef scheme;
+  llvm::StringRef hostname;
+  int port;
+  llvm::StringRef path;
+  std::string uri(socket_a_up->GetRemoteConnectionURI());
+  EXPECT_TRUE(UriParser::Parse(uri, scheme, hostname, port, path));
+  EXPECT_EQ(scheme, "connect");
+  EXPECT_EQ(port, socket_a_up->GetRemotePortNumber());
+}
+
+TEST_F(SocketTest, UDPGetConnectURI) {
+  if (!IsAddressFamilySupported("127.0.0.1")) {
+GTEST_LOG_(WARNING) << "Skipping test due to missing IPv4 support.";
+return;
+  }
+  Socket *socket;
+  bool child_processes_inherit = false;
+  auto error =
+  UDPSocket::Connect("127.0.0.1:0", child_processes_inherit, socket);
+
+  llvm::StringRef scheme;
+  llvm::StringRef hostname;
+  int port;
+  llvm::StringRef path;
+  std::string uri(socket->GetRemoteConnectionURI());
+  EXPECT_TRUE(UriParser::Parse(uri, scheme, hostname, port, path));
+  EXPECT_EQ(scheme, "udp");
+}
+
+#ifndef LLDB_DISABLE_POSIX
+TEST_F(SocketTest, DomainGetConnectURI) {
+  llvm::SmallString<64> domain_path;
+  std::error_code EC =
+  llvm::sys::fs::createUniqueDirectory("DomainListenConnectAccept", domain_path);
+  ASSERT_FALSE(EC);
+  llvm::sys::path::append(domain_path, "test");
+
+  std::unique_ptr socket_a_up;
+  std::unique_ptr socket_b_up;
+  CreateDomainConnectedSockets(domain_path, &socket_a_up, &socket_b_up);
+
+  llvm::StringRef scheme;
+  llvm::StringRef hostname;
+  int port;
+  llvm::StringRef path;
+  std::string uri(socket_a_up->GetRemoteConnectionURI());
+  EXPECT_TRUE(UriParser::Parse(uri, scheme, hostname, port, path));
+  EXPECT_EQ(scheme, "unix-connect");
+  EXPECT_EQ(path, domain_path);
+}
+#endif
\ No newline at end of file
Index: lldb/trunk/source/Host/common/TCPSocket.cpp
===
--- lldb/trunk/source/Host/common/TCPSocket.cpp
+++ lldb/trunk/source/Host/common/TCPSocket.cpp
@@ -118,6 +118,14 @@
   return "";
 }
 
+std::string TCPSocket::GetRemoteConnectionURI() const {
+  if (m_socket != kInvalidSocketValue) {
+return llvm::formatv("connect://[{0}]:{1}", GetRemoteIPAddress(),
+ GetRemotePortNumber());
+  }
+  return "";
+};
+
 Status TCPSocket::CreateSocket(int domain) {
   Status error;
   if (IsValid())
Index: lldb/trunk/source/Host/common/UDPSocket.cpp
===
--- lldb/trunk/source/Host/common/UDPSocket.cpp
+++ lldb/trunk/source/Host/common/UDPSocket.cpp
@@ -134,3 +134,11 @@
   error.Clear();
   return error;
 }
+
+std::string UDPSocket::GetRemoteConnectionURI() const {
+  if (m_socket != kInvalidSocketValue) {
+return llvm::formatv("udp://[{0}]:{1}", m_sockaddr.GetIPAddress(),
+ m_sockaddr.GetPort());
+  }
+  return "";
+}
Index: lldb/trunk/source/Host/posix/ConnectionFileDescriptorPosix.cpp
===
--- lldb/trunk/source/Host/posix/ConnectionFileDescriptorPosix.cpp
+++ lldb/trunk/source/Host/posix/ConnectionFileDescriptorPosix.cpp
@@ -758,13 +758,7 @@
 }
 
 void ConnectionFileDescriptor::InitializeSocket(Socket *socket

[Lldb-commits] [PATCH] D62702: [ABI] Fix SystemV ABI to handle nested aggregate type returned in register

[Saleem Abdulrasool via Phabricator via lldb-commits]

compnerd added inline comments.



Comment at: lldb/source/Plugins/ABI/SysV-x86_64/ABISysV_x86_64.cpp:1563
 
+// The compiler will faltten the nested aggregate type into single
+// layer and push the value to stack

NIT: `faltten` -> `flatten`



Comment at: lldb/source/Plugins/ABI/SysV-x86_64/ABISysV_x86_64.cpp:1568
+// return false if the aggregate is in memory
+static bool FlattenAggregateType(
+ExecutionContext &exe_ctx,

I really wish that we could just use `CGFunctionInfo::getReturnInfo().getKind()`



Comment at: lldb/source/Plugins/ABI/SysV-x86_64/ABISysV_x86_64.cpp:1678
 
-  const uint32_t num_children = return_compiler_type.GetNumFields();
+  // const uint32_t num_children = return_compiler_type.GetNumFields();
+  const uint32_t num_children = aggregate_compiler_types.size();

Hmm, dead code?



Comment at: lldb/source/Plugins/ABI/SysV-x86_64/ABISysV_x86_64.cpp:1810
   return return_valobj_sp;
+}
 if (copy_from_offset + field_byte_width >

The braces are extraneous



Comment at: lldb/source/Plugins/ABI/SysV-x86_64/ABISysV_x86_64.cpp:1811
+}
 if (copy_from_offset + field_byte_width >
+copy_from_extractor->GetByteSize()) {

The braces are extraneous


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D62702



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

[Lldb-commits] [PATCH] D62702: [ABI] Fix SystemV ABI to handle nested aggregate type returned in register

[Wanyi Ye via Phabricator via lldb-commits]

kusmour marked an inline comment as done.
kusmour added inline comments.



Comment at: lldb/source/Plugins/ABI/SysV-x86_64/ABISysV_x86_64.cpp:1568
+// return false if the aggregate is in memory
+static bool FlattenAggregateType(
+ExecutionContext &exe_ctx,

compnerd wrote:
> I really wish that we could just use 
> `CGFunctionInfo::getReturnInfo().getKind()`
that's clang. I can try simplify it using ·GetFieldAtIndex·. this just incase 
we need to check `child_is_base_class`


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D62702



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

[Lldb-commits] [lldb] r362177 - Make CPlusPlusNameParser robust against nullptr StringRefs.

[Adrian Prantl via lldb-commits]

Author: adrian
Date: Thu May 30 17:18:42 2019
New Revision: 362177

URL: http://llvm.org/viewvc/llvm-project?rev=362177&view=rev
Log:
Make CPlusPlusNameParser robust against nullptr StringRefs.

There is likely also an underlying bug in all code that calls
CPlusPlusNameParser with nullptrs, but this patch can also stand for
itself.

rdar://problem/49072829

Modified:
lldb/trunk/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
lldb/trunk/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp

Modified: lldb/trunk/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp?rev=362177&r1=362176&r2=362177&view=diff
==
--- lldb/trunk/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp 
(original)
+++ lldb/trunk/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp Thu 
May 30 17:18:42 2019
@@ -640,6 +640,8 @@ static const llvm::StringMaphttp://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp?rev=362177&r1=362176&r2=362177&view=diff
==
--- lldb/trunk/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp (original)
+++ lldb/trunk/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp Thu May 
30 17:18:42 2019
@@ -6,6 +6,7 @@
 //
 
//===--===//
 #include "Plugins/Language/CPlusPlus/CPlusPlusLanguage.h"
+#include "Plugins/Language/CPlusPlus/CPlusPlusNameParser.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
@@ -192,3 +193,8 @@ TEST(CPlusPlusLanguage, FindAlternateFun
   EXPECT_THAT(FindAlternate("_ZN1A1fEai"), Contains("_ZN1A1fEci"));
   EXPECT_THAT(FindAlternate("_bogus"), IsEmpty());
 }
+
+TEST(CPlusPlusLanguage, CPlusPlusNameParser) {
+  // Don't crash.
+  CPlusPlusNameParser((const char *)nullptr);
+}


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

[Lldb-commits] [PATCH] D62702: [ABI] Fix SystemV ABI to handle nested aggregate type returned in register

[Wanyi Ye via Phabricator via lldb-commits]

kusmour updated this revision to Diff 202340.
kusmour added a comment.

simplify the method in 'FlattenAggregateType'
added test for nested struct returned in registers


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D62702

Files:
  
lldb/packages/Python/lldbsuite/test/functionalities/return-value/TestReturnValue.py
  lldb/packages/Python/lldbsuite/test/functionalities/return-value/call-func.c
  lldb/source/Plugins/ABI/SysV-x86_64/ABISysV_x86_64.cpp

Index: lldb/source/Plugins/ABI/SysV-x86_64/ABISysV_x86_64.cpp
===
--- lldb/source/Plugins/ABI/SysV-x86_64/ABISysV_x86_64.cpp
+++ lldb/source/Plugins/ABI/SysV-x86_64/ABISysV_x86_64.cpp
@@ -30,6 +30,8 @@
 #include "lldb/Utility/RegisterValue.h"
 #include "lldb/Utility/Status.h"
 
+#include 
+
 using namespace lldb;
 using namespace lldb_private;
 
@@ -1558,6 +1560,47 @@
   return return_valobj_sp;
 }
 
+// The compiler will flatten the nested aggregate type into single
+// layer and push the value to stack
+// This helper function will flatten an aggregate type
+// and return true if it can be returned in register(s) by value
+// return false if the aggregate is in memory
+static bool FlattenAggregateType(
+ExecutionContext &exe_ctx,
+CompilerType &return_compiler_type,
+uint32_t data_byte_offset,
+std::vector &aggregate_field_offsets,
+std::vector &aggregate_compiler_types) {
+  const uint32_t num_children = return_compiler_type.GetNumFields();
+  for (uint32_t idx = 0; idx < num_children; ++idx) {
+std::string name;
+bool is_signed;
+uint32_t count;
+bool is_complex;
+
+uint64_t field_bit_offset = 0;
+CompilerType field_compiler_type = return_compiler_type.GetFieldAtIndex(
+idx, name, &field_bit_offset, nullptr, nullptr);
+
+uint32_t field_byte_offset = field_bit_offset / 8 + data_byte_offset;
+
+const uint32_t field_type_flags = field_compiler_type.GetTypeInfo();
+if (field_compiler_type.IsIntegerOrEnumerationType(is_signed) ||
+field_compiler_type.IsPointerType() ||
+field_compiler_type.IsFloatingPointType(count, is_complex)) {
+  aggregate_field_offsets.push_back(field_byte_offset);
+  aggregate_compiler_types.push_back(field_compiler_type);
+} else if (field_type_flags & eTypeHasChildren) {
+  if (!FlattenAggregateType(exe_ctx, field_compiler_type,
+field_byte_offset, aggregate_field_offsets,
+aggregate_compiler_types)) {
+return false;
+  }
+}
+  }
+  return true;
+}
+
 ValueObjectSP ABISysV_x86_64::GetReturnValueObjectImpl(
 Thread &thread, CompilerType &return_compiler_type) const {
   ValueObjectSP return_valobj_sp;
@@ -1580,10 +1623,14 @@
   if (return_compiler_type.IsAggregateType()) {
 Target *target = exe_ctx.GetTargetPtr();
 bool is_memory = true;
-if (*bit_width <= 128) {
-  ByteOrder target_byte_order = target->GetArchitecture().GetByteOrder();
+std::vector aggregate_field_offsets;
+std::vector aggregate_compiler_types;
+if (*bit_width <= 128 && FlattenAggregateType(exe_ctx, return_compiler_type,
+  0, aggregate_field_offsets,
+  aggregate_compiler_types)) {
+  ByteOrder byte_order = target->GetArchitecture().GetByteOrder();
   DataBufferSP data_sp(new DataBufferHeap(16, 0));
-  DataExtractor return_ext(data_sp, target_byte_order,
+  DataExtractor return_ext(data_sp, byte_order,
target->GetArchitecture().GetAddressByteSize());
 
   const RegisterInfo *rax_info =
@@ -1613,40 +1660,26 @@
   uint32_t integer_bytes =
   0; // Tracks how much of the rax/rds registers we've consumed so far
 
-  const uint32_t num_children = return_compiler_type.GetNumFields();
+  const uint32_t num_children = aggregate_compiler_types.size();
 
   // Since we are in the small struct regime, assume we are not in memory.
   is_memory = false;
-
   for (uint32_t idx = 0; idx < num_children; idx++) {
-std::string name;
-uint64_t field_bit_offset = 0;
 bool is_signed;
-bool is_complex;
 uint32_t count;
+bool is_complex;
 
-CompilerType field_compiler_type = return_compiler_type.GetFieldAtIndex(
-idx, name, &field_bit_offset, nullptr, nullptr);
-llvm::Optional field_bit_width =
-field_compiler_type.GetBitSize(&thread);
-
-// if we don't know the size of the field (e.g. invalid type), just
-// bail out
-if (!field_bit_width || *field_bit_width == 0)
-  break;
+CompilerType field_compiler_type = aggregate_compiler_types[idx];
+uint32_t field_byte_width = (uint32_t) (*field_compiler_type.GetByteSize(&thread));
+uin

[Lldb-commits] [PATCH] D62702: [ABI] Fix SystemV ABI to handle nested aggregate type returned in register

[Saleem Abdulrasool via Phabricator via lldb-commits]

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

Thanks, this looks much better.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D62702



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

[Lldb-commits] [PATCH] D62714: [FormatEntity] Ignore ASCII escape sequences when colors are disabled.

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added reviewers: labath, jingham, xiaobai, teemperor.
Herald added a project: LLDB.

This patch makes the FormatEntity honor the debugger's color settings by not 
inserting ASCII escape sequences when colors are disabled.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D62714

Files:
  lldb/include/lldb/Core/FormatEntity.h
  lldb/lit/Settings/Inputs/main.c
  lldb/lit/Settings/TestFrameFormatColor.test
  lldb/lit/Settings/TestFrameFormatNoColor.test
  lldb/source/Core/FormatEntity.cpp

Index: lldb/source/Core/FormatEntity.cpp
===
--- lldb/source/Core/FormatEntity.cpp
+++ lldb/source/Core/FormatEntity.cpp
@@ -94,7 +94,7 @@
 static_cast(llvm::array_lengthof(c)), c, true\
   }
 #define ENTRY_STRING(n, s) \
-  { n, s, FormatEntity::Entry::Type::InsertString, 0, 0, nullptr, false }
+  { n, s, FormatEntity::Entry::Type::EscapeCode, 0, 0, nullptr, false }
 static FormatEntity::Entry::Definition g_string_entry[] = {
 ENTRY("*", ParentString)};
 
@@ -307,7 +307,7 @@
 ENUM_TO_CSTR(Invalid);
 ENUM_TO_CSTR(ParentNumber);
 ENUM_TO_CSTR(ParentString);
-ENUM_TO_CSTR(InsertString);
+ENUM_TO_CSTR(EscapeCode);
 ENUM_TO_CSTR(Root);
 ENUM_TO_CSTR(String);
 ENUM_TO_CSTR(Scope);
@@ -1102,8 +1102,17 @@
   // FormatEntity::Entry::Definition encoding
   case Entry::Type::ParentString: // Only used for
   // FormatEntity::Entry::Definition encoding
-  case Entry::Type::InsertString: // Only used for
-  // FormatEntity::Entry::Definition encoding
+return false;
+  case Entry::Type::EscapeCode:
+if (exe_ctx) {
+  if (Target *target = exe_ctx->GetTargetPtr()) {
+Debugger &debugger = target->GetDebugger();
+if (debugger.GetUseColor()) {
+  s.PutCString(entry.string);
+  return true;
+}
+  }
+}
 return false;
 
   case Entry::Type::Root:
@@ -1896,7 +1905,7 @@
 entry.number = entry_def->data;
 return error; // Success
 
-  case FormatEntity::Entry::Type::InsertString:
+  case FormatEntity::Entry::Type::EscapeCode:
 entry.type = entry_def->type;
 entry.string = entry_def->string;
 return error; // Success
@@ -2265,13 +2274,7 @@
   return error;
 }
   }
-  // Check if this entry just wants to insert a constant string value
-  // into the parent_entry, if so, insert the string with AppendText,
-  // else append the entry to the parent_entry.
-  if (entry.type == Entry::Type::InsertString)
-parent_entry.AppendText(entry.string.c_str());
-  else
-parent_entry.AppendEntry(std::move(entry));
+  parent_entry.AppendEntry(std::move(entry));
 }
   }
   break;
Index: lldb/lit/Settings/TestFrameFormatNoColor.test
===
--- /dev/null
+++ lldb/lit/Settings/TestFrameFormatNoColor.test
@@ -0,0 +1,12 @@
+# RUN: %clang -g -O0 %S/Inputs/main.c -o %t.out
+# RUN: %lldb -x -b -s %s %t.out | FileCheck %s
+settings set use-color false
+settings set -f frame-format "frame #${frame.index}: \`${ansi.fg.green}{${function.name-with-args}${ansi.normal}\n"
+b foo
+run
+bt
+c
+q
+
+# Check the ASCII escape code
+# CHECK-NOT: 
Index: lldb/lit/Settings/TestFrameFormatColor.test
===
--- /dev/null
+++ lldb/lit/Settings/TestFrameFormatColor.test
@@ -0,0 +1,12 @@
+# RUN: %clang -g -O0 %S/Inputs/main.c -o %t.out
+# RUN: %lldb -x -b -s %s %t.out | FileCheck %s
+settings set use-color true
+settings set -f frame-format "frame #${frame.index}: \`${ansi.fg.green}{${function.name-with-args}${ansi.normal}\n"
+b foo
+run
+bt
+c
+q
+
+# Check the ASCII escape code
+# CHECK: 
Index: lldb/lit/Settings/Inputs/main.c
===
--- /dev/null
+++ lldb/lit/Settings/Inputs/main.c
@@ -0,0 +1,2 @@
+int foo() { return 0; }
+int main() { return foo(); }
Index: lldb/include/lldb/Core/FormatEntity.h
===
--- lldb/include/lldb/Core/FormatEntity.h
+++ lldb/include/lldb/Core/FormatEntity.h
@@ -41,7 +41,7 @@
   Invalid,
   ParentNumber,
   ParentString,
-  InsertString,
+  EscapeCode,
   Root,
   String,
   Scope,
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D62714: [FormatEntity] Ignore ASCII escape sequences when colors are disabled.

[Alex Langford via Phabricator via lldb-commits]

xiaobai added a comment.

Seems good to me.




Comment at: lldb/lit/Settings/TestFrameFormatColor.test:12
+# Check the ASCII escape code
+# CHECK: 

I don't see any escape code here, I assume phabricator is just not showing 
them? 😛 


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D62714



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

[Lldb-commits] [PATCH] D62715: [NativeProcessLinux] Reuse memory read by process_vm_readv before calling ptrace

[António Afonso via Phabricator via lldb-commits]

aadsm created this revision.
aadsm added reviewers: clayborg, xiaobai, labath.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

I'm putting this up as discussed per D62503 .
Today when process_vm_readv fails to read the entire range we fallback to 
ptrace and try to read the same range.
However, process_vm_readv does partial reads so there's no need to read again 
what has already been read by process_vm_readv.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D62715

Files:
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp


Index: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
===
--- lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
+++ lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
@@ -1441,6 +1441,7 @@
 
 Status NativeProcessLinux::ReadMemory(lldb::addr_t addr, void *buf, size_t 
size,
   size_t &bytes_read) {
+  bytes_read = 0;
   if (ProcessVmReadvSupported()) {
 // The process_vm_readv path is about 50 times faster than ptrace api. We
 // want to use this syscall if it is supported.
@@ -1453,8 +1454,21 @@
 remote_iov.iov_base = reinterpret_cast(addr);
 remote_iov.iov_len = size;
 
-bytes_read = process_vm_readv(pid, &local_iov, 1, &remote_iov, 1, 0);
-const bool success = bytes_read == size;
+auto vm_bytes_read =
+process_vm_readv(pid, &local_iov, 1, &remote_iov, 1, 0);
+const bool success = vm_bytes_read == size;
+if (vm_bytes_read > 0) {
+  bytes_read = vm_bytes_read;
+  // When process_vm_readv is not able to read all the data we fallback to
+  // ptrace. However, if process_vm_readv didn't fail (vm_bytes_read != -1)
+  // and was able to some bytes avoid re-reading them with ptrace by
+  // updating the addr and size to point to the rest of the data.
+  if (!success) {
+addr = reinterpret_cast(reinterpret_cast(addr) +
+vm_bytes_read);
+size -= vm_bytes_read;
+  }
+}
 
 Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_PROCESS));
 LLDB_LOG(log,
@@ -1468,14 +1482,14 @@
 // api.
   }
 
-  unsigned char *dst = static_cast(buf);
+  unsigned char *dst = static_cast(buf) + bytes_read;
   size_t remainder;
   long data;
 
   Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_MEMORY));
   LLDB_LOG(log, "addr = {0}, buf = {1}, size = {2}", addr, buf, size);
 
-  for (bytes_read = 0; bytes_read < size; bytes_read += remainder) {
+  for (; bytes_read < size; bytes_read += remainder) {
 Status error = NativeProcessLinux::PtraceWrapper(
 PTRACE_PEEKDATA, GetID(), (void *)addr, nullptr, 0, &data);
 if (error.Fail())


Index: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
===
--- lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
+++ lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
@@ -1441,6 +1441,7 @@
 
 Status NativeProcessLinux::ReadMemory(lldb::addr_t addr, void *buf, size_t size,
   size_t &bytes_read) {
+  bytes_read = 0;
   if (ProcessVmReadvSupported()) {
 // The process_vm_readv path is about 50 times faster than ptrace api. We
 // want to use this syscall if it is supported.
@@ -1453,8 +1454,21 @@
 remote_iov.iov_base = reinterpret_cast(addr);
 remote_iov.iov_len = size;
 
-bytes_read = process_vm_readv(pid, &local_iov, 1, &remote_iov, 1, 0);
-const bool success = bytes_read == size;
+auto vm_bytes_read =
+process_vm_readv(pid, &local_iov, 1, &remote_iov, 1, 0);
+const bool success = vm_bytes_read == size;
+if (vm_bytes_read > 0) {
+  bytes_read = vm_bytes_read;
+  // When process_vm_readv is not able to read all the data we fallback to
+  // ptrace. However, if process_vm_readv didn't fail (vm_bytes_read != -1)
+  // and was able to some bytes avoid re-reading them with ptrace by
+  // updating the addr and size to point to the rest of the data.
+  if (!success) {
+addr = reinterpret_cast(reinterpret_cast(addr) +
+vm_bytes_read);
+size -= vm_bytes_read;
+  }
+}
 
 Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_PROCESS));
 LLDB_LOG(log,
@@ -1468,14 +1482,14 @@
 // api.
   }
 
-  unsigned char *dst = static_cast(buf);
+  unsigned char *dst = static_cast(buf) + bytes_read;
   size_t remainder;
   long data;
 
   Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_MEMORY));
   LLDB_LOG(log, "addr = {0}, buf = {1}, size = {2}", addr, buf, size);
 
-  for (bytes_read = 0; bytes_read < size; bytes_read += remainder) {
+  for (; bytes_read < size; bytes_read += remainder) {
 Status error = NativeProcessLinux::PtraceWrapper(
 PT

[Lldb-commits] [PATCH] D62714: [FormatEntity] Ignore ASCII escape sequences when colors are disabled.

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere marked an inline comment as done.
JDevlieghere added inline comments.



Comment at: lldb/lit/Settings/TestFrameFormatColor.test:12
+# Check the ASCII escape code
+# CHECK: 

xiaobai wrote:
> I don't see any escape code here, I assume phabricator is just not showing 
> them? 😛 
Yup!


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D62714



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