date:20190910

[Lldb-commits] [lldb] r371491 - [LLDB] FreeBSD fix new SetFile call.

2019-09-10 Thread David Carlier via lldb-commits

Author: devnexen
Date: Tue Sep 10 00:33:39 2019
New Revision: 371491

URL: http://llvm.org/viewvc/llvm-project?rev=371491&view=rev
Log:
[LLDB] FreeBSD fix new SetFile call.

Modified:
lldb/trunk/source/Host/freebsd/HostInfoFreeBSD.cpp

Modified: lldb/trunk/source/Host/freebsd/HostInfoFreeBSD.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/freebsd/HostInfoFreeBSD.cpp?rev=371491&r1=371490&r2=371491&view=diff
==
--- lldb/trunk/source/Host/freebsd/HostInfoFreeBSD.cpp (original)
+++ lldb/trunk/source/Host/freebsd/HostInfoFreeBSD.cpp Tue Sep 10 00:33:39 2019
@@ -67,7 +67,7 @@ FileSpec HostInfoFreeBSD::GetProgramFile
 char exe_path[PATH_MAX];
 size_t exe_path_size = sizeof(exe_path);
 if (sysctl(exe_path_mib, 4, exe_path, &exe_path_size, NULL, 0) == 0)
-  g_program_filespec.SetFile(exe_path, false);
+  g_program_filespec.SetFile(exe_path, FileSpec::Style::native);
   }
   return g_program_filespec;
 }


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

[Lldb-commits] [PATCH] D67390: [LLDB][ELF] Load both, .symtab and .dynsym sections

2019-09-10 Thread Konrad Wilhelm Kleine via Phabricator via lldb-commits

kwk created this revision.
kwk added a reviewer: labath.
Herald added subscribers: lldb-commits, MaskRay, arichardson, emaste.
Herald added a reviewer: espindola.
Herald added a reviewer: alexshap.
Herald added a project: LLDB.

This change ensures that the .dynsym section will be parsed even when
there's already is a .symtab section.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D67390

Files:
  lldb/lit/Modules/ELF/Inputs/load-from-dynsym-alone.c
  lldb/lit/Modules/ELF/Inputs/load-symtab-and-dynsym.c
  lldb/lit/Modules/ELF/load-from-dynsym-alone.test
  lldb/lit/Modules/ELF/load-symtab-and-dynsym.test
  lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp

Index: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===
--- lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -2649,19 +2649,25 @@
 // while the reverse is not necessarily true.
 Section *symtab =
 section_list->FindSectionByType(eSectionTypeELFSymbolTable, true).get();
-if (!symtab) {
-  // The symtab section is non-allocable and can be stripped, so if it
-  // doesn't exist then use the dynsym section which should always be
-  // there.
-  symtab =
-  section_list->FindSectionByType(eSectionTypeELFDynamicSymbols, true)
-  .get();
-}
 if (symtab) {
   m_symtab_up.reset(new Symtab(symtab->GetObjectFile()));
   symbol_id += ParseSymbolTable(m_symtab_up.get(), symbol_id, symtab);
 }
 
+// The symtab section is non-allocable and can be stripped. If both, .symtab
+// and .dynsym exist, we load both. And if only .dymsym exists, we load it
+// alone.
+Section *dynsym =
+section_list->FindSectionByType(eSectionTypeELFDynamicSymbols, true)
+.get();
+if (dynsym) {
+  if (!m_symtab_up) {
+auto sec = symtab ? symtab : dynsym;
+m_symtab_up.reset(new Symtab(sec->GetObjectFile()));
+  }
+  symbol_id += ParseSymbolTable(m_symtab_up.get(), symbol_id, dynsym);
+}
+
 // DT_JMPREL
 //  If present, this entry's d_ptr member holds the address of
 //  relocation
Index: lldb/lit/Modules/ELF/load-symtab-and-dynsym.test
===
--- /dev/null
+++ lldb/lit/Modules/ELF/load-symtab-and-dynsym.test
@@ -0,0 +1,48 @@
+# REQUIRES: system-linux
+
+# This test ensures that we will load .dynsym even if there's a .symtab section.
+# We do this by compiling a small C program with two functions and we direct the
+# linker where to put the symbols so that in the end the layout is as follows:
+#
+#   Symbol table '.dynsym' contains 4 entries:
+#   Num:Value  Size TypeBind   Vis  Ndx Name
+# 0:  0 NOTYPE  LOCAL  DEFAULT  UND 
+# 1:  0 FUNCGLOBAL DEFAULT  UND __libc_start_main@GLIBC_2.2.5
+# 2:  0 NOTYPE  WEAK   DEFAULT  UND __gmon_start__
+# 3: 0040112013 FUNCGLOBAL DEFAULT   10 functionInDynsym
+#   
+#   Symbol table '.symtab' contains 2 entries:
+#   Num:Value  Size TypeBind   Vis  Ndx Name
+# 0:  0 NOTYPE  LOCAL  DEFAULT  UND 
+# 1: 0040111015 FUNCGLOBAL DEFAULT   10 functionInSymtab
+
+# We want to keep the symbol "functionInDynsym" in the .dynamic section and not
+# have it put the default .symtab section.
+# RUN: echo "{functionInDynsym;};" > %T/dynmic-symbols.txt
+# RUN: %clang -Wl,--dynamic-list=%T/dynmic-symbols.txt -g -o %t.binary %p/Inputs/load-symtab-and-dynsym.c
+
+# Remove not needed symbols
+# RUN: echo "functionInSymtab" > %t.keep_symbols
+# RUN: echo "functionInDynsym" >> %t.keep_symbols
+# RUN: llvm-objcopy --strip-all --remove-section .gdb_index --remove-section .comment --keep-symbols=%t.keep_symbols %t.binary
+
+# Remove functionInDynsym symbol from .symtab (will leave symbol in .dynsym intact)
+# RUN: llvm-strip --strip-symbol=functionInDynsym %t.binary
+
+# RUN: %lldb -b -o 'b functionInSymtab' -o 'b functionInDynsym' -o 'run' -o 'continue' %t.binary | FileCheck %s
+
+# CHECK: (lldb) b functionInSymtab
+# CHECK-NEXT: Breakpoint 1: where = {{.*}}.binary`functionInSymtab, address = 0x{{.*}}
+
+# CHECK: (lldb) b functionInDynsym
+# CHECK-NEXT: Breakpoint 2: where = {{.*}}.binary`functionInDynsym, address = 0x{{.*}}
+
+# CHECK: (lldb) run
+# CHECK-NEXT: Process {{.*}} stopped
+# CHECK-NEXT: * thread #1, name = 'load-symtab-and', stop reason = breakpoint 1.1
+
+# CHECK: (lldb) continue
+# CHECK-NEXT: Process {{.*}} resuming
+# CHECK-NEXT: Process {{.*}} stopped
+# CHECK-NEXT: * thread #1, name = 'load-symtab-and', stop reason = breakpoint 2.1
+
Index: lldb/lit/Modules/ELF/load-from-dynsym-alone.test
===
--- /dev/null
+++ lldb/lit/Modules/ELF/load-from-dynsym-al

[Lldb-commits] [PATCH] D67347: [Windows] Use information from the PE32 exceptions directory to construct unwind plans

2019-09-10 Thread Aleksandr Urakov via Phabricator via lldb-commits

aleksandr.urakov added a comment.

It's a good point, thank you! I had the same thoughts when I done it, but I'm 
not completely sure. The problem is that an object file can't be completely 
responsible for choosing an unwind plan, because some plans are produced by 
symbol files (and an object file knows nothing about that, if I understand 
right). So we can move only a part of the plan-choosing-functionality from 
`FuncUnwinders` to `ObjectFile`s, but will it be better? In that case both 
`FuncUnwinders` and `ObjectFile`s will be responsible for managing plans, won't 
it add an additional complexity to the solution?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D67347



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

[Lldb-commits] [PATCH] D67376: [DWARF] Evaluate DW_OP_entry_value

2019-09-10 Thread Adrian Prantl via Phabricator via lldb-commits

aprantl added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:3752
+LocationInCaller = parse_simple_location(i);
+break;
+  }

vsk wrote:
> aprantl wrote:
> > default?
> I don't believe any action is needed in the default case: we do not want to 
> log or report an error.
Doesn't this trigger the switch-doesn't-cover-all-cases warning otherwise?


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

https://reviews.llvm.org/D67376



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

[Lldb-commits] [PATCH] D67369: Implement DW_OP_convert

2019-09-10 Thread Adrian Prantl via Phabricator via lldb-commits

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



Comment at: lldb/include/lldb/Utility/Scalar.h:107
+  /// Return the most efficient Scalar::Type for the requested size.
+  static Type GetBestType(size_t bit_size, bool sign);
+

vsk wrote:
> JDevlieghere wrote:
> > How about `GetTypeForBitSize`? 
> nit: bool signed ?
that name conflicts with the actual type `signed`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67369



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

[Lldb-commits] [PATCH] D67369: Implement DW_OP_convert

2019-09-10 Thread Adrian Prantl via Phabricator via lldb-commits

aprantl marked 5 inline comments as done.
aprantl added inline comments.



Comment at: lldb/source/Expression/DWARFExpression.cpp:2571
+  if (stack.size() < 1) {
+if (error_ptr)
+  error_ptr->SetErrorString(

vsk wrote:
> JDevlieghere wrote:
> > aprantl wrote:
> > > JDevlieghere wrote:
> > > > Can we wrap this in a lambda? 
> > > Which part specifically do you mean?
> > ```auto SetErrorString = [&](llvm::StringRef msg) { if (error_ptr) 
> > error_ptr->SetErrorString(msg); })```
> > 
> > We could deduplicate even more code by using a macro that also covers the 
> > check around it and does the return, but I'm not sure if that's really 
> > worth it. 
> Oh, like LLDB_LOG? Seems like a nice idea. I think I need this for D67376, so 
> I might do it if no one else has started already. It'd be great to use that 
> here.
I'll leave this as an NFC refactoring for later since it will affect code 
outside of this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67369



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

[Lldb-commits] [lldb] r371532 - Implement DW_OP_convert

2019-09-10 Thread Adrian Prantl via lldb-commits

Author: adrian
Date: Tue Sep 10 09:17:38 2019
New Revision: 371532

URL: http://llvm.org/viewvc/llvm-project?rev=371532&view=rev
Log:
Implement DW_OP_convert

This patch adds basic support for DW_OP_convert[1] for integer
types. Recent versions of LLVM's optimizer may insert this opcode into
DWARF expressions. DW_OP_convert is effectively a type cast operation
that takes a reference to a base type DIE (or zero) and then casts the
value at the top of the DWARF stack to that type. Internally this
works by changing the bit size of the APInt that is used as backing
storage for LLDB's DWARF stack.

I managed to write a unit test for this by implementing a mock YAML
object file / module that takes debug info sections in yaml2obj
format.

[1] Typed DWARF stack. http://www.dwarfstd.org/ShowIssue.php?issue=140425.1



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

Modified:
lldb/trunk/include/lldb/Core/Section.h
lldb/trunk/include/lldb/Utility/Scalar.h
lldb/trunk/source/Core/Section.cpp
lldb/trunk/source/Expression/DWARFExpression.cpp
lldb/trunk/source/Utility/Scalar.cpp
lldb/trunk/unittests/Expression/DWARFExpressionTest.cpp
lldb/trunk/unittests/Utility/ScalarTest.cpp

Modified: lldb/trunk/include/lldb/Core/Section.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/Section.h?rev=371532&r1=371531&r2=371532&view=diff
==
--- lldb/trunk/include/lldb/Core/Section.h (original)
+++ lldb/trunk/include/lldb/Core/Section.h Tue Sep 10 09:17:38 2019
@@ -43,9 +43,8 @@ public:
   const_iterator begin() { return m_sections.begin(); }
   const_iterator end() { return m_sections.end(); }
 
-  SectionList();
-
-  ~SectionList();
+  /// Create an empty list.
+  SectionList() = default;
 
   SectionList &operator=(const SectionList &rhs);
 

Modified: lldb/trunk/include/lldb/Utility/Scalar.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Utility/Scalar.h?rev=371532&r1=371531&r2=371532&view=diff
==
--- lldb/trunk/include/lldb/Utility/Scalar.h (original)
+++ lldb/trunk/include/lldb/Utility/Scalar.h Tue Sep 10 09:17:38 2019
@@ -38,6 +38,7 @@ namespace lldb_private {
 // follows the ANSI C type promotion rules.
 class Scalar {
 public:
+  // FIXME: These are host types which seems to be an odd choice.
   enum Type {
 e_void = 0,
 e_sint,
@@ -98,30 +99,14 @@ public:
   }
   Scalar(llvm::APInt v) : m_type(), m_float(static_cast(0)) {
 m_integer = llvm::APInt(v);
-switch (m_integer.getBitWidth()) {
-case 8:
-case 16:
-case 32:
-  m_type = e_sint;
-  return;
-case 64:
-  m_type = e_slonglong;
-  return;
-case 128:
-  m_type = e_sint128;
-  return;
-case 256:
-  m_type = e_sint256;
-  return;
-case 512:
-  m_type = e_sint512;
-  return;
-}
-lldbassert(false && "unsupported bitwidth");
+m_type = GetBestTypeForBitSize(m_integer.getBitWidth(), true);
   }
   // Scalar(const RegisterValue& reg_value);
   virtual ~Scalar();
 
+  /// Return the most efficient Scalar::Type for the requested bit size.
+  static Type GetBestTypeForBitSize(size_t bit_size, bool sign);
+
   bool SignExtend(uint32_t bit_pos);
 
   bool ExtractBitfield(uint32_t bit_size, uint32_t bit_offset);
@@ -154,6 +139,9 @@ public:
 return (m_type >= e_sint) && (m_type <= e_long_double);
   }
 
+  /// Convert integer to \p type, limited to \p bits size.
+  void TruncOrExtendTo(Scalar::Type type, uint16_t bits);
+
   bool Promote(Scalar::Type type);
 
   bool MakeSigned();

Modified: lldb/trunk/source/Core/Section.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/Section.cpp?rev=371532&r1=371531&r2=371532&view=diff
==
--- lldb/trunk/source/Core/Section.cpp (original)
+++ lldb/trunk/source/Core/Section.cpp Tue Sep 10 09:17:38 2019
@@ -417,10 +417,6 @@ lldb::offset_t Section::GetSectionData(D
 
 #pragma mark SectionList
 
-SectionList::SectionList() : m_sections() {}
-
-SectionList::~SectionList() {}
-
 SectionList &SectionList::operator=(const SectionList &rhs) {
   if (this != &rhs)
 m_sections = rhs.m_sections;

Modified: lldb/trunk/source/Expression/DWARFExpression.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Expression/DWARFExpression.cpp?rev=371532&r1=371531&r2=371532&view=diff
==
--- lldb/trunk/source/Expression/DWARFExpression.cpp (original)
+++ lldb/trunk/source/Expression/DWARFExpression.cpp Tue Sep 10 09:17:38 2019
@@ -2559,6 +2559,83 @@ bool DWARFExpression::Evaluate(
   stack.back().SetValueType(Value::eValueTypeScalar);
   break;
 
+// OPCODE: DW_OP_convert
+// OPERANDS: 1
+//  A ULEB128 that is either a DIE offset of a
+//  DW

[Lldb-commits] [PATCH] D67369: Implement DW_OP_convert

2019-09-10 Thread Phabricator via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL371532: Implement DW_OP_convert (authored by adrian, 
committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D67369?vs=219422&id=219559#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D67369

Files:
  lldb/trunk/include/lldb/Core/Section.h
  lldb/trunk/include/lldb/Utility/Scalar.h
  lldb/trunk/source/Core/Section.cpp
  lldb/trunk/source/Expression/DWARFExpression.cpp
  lldb/trunk/source/Utility/Scalar.cpp
  lldb/trunk/unittests/Expression/DWARFExpressionTest.cpp
  lldb/trunk/unittests/Utility/ScalarTest.cpp

Index: lldb/trunk/unittests/Utility/ScalarTest.cpp
===
--- lldb/trunk/unittests/Utility/ScalarTest.cpp
+++ lldb/trunk/unittests/Utility/ScalarTest.cpp
@@ -272,7 +272,7 @@
   }
 
   Scalar B(APInt(64, 42));
-  EXPECT_EQ(B.GetType(), Scalar::e_slonglong);
+  EXPECT_EQ(B.GetType(), Scalar::GetBestTypeForBitSize(64, true));
   Scalar C(APInt(128, 96));
   EXPECT_EQ(C.GetType(), Scalar::e_sint128);
   Scalar D(APInt(256, 156));
Index: lldb/trunk/unittests/Expression/DWARFExpressionTest.cpp
===
--- lldb/trunk/unittests/Expression/DWARFExpressionTest.cpp
+++ lldb/trunk/unittests/Expression/DWARFExpressionTest.cpp
@@ -7,24 +7,108 @@
 //===--===//
 
 #include "lldb/Expression/DWARFExpression.h"
+#include "../../source/Plugins/SymbolFile/DWARF/DWARFUnit.h"
+#include "../../source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h"
+#include "lldb/Core/Module.h"
+#include "lldb/Core/Section.h"
 #include "lldb/Core/Value.h"
 #include "lldb/Core/dwarf.h"
+#include "lldb/Symbol/ObjectFile.h"
 #include "lldb/Utility/StreamString.h"
 #include "llvm/ADT/StringExtras.h"
+#include "llvm/ObjectYAML/DWARFEmitter.h"
 #include "llvm/Testing/Support/Error.h"
 #include "gtest/gtest.h"
 
 using namespace lldb_private;
 
-static llvm::Expected Evaluate(llvm::ArrayRef expr) {
+/// A mock module holding an object file parsed from YAML.
+class YAMLModule : public lldb_private::Module {
+public:
+  YAMLModule(ArchSpec &arch) : Module(FileSpec("test"), arch) {}
+  void SetObjectFile(lldb::ObjectFileSP obj_file) { m_objfile_sp = obj_file; }
+  ObjectFile *GetObjectFile() override { return m_objfile_sp.get(); }
+};
+
+/// A mock object file that can be parsed from YAML.
+class YAMLObjectFile : public lldb_private::ObjectFile {
+  const lldb::ModuleSP m_module_sp;
+  llvm::StringMap> &m_section_map;
+  /// Because there is only one DataExtractor in the ObjectFile
+  /// interface, all sections are copied into a contiguous buffer.
+  std::vector m_buffer;
+
+public:
+  YAMLObjectFile(const lldb::ModuleSP &module_sp,
+ llvm::StringMap> &map)
+  : ObjectFile(module_sp, &module_sp->GetFileSpec(), /*file_offset*/ 0,
+   /*length*/ 0, /*data_sp*/ nullptr, /*data_offset*/ 0),
+m_module_sp(module_sp), m_section_map(map) {}
+
+  /// Callback for initializing the module's list of sections.
+  void CreateSections(SectionList &unified_section_list) override {
+lldb::offset_t total_bytes = 0;
+for (auto &entry : m_section_map)
+  total_bytes += entry.getValue()->getBufferSize();
+m_buffer.reserve(total_bytes);
+m_data =
+DataExtractor(m_buffer.data(), total_bytes, lldb::eByteOrderLittle, 4);
+
+lldb::user_id_t sect_id = 1;
+for (auto &entry : m_section_map) {
+  llvm::StringRef name = entry.getKey();
+  lldb::SectionType sect_type =
+  llvm::StringSwitch(name)
+  .Case("debug_info", lldb::eSectionTypeDWARFDebugInfo)
+  .Case("debug_abbrev", lldb::eSectionTypeDWARFDebugAbbrev);
+  auto &membuf = entry.getValue();
+  lldb::addr_t file_vm_addr = 0;
+  lldb::addr_t vm_size = 0;
+  lldb::offset_t file_offset = m_buffer.size();
+  lldb::offset_t file_size = membuf->getBufferSize();
+  m_buffer.resize(file_offset + file_size);
+  memcpy(m_buffer.data() + file_offset, membuf->getBufferStart(),
+ file_size);
+  uint32_t log2align = 0;
+  uint32_t flags = 0;
+  auto section_sp = std::make_shared(
+  m_module_sp, this, sect_id++, ConstString(name), sect_type,
+  file_vm_addr, vm_size, file_offset, file_size, log2align, flags);
+  unified_section_list.AddSection(section_sp);
+}
+  }
+
+  /// \{
+  /// Stub methods that aren't needed here.
+  ConstString GetPluginName() override { return ConstString("YAMLObjectFile"); }
+  uint32_t GetPluginVersion() override { return 0; }
+  void Dump(Stream *s) override {}
+  uint32_t GetAddressByteSize() const override { return 8; }
+  uint32_t GetDependentModules(FileSpecList &file_list) override { retu

[Lldb-commits] [PATCH] D67376: [DWARF] Evaluate DW_OP_entry_value

2019-09-10 Thread Vedant Kumar via Phabricator via lldb-commits

vsk updated this revision to Diff 219573.
vsk marked an inline comment as done.
vsk edited the summary of this revision.
vsk added a comment.

- Addressed review feedback, split out unrelated changes, and improved test 
coverage.


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

https://reviews.llvm.org/D67376

Files:
  lldb/include/lldb/Symbol/Function.h
  lldb/packages/Python/lldbsuite/test/decorators.py
  
lldb/packages/Python/lldbsuite/test/functionalities/param_entry_vals/basic_entry_values_x86_64/Makefile
  
lldb/packages/Python/lldbsuite/test/functionalities/param_entry_vals/basic_entry_values_x86_64/TestBasicEntryValuesX86_64.py
  
lldb/packages/Python/lldbsuite/test/functionalities/param_entry_vals/basic_entry_values_x86_64/main.cpp
  lldb/source/Expression/DWARFExpression.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDefines.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Symbol/Function.cpp

Index: lldb/source/Symbol/Function.cpp
===
--- lldb/source/Symbol/Function.cpp
+++ lldb/source/Symbol/Function.cpp
@@ -127,11 +127,18 @@
 }
 
 //
-CallEdge::CallEdge(const char *symbol_name, lldb::addr_t return_pc)
-: return_pc(return_pc), resolved(false) {
+CallEdge::CallEdge(const char *symbol_name, lldb::addr_t return_pc,
+   CallSiteParameterArray parameters)
+: return_pc(return_pc), parameters(std::move(parameters)), resolved(false) {
   lazy_callee.symbol_name = symbol_name;
 }
 
+llvm::ArrayRef CallEdge::GetCallSiteParameters() const {
+  if (parameters)
+return *parameters.get();
+  return {};
+}
+
 void CallEdge::ParseSymbolFileAndResolve(ModuleList &images) {
   if (resolved)
 return;
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -8,6 +8,7 @@
 
 #include "SymbolFileDWARF.h"
 
+#include "llvm/ADT/Optional.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/Threading.h"
 
@@ -3708,9 +3709,59 @@
   return vars_added;
 }
 
+/// Collect call site parameters in a DW_TAG_call_site DIE.
+static CallSiteParameterArray
+CollectCallSiteParameters(ModuleSP module, DWARFDIE call_site_die) {
+  CallSiteParameterArray parameters = nullptr;
+  for (DWARFDIE child = call_site_die.GetFirstChild(); child.IsValid();
+   child = child.GetSibling()) {
+if (child.Tag() != DW_TAG_call_site_parameter)
+  continue;
+
+llvm::Optional LocationInCallee = {};
+llvm::Optional LocationInCaller = {};
+
+DWARFAttributes attributes;
+const size_t num_attributes = child.GetAttributes(attributes);
+
+// Parse the location at index \p attr_index within this call site parameter
+// DIE, or return None on failure.
+auto parse_simple_location =
+[&](int attr_index) -> llvm::Optional {
+  DWARFFormValue form_value;
+  if (!attributes.ExtractFormValueAtIndex(attr_index, form_value))
+return {};
+  if (!DWARFFormValue::IsBlockForm(form_value.Form()))
+return {};
+  auto data = child.GetData();
+  uint32_t block_offset = form_value.BlockData() - data.GetDataStart();
+  uint32_t block_length = form_value.Unsigned();
+  return DWARFExpression(module,
+ DataExtractor(data, block_offset, block_length),
+ child.GetCU());
+};
+
+for (size_t i = 0; i < num_attributes; ++i) {
+  dw_attr_t attr = attributes.AttributeAtIndex(i);
+  if (attr == DW_AT_location)
+LocationInCallee = parse_simple_location(i);
+  if (attr == DW_AT_call_value)
+LocationInCaller = parse_simple_location(i);
+}
+
+if (LocationInCallee && LocationInCaller) {
+  if (!parameters)
+parameters.reset(new std::vector);
+  CallSiteParameter param = {*LocationInCallee, *LocationInCaller};
+  parameters->push_back(param);
+}
+  }
+  return parameters;
+}
+
 /// Collect call graph edges present in a function DIE.
 static std::vector
-CollectCallEdges(DWARFDIE function_die) {
+CollectCallEdges(ModuleSP module, DWARFDIE function_die) {
   // Check if the function has a supported call site-related attribute.
   // TODO: In the future it may be worthwhile to support call_all_source_calls.
   uint64_t has_call_edges =
@@ -3749,9 +3800,28 @@
 addr_t return_pc = child.GetAttributeValueAsAddress(DW_AT_call_return_pc,
 LLDB_INVALID_ADDRESS);
 
+// Extract call site parameters.
+CallSiteParameterArray parameters =
+CollectCallSiteParameters(module, child);
+
 LLDB_LOG(log, "CollectCallEdges: Found call origin: {0} (retn-PC: {1:x})",
  call_origin.GetPubname(), return_pc);
-call_edges.emplace_back(call_origin.GetMangledName(), return_p

[Lldb-commits] [PATCH] D67376: [DWARF] Evaluate DW_OP_entry_value

2019-09-10 Thread Vedant Kumar via Phabricator via lldb-commits

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



Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:3752
+LocationInCaller = parse_simple_location(i);
+break;
+  }

aprantl wrote:
> vsk wrote:
> > aprantl wrote:
> > > default?
> > I don't believe any action is needed in the default case: we do not want to 
> > log or report an error.
> Doesn't this trigger the switch-doesn't-cover-all-cases warning otherwise?
No warning because dw_attr_t is a uint16_t, but it's probably simpler to avoid 
the switch here.


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

https://reviews.llvm.org/D67376



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

[Lldb-commits] [lldb] r371544 - [lldbtest] Add an "expected_cmd_failure" option to the filecheck helper

2019-09-10 Thread Vedant Kumar via lldb-commits

Author: vedantk
Date: Tue Sep 10 11:36:53 2019
New Revision: 371544

URL: http://llvm.org/viewvc/llvm-project?rev=371544&view=rev
Log:
[lldbtest] Add an "expected_cmd_failure" option to the filecheck helper

Modified:
lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py

Modified: lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py?rev=371544&r1=371543&r2=371544&view=diff
==
--- lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py (original)
+++ lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py Tue Sep 10 11:36:53 
2019
@@ -2207,12 +2207,16 @@ class TestBase(Base):
 self,
 command,
 check_file,
-filecheck_options = ''):
+filecheck_options = '',
+expect_cmd_failure = False):
 # Run the command.
 self.runCmd(
 command,
+check=(not expect_cmd_failure),
 msg="FileCheck'ing result of `{0}`".format(command))
 
+self.assertTrue((not expect_cmd_failure) == self.res.Succeeded())
+
 # Get the error text if there was an error, and the regular text if 
not.
 output = self.res.GetOutput() if self.res.Succeeded() \
 else self.res.GetError()


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

[Lldb-commits] [lldb] r371543 - [Function] Factor out GetCallEdgeForReturnAddress, NFC

2019-09-10 Thread Vedant Kumar via lldb-commits

Author: vedantk
Date: Tue Sep 10 11:36:50 2019
New Revision: 371543

URL: http://llvm.org/viewvc/llvm-project?rev=371543&view=rev
Log:
[Function] Factor out GetCallEdgeForReturnAddress, NFC

Finding the call edge in a function which corresponds to a particular
return address is a generic/useful operation.

Modified:
lldb/trunk/include/lldb/Symbol/Function.h
lldb/trunk/source/Symbol/Function.cpp
lldb/trunk/source/Target/StackFrameList.cpp

Modified: lldb/trunk/include/lldb/Symbol/Function.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Symbol/Function.h?rev=371543&r1=371542&r2=371543&view=diff
==
--- lldb/trunk/include/lldb/Symbol/Function.h (original)
+++ lldb/trunk/include/lldb/Symbol/Function.h Tue Sep 10 11:36:50 2019
@@ -402,6 +402,11 @@ public:
   /// return None.
   llvm::MutableArrayRef GetTailCallingEdges();
 
+  /// Get the outgoing call edge from this function which has the given return
+  /// address \p return_pc, or return nullptr. Note that this will not return a
+  /// tail-calling edge.
+  CallEdge *GetCallEdgeForReturnAddress(lldb::addr_t return_pc, Target 
&target);
+
   /// Get accessor for the block list.
   ///
   /// \return

Modified: lldb/trunk/source/Symbol/Function.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Symbol/Function.cpp?rev=371543&r1=371542&r2=371543&view=diff
==
--- lldb/trunk/source/Symbol/Function.cpp (original)
+++ lldb/trunk/source/Symbol/Function.cpp Tue Sep 10 11:36:50 2019
@@ -276,6 +276,20 @@ llvm::MutableArrayRef Function
   });
 }
 
+CallEdge *Function::GetCallEdgeForReturnAddress(addr_t return_pc,
+Target &target) {
+  auto edges = GetCallEdges();
+  auto edge_it =
+  std::lower_bound(edges.begin(), edges.end(), return_pc,
+   [&](const CallEdge &edge, addr_t pc) {
+ return edge.GetReturnPCAddress(*this, target) < pc;
+   });
+  if (edge_it == edges.end() ||
+  edge_it->GetReturnPCAddress(*this, target) != return_pc)
+return nullptr;
+  return &const_cast(*edge_it);
+}
+
 Block &Function::GetBlock(bool can_create) {
   if (!m_block.BlockInfoHasBeenParsed() && can_create) {
 ModuleSP module_sp = CalculateSymbolContextModule();

Modified: lldb/trunk/source/Target/StackFrameList.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/StackFrameList.cpp?rev=371543&r1=371542&r2=371543&view=diff
==
--- lldb/trunk/source/Target/StackFrameList.cpp (original)
+++ lldb/trunk/source/Target/StackFrameList.cpp Tue Sep 10 11:36:50 2019
@@ -250,26 +250,19 @@ static void FindInterveningFrames(Functi
begin.GetDisplayName(), end.GetDisplayName(), return_pc);
 
   // Find a non-tail calling edge with the correct return PC.
-  auto first_level_edges = begin.GetCallEdges();
   if (log)
-for (const CallEdge &edge : first_level_edges)
+for (const CallEdge &edge : begin.GetCallEdges())
   LLDB_LOG(log, "FindInterveningFrames: found call with retn-PC = {0:x}",
edge.GetReturnPCAddress(begin, target));
-  auto first_edge_it = std::lower_bound(
-  first_level_edges.begin(), first_level_edges.end(), return_pc,
-  [&](const CallEdge &edge, addr_t target_pc) {
-return edge.GetReturnPCAddress(begin, target) < target_pc;
-  });
-  if (first_edge_it == first_level_edges.end() ||
-  first_edge_it->GetReturnPCAddress(begin, target) != return_pc) {
+  CallEdge *first_edge = begin.GetCallEdgeForReturnAddress(return_pc, target);
+  if (!first_edge) {
 LLDB_LOG(log, "No call edge outgoing from {0} with retn-PC == {1:x}",
  begin.GetDisplayName(), return_pc);
 return;
   }
-  CallEdge &first_edge = const_cast(*first_edge_it);
 
   // The first callee may not be resolved, or there may be nothing to fill in.
-  Function *first_callee = first_edge.GetCallee(images);
+  Function *first_callee = first_edge->GetCallee(images);
   if (!first_callee) {
 LLDB_LOG(log, "Could not resolve callee");
 return;


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

[Lldb-commits] [PATCH] D67390: [LLDB][ELF] Load both, .symtab and .dynsym sections

2019-09-10 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere added inline comments.



Comment at: lldb/lit/Modules/ELF/load-from-dynsym-alone.test:24
+# Remove functionInDynsym symbol from .symtab (will leave symbol in .dynsym 
intact)
+# RUN: llvm-strip --strip-symbol=functionInDynsym %t.binary
+

Please also add `llvm-strip` to the list of support tools (`toolchain.py:127`).



Comment at: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:2645
 
 // Sharable objects and dynamic executables usually have 2 distinct symbol
 // tables, one named ".symtab", and the other ".dynsym". The dynsym is a

Can you motivate the need for this change? This comment seems to suggest that 
reading the symtab table should be sufficient as it should contain all the 
information from the dynsym. If that is not true, it would be worth updating 
this comment. 



Comment at: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:2657
 
+// The symtab section is non-allocable and can be stripped. If both, 
.symtab
+// and .dynsym exist, we load both. And if only .dymsym exists, we load it

Why did you remove the last part of the original comment? This seemed to be the 
most useful part... The newly added sentences explain what we are doing (which 
is relatively clear from the code). I'd rather see a comment explaining "why" 
something needs to happen. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67390



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

[Lldb-commits] [PATCH] D67390: [LLDB][ELF] Load both, .symtab and .dynsym sections

2019-09-10 Thread Konrad Wilhelm Kleine via Phabricator via lldb-commits

kwk updated this revision to Diff 219587.
kwk added a comment.

- Added llvm-strip to the list of support tools


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67390

Files:
  lldb/lit/Modules/ELF/Inputs/load-from-dynsym-alone.c
  lldb/lit/Modules/ELF/Inputs/load-symtab-and-dynsym.c
  lldb/lit/Modules/ELF/load-from-dynsym-alone.test
  lldb/lit/Modules/ELF/load-symtab-and-dynsym.test
  lldb/lit/helper/toolchain.py
  lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp

Index: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===
--- lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -2649,19 +2649,25 @@
 // while the reverse is not necessarily true.
 Section *symtab =
 section_list->FindSectionByType(eSectionTypeELFSymbolTable, true).get();
-if (!symtab) {
-  // The symtab section is non-allocable and can be stripped, so if it
-  // doesn't exist then use the dynsym section which should always be
-  // there.
-  symtab =
-  section_list->FindSectionByType(eSectionTypeELFDynamicSymbols, true)
-  .get();
-}
 if (symtab) {
   m_symtab_up.reset(new Symtab(symtab->GetObjectFile()));
   symbol_id += ParseSymbolTable(m_symtab_up.get(), symbol_id, symtab);
 }
 
+// The symtab section is non-allocable and can be stripped. If both, .symtab
+// and .dynsym exist, we load both. And if only .dymsym exists, we load it
+// alone.
+Section *dynsym =
+section_list->FindSectionByType(eSectionTypeELFDynamicSymbols, true)
+.get();
+if (dynsym) {
+  if (!m_symtab_up) {
+auto sec = symtab ? symtab : dynsym;
+m_symtab_up.reset(new Symtab(sec->GetObjectFile()));
+  }
+  symbol_id += ParseSymbolTable(m_symtab_up.get(), symbol_id, dynsym);
+}
+
 // DT_JMPREL
 //  If present, this entry's d_ptr member holds the address of
 //  relocation
Index: lldb/lit/helper/toolchain.py
===
--- lldb/lit/helper/toolchain.py
+++ lldb/lit/helper/toolchain.py
@@ -126,6 +126,6 @@
 
 support_tools = ['yaml2obj', 'obj2yaml', 'llvm-pdbutil',
  'llvm-mc', 'llvm-readobj', 'llvm-objdump',
- 'llvm-objcopy', 'lli']
+ 'llvm-objcopy', 'lli', 'llvm-strip']
 additional_tool_dirs += [config.lldb_tools_dir, config.llvm_tools_dir]
 llvm_config.add_tool_substitutions(support_tools, additional_tool_dirs)
Index: lldb/lit/Modules/ELF/load-symtab-and-dynsym.test
===
--- /dev/null
+++ lldb/lit/Modules/ELF/load-symtab-and-dynsym.test
@@ -0,0 +1,48 @@
+# REQUIRES: system-linux
+
+# This test ensures that we will load .dynsym even if there's a .symtab section.
+# We do this by compiling a small C program with two functions and we direct the
+# linker where to put the symbols so that in the end the layout is as follows:
+#
+#   Symbol table '.dynsym' contains 4 entries:
+#   Num:Value  Size TypeBind   Vis  Ndx Name
+# 0:  0 NOTYPE  LOCAL  DEFAULT  UND 
+# 1:  0 FUNCGLOBAL DEFAULT  UND __libc_start_main@GLIBC_2.2.5
+# 2:  0 NOTYPE  WEAK   DEFAULT  UND __gmon_start__
+# 3: 0040112013 FUNCGLOBAL DEFAULT   10 functionInDynsym
+#   
+#   Symbol table '.symtab' contains 2 entries:
+#   Num:Value  Size TypeBind   Vis  Ndx Name
+# 0:  0 NOTYPE  LOCAL  DEFAULT  UND 
+# 1: 0040111015 FUNCGLOBAL DEFAULT   10 functionInSymtab
+
+# We want to keep the symbol "functionInDynsym" in the .dynamic section and not
+# have it put the default .symtab section.
+# RUN: echo "{functionInDynsym;};" > %T/dynmic-symbols.txt
+# RUN: %clang -Wl,--dynamic-list=%T/dynmic-symbols.txt -g -o %t.binary %p/Inputs/load-symtab-and-dynsym.c
+
+# Remove not needed symbols
+# RUN: echo "functionInSymtab" > %t.keep_symbols
+# RUN: echo "functionInDynsym" >> %t.keep_symbols
+# RUN: llvm-objcopy --strip-all --remove-section .gdb_index --remove-section .comment --keep-symbols=%t.keep_symbols %t.binary
+
+# Remove functionInDynsym symbol from .symtab (will leave symbol in .dynsym intact)
+# RUN: llvm-strip --strip-symbol=functionInDynsym %t.binary
+
+# RUN: %lldb -b -o 'b functionInSymtab' -o 'b functionInDynsym' -o 'run' -o 'continue' %t.binary | FileCheck %s
+
+# CHECK: (lldb) b functionInSymtab
+# CHECK-NEXT: Breakpoint 1: where = {{.*}}.binary`functionInSymtab, address = 0x{{.*}}
+
+# CHECK: (lldb) b functionInDynsym
+# CHECK-NEXT: Breakpoint 2: where = {{.*}}.binary`functionInDynsym, address = 0x{{.*}}
+
+# CHECK: (lldb) run
+# CHECK-NEXT: Process {{.*}} stopped
+# CHEC

[Lldb-commits] [PATCH] D67390: [LLDB][ELF] Load both, .symtab and .dynsym sections

2019-09-10 Thread Konrad Wilhelm Kleine via Phabricator via lldb-commits

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

@JDevlieghere I change the support tool. It was @labath who requested 
(D66791#inline-601050 ) to 
outsource this patch. In that change you can find the whole reason which is 
somewhat more longish:

> Let's put this bit into a separate patch. As I said over IRC, I view this bit 
> as functionally independent of the debugdata stuff (it's definitely 
> independent in it's current form, as it will kick in for non-debugdata files 
> too, which I think is fine).

But apart from that I think the comment indeed needs to change. Anyway, please 
let me know if the connection to the other change D66791 
 does make sense to you.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67390



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

[Lldb-commits] [PATCH] D67390: [LLDB][ELF] Load both, .symtab and .dynsym sections

2019-09-10 Thread Konrad Wilhelm Kleine via Phabricator via lldb-commits

kwk updated this revision to Diff 219594.
kwk marked an inline comment as done.
kwk added a comment.

- update comment for .symtab section with minidebuginfo


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67390

Files:
  lldb/lit/Modules/ELF/Inputs/load-from-dynsym-alone.c
  lldb/lit/Modules/ELF/Inputs/load-symtab-and-dynsym.c
  lldb/lit/Modules/ELF/load-from-dynsym-alone.test
  lldb/lit/Modules/ELF/load-symtab-and-dynsym.test
  lldb/lit/helper/toolchain.py
  lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp

Index: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===
--- lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -2647,21 +2647,30 @@
 // smaller version of the symtab that only contains global symbols. The
 // information found in the dynsym is therefore also found in the symtab,
 // while the reverse is not necessarily true.
+// One exception to the above rule is when we have minidebuginfo embedded
+// into a compressed .gnu_debugdata section. This section contains a .symtab
+// from which all symbols already contained in the .dynsym are stripped.
 Section *symtab =
 section_list->FindSectionByType(eSectionTypeELFSymbolTable, true).get();
-if (!symtab) {
-  // The symtab section is non-allocable and can be stripped, so if it
-  // doesn't exist then use the dynsym section which should always be
-  // there.
-  symtab =
-  section_list->FindSectionByType(eSectionTypeELFDynamicSymbols, true)
-  .get();
-}
 if (symtab) {
   m_symtab_up.reset(new Symtab(symtab->GetObjectFile()));
   symbol_id += ParseSymbolTable(m_symtab_up.get(), symbol_id, symtab);
 }
 
+// The symtab section is non-allocable and can be stripped. If both, .symtab
+// and .dynsym exist, we load both. And if only .dymsym exists, we load it
+// alone.
+Section *dynsym =
+section_list->FindSectionByType(eSectionTypeELFDynamicSymbols, true)
+.get();
+if (dynsym) {
+  if (!m_symtab_up) {
+auto sec = symtab ? symtab : dynsym;
+m_symtab_up.reset(new Symtab(sec->GetObjectFile()));
+  }
+  symbol_id += ParseSymbolTable(m_symtab_up.get(), symbol_id, dynsym);
+}
+
 // DT_JMPREL
 //  If present, this entry's d_ptr member holds the address of
 //  relocation
Index: lldb/lit/helper/toolchain.py
===
--- lldb/lit/helper/toolchain.py
+++ lldb/lit/helper/toolchain.py
@@ -126,6 +126,6 @@
 
 support_tools = ['yaml2obj', 'obj2yaml', 'llvm-pdbutil',
  'llvm-mc', 'llvm-readobj', 'llvm-objdump',
- 'llvm-objcopy', 'lli']
+ 'llvm-objcopy', 'lli', 'llvm-strip']
 additional_tool_dirs += [config.lldb_tools_dir, config.llvm_tools_dir]
 llvm_config.add_tool_substitutions(support_tools, additional_tool_dirs)
Index: lldb/lit/Modules/ELF/load-symtab-and-dynsym.test
===
--- /dev/null
+++ lldb/lit/Modules/ELF/load-symtab-and-dynsym.test
@@ -0,0 +1,48 @@
+# REQUIRES: system-linux
+
+# This test ensures that we will load .dynsym even if there's a .symtab section.
+# We do this by compiling a small C program with two functions and we direct the
+# linker where to put the symbols so that in the end the layout is as follows:
+#
+#   Symbol table '.dynsym' contains 4 entries:
+#   Num:Value  Size TypeBind   Vis  Ndx Name
+# 0:  0 NOTYPE  LOCAL  DEFAULT  UND 
+# 1:  0 FUNCGLOBAL DEFAULT  UND __libc_start_main@GLIBC_2.2.5
+# 2:  0 NOTYPE  WEAK   DEFAULT  UND __gmon_start__
+# 3: 0040112013 FUNCGLOBAL DEFAULT   10 functionInDynsym
+#   
+#   Symbol table '.symtab' contains 2 entries:
+#   Num:Value  Size TypeBind   Vis  Ndx Name
+# 0:  0 NOTYPE  LOCAL  DEFAULT  UND 
+# 1: 0040111015 FUNCGLOBAL DEFAULT   10 functionInSymtab
+
+# We want to keep the symbol "functionInDynsym" in the .dynamic section and not
+# have it put the default .symtab section.
+# RUN: echo "{functionInDynsym;};" > %T/dynmic-symbols.txt
+# RUN: %clang -Wl,--dynamic-list=%T/dynmic-symbols.txt -g -o %t.binary %p/Inputs/load-symtab-and-dynsym.c
+
+# Remove not needed symbols
+# RUN: echo "functionInSymtab" > %t.keep_symbols
+# RUN: echo "functionInDynsym" >> %t.keep_symbols
+# RUN: llvm-objcopy --strip-all --remove-section .gdb_index --remove-section .comment --keep-symbols=%t.keep_symbols %t.binary
+
+# Remove functionInDynsym symbol from .symtab (will leave symbol in .dynsym intact)
+# RUN: llvm-strip --strip-symbol=functionInDynsym %t.b

[Lldb-commits] [PATCH] D67390: [LLDB][ELF] Load both, .symtab and .dynsym sections

2019-09-10 Thread Konrad Wilhelm Kleine via Phabricator via lldb-commits

kwk added a comment.

Fixed the comment as per request.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67390



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

[Lldb-commits] [PATCH] D67376: [DWARF] Evaluate DW_OP_entry_value

2019-09-10 Thread Noel Grandin via Phabricator via lldb-commits

grandinj added inline comments.



Comment at: lldb/include/lldb/Symbol/Function.h:258
+
+using CallSiteParameterArray = std::unique_ptr>;
+

the way this is being used seems to indicate it can be 
   std::vector
no need for unique_ptr


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

https://reviews.llvm.org/D67376



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

[Lldb-commits] [PATCH] D67390: [LLDB][ELF] Load both, .symtab and .dynsym sections

2019-09-10 Thread Jonas Devlieghere via Phabricator via lldb-commits

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

Thanks! This LGTM with the comment on line 2660 rephrased and the motivation as 
part of the summary/commit message.




Comment at: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:2660
 
+// The symtab section is non-allocable and can be stripped. If both, 
.symtab
+// and .dynsym exist, we load both. And if only .dymsym exists, we load it

> The symtab section is non-allocable and can be stripped, while the dynsym 
> section which should always be always be there. If both exist we load both to 
> support the minidebuginfo case. Otherwise we just load the dynsym section.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67390



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

[Lldb-commits] [PATCH] D67390: [LLDB][ELF] Load both, .symtab and .dynsym sections

2019-09-10 Thread Konrad Wilhelm Kleine via Phabricator via lldb-commits

kwk updated this revision to Diff 219605.
kwk added a comment.

- Rephrase comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67390

Files:
  lldb/lit/Modules/ELF/Inputs/load-from-dynsym-alone.c
  lldb/lit/Modules/ELF/Inputs/load-symtab-and-dynsym.c
  lldb/lit/Modules/ELF/load-from-dynsym-alone.test
  lldb/lit/Modules/ELF/load-symtab-and-dynsym.test
  lldb/lit/helper/toolchain.py
  lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp

Index: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===
--- lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -2647,21 +2647,31 @@
 // smaller version of the symtab that only contains global symbols. The
 // information found in the dynsym is therefore also found in the symtab,
 // while the reverse is not necessarily true.
+// One exception to the above rule is when we have minidebuginfo embedded
+// into a compressed .gnu_debugdata section. This section contains a .symtab
+// from which all symbols already contained in the .dynsym are stripped.
 Section *symtab =
 section_list->FindSectionByType(eSectionTypeELFSymbolTable, true).get();
-if (!symtab) {
-  // The symtab section is non-allocable and can be stripped, so if it
-  // doesn't exist then use the dynsym section which should always be
-  // there.
-  symtab =
-  section_list->FindSectionByType(eSectionTypeELFDynamicSymbols, true)
-  .get();
-}
 if (symtab) {
   m_symtab_up.reset(new Symtab(symtab->GetObjectFile()));
   symbol_id += ParseSymbolTable(m_symtab_up.get(), symbol_id, symtab);
 }
 
+// The symtab section is non-allocable and can be stripped, while the dynsym
+// section which should always be always be there. If both exist we load
+// both to support the minidebuginfo case. Otherwise we just load the dynsym
+// section.
+Section *dynsym =
+section_list->FindSectionByType(eSectionTypeELFDynamicSymbols, true)
+.get();
+if (dynsym) {
+  if (!m_symtab_up) {
+auto sec = symtab ? symtab : dynsym;
+m_symtab_up.reset(new Symtab(sec->GetObjectFile()));
+  }
+  symbol_id += ParseSymbolTable(m_symtab_up.get(), symbol_id, dynsym);
+}
+
 // DT_JMPREL
 //  If present, this entry's d_ptr member holds the address of
 //  relocation
Index: lldb/lit/helper/toolchain.py
===
--- lldb/lit/helper/toolchain.py
+++ lldb/lit/helper/toolchain.py
@@ -126,6 +126,6 @@
 
 support_tools = ['yaml2obj', 'obj2yaml', 'llvm-pdbutil',
  'llvm-mc', 'llvm-readobj', 'llvm-objdump',
- 'llvm-objcopy', 'lli']
+ 'llvm-objcopy', 'lli', 'llvm-strip']
 additional_tool_dirs += [config.lldb_tools_dir, config.llvm_tools_dir]
 llvm_config.add_tool_substitutions(support_tools, additional_tool_dirs)
Index: lldb/lit/Modules/ELF/load-symtab-and-dynsym.test
===
--- /dev/null
+++ lldb/lit/Modules/ELF/load-symtab-and-dynsym.test
@@ -0,0 +1,48 @@
+# REQUIRES: system-linux
+
+# This test ensures that we will load .dynsym even if there's a .symtab section.
+# We do this by compiling a small C program with two functions and we direct the
+# linker where to put the symbols so that in the end the layout is as follows:
+#
+#   Symbol table '.dynsym' contains 4 entries:
+#   Num:Value  Size TypeBind   Vis  Ndx Name
+# 0:  0 NOTYPE  LOCAL  DEFAULT  UND 
+# 1:  0 FUNCGLOBAL DEFAULT  UND __libc_start_main@GLIBC_2.2.5
+# 2:  0 NOTYPE  WEAK   DEFAULT  UND __gmon_start__
+# 3: 0040112013 FUNCGLOBAL DEFAULT   10 functionInDynsym
+#   
+#   Symbol table '.symtab' contains 2 entries:
+#   Num:Value  Size TypeBind   Vis  Ndx Name
+# 0:  0 NOTYPE  LOCAL  DEFAULT  UND 
+# 1: 0040111015 FUNCGLOBAL DEFAULT   10 functionInSymtab
+
+# We want to keep the symbol "functionInDynsym" in the .dynamic section and not
+# have it put the default .symtab section.
+# RUN: echo "{functionInDynsym;};" > %T/dynmic-symbols.txt
+# RUN: %clang -Wl,--dynamic-list=%T/dynmic-symbols.txt -g -o %t.binary %p/Inputs/load-symtab-and-dynsym.c
+
+# Remove not needed symbols
+# RUN: echo "functionInSymtab" > %t.keep_symbols
+# RUN: echo "functionInDynsym" >> %t.keep_symbols
+# RUN: llvm-objcopy --strip-all --remove-section .gdb_index --remove-section .comment --keep-symbols=%t.keep_symbols %t.binary
+
+# Remove functionInDynsym symbol from .symtab (will leave symbol in .dynsym intact)
+# RUN: llvm-strip --strip-symbol=functionInDyns

[Lldb-commits] [PATCH] D67111: Adding caching to libc++ std::function formatter for lookups that require scanning symbols

2019-09-10 Thread Shafik Yaghmour via Phabricator via lldb-commits

shafik updated this revision to Diff 219604.
shafik marked an inline comment as done.
shafik added a comment.

- Refactored test to use a function to do repetitive task


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

https://reviews.llvm.org/D67111

Files:
  
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/function/TestLibCxxFunction.py
  
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/function/main.cpp
  source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
  source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.h

Index: source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.h
===
--- source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.h
+++ source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.h
@@ -10,6 +10,9 @@
 #define liblldb_CPPLanguageRuntime_h_
 
 #include 
+
+#include "llvm/ADT/StringMap.h"
+
 #include "lldb/Core/PluginInterface.h"
 #include "lldb/Target/LanguageRuntime.h"
 #include "lldb/lldb-private.h"
@@ -82,6 +85,11 @@
   CPPLanguageRuntime(Process *process);
 
 private:
+  using OperatorStringToCallableInfoMap =
+llvm::StringMap;
+
+  OperatorStringToCallableInfoMap CallableLookupCache;
+
   DISALLOW_COPY_AND_ASSIGN(CPPLanguageRuntime);
 };
 
Index: source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
===
--- source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
+++ source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
@@ -193,13 +193,30 @@
 symbol = sc.symbol;
   }
 
+  // Case 4 or 5
+  // We eliminate these cases early because they don't need the potentially
+  // expensive lookup through the symbol table.
+  if (symbol && !symbol->GetName().GetStringRef().startswith("vtable for") &&
+  !first_template_parameter.contains("$_") &&
+  !first_template_parameter.contains("'lambda'") &&
+  !symbol->GetName().GetStringRef().contains("__invoke")) {
+optional_info.callable_case =
+LibCppStdFunctionCallableCase::FreeOrMemberFunction;
+optional_info.callable_address = function_address_resolved;
+optional_info.callable_symbol = *symbol;
+
+return optional_info;
+  }
+
   auto get_name = [&first_template_parameter, &symbol]() {
 // Given case 1:
 //
 //main::$_0
+//Bar::add_num2(int)::'lambda'(int)
 //
 // we want to append ::operator()()
-if (first_template_parameter.contains("$_"))
+if (first_template_parameter.contains("$_") ||
+first_template_parameter.contains("'lambda'"))
   return llvm::Regex::escape(first_template_parameter.str()) +
  R"(::operator\(\)\(.*\))";
 
@@ -228,6 +245,9 @@
 
   std::string func_to_match = get_name();
 
+  if (CallableLookupCache.count(func_to_match))
+return CallableLookupCache[func_to_match];
+
   SymbolContextList scl;
 
   target.GetImages().FindSymbolsMatchingRegExAndType(
@@ -249,6 +269,7 @@
   addr.CalculateSymbolContextLineEntry(line_entry);
 
   if (first_template_parameter.contains("$_") ||
+  first_template_parameter.contains("'lambda'") ||
   (symbol != nullptr &&
symbol->GetName().GetStringRef().contains("__invoke"))) {
 // Case 1 and 2
@@ -262,19 +283,10 @@
   optional_info.callable_symbol = *symbol;
   optional_info.callable_line_entry = line_entry;
   optional_info.callable_address = addr;
-  return optional_info;
 }
   }
 
-  // Case 4 or 5
-  if (symbol && !symbol->GetName().GetStringRef().startswith("vtable for")) {
-optional_info.callable_case =
-LibCppStdFunctionCallableCase::FreeOrMemberFunction;
-optional_info.callable_address = function_address_resolved;
-optional_info.callable_symbol = *symbol;
-
-return optional_info;
-  }
+  CallableLookupCache[func_to_match] = optional_info;
 
   return optional_info;
 }
Index: source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
===
--- source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
+++ source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
@@ -568,6 +568,11 @@
   "weak_ptr synthetic children",
   ConstString("^(std::__[[:alnum:]]+::)weak_ptr<.+>(( )?&)?$"),
   stl_synth_flags, true);
+  AddCXXSummary(cpp_category_sp,
+lldb_private::formatters::LibcxxFunctionSummaryProvider,
+"libc++ std::function summary provider",
+ConstString("^std::__[[:alnum:]]+::function<.+>$"),
+stl_summary_flags, true);
 
   stl_summary_flags.SetDontShowChildren(false);
   stl_summary_flags.SetSkipPointers(false);
Index: packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/function/main.cpp

[Lldb-commits] [lldb] r371560 - Fix a thinko in handling the QSetLogging packet.

2019-09-10 Thread Jim Ingham via lldb-commits

Author: jingham
Date: Tue Sep 10 14:58:22 2019
New Revision: 371560

URL: http://llvm.org/viewvc/llvm-project?rev=371560&view=rev
Log:
Fix a thinko in handling the QSetLogging packet.

The comparison against LOG_MEMORY shortcut all the LOG_MEMORY_*
log channels.  It has to come last.

Modified:
lldb/trunk/tools/debugserver/source/RNBRemote.cpp

Modified: lldb/trunk/tools/debugserver/source/RNBRemote.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/debugserver/source/RNBRemote.cpp?rev=371560&r1=371559&r2=371560&view=diff
==
--- lldb/trunk/tools/debugserver/source/RNBRemote.cpp (original)
+++ lldb/trunk/tools/debugserver/source/RNBRemote.cpp Tue Sep 10 14:58:22 2019
@@ -2091,9 +2091,6 @@ rnb_err_t set_logging(const char *p) {
 } else if (strncmp(p, "LOG_SHLIB", sizeof("LOG_SHLIB") - 1) == 0) {
   p += sizeof("LOG_SHLIB") - 1;
   bitmask |= LOG_SHLIB;
-} else if (strncmp(p, "LOG_MEMORY", sizeof("LOG_MEMORY") - 1) == 0) {
-  p += sizeof("LOG_MEMORY") - 1;
-  bitmask |= LOG_MEMORY;
 } else if (strncmp(p, "LOG_MEMORY_DATA_SHORT",
sizeof("LOG_MEMORY_DATA_SHORT") - 1) == 0) {
   p += sizeof("LOG_MEMORY_DATA_SHORT") - 1;
@@ -2106,6 +2103,9 @@ rnb_err_t set_logging(const char *p) {
sizeof("LOG_MEMORY_PROTECTIONS") - 1) == 0) {
   p += sizeof("LOG_MEMORY_PROTECTIONS") - 1;
   bitmask |= LOG_MEMORY_PROTECTIONS;
+} else if (strncmp(p, "LOG_MEMORY", sizeof("LOG_MEMORY") - 1) == 0) {
+  p += sizeof("LOG_MEMORY") - 1;
+  bitmask |= LOG_MEMORY;
 } else if (strncmp(p, "LOG_BREAKPOINTS",
sizeof("LOG_BREAKPOINTS") - 1) == 0) {
   p += sizeof("LOG_BREAKPOINTS") - 1;


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

[Lldb-commits] [PATCH] D67111: Adding caching to libc++ std::function formatter for lookups that require scanning symbols

2019-09-10 Thread Adrian Prantl via Phabricator via lldb-commits

aprantl added inline comments.



Comment at: source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp:248
 
+  if (CallableLookupCache.count(func_to_match))
+return CallableLookupCache[func_to_match];

This is performing the lookup twice.
```
auto it = CallableLookupCache.find(func_to_match);
if (it != CallableLookupCache.end)
  return *it;
```



Comment at: source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp:272
   if (first_template_parameter.contains("$_") ||
+  first_template_parameter.contains("'lambda'") ||
   (symbol != nullptr &&

Factor these conditions out into a helper function with a descriptive name 
since it's also used above?



Comment at: source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.h:91
+
+  OperatorStringToCallableInfoMap CallableLookupCache;
+

Since this is used in only one place, I would skip the using statement, but 
that's more a personal style preference.


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

https://reviews.llvm.org/D67111



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

[Lldb-commits] [PATCH] D67111: Adding caching to libc++ std::function formatter for lookups that require scanning symbols

2019-09-10 Thread Frederic Riss via Phabricator via lldb-commits

friss added inline comments.



Comment at: 
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/function/TestLibCxxFunction.py:31
+self.expect("log timers dump",
+   
patterns=["(?!lldb_private::Module::FindSymbolsMatchingRegExAndType).*"])
+

Does this negative lookahead regex actually fail if 
FindSymbolsMatchingRegExAndType is in the output? I would expect the `.*` 
afterwards to match basically anything, including what you have in your 
negative lookahead or another line of the output.


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

https://reviews.llvm.org/D67111



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

[Lldb-commits] [PATCH] D67376: [DWARF] Evaluate DW_OP_entry_value

2019-09-10 Thread Vedant Kumar via Phabricator via lldb-commits

vsk planned changes to this revision.
vsk marked an inline comment as done.
vsk added a comment.

While tightening up the test case I think I found an issue with the way inlined 
frames are handled. I need to take a closer look.

Comment at: lldb/include/lldb/Symbol/Function.h:258
+
+using CallSiteParameterArray = std::unique_ptr>;
+

grandinj wrote:
> the way this is being used seems to indicate it can be 
>std::vector
> no need for unique_ptr
That's a totally fair point. The reason I've used unique_ptr here is to save 
space in CallEdge in the common case, where no call site information is loaded 
for the function. Call site info is lazily parsed, so we'd like to take a 
minimal memory hit for functions that aren't in a backtrace.

Also, note that using a pointer allows for a further PointerIntPair memory 
optimization mentioned below.

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

https://reviews.llvm.org/D67376

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

[Lldb-commits] [PATCH] D67376: [DWARF] Evaluate DW_OP_entry_value

2019-09-10 Thread Adrian Prantl via Phabricator via lldb-commits

aprantl added inline comments.



Comment at: lldb/include/lldb/Symbol/Function.h:258
+
+using CallSiteParameterArray = std::unique_ptr>;
+

vsk wrote:
> grandinj wrote:
> > the way this is being used seems to indicate it can be 
> >std::vector
> > no need for unique_ptr
> That's a totally fair point. The reason I've used unique_ptr here is to save 
> space in CallEdge in the common case, where no call site information is 
> loaded for the function. Call site info is lazily parsed, so we'd like to 
> take a minimal memory hit for functions that aren't in a backtrace.
> 
> Also, note that using a pointer allows for a further PointerIntPair memory 
> optimization mentioned below.
Can you document this decision up there?



Comment at: lldb/include/lldb/Symbol/Function.h:249
 
+/// \class CallSiteParameter Function.h "lldb/Symbol/Function.h"
+///

Out of curiosity: What's the effect of this line? It appears to have totally 
redundant information in it that Doxygen should already know about.



Comment at: lldb/source/Expression/DWARFExpression.cpp:1136
+if (parent_frame && !parent_frame->IsInlined())
+  break;
+  }

What does it mean if there is a null parent_frame and shouldn't we return false 
in that case?



Comment at: lldb/source/Expression/DWARFExpression.cpp:2672
 case DW_OP_push_object_address:
+  // TODO: Reject DW_OP_push_object_address within entry value exprs.
   if (object_address_ptr)

because...?


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

https://reviews.llvm.org/D67376



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

[Lldb-commits] [PATCH] D67111: Adding caching to libc++ std::function formatter for lookups that require scanning symbols

2019-09-10 Thread Shafik Yaghmour via Phabricator via lldb-commits

shafik updated this revision to Diff 219621.
shafik marked 2 inline comments as done.
shafik added a comment.

- Fixing test based on comments


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

https://reviews.llvm.org/D67111

Files:
  
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/function/TestLibCxxFunction.py
  
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/function/main.cpp
  source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
  source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.h

Index: source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.h
===
--- source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.h
+++ source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.h
@@ -10,6 +10,9 @@
 #define liblldb_CPPLanguageRuntime_h_
 
 #include 
+
+#include "llvm/ADT/StringMap.h"
+
 #include "lldb/Core/PluginInterface.h"
 #include "lldb/Target/LanguageRuntime.h"
 #include "lldb/lldb-private.h"
@@ -82,6 +85,11 @@
   CPPLanguageRuntime(Process *process);
 
 private:
+  using OperatorStringToCallableInfoMap =
+llvm::StringMap;
+
+  OperatorStringToCallableInfoMap CallableLookupCache;
+
   DISALLOW_COPY_AND_ASSIGN(CPPLanguageRuntime);
 };
 
Index: source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
===
--- source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
+++ source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
@@ -193,13 +193,30 @@
 symbol = sc.symbol;
   }
 
+  // Case 4 or 5
+  // We eliminate these cases early because they don't need the potentially
+  // expensive lookup through the symbol table.
+  if (symbol && !symbol->GetName().GetStringRef().startswith("vtable for") &&
+  !first_template_parameter.contains("$_") &&
+  !first_template_parameter.contains("'lambda'") &&
+  !symbol->GetName().GetStringRef().contains("__invoke")) {
+optional_info.callable_case =
+LibCppStdFunctionCallableCase::FreeOrMemberFunction;
+optional_info.callable_address = function_address_resolved;
+optional_info.callable_symbol = *symbol;
+
+return optional_info;
+  }
+
   auto get_name = [&first_template_parameter, &symbol]() {
 // Given case 1:
 //
 //main::$_0
+//Bar::add_num2(int)::'lambda'(int)
 //
 // we want to append ::operator()()
-if (first_template_parameter.contains("$_"))
+if (first_template_parameter.contains("$_") ||
+first_template_parameter.contains("'lambda'"))
   return llvm::Regex::escape(first_template_parameter.str()) +
  R"(::operator\(\)\(.*\))";
 
@@ -228,6 +245,9 @@
 
   std::string func_to_match = get_name();
 
+  if (CallableLookupCache.count(func_to_match))
+return CallableLookupCache[func_to_match];
+
   SymbolContextList scl;
 
   target.GetImages().FindSymbolsMatchingRegExAndType(
@@ -249,6 +269,7 @@
   addr.CalculateSymbolContextLineEntry(line_entry);
 
   if (first_template_parameter.contains("$_") ||
+  first_template_parameter.contains("'lambda'") ||
   (symbol != nullptr &&
symbol->GetName().GetStringRef().contains("__invoke"))) {
 // Case 1 and 2
@@ -262,19 +283,10 @@
   optional_info.callable_symbol = *symbol;
   optional_info.callable_line_entry = line_entry;
   optional_info.callable_address = addr;
-  return optional_info;
 }
   }
 
-  // Case 4 or 5
-  if (symbol && !symbol->GetName().GetStringRef().startswith("vtable for")) {
-optional_info.callable_case =
-LibCppStdFunctionCallableCase::FreeOrMemberFunction;
-optional_info.callable_address = function_address_resolved;
-optional_info.callable_symbol = *symbol;
-
-return optional_info;
-  }
+  CallableLookupCache[func_to_match] = optional_info;
 
   return optional_info;
 }
Index: source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
===
--- source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
+++ source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
@@ -568,6 +568,11 @@
   "weak_ptr synthetic children",
   ConstString("^(std::__[[:alnum:]]+::)weak_ptr<.+>(( )?&)?$"),
   stl_synth_flags, true);
+  AddCXXSummary(cpp_category_sp,
+lldb_private::formatters::LibcxxFunctionSummaryProvider,
+"libc++ std::function summary provider",
+ConstString("^std::__[[:alnum:]]+::function<.+>$"),
+stl_summary_flags, true);
 
   stl_summary_flags.SetDontShowChildren(false);
   stl_summary_flags.SetSkipPointers(false);
Index: packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/function/main.cpp
==

[Lldb-commits] [PATCH] D67111: Adding caching to libc++ std::function formatter for lookups that require scanning symbols

2019-09-10 Thread Shafik Yaghmour via Phabricator via lldb-commits

shafik added inline comments.



Comment at: 
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/function/TestLibCxxFunction.py:31
+self.expect("log timers dump",
+   
patterns=["(?!lldb_private::Module::FindSymbolsMatchingRegExAndType).*"])
+

friss wrote:
> Does this negative lookahead regex actually fail if 
> FindSymbolsMatchingRegExAndType is in the output? I would expect the `.*` 
> afterwards to match basically anything, including what you have in your 
> negative lookahead or another line of the output.
Good catch, I was trying to be too clever and forgot how it worked.

This is more easily done using `matching=False`


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

https://reviews.llvm.org/D67111



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

[Lldb-commits] [PATCH] D67111: Adding caching to libc++ std::function formatter for lookups that require scanning symbols

2019-09-10 Thread Shafik Yaghmour via Phabricator via lldb-commits

shafik updated this revision to Diff 219633.
shafik marked 3 inline comments as done.
shafik added a comment.

Changes based on comments:

- Removed extra lookup
- Refactored redundant code


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

https://reviews.llvm.org/D67111

Files:
  
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/function/TestLibCxxFunction.py
  
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/function/main.cpp
  source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
  source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.h

Index: source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.h
===
--- source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.h
+++ source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.h
@@ -10,6 +10,9 @@
 #define liblldb_CPPLanguageRuntime_h_
 
 #include 
+
+#include "llvm/ADT/StringMap.h"
+
 #include "lldb/Core/PluginInterface.h"
 #include "lldb/Target/LanguageRuntime.h"
 #include "lldb/lldb-private.h"
@@ -82,6 +85,11 @@
   CPPLanguageRuntime(Process *process);
 
 private:
+  using OperatorStringToCallableInfoMap =
+llvm::StringMap;
+
+  OperatorStringToCallableInfoMap CallableLookupCache;
+
   DISALLOW_COPY_AND_ASSIGN(CPPLanguageRuntime);
 };
 
Index: source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
===
--- source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
+++ source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
@@ -192,14 +192,33 @@
 function_address_resolved, eSymbolContextEverything, sc);
 symbol = sc.symbol;
   }
+
+auto contains_lambda_identifier = []( llvm::StringRef & str_ref ) {
+return str_ref.contains("$_") || str_ref.contains("'lambda'");
+};
 
-  auto get_name = [&first_template_parameter, &symbol]() {
+  // Case 4 or 5
+  // We eliminate these cases early because they don't need the potentially
+  // expensive lookup through the symbol table.
+  if (symbol && !symbol->GetName().GetStringRef().startswith("vtable for") &&
+  !contains_lambda_identifier(first_template_parameter) &&
+  !symbol->GetName().GetStringRef().contains("__invoke")) {
+optional_info.callable_case =
+LibCppStdFunctionCallableCase::FreeOrMemberFunction;
+optional_info.callable_address = function_address_resolved;
+optional_info.callable_symbol = *symbol;
+
+return optional_info;
+  }
+
+  auto get_name = [&first_template_parameter, &symbol, contains_lambda_identifier]() {
 // Given case 1:
 //
 //main::$_0
+//Bar::add_num2(int)::'lambda'(int)
 //
 // we want to append ::operator()()
-if (first_template_parameter.contains("$_"))
+if (contains_lambda_identifier(first_template_parameter))
   return llvm::Regex::escape(first_template_parameter.str()) +
  R"(::operator\(\)\(.*\))";
 
@@ -228,6 +247,10 @@
 
   std::string func_to_match = get_name();
 
+  auto it = CallableLookupCache.find(func_to_match);
+  if (it != CallableLookupCache.end())
+return it->second;
+
   SymbolContextList scl;
 
   target.GetImages().FindSymbolsMatchingRegExAndType(
@@ -248,7 +271,7 @@
   LineEntry line_entry;
   addr.CalculateSymbolContextLineEntry(line_entry);
 
-  if (first_template_parameter.contains("$_") ||
+  if (contains_lambda_identifier(first_template_parameter) ||
   (symbol != nullptr &&
symbol->GetName().GetStringRef().contains("__invoke"))) {
 // Case 1 and 2
@@ -262,19 +285,10 @@
   optional_info.callable_symbol = *symbol;
   optional_info.callable_line_entry = line_entry;
   optional_info.callable_address = addr;
-  return optional_info;
 }
   }
 
-  // Case 4 or 5
-  if (symbol && !symbol->GetName().GetStringRef().startswith("vtable for")) {
-optional_info.callable_case =
-LibCppStdFunctionCallableCase::FreeOrMemberFunction;
-optional_info.callable_address = function_address_resolved;
-optional_info.callable_symbol = *symbol;
-
-return optional_info;
-  }
+  CallableLookupCache[func_to_match] = optional_info;
 
   return optional_info;
 }
Index: packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/function/main.cpp
===
--- packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/function/main.cpp
+++ packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/function/main.cpp
@@ -17,8 +17,25 @@
return 66 ;
}
int add_num(int i) const { return i + 3 ; }
+   int add_num2(int i) {
+ std::function add_num2_f = [](int x) {
+ return x+1;
+  };
+
+  return add_num2_f(i); // Set break point at this line.
+

[Lldb-commits] [PATCH] D67427: [Plugins/Process] Remove direct use of ClangASTContext from InferiorCallPOSIX

2019-09-10 Thread Alex Langford via Phabricator via lldb-commits

xiaobai created this revision.
xiaobai added reviewers: JDevlieghere, compnerd, davide, clayborg.
Herald added a project: LLDB.

InferiorCallPOSIX directly grabs a ClangASTContext from the Target it
has and does no error checking. I don't think these functions have a
reason to know about clang specifically. Additionally, using
`GetScratchTypeSystemForLanguage` forces us to do error checking since
it returns an Expected.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D67427

Files:
  lldb/source/Plugins/Process/Utility/InferiorCallPOSIX.cpp


Index: lldb/source/Plugins/Process/Utility/InferiorCallPOSIX.cpp
===
--- lldb/source/Plugins/Process/Utility/InferiorCallPOSIX.cpp
+++ lldb/source/Plugins/Process/Utility/InferiorCallPOSIX.cpp
@@ -12,7 +12,7 @@
 #include "lldb/Core/ValueObject.h"
 #include "lldb/Expression/DiagnosticManager.h"
 #include "lldb/Host/Config.h"
-#include "lldb/Symbol/ClangASTContext.h"
+#include "lldb/Symbol/TypeSystem.h"
 #include "lldb/Symbol/SymbolContext.h"
 #include "lldb/Target/ExecutionContext.h"
 #include "lldb/Target/Platform.h"
@@ -79,17 +79,23 @@
   AddressRange mmap_range;
   if (sc.GetAddressRange(range_scope, 0, use_inline_block_range,
  mmap_range)) {
-ClangASTContext *clang_ast_context =
-process->GetTarget().GetScratchClangASTContext();
-CompilerType clang_void_ptr_type =
-clang_ast_context->GetBasicType(eBasicTypeVoid).GetPointerType();
+auto type_system_or_err =
+process->GetTarget().GetScratchTypeSystemForLanguage(
+eLanguageTypeC);
+if (!type_system_or_err) {
+  llvm::consumeError(type_system_or_err.takeError());
+  return false;
+}
+CompilerType void_ptr_type =
+type_system_or_err->GetBasicTypeFromAST(eBasicTypeVoid)
+.GetPointerType();
 const ArchSpec arch = process->GetTarget().GetArchitecture();
 MmapArgList args =
 process->GetTarget().GetPlatform()->GetMmapArgumentList(
 arch, addr, length, prot_arg, flags, fd, offset);
 lldb::ThreadPlanSP call_plan_sp(
 new ThreadPlanCallFunction(*thread, mmap_range.GetBaseAddress(),
-   clang_void_ptr_type, args, options));
+   void_ptr_type, args, options));
 if (call_plan_sp) {
   DiagnosticManager diagnostics;
 
@@ -200,12 +206,16 @@
   options.SetTimeout(process->GetUtilityExpressionTimeout());
   options.SetTrapExceptions(trap_exceptions);
 
-  ClangASTContext *clang_ast_context =
-  process->GetTarget().GetScratchClangASTContext();
-  CompilerType clang_void_ptr_type =
-  clang_ast_context->GetBasicType(eBasicTypeVoid).GetPointerType();
+  auto type_system_or_err =
+  process->GetTarget().GetScratchTypeSystemForLanguage(eLanguageTypeC);
+  if (!type_system_or_err) {
+llvm::consumeError(type_system_or_err.takeError());
+return false;
+  }
+  CompilerType void_ptr_type =
+  type_system_or_err->GetBasicTypeFromAST(eBasicTypeVoid).GetPointerType();
   lldb::ThreadPlanSP call_plan_sp(
-  new ThreadPlanCallFunction(*thread, *address, clang_void_ptr_type,
+  new ThreadPlanCallFunction(*thread, *address, void_ptr_type,
  llvm::ArrayRef(), options));
   if (call_plan_sp) {
 DiagnosticManager diagnostics;


Index: lldb/source/Plugins/Process/Utility/InferiorCallPOSIX.cpp
===
--- lldb/source/Plugins/Process/Utility/InferiorCallPOSIX.cpp
+++ lldb/source/Plugins/Process/Utility/InferiorCallPOSIX.cpp
@@ -12,7 +12,7 @@
 #include "lldb/Core/ValueObject.h"
 #include "lldb/Expression/DiagnosticManager.h"
 #include "lldb/Host/Config.h"
-#include "lldb/Symbol/ClangASTContext.h"
+#include "lldb/Symbol/TypeSystem.h"
 #include "lldb/Symbol/SymbolContext.h"
 #include "lldb/Target/ExecutionContext.h"
 #include "lldb/Target/Platform.h"
@@ -79,17 +79,23 @@
   AddressRange mmap_range;
   if (sc.GetAddressRange(range_scope, 0, use_inline_block_range,
  mmap_range)) {
-ClangASTContext *clang_ast_context =
-process->GetTarget().GetScratchClangASTContext();
-CompilerType clang_void_ptr_type =
-clang_ast_context->GetBasicType(eBasicTypeVoid).GetPointerType();
+auto type_system_or_err =
+process->GetTarget().GetScratchTypeSystemForLanguage(
+eLanguageTypeC);
+if (!type_system_or_err) {
+  llvm::consumeError(type_system_or_err.takeError());
+  return false;
+}
+CompilerType void_ptr_type =
+type_system_or_err->GetBasicTypeFromAST(eBasicTypeVoid)
+.GetPointerType();
 const ArchSpec arch = process->GetTarget().GetArchitecture();

[Lldb-commits] [lldb] r371582 - Skip a test in TestProcessIO.py when running against ios devices.

2019-09-10 Thread Jason Molenda via lldb-commits

Author: jmolenda
Date: Tue Sep 10 18:02:30 2019
New Revision: 371582

URL: http://llvm.org/viewvc/llvm-project?rev=371582&view=rev
Log:
Skip a test in TestProcessIO.py when running against ios devices.

Modified:

lldb/trunk/packages/Python/lldbsuite/test/python_api/process/io/TestProcessIO.py

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/python_api/process/io/TestProcessIO.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/python_api/process/io/TestProcessIO.py?rev=371582&r1=371581&r2=371582&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/python_api/process/io/TestProcessIO.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/python_api/process/io/TestProcessIO.py
 Tue Sep 10 18:02:30 2019
@@ -36,6 +36,7 @@ class ProcessIOTestCase(TestBase):
 @skipIfWindows  # stdio manipulation unsupported on Windows
 @add_test_categories(['pyapi'])
 @expectedFlakeyLinux(bugnumber="llvm.org/pr26437")
+@skipIfRemote # I/O redirection like this is not supported on remote iOS 
devices yet 
 def test_stdin_by_api(self):
 """Exercise SBProcess.PutSTDIN()."""
 self.setup_test()


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

[Lldb-commits] [lldb] r371583 - Ah, only skip this for embedded darwin targets.

2019-09-10 Thread Jason Molenda via lldb-commits

Author: jmolenda
Date: Tue Sep 10 18:03:24 2019
New Revision: 371583

URL: http://llvm.org/viewvc/llvm-project?rev=371583&view=rev
Log:
Ah, only skip this for embedded darwin targets.

Modified:

lldb/trunk/packages/Python/lldbsuite/test/python_api/process/io/TestProcessIO.py

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/python_api/process/io/TestProcessIO.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/python_api/process/io/TestProcessIO.py?rev=371583&r1=371582&r2=371583&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/python_api/process/io/TestProcessIO.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/python_api/process/io/TestProcessIO.py
 Tue Sep 10 18:03:24 2019
@@ -36,7 +36,7 @@ class ProcessIOTestCase(TestBase):
 @skipIfWindows  # stdio manipulation unsupported on Windows
 @add_test_categories(['pyapi'])
 @expectedFlakeyLinux(bugnumber="llvm.org/pr26437")
-@skipIfRemote # I/O redirection like this is not supported on remote iOS 
devices yet 
+@skipIfDarwinEmbedded # I/O redirection like this is not supported on 
remote iOS devices yet 
 def test_stdin_by_api(self):
 """Exercise SBProcess.PutSTDIN()."""
 self.setup_test()


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

[Lldb-commits] [PATCH] D67376: [DWARF] Evaluate DW_OP_entry_value

2019-09-10 Thread Vedant Kumar via Phabricator via lldb-commits

vsk updated this revision to Diff 219646.
vsk marked 4 inline comments as done.
vsk added a comment.

- Fix return address lookup when the immediate parent frame is inlined.
- Tighten the test so that it actually verifies that tail calls, inlining, etc. 
occur, instead of assuming :).
- Add/move various TODOs as pointed out by reviewers.

I'm happy with the test coverage now, and think that I've addressed all the 
feedback. PTAL, thanks!


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

https://reviews.llvm.org/D67376

Files:
  lldb/include/lldb/Symbol/Function.h
  lldb/packages/Python/lldbsuite/test/decorators.py
  
lldb/packages/Python/lldbsuite/test/functionalities/param_entry_vals/basic_entry_values_x86_64/Makefile
  
lldb/packages/Python/lldbsuite/test/functionalities/param_entry_vals/basic_entry_values_x86_64/TestBasicEntryValuesX86_64.py
  
lldb/packages/Python/lldbsuite/test/functionalities/param_entry_vals/basic_entry_values_x86_64/main.cpp
  lldb/source/Expression/DWARFExpression.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDefines.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Symbol/Function.cpp

Index: lldb/source/Symbol/Function.cpp
===
--- lldb/source/Symbol/Function.cpp
+++ lldb/source/Symbol/Function.cpp
@@ -127,11 +127,18 @@
 }
 
 //
-CallEdge::CallEdge(const char *symbol_name, lldb::addr_t return_pc)
-: return_pc(return_pc), resolved(false) {
+CallEdge::CallEdge(const char *symbol_name, lldb::addr_t return_pc,
+   CallSiteParameterArray parameters)
+: return_pc(return_pc), parameters(std::move(parameters)), resolved(false) {
   lazy_callee.symbol_name = symbol_name;
 }
 
+llvm::ArrayRef CallEdge::GetCallSiteParameters() const {
+  if (parameters)
+return *parameters.get();
+  return {};
+}
+
 void CallEdge::ParseSymbolFileAndResolve(ModuleList &images) {
   if (resolved)
 return;
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -8,6 +8,7 @@
 
 #include "SymbolFileDWARF.h"
 
+#include "llvm/ADT/Optional.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/Threading.h"
 
@@ -3708,9 +3709,59 @@
   return vars_added;
 }
 
+/// Collect call site parameters in a DW_TAG_call_site DIE.
+static CallSiteParameterArray
+CollectCallSiteParameters(ModuleSP module, DWARFDIE call_site_die) {
+  CallSiteParameterArray parameters = nullptr;
+  for (DWARFDIE child = call_site_die.GetFirstChild(); child.IsValid();
+   child = child.GetSibling()) {
+if (child.Tag() != DW_TAG_call_site_parameter)
+  continue;
+
+llvm::Optional LocationInCallee = {};
+llvm::Optional LocationInCaller = {};
+
+DWARFAttributes attributes;
+const size_t num_attributes = child.GetAttributes(attributes);
+
+// Parse the location at index \p attr_index within this call site parameter
+// DIE, or return None on failure.
+auto parse_simple_location =
+[&](int attr_index) -> llvm::Optional {
+  DWARFFormValue form_value;
+  if (!attributes.ExtractFormValueAtIndex(attr_index, form_value))
+return {};
+  if (!DWARFFormValue::IsBlockForm(form_value.Form()))
+return {};
+  auto data = child.GetData();
+  uint32_t block_offset = form_value.BlockData() - data.GetDataStart();
+  uint32_t block_length = form_value.Unsigned();
+  return DWARFExpression(module,
+ DataExtractor(data, block_offset, block_length),
+ child.GetCU());
+};
+
+for (size_t i = 0; i < num_attributes; ++i) {
+  dw_attr_t attr = attributes.AttributeAtIndex(i);
+  if (attr == DW_AT_location)
+LocationInCallee = parse_simple_location(i);
+  if (attr == DW_AT_call_value)
+LocationInCaller = parse_simple_location(i);
+}
+
+if (LocationInCallee && LocationInCaller) {
+  if (!parameters)
+parameters.reset(new std::vector);
+  CallSiteParameter param = {*LocationInCallee, *LocationInCaller};
+  parameters->push_back(param);
+}
+  }
+  return parameters;
+}
+
 /// Collect call graph edges present in a function DIE.
 static std::vector
-CollectCallEdges(DWARFDIE function_die) {
+CollectCallEdges(ModuleSP module, DWARFDIE function_die) {
   // Check if the function has a supported call site-related attribute.
   // TODO: In the future it may be worthwhile to support call_all_source_calls.
   uint64_t has_call_edges =
@@ -3747,9 +3798,28 @@
 addr_t return_pc = child.GetAttributeValueAsAddress(DW_AT_call_return_pc,
 LLDB_INVALID_ADDRESS);
 
+// Extract call site parameters.
+CallSiteParameterArray parameters =
+CollectCallSiteParameters(modul

[Lldb-commits] [PATCH] D67376: [DWARF] Evaluate DW_OP_entry_value

2019-09-10 Thread Vedant Kumar via Phabricator via lldb-commits

vsk added inline comments.



Comment at: lldb/include/lldb/Symbol/Function.h:258
+
+using CallSiteParameterArray = std::unique_ptr>;
+

aprantl wrote:
> vsk wrote:
> > grandinj wrote:
> > > the way this is being used seems to indicate it can be 
> > >std::vector
> > > no need for unique_ptr
> > That's a totally fair point. The reason I've used unique_ptr here is to 
> > save space in CallEdge in the common case, where no call site information 
> > is loaded for the function. Call site info is lazily parsed, so we'd like 
> > to take a minimal memory hit for functions that aren't in a backtrace.
> > 
> > Also, note that using a pointer allows for a further PointerIntPair memory 
> > optimization mentioned below.
> Can you document this decision up there?
Done. And, thanks @grandinj for pointing this out, I dug a bit more and found 
that we're *not* doing this in Function for the CallEdge vector, but probably 
should be. Added a TODO there.



Comment at: lldb/include/lldb/Symbol/Function.h:249
 
+/// \class CallSiteParameter Function.h "lldb/Symbol/Function.h"
+///

aprantl wrote:
> Out of curiosity: What's the effect of this line? It appears to have totally 
> redundant information in it that Doxygen should already know about.
No clue. I saw it elsewhere in this file and wanted to stick to the established 
format. It could be worth simplifying later, though.



Comment at: lldb/source/Expression/DWARFExpression.cpp:1136
+if (parent_frame && !parent_frame->IsInlined())
+  break;
+  }

aprantl wrote:
> What does it mean if there is a null parent_frame and shouldn't we return 
> false in that case?
Yes, we should detect this and fail early.



Comment at: lldb/source/Expression/DWARFExpression.cpp:2672
 case DW_OP_push_object_address:
+  // TODO: Reject DW_OP_push_object_address within entry value exprs.
   if (object_address_ptr)

aprantl wrote:
> because...?
Actually, rejecting DW_OP_push_object_address requires no special handling. 
Instead we need a TODO about actually supporting it: that belongs in the 
Evaluate_DW_OP_entry_value.


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

https://reviews.llvm.org/D67376



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

[Lldb-commits] [PATCH] D67376: [DWARF] Evaluate DW_OP_entry_value

2019-09-10 Thread Vedant Kumar via Phabricator via lldb-commits

vsk added inline comments.



Comment at: lldb/include/lldb/Symbol/Function.h:258
+
+using CallSiteParameterArray = std::unique_ptr>;
+

vsk wrote:
> aprantl wrote:
> > vsk wrote:
> > > grandinj wrote:
> > > > the way this is being used seems to indicate it can be 
> > > >std::vector
> > > > no need for unique_ptr
> > > That's a totally fair point. The reason I've used unique_ptr here is to 
> > > save space in CallEdge in the common case, where no call site information 
> > > is loaded for the function. Call site info is lazily parsed, so we'd like 
> > > to take a minimal memory hit for functions that aren't in a backtrace.
> > > 
> > > Also, note that using a pointer allows for a further PointerIntPair 
> > > memory optimization mentioned below.
> > Can you document this decision up there?
> Done. And, thanks @grandinj for pointing this out, I dug a bit more and found 
> that we're *not* doing this in Function for the CallEdge vector, but probably 
> should be. Added a TODO there.
Actually, there's no need to do this in both CallEdge and Function: edges are 
parsed lazily, but parameters aren't. Let's just leave a note about this in 
Function.


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

https://reviews.llvm.org/D67376



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

[Lldb-commits] [PATCH] D67376: [DWARF] Evaluate DW_OP_entry_value

2019-09-10 Thread Vedant Kumar via Phabricator via lldb-commits

vsk updated this revision to Diff 219648.
vsk marked an inline comment as done.

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

https://reviews.llvm.org/D67376

Files:
  lldb/include/lldb/Symbol/Function.h
  lldb/packages/Python/lldbsuite/test/decorators.py
  
lldb/packages/Python/lldbsuite/test/functionalities/param_entry_vals/basic_entry_values_x86_64/Makefile
  
lldb/packages/Python/lldbsuite/test/functionalities/param_entry_vals/basic_entry_values_x86_64/TestBasicEntryValuesX86_64.py
  
lldb/packages/Python/lldbsuite/test/functionalities/param_entry_vals/basic_entry_values_x86_64/main.cpp
  lldb/source/Expression/DWARFExpression.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDefines.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Symbol/Function.cpp

Index: lldb/source/Symbol/Function.cpp
===
--- lldb/source/Symbol/Function.cpp
+++ lldb/source/Symbol/Function.cpp
@@ -127,11 +127,16 @@
 }
 
 //
-CallEdge::CallEdge(const char *symbol_name, lldb::addr_t return_pc)
-: return_pc(return_pc), resolved(false) {
+CallEdge::CallEdge(const char *symbol_name, lldb::addr_t return_pc,
+   CallSiteParameterArray parameters)
+: return_pc(return_pc), parameters(std::move(parameters)), resolved(false) {
   lazy_callee.symbol_name = symbol_name;
 }
 
+llvm::ArrayRef CallEdge::GetCallSiteParameters() const {
+  return parameters;
+}
+
 void CallEdge::ParseSymbolFileAndResolve(ModuleList &images) {
   if (resolved)
 return;
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -8,6 +8,7 @@
 
 #include "SymbolFileDWARF.h"
 
+#include "llvm/ADT/Optional.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/Threading.h"
 
@@ -3708,9 +3709,57 @@
   return vars_added;
 }
 
+/// Collect call site parameters in a DW_TAG_call_site DIE.
+static CallSiteParameterArray
+CollectCallSiteParameters(ModuleSP module, DWARFDIE call_site_die) {
+  CallSiteParameterArray parameters;
+  for (DWARFDIE child = call_site_die.GetFirstChild(); child.IsValid();
+   child = child.GetSibling()) {
+if (child.Tag() != DW_TAG_call_site_parameter)
+  continue;
+
+llvm::Optional LocationInCallee = {};
+llvm::Optional LocationInCaller = {};
+
+DWARFAttributes attributes;
+const size_t num_attributes = child.GetAttributes(attributes);
+
+// Parse the location at index \p attr_index within this call site parameter
+// DIE, or return None on failure.
+auto parse_simple_location =
+[&](int attr_index) -> llvm::Optional {
+  DWARFFormValue form_value;
+  if (!attributes.ExtractFormValueAtIndex(attr_index, form_value))
+return {};
+  if (!DWARFFormValue::IsBlockForm(form_value.Form()))
+return {};
+  auto data = child.GetData();
+  uint32_t block_offset = form_value.BlockData() - data.GetDataStart();
+  uint32_t block_length = form_value.Unsigned();
+  return DWARFExpression(module,
+ DataExtractor(data, block_offset, block_length),
+ child.GetCU());
+};
+
+for (size_t i = 0; i < num_attributes; ++i) {
+  dw_attr_t attr = attributes.AttributeAtIndex(i);
+  if (attr == DW_AT_location)
+LocationInCallee = parse_simple_location(i);
+  if (attr == DW_AT_call_value)
+LocationInCaller = parse_simple_location(i);
+}
+
+if (LocationInCallee && LocationInCaller) {
+  CallSiteParameter param = {*LocationInCallee, *LocationInCaller};
+  parameters.push_back(param);
+}
+  }
+  return parameters;
+}
+
 /// Collect call graph edges present in a function DIE.
 static std::vector
-CollectCallEdges(DWARFDIE function_die) {
+CollectCallEdges(ModuleSP module, DWARFDIE function_die) {
   // Check if the function has a supported call site-related attribute.
   // TODO: In the future it may be worthwhile to support call_all_source_calls.
   uint64_t has_call_edges =
@@ -3747,9 +3796,28 @@
 addr_t return_pc = child.GetAttributeValueAsAddress(DW_AT_call_return_pc,
 LLDB_INVALID_ADDRESS);
 
+// Extract call site parameters.
+CallSiteParameterArray parameters =
+CollectCallSiteParameters(module, child);
+
 LLDB_LOG(log, "CollectCallEdges: Found call origin: {0} (retn-PC: {1:x})",
  call_origin.GetPubname(), return_pc);
-call_edges.emplace_back(call_origin.GetMangledName(), return_pc);
+if (log && parameters.size()) {
+  for (const CallSiteParameter ¶m : parameters) {
+StreamString callee_loc_desc, caller_loc_desc;
+param.LocationInCallee.GetDescription(&callee_loc_desc,
+  eDescrip

[Lldb-commits] [lldb] r371491 - [LLDB] FreeBSD fix new SetFile call.

[Lldb-commits] [PATCH] D67390: [LLDB][ELF] Load both, .symtab and .dynsym sections

[Lldb-commits] [PATCH] D67347: [Windows] Use information from the PE32 exceptions directory to construct unwind plans

[Lldb-commits] [PATCH] D67376: [DWARF] Evaluate DW_OP_entry_value

[Lldb-commits] [PATCH] D67369: Implement DW_OP_convert

[Lldb-commits] [PATCH] D67369: Implement DW_OP_convert

[Lldb-commits] [lldb] r371532 - Implement DW_OP_convert

[Lldb-commits] [PATCH] D67369: Implement DW_OP_convert

[Lldb-commits] [PATCH] D67376: [DWARF] Evaluate DW_OP_entry_value

[Lldb-commits] [PATCH] D67376: [DWARF] Evaluate DW_OP_entry_value

[Lldb-commits] [lldb] r371544 - [lldbtest] Add an "expected_cmd_failure" option to the filecheck helper

[Lldb-commits] [lldb] r371543 - [Function] Factor out GetCallEdgeForReturnAddress, NFC

[Lldb-commits] [PATCH] D67390: [LLDB][ELF] Load both, .symtab and .dynsym sections

[Lldb-commits] [PATCH] D67390: [LLDB][ELF] Load both, .symtab and .dynsym sections

[Lldb-commits] [PATCH] D67390: [LLDB][ELF] Load both, .symtab and .dynsym sections

[Lldb-commits] [PATCH] D67390: [LLDB][ELF] Load both, .symtab and .dynsym sections

[Lldb-commits] [PATCH] D67390: [LLDB][ELF] Load both, .symtab and .dynsym sections

[Lldb-commits] [PATCH] D67376: [DWARF] Evaluate DW_OP_entry_value

[Lldb-commits] [PATCH] D67390: [LLDB][ELF] Load both, .symtab and .dynsym sections

[Lldb-commits] [PATCH] D67390: [LLDB][ELF] Load both, .symtab and .dynsym sections

[Lldb-commits] [PATCH] D67111: Adding caching to libc++ std::function formatter for lookups that require scanning symbols

[Lldb-commits] [lldb] r371560 - Fix a thinko in handling the QSetLogging packet.

[Lldb-commits] [PATCH] D67111: Adding caching to libc++ std::function formatter for lookups that require scanning symbols

[Lldb-commits] [PATCH] D67111: Adding caching to libc++ std::function formatter for lookups that require scanning symbols

[Lldb-commits] [PATCH] D67376: [DWARF] Evaluate DW_OP_entry_value

[Lldb-commits] [PATCH] D67376: [DWARF] Evaluate DW_OP_entry_value

[Lldb-commits] [PATCH] D67111: Adding caching to libc++ std::function formatter for lookups that require scanning symbols

[Lldb-commits] [PATCH] D67111: Adding caching to libc++ std::function formatter for lookups that require scanning symbols

[Lldb-commits] [PATCH] D67111: Adding caching to libc++ std::function formatter for lookups that require scanning symbols

[Lldb-commits] [PATCH] D67427: [Plugins/Process] Remove direct use of ClangASTContext from InferiorCallPOSIX

[Lldb-commits] [lldb] r371582 - Skip a test in TestProcessIO.py when running against ios devices.

[Lldb-commits] [lldb] r371583 - Ah, only skip this for embedded darwin targets.

[Lldb-commits] [PATCH] D67376: [DWARF] Evaluate DW_OP_entry_value

[Lldb-commits] [PATCH] D67376: [DWARF] Evaluate DW_OP_entry_value

[Lldb-commits] [PATCH] D67376: [DWARF] Evaluate DW_OP_entry_value

[Lldb-commits] [PATCH] D67376: [DWARF] Evaluate DW_OP_entry_value

36 matches

Site Navigation

Mail list logo

Footer information