[Lldb-commits] [PATCH] D56124: PECOFF: Fix section name computation

2018-12-28 Thread Pavel Labath via Phabricator via lldb-commits 
labath created this revision.
labath added reviewers: zturner, stella.stamenova.
Herald added a subscriber: abidh.

  If a section name is exactly 8 bytes long (or has been truncated to 8
  bytes), it will not contain the terminating nul character. This means
  reading the name as a c string will pick up random data following the
  name field (which happens to be the section vm size).
  
  This fixes the name computation to avoid out-of-bounds access and adds a
  test.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D56124

Files:
  lit/Modules/PECOFF/sections-names.yaml
  source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
  source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h

Index: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
===
--- source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
+++ source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
@@ -270,7 +270,7 @@
   void DumpSectionHeader(lldb_private::Stream *s, const section_header_t &sh);
   void DumpDependentModules(lldb_private::Stream *s);
 
-  bool GetSectionName(std::string §_name, const section_header_t §);
+  std::string GetSectionName(const section_header_t §);
 
   typedef std::vector SectionHeaderColl;
   typedef SectionHeaderColl::iterator SectionHeaderCollIter;
Index: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
===
--- source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -530,22 +530,20 @@
   return !m_sect_headers.empty();
 }
 
-bool ObjectFilePECOFF::GetSectionName(std::string §_name,
-  const section_header_t §) {
-  if (sect.name[0] == '/') {
-lldb::offset_t stroff = strtoul(§.name[1], NULL, 10);
+std::string ObjectFilePECOFF::GetSectionName(const section_header_t §) {
+  llvm::StringRef hdr_name(sect.name, llvm::array_lengthof(sect.name));
+  hdr_name = hdr_name.split('\0').first;
+  if (hdr_name.consume_front("/")) {
+lldb::offset_t stroff;
+if (!to_integer(hdr_name, stroff, 10))
+  return "";
 lldb::offset_t string_file_offset =
 m_coff_header.symoff + (m_coff_header.nsyms * 18) + stroff;
-const char *name = m_data.GetCStr(&string_file_offset);
-if (name) {
-  sect_name = name;
-  return true;
-}
-
-return false;
+if (const char *name = m_data.GetCStr(&string_file_offset))
+  return name;
+return "";
   }
-  sect_name = sect.name;
-  return true;
+  return hdr_name;
 }
 
 //--
@@ -712,8 +710,7 @@
 const uint32_t nsects = m_sect_headers.size();
 ModuleSP module_sp(GetModule());
 for (uint32_t idx = 0; idx < nsects; ++idx) {
-  std::string sect_name;
-  GetSectionName(sect_name, m_sect_headers[idx]);
+  std::string sect_name = GetSectionName(m_sect_headers[idx]);
   ConstString const_sect_name(sect_name.c_str());
   static ConstString g_code_sect_name(".code");
   static ConstString g_CODE_sect_name("CODE");
@@ -1087,8 +1084,7 @@
 //--
 void ObjectFilePECOFF::DumpSectionHeader(Stream *s,
  const section_header_t &sh) {
-  std::string name;
-  GetSectionName(name, sh);
+  std::string name = GetSectionName(sh);
   s->Printf("%-16s 0x%8.8x 0x%8.8x 0x%8.8x 0x%8.8x 0x%8.8x 0x%8.8x 0x%4.4x "
 "0x%4.4x 0x%8.8x\n",
 name.c_str(), sh.vmaddr, sh.vmsize, sh.offset, sh.size, sh.reloff,
Index: lit/Modules/PECOFF/sections-names.yaml
===
--- /dev/null
+++ lit/Modules/PECOFF/sections-names.yaml
@@ -0,0 +1,52 @@
+# RUN: yaml2obj %s > %t
+# RUN: lldb-test object-file %t | FileCheck %s
+
+# CHECK: Name: .text{{$}}
+# CHECK: Name: 1234567{{$}}
+# CHECK: Name: 12345678{{$}}
+# CHECK: Name: 123456789{{$}}
+
+--- !COFF
+OptionalHeader:
+  AddressOfEntryPoint: 4616
+  ImageBase:   1073741824
+  SectionAlignment: 4096
+  FileAlignment:   512
+  MajorOperatingSystemVersion: 6
+  MinorOperatingSystemVersion: 0
+  MajorImageVersion: 0
+  MinorImageVersion: 0
+  MajorSubsystemVersion: 6
+  MinorSubsystemVersion: 0
+  Subsystem:   IMAGE_SUBSYSTEM_WINDOWS_CUI
+  DLLCharacteristics: [ IMAGE_DLL_CHARACTERISTICS_HIGH_ENTROPY_VA, IMAGE_DLL_CHARACTERISTICS_DYNAMIC_BASE, IMAGE_DLL_CHARACTERISTICS_NX_COMPAT, IMAGE_DLL_CHARACTERISTICS_TERMINAL_SERVER_AWARE ]
+  SizeOfStackReserve: 1048576
+  SizeOfStackCommit: 4096
+  SizeOfHeapReserve: 1048576
+  SizeOfHeapCommit: 4096
+header:
+  Machine: IMAGE_FILE_MACHINE_AMD64
+  Characteristics: [ IMAGE_FILE_EXECUTABLE_IMAGE, IMAGE_FILE_LARGE_ADDRESS_AWARE ]
+sections:
+  - Name:.text
+Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ]
+VirtualAddress:  4096
+Virt

[Lldb-commits] [PATCH] D56126: [NativePDB] Add basic support of methods recostruction in AST

2018-12-28 Thread Aleksandr Urakov via Phabricator via lldb-commits 
aleksandr.urakov created this revision.
aleksandr.urakov added reviewers: zturner, labath, lemo, stella.stamenova.
aleksandr.urakov added a project: LLDB.
Herald added subscribers: lldb-commits, teemperor, abidh.

This patch adds the basic support of methods reconstruction by native PDB 
plugin. It contains only most obvious changes (it processes `LF_ONEMETHOD` and 
`LF_METHOD` records), some things still remain unsolved:

- mangled names retrieving;
- support of template methods (they are not presented as `LF_ONEMETHOD` records 
at all).

This info is contained in the `Symbols` stream, not in the `TPI` stream. As far 
as I understand, we can't find in a simple way the link between a 
`LF_ONEMETHOD` record and a corresponding `S_GPROC32` record. I think it's the 
place, where we need to use the approach similar to D54053 
 (to parse mangled names and to build some 
part of AST based on this info). That's what I'm planning to implement next. Do 
you have any objections on this? Or may be there's a better way to solve this?


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D56126

Files:
  lit/SymbolFile/NativePDB/Inputs/ast-methods.lldbinit
  lit/SymbolFile/NativePDB/ast-methods.cpp
  source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
  source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.h
  source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
  source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.h

Index: source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.h
===
--- source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.h
+++ source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.h
@@ -67,6 +67,10 @@
 private:
   clang::QualType AddBaseClassForTypeIndex(llvm::codeview::TypeIndex ti,
llvm::codeview::MemberAccess access);
+  void AddMethod(llvm::StringRef name, llvm::codeview::TypeIndex type_idx,
+ llvm::codeview::MemberAccess access,
+ llvm::codeview::MethodOptions options,
+ llvm::codeview::MemberAttributes attrs);
 };
 
 } // namespace npdb
Index: source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
===
--- source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
+++ source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
@@ -65,6 +65,22 @@
   return qt;
 }
 
+void UdtRecordCompleter::AddMethod(llvm::StringRef name, TypeIndex type_idx,
+   MemberAccess access, MethodOptions options,
+   MemberAttributes attrs) {
+  clang::QualType method_qt =
+  m_ast_builder.GetOrCreateType(PdbTypeSymId(type_idx));
+  m_ast_builder.CompleteType(method_qt);
+
+  lldb::AccessType access_type = TranslateMemberAccess(access);
+  bool is_artificial = (options & MethodOptions::CompilerGenerated) ==
+   MethodOptions::CompilerGenerated;
+  m_ast_builder.clang().AddMethodToCXXRecordType(
+  m_derived_ct.GetOpaqueQualType(), name.data(), nullptr,
+  m_ast_builder.ToCompilerType(method_qt), access_type, attrs.isVirtual(),
+  attrs.isStatic(), false, false, false, is_artificial);
+}
+
 Error UdtRecordCompleter::visitKnownMember(CVMemberRecord &cvr,
BaseClassRecord &base) {
   clang::QualType base_qt =
@@ -158,11 +174,27 @@
 
 Error UdtRecordCompleter::visitKnownMember(CVMemberRecord &cvr,
OneMethodRecord &one_method) {
+  AddMethod(one_method.Name, one_method.Type, one_method.getAccess(),
+one_method.getOptions(), one_method.Attrs);
+
   return Error::success();
 }
 
 Error UdtRecordCompleter::visitKnownMember(CVMemberRecord &cvr,
OverloadedMethodRecord &overloaded) {
+  TypeIndex method_list_idx = overloaded.MethodList;
+
+  CVType method_list_type = m_tpi.getType(method_list_idx);
+  assert(method_list_type.Type == LF_METHODLIST);
+
+  MethodOverloadListRecord method_list;
+  llvm::cantFail(TypeDeserializer::deserializeAs(
+  method_list_type, method_list));
+
+  for (const OneMethodRecord &method : method_list.Methods)
+AddMethod(overloaded.Name, method.Type, method.getAccess(),
+  method.getOptions(), method.Attrs);
+
   return Error::success();
 }
 
Index: source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.h
===
--- source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.h
+++ source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.h
@@ -100,7 +100,8 @@
   clang::QualType CreateEnumType(PdbTypeSymId id,
  const llvm::codeview::EnumRecord &record);
   clang::QualType
-  CreateProcedureType(const llvm::codeview::ProcedureRecord &proc);
+  CreateFunctionType(TypeIndex args_type_idx, TypeIndex return_type_idx,
+

[Lldb-commits] [PATCH] D56129: Simplify ObjectFile::GetArchitecture

2018-12-28 Thread Pavel Labath via Phabricator via lldb-commits 
labath created this revision.
labath added reviewers: clayborg, zturner.
Herald added subscribers: JDevlieghere, arichardson, emaste.
Herald added a reviewer: espindola.

instead of returning the architecture through by-ref argument and a
boolean value indicating success, we can just return the ArchSpec
directly. Since the ArchSpec already has an invalid state, it can be
used to denote the failure without the additional bool.


https://reviews.llvm.org/D56129

Files:
  include/lldb/Core/Module.h
  include/lldb/Expression/IRExecutionUnit.h
  include/lldb/Symbol/ObjectFile.h
  include/lldb/Symbol/UnwindTable.h
  include/lldb/Utility/ArchSpec.h
  source/Core/Module.cpp
  source/Expression/IRExecutionUnit.cpp
  source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.cpp
  source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.h
  source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  source/Plugins/ObjectFile/ELF/ObjectFileELF.h
  source/Plugins/ObjectFile/JIT/ObjectFileJIT.cpp
  source/Plugins/ObjectFile/JIT/ObjectFileJIT.h
  source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
  source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h
  source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
  source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
  source/Plugins/Process/elf-core/ProcessElfCore.cpp
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  source/Symbol/CompactUnwindInfo.cpp
  source/Symbol/DWARFCallFrameInfo.cpp
  source/Symbol/FuncUnwinders.cpp
  source/Symbol/Type.cpp
  source/Symbol/UnwindTable.cpp

Index: source/Symbol/UnwindTable.cpp
===
--- source/Symbol/UnwindTable.cpp
+++ source/Symbol/UnwindTable.cpp
@@ -180,8 +180,8 @@
   return m_arm_unwind_up.get();
 }
 
-bool UnwindTable::GetArchitecture(lldb_private::ArchSpec &arch) {
-  return m_object_file.GetArchitecture(arch);
+ArchSpec UnwindTable::GetArchitecture() {
+  return m_object_file.GetArchitecture();
 }
 
 bool UnwindTable::GetAllowAssemblyEmulationUnwindPlans() {
Index: source/Symbol/Type.cpp
===
--- source/Symbol/Type.cpp
+++ source/Symbol/Type.cpp
@@ -328,8 +328,7 @@
 case eEncodingIsPointerUID:
 case eEncodingIsLValueReferenceUID:
 case eEncodingIsRValueReferenceUID: {
-  ArchSpec arch;
-  if (m_symbol_file->GetObjectFile()->GetArchitecture(arch))
+  if (ArchSpec arch = m_symbol_file->GetObjectFile()->GetArchitecture())
 m_byte_size = arch.GetAddressByteSize();
 } break;
 }
Index: source/Symbol/FuncUnwinders.cpp
===
--- source/Symbol/FuncUnwinders.cpp
+++ source/Symbol/FuncUnwinders.cpp
@@ -443,8 +443,7 @@
 lldb::UnwindAssemblySP
 FuncUnwinders::GetUnwindAssemblyProfiler(Target &target) {
   UnwindAssemblySP assembly_profiler_sp;
-  ArchSpec arch;
-  if (m_unwind_table.GetArchitecture(arch)) {
+  if (ArchSpec arch = m_unwind_table.GetArchitecture()) {
 arch.MergeFrom(target.GetArchitecture());
 assembly_profiler_sp = UnwindAssembly::FindPlugin(arch);
   }
Index: source/Symbol/DWARFCallFrameInfo.cpp
===
--- source/Symbol/DWARFCallFrameInfo.cpp
+++ source/Symbol/DWARFCallFrameInfo.cpp
@@ -423,8 +423,7 @@
  m_objfile.GetFileSpec().GetFilename().AsCString(""));
 
   bool clear_address_zeroth_bit = false;
-  ArchSpec arch;
-  if (m_objfile.GetArchitecture(arch)) {
+  if (ArchSpec arch = m_objfile.GetArchitecture()) {
 if (arch.GetTriple().getArch() == llvm::Triple::arm ||
 arch.GetTriple().getArch() == llvm::Triple::thumb)
   clear_address_zeroth_bit = true;
Index: source/Symbol/CompactUnwindInfo.cpp
===
--- source/Symbol/CompactUnwindInfo.cpp
+++ source/Symbol/CompactUnwindInfo.cpp
@@ -183,8 +183,7 @@
 if (function_info.encoding == 0)
   return false;
 
-ArchSpec arch;
-if (m_objfile.GetArchitecture(arch)) {
+if (ArchSpec arch = m_objfile.GetArchitecture()) {
 
   Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_UNWIND));
   if (log && log->GetVerbose()) {
@@ -338,8 +337,7 @@
 // };
 
 bool clear_address_zeroth_bit = false;
-ArchSpec arch;
-if (m_objfile.GetArchitecture(arch)) {
+if (ArchSpec arch = m_objfile.GetArchitecture()) {
   if (arch.GetTriple().getArch() == llvm::Triple::arm ||
   arch.GetTriple().getArch() == llvm::Triple::thumb)
 clear_address_zeroth_bit = true;
Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -1084,9 +1084,7 @@
* #0
* for MIPS. Use ArchSpec to clear the bit #0.
   */
-  ArchSpec arch;
-  GetObjectFile()->GetArchitecture(arch

[Lldb-commits] [lldb] r350122 - Fix signed-unsigned comparisons in MinidumpParserTest

2018-12-28 Thread Pavel Labath via lldb-commits 
Author: labath
Date: Fri Dec 28 05:34:50 2018
New Revision: 350122

URL: http://llvm.org/viewvc/llvm-project?rev=350122&view=rev
Log:
Fix signed-unsigned comparisons in MinidumpParserTest

Modified:
lldb/trunk/unittests/Process/minidump/MinidumpParserTest.cpp

Modified: lldb/trunk/unittests/Process/minidump/MinidumpParserTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Process/minidump/MinidumpParserTest.cpp?rev=350122&r1=350121&r2=350122&view=diff
==
--- lldb/trunk/unittests/Process/minidump/MinidumpParserTest.cpp (original)
+++ lldb/trunk/unittests/Process/minidump/MinidumpParserTest.cpp Fri Dec 28 
05:34:50 2018
@@ -636,8 +636,8 @@ TEST_F(MinidumpParserTest, MinidumpDupli
   // one time, we pick the one with the lowest base_of_image.
   std::vector filtered_modules =
   parser->GetFilteredModuleList();
-  EXPECT_EQ(1, filtered_modules.size());
-  EXPECT_EQ(0x1000, filtered_modules[0]->base_of_image);
+  EXPECT_EQ(1u, filtered_modules.size());
+  EXPECT_EQ(0x1000u, filtered_modules[0]->base_of_image);
 }
 
 TEST_F(MinidumpParserTest, MinidumpModuleOrder) {
@@ -652,12 +652,12 @@ TEST_F(MinidumpParserTest, MinidumpModul
   std::vector filtered_modules =
   parser->GetFilteredModuleList();
   llvm::Optional name;
-  EXPECT_EQ(2, filtered_modules.size());
-  EXPECT_EQ(0x2000, filtered_modules[0]->base_of_image);
+  EXPECT_EQ(2u, filtered_modules.size());
+  EXPECT_EQ(0x2000u, filtered_modules[0]->base_of_image);
   name = parser->GetMinidumpString(filtered_modules[0]->module_name_rva);
   ASSERT_TRUE((bool)name);
   EXPECT_EQ(std::string("/tmp/a"), *name);
-  EXPECT_EQ(0x1000, filtered_modules[1]->base_of_image);
+  EXPECT_EQ(0x1000u, filtered_modules[1]->base_of_image);
   name = parser->GetMinidumpString(filtered_modules[1]->module_name_rva);
   ASSERT_TRUE((bool)name);
   EXPECT_EQ(std::string("/tmp/b"), *name);


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

[Lldb-commits] [lldb] r350121 - Remove unused variable from ClangASTContext

2018-12-28 Thread Pavel Labath via lldb-commits 
Author: labath
Date: Fri Dec 28 05:34:44 2018
New Revision: 350121

URL: http://llvm.org/viewvc/llvm-project?rev=350121&view=rev
Log:
Remove unused variable from ClangASTContext

Modified:
lldb/trunk/source/Symbol/ClangASTContext.cpp

Modified: lldb/trunk/source/Symbol/ClangASTContext.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Symbol/ClangASTContext.cpp?rev=350121&r1=350120&r2=350121&view=diff
==
--- lldb/trunk/source/Symbol/ClangASTContext.cpp (original)
+++ lldb/trunk/source/Symbol/ClangASTContext.cpp Fri Dec 28 05:34:44 2018
@@ -5533,9 +5533,8 @@ GetDynamicArrayInfo(ClangASTContext &ast
 const ExecutionContext *exe_ctx) {
   if (qual_type->isIncompleteArrayType())
 if (auto *metadata = ast.GetMetadata(qual_type.getAsOpaquePtr()))
-  if (auto *dwarf_parser = ast.GetDWARFParser())
-return sym_file->GetDynamicArrayInfoForUID(metadata->GetUserID(),
-   exe_ctx);
+  return sym_file->GetDynamicArrayInfoForUID(metadata->GetUserID(),
+ exe_ctx);
   return llvm::None;
 }
 


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

[Lldb-commits] [PATCH] D56132: Symtab: Remove one copy of symbol size computation code

2018-12-28 Thread Pavel Labath via Phabricator via lldb-commits 
labath created this revision.
labath added reviewers: clayborg, jasonmolenda, tberghammer.

The implementation in CalculateSymbolSizes has been made redundant in
D19004 , as this patch added another copy of 
size computation code into
InitAddressIndexes (which is called by CalculateSymbolSizes).


https://reviews.llvm.org/D56132

Files:
  source/Symbol/Symtab.cpp


Index: source/Symbol/Symtab.cpp
===
--- source/Symbol/Symtab.cpp
+++ source/Symbol/Symtab.cpp
@@ -976,32 +976,8 @@
 
 void Symtab::CalculateSymbolSizes() {
   std::lock_guard guard(m_mutex);
-
-  if (!m_symbols.empty()) {
-if (!m_file_addr_to_index_computed)
-  InitAddressIndexes();
-
-const size_t num_entries = m_file_addr_to_index.GetSize();
-
-for (size_t i = 0; i < num_entries; ++i) {
-  // The entries in the m_file_addr_to_index have calculated the sizes
-  // already so we will use this size if we need to.
-  const FileRangeToIndexMap::Entry &entry =
-  m_file_addr_to_index.GetEntryRef(i);
-
-  Symbol &symbol = m_symbols[entry.data];
-
-  // If the symbol size is already valid, no need to do anything
-  if (symbol.GetByteSizeIsValid())
-continue;
-
-  const addr_t range_size = entry.GetByteSize();
-  if (range_size > 0) {
-symbol.SetByteSize(range_size);
-symbol.SetSizeIsSynthesized(true);
-  }
-}
-  }
+  // Size computation happens inside InitAddressIndexes.
+  InitAddressIndexes();
 }
 
 Symbol *Symtab::FindSymbolAtFileAddress(addr_t file_addr) {


Index: source/Symbol/Symtab.cpp
===
--- source/Symbol/Symtab.cpp
+++ source/Symbol/Symtab.cpp
@@ -976,32 +976,8 @@
 
 void Symtab::CalculateSymbolSizes() {
   std::lock_guard guard(m_mutex);
-
-  if (!m_symbols.empty()) {
-if (!m_file_addr_to_index_computed)
-  InitAddressIndexes();
-
-const size_t num_entries = m_file_addr_to_index.GetSize();
-
-for (size_t i = 0; i < num_entries; ++i) {
-  // The entries in the m_file_addr_to_index have calculated the sizes
-  // already so we will use this size if we need to.
-  const FileRangeToIndexMap::Entry &entry =
-  m_file_addr_to_index.GetEntryRef(i);
-
-  Symbol &symbol = m_symbols[entry.data];
-
-  // If the symbol size is already valid, no need to do anything
-  if (symbol.GetByteSizeIsValid())
-continue;
-
-  const addr_t range_size = entry.GetByteSize();
-  if (range_size > 0) {
-symbol.SetByteSize(range_size);
-symbol.SetSizeIsSynthesized(true);
-  }
-}
-  }
+  // Size computation happens inside InitAddressIndexes.
+  InitAddressIndexes();
 }
 
 Symbol *Symtab::FindSymbolAtFileAddress(addr_t file_addr) {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D56124: PECOFF: Fix section name computation

2018-12-28 Thread Zachary Turner via Phabricator via lldb-commits 
zturner added a comment.

What about just making this function return a StringRef?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D56124



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

[Lldb-commits] [PATCH] D56124: PECOFF: Fix section name computation

2018-12-28 Thread Pavel Labath via Phabricator via lldb-commits 
labath updated this revision to Diff 179644.
labath added a comment.

An excellent idea. Updating to use StringRef.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D56124

Files:
  lit/Modules/PECOFF/sections-names.yaml
  source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
  source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h

Index: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
===
--- source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
+++ source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
@@ -270,7 +270,7 @@
   void DumpSectionHeader(lldb_private::Stream *s, const section_header_t &sh);
   void DumpDependentModules(lldb_private::Stream *s);
 
-  bool GetSectionName(std::string §_name, const section_header_t §);
+  llvm::StringRef GetSectionName(const section_header_t §);
 
   typedef std::vector SectionHeaderColl;
   typedef SectionHeaderColl::iterator SectionHeaderCollIter;
Index: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
===
--- source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -530,22 +530,20 @@
   return !m_sect_headers.empty();
 }
 
-bool ObjectFilePECOFF::GetSectionName(std::string §_name,
-  const section_header_t §) {
-  if (sect.name[0] == '/') {
-lldb::offset_t stroff = strtoul(§.name[1], NULL, 10);
+llvm::StringRef ObjectFilePECOFF::GetSectionName(const section_header_t §) {
+  llvm::StringRef hdr_name(sect.name, llvm::array_lengthof(sect.name));
+  hdr_name = hdr_name.split('\0').first;
+  if (hdr_name.consume_front("/")) {
+lldb::offset_t stroff;
+if (!to_integer(hdr_name, stroff, 10))
+  return "";
 lldb::offset_t string_file_offset =
 m_coff_header.symoff + (m_coff_header.nsyms * 18) + stroff;
-const char *name = m_data.GetCStr(&string_file_offset);
-if (name) {
-  sect_name = name;
-  return true;
-}
-
-return false;
+if (const char *name = m_data.GetCStr(&string_file_offset))
+  return name;
+return "";
   }
-  sect_name = sect.name;
-  return true;
+  return hdr_name;
 }
 
 //--
@@ -712,9 +710,7 @@
 const uint32_t nsects = m_sect_headers.size();
 ModuleSP module_sp(GetModule());
 for (uint32_t idx = 0; idx < nsects; ++idx) {
-  std::string sect_name;
-  GetSectionName(sect_name, m_sect_headers[idx]);
-  ConstString const_sect_name(sect_name.c_str());
+  ConstString const_sect_name(GetSectionName(m_sect_headers[idx]));
   static ConstString g_code_sect_name(".code");
   static ConstString g_CODE_sect_name("CODE");
   static ConstString g_data_sect_name(".data");
@@ -1087,8 +1083,7 @@
 //--
 void ObjectFilePECOFF::DumpSectionHeader(Stream *s,
  const section_header_t &sh) {
-  std::string name;
-  GetSectionName(name, sh);
+  std::string name = GetSectionName(sh);
   s->Printf("%-16s 0x%8.8x 0x%8.8x 0x%8.8x 0x%8.8x 0x%8.8x 0x%8.8x 0x%4.4x "
 "0x%4.4x 0x%8.8x\n",
 name.c_str(), sh.vmaddr, sh.vmsize, sh.offset, sh.size, sh.reloff,
Index: lit/Modules/PECOFF/sections-names.yaml
===
--- /dev/null
+++ lit/Modules/PECOFF/sections-names.yaml
@@ -0,0 +1,52 @@
+# RUN: yaml2obj %s > %t
+# RUN: lldb-test object-file %t | FileCheck %s
+
+# CHECK: Name: .text{{$}}
+# CHECK: Name: 1234567{{$}}
+# CHECK: Name: 12345678{{$}}
+# CHECK: Name: 123456789{{$}}
+
+--- !COFF
+OptionalHeader:
+  AddressOfEntryPoint: 4616
+  ImageBase:   1073741824
+  SectionAlignment: 4096
+  FileAlignment:   512
+  MajorOperatingSystemVersion: 6
+  MinorOperatingSystemVersion: 0
+  MajorImageVersion: 0
+  MinorImageVersion: 0
+  MajorSubsystemVersion: 6
+  MinorSubsystemVersion: 0
+  Subsystem:   IMAGE_SUBSYSTEM_WINDOWS_CUI
+  DLLCharacteristics: [ IMAGE_DLL_CHARACTERISTICS_HIGH_ENTROPY_VA, IMAGE_DLL_CHARACTERISTICS_DYNAMIC_BASE, IMAGE_DLL_CHARACTERISTICS_NX_COMPAT, IMAGE_DLL_CHARACTERISTICS_TERMINAL_SERVER_AWARE ]
+  SizeOfStackReserve: 1048576
+  SizeOfStackCommit: 4096
+  SizeOfHeapReserve: 1048576
+  SizeOfHeapCommit: 4096
+header:
+  Machine: IMAGE_FILE_MACHINE_AMD64
+  Characteristics: [ IMAGE_FILE_EXECUTABLE_IMAGE, IMAGE_FILE_LARGE_ADDRESS_AWARE ]
+sections:
+  - Name:.text
+Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ]
+VirtualAddress:  4096
+VirtualSize: 64 # '@', if it makes its way into the name field
+SectionData: DEADBEEFBAADF00D
+  - Name:1234567
+Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ ]
+VirtualAddre

[Lldb-commits] [PATCH] D56126: [NativePDB] Add basic support of methods recostruction in AST

2018-12-28 Thread Zachary Turner via Phabricator via lldb-commits 
zturner added a comment.

> This info is contained in the Symbols stream, not in the TPI stream. As far 
> as I understand, we can't find in a simple way the link between a 
> LF_ONEMETHOD record and a corresponding S_GPROC32 record. I think it's the 
> place, where we need to use the approach similar to D54053 
>  (to parse mangled names and to build some 
> part of AST based on this info). That's what I'm planning to implement next. 
> Do you have any objections on this? Or may be there's a better way to solve 
> this?

I think it's probably best to skip this part for now and come back to it later. 
 The only thing that will be missing is the ability to use member function 
templates in expressions.  Of course we need this eventually, but probably 
there is more useful stuff to work on first.

One idea for implementing this though might be to add a pre-processing step of 
the publics stream similar to how we pre-process the TPI stream.  This would 
also allow us to find mangled names of global functions as well (currently even 
for global functions, we create an empty `Mangled` structure when LLDB asks us 
for the mangled name).  One pass over the publics stream should be simpler and 
more straightforward than a pass over every single module's symbol stream.  
From there we have the address, which tells us the module, and then we can use 
the `CompilandIndexItem::FindSymbolsByVa()` to find the `S_GPROC32` record, and 
from there find the type.  But I think this is a large effort for low value, so 
we should maybe wait until the easier stuff is done first.  I haven't looked at 
the rest of the patch yet, but I will in a little bit.  Thanks!


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D56126



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

[Lldb-commits] [lldb] r350146 - [SymbolContext] Rewrite operator== to be more concise.

2018-12-28 Thread Davide Italiano via lldb-commits 
Author: davide
Date: Fri Dec 28 20:57:00 2018
New Revision: 350146

URL: http://llvm.org/viewvc/llvm-project?rev=350146&view=rev
Log:
[SymbolContext] Rewrite operator== to be more concise.

And probably, less error prone. NFCI.

Modified:
lldb/trunk/source/Symbol/SymbolContext.cpp

Modified: lldb/trunk/source/Symbol/SymbolContext.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Symbol/SymbolContext.cpp?rev=350146&r1=350145&r2=350146&view=diff
==
--- lldb/trunk/source/Symbol/SymbolContext.cpp (original)
+++ lldb/trunk/source/Symbol/SymbolContext.cpp Fri Dec 28 20:57:00 2018
@@ -393,12 +393,7 @@ bool lldb_private::operator==(const Symb
 
 bool lldb_private::operator!=(const SymbolContext &lhs,
   const SymbolContext &rhs) {
-  return lhs.function != rhs.function || lhs.symbol != rhs.symbol ||
- lhs.module_sp.get() != rhs.module_sp.get() ||
- lhs.comp_unit != rhs.comp_unit ||
- lhs.target_sp.get() != rhs.target_sp.get() ||
- LineEntry::Compare(lhs.line_entry, rhs.line_entry) != 0 ||
- lhs.variable != rhs.variable;
+  return !(lhs == rhs);
 }
 
 bool SymbolContext::GetAddressRange(uint32_t scope, uint32_t range_idx,


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

[Lldb-commits] [lldb] r350147 - [CompilerType] Remove dead code. NFCI.

2018-12-28 Thread Davide Italiano via lldb-commits 
Author: davide
Date: Fri Dec 28 20:59:07 2018
New Revision: 350147

URL: http://llvm.org/viewvc/llvm-project?rev=350147&view=rev
Log:
[CompilerType] Remove dead code. NFCI.

Modified:
lldb/trunk/source/Symbol/CompilerType.cpp

Modified: lldb/trunk/source/Symbol/CompilerType.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Symbol/CompilerType.cpp?rev=350147&r1=350146&r2=350147&view=diff
==
--- lldb/trunk/source/Symbol/CompilerType.cpp (original)
+++ lldb/trunk/source/Symbol/CompilerType.cpp Fri Dec 28 20:59:07 2018
@@ -1058,16 +1058,6 @@ bool CompilerType::WriteToMemory(lldb_pr
   return false;
 }
 
-// clang::CXXRecordDecl *
-// CompilerType::GetAsCXXRecordDecl (lldb::opaque_compiler_type_t
-// opaque_compiler_qual_type)
-//{
-//if (opaque_compiler_qual_type)
-//return
-//
clang::QualType::getFromOpaquePtr(opaque_compiler_qual_type)->getAsCXXRecordDecl();
-//return NULL;
-//}
-
 bool lldb_private::operator==(const lldb_private::CompilerType &lhs,
   const lldb_private::CompilerType &rhs) {
   return lhs.GetTypeSystem() == rhs.GetTypeSystem() &&


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

[Lldb-commits] [lldb] r350148 - [CompilerType] Simplify operator!=. NFCI.

2018-12-28 Thread Davide Italiano via lldb-commits 
Author: davide
Date: Fri Dec 28 21:00:33 2018
New Revision: 350148

URL: http://llvm.org/viewvc/llvm-project?rev=350148&view=rev
Log:
[CompilerType] Simplify operator!=. NFCI.

Modified:
lldb/trunk/source/Symbol/CompilerType.cpp

Modified: lldb/trunk/source/Symbol/CompilerType.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Symbol/CompilerType.cpp?rev=350148&r1=350147&r2=350148&view=diff
==
--- lldb/trunk/source/Symbol/CompilerType.cpp (original)
+++ lldb/trunk/source/Symbol/CompilerType.cpp Fri Dec 28 21:00:33 2018
@@ -1066,6 +1066,5 @@ bool lldb_private::operator==(const lldb
 
 bool lldb_private::operator!=(const lldb_private::CompilerType &lhs,
   const lldb_private::CompilerType &rhs) {
-  return lhs.GetTypeSystem() != rhs.GetTypeSystem() ||
- lhs.GetOpaqueQualType() != rhs.GetOpaqueQualType();
+  return !(lhs == rhs);
 }


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

[Lldb-commits] [lldb] r350149 - [RegisterValue] Rewrite operator!= in terms of operator==. NFCI.

2018-12-28 Thread Davide Italiano via lldb-commits 
Author: davide
Date: Fri Dec 28 21:05:23 2018
New Revision: 350149

URL: http://llvm.org/viewvc/llvm-project?rev=350149&view=rev
Log:
[RegisterValue] Rewrite operator!= in terms of operator==. NFCI.

Modified:
lldb/trunk/source/Utility/RegisterValue.cpp

Modified: lldb/trunk/source/Utility/RegisterValue.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Utility/RegisterValue.cpp?rev=350149&r1=350148&r2=350149&view=diff
==
--- lldb/trunk/source/Utility/RegisterValue.cpp (original)
+++ lldb/trunk/source/Utility/RegisterValue.cpp Fri Dec 28 21:05:23 2018
@@ -793,32 +793,7 @@ bool RegisterValue::operator==(const Reg
 }
 
 bool RegisterValue::operator!=(const RegisterValue &rhs) const {
-  if (m_type != rhs.m_type)
-return true;
-  switch (m_type) {
-  case eTypeInvalid:
-return false;
-  case eTypeUInt8:
-  case eTypeUInt16:
-  case eTypeUInt32:
-  case eTypeUInt64:
-  case eTypeUInt128:
-  case eTypeFloat:
-  case eTypeDouble:
-  case eTypeLongDouble:
-return m_scalar != rhs.m_scalar;
-  case eTypeBytes:
-if (buffer.length != rhs.buffer.length) {
-  return true;
-} else {
-  uint8_t length = buffer.length;
-  if (length > kMaxRegisterByteSize)
-length = kMaxRegisterByteSize;
-  return memcmp(buffer.bytes, rhs.buffer.bytes, length) != 0;
-}
-break;
-  }
-  return true;
+  return !(*this == rhs);
 }
 
 bool RegisterValue::ClearBit(uint32_t bit) {


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

[Lldb-commits] [PATCH] D54747: Discard debuginfo for object files empty after GC

2018-12-28 Thread Robert O'Callahan via Phabricator via lldb-commits 
rocallahan added a comment.

Sorry for the delayed reply. I just discovered that Phabricator nofications 
were being buried by my mail filters.

In D54747#1312161 , @ruiu wrote:

> What you are doing in this patch is not too complicated and makes sense to 
> me. That said, if actual size saving is not significant as you said in 
> https://github.com/rust-lang/rust/issues/56068#issuecomment-440160568, it may 
> not be worth doing. It seems to me that if debug info is already 2.4GB, 
> shrinking it to 2GB doesn't make much difference.


17% saving for a pretty small and conservative patch seems good to me, but I 
guess it's subjective.

In D54747#1315241 , @ruiu wrote:

> rocallahan, can I ask why Rust passes all object files to the linker and use 
> `-gc-sections` to eliminate unused part of these files?


I'm not a Rust compiler contributor, but my guess is developer ergonomics. Once 
upon a time users manually divided code into translation units, each producing 
an object, file and garbage collection occurred only at the granularity of 
object files. Later `--gc-sections` and `-ffunction-sections` let compilers 
automatically subdivide object files into smaller units and GC with a 
granularity of functions. In Rust each translation unit is a whole module 
(crate); users aren't expected to subdivide libraries into smaller units for GC 
purposes, and in fact aren't able to. Emitting one section per function mostly 
means they don't need to ... except it breaks down with DWARF here.

> One possible way to fix it is to pass `--start-lib` to the linker before any 
> object file paths in the command line. That option gives archive file 
> semantics to object files, so if you option, each object file is treated as 
> if that were in an archive file containing that the single object file. (Note 
> that `--start-lib` is a positional option just like `--start-group`, so all 
> files between `--start-lib` and `--end-lib` get that special treament.)

I will try that and follow up but I don't see how it would make any difference.


Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D54747



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