Re: [Lldb-commits] [PATCH] D49980: [PDB] Parse UDT symbols and pointers to members (combined patch)

2018-08-22 Thread Zachary Turner via lldb-commits
On Wed, Aug 22, 2018 at 12:34 AM Aleksandr Urakov via Phabricator <
revi...@reviews.llvm.org> wrote:

> aleksandr.urakov added a subscriber: labath.
> aleksandr.urakov added a comment.
>
> It seems that the cause of the failure is
> https://reviews.llvm.org/rL340206, but I'm not sure if the adding of
> `-gpubnames` switch to `clang` will be a correct fix for that?
>
>
> Repository:
>   rL LLVM
>
> https://reviews.llvm.org/D49980
>
>
>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D49980: [PDB] Parse UDT symbols and pointers to members (combined patch)

2018-08-22 Thread Aleksandr Urakov via Phabricator via lldb-commits
aleksandr.urakov added a subscriber: labath.
aleksandr.urakov added a comment.

It seems that the cause of the failure is https://reviews.llvm.org/rL340206, 
but I'm not sure if the adding of `-gpubnames` switch to `clang` will be a 
correct fix for that?


Repository:
  rL LLVM

https://reviews.llvm.org/D49980



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


[Lldb-commits] [PATCH] D49980: [PDB] Parse UDT symbols and pointers to members (combined patch)

2018-08-21 Thread Stella Stamenova via Phabricator via lldb-commits
stella.stamenova added a comment.

In https://reviews.llvm.org/D49980#1208096, @aleksandr.urakov wrote:

> It's also possible that the test has been failing since some commit in 
> another repository (e.g. `clang`, `lld` or `llvm`). It seems that it was ok 
> several days ago...


I think you are right. I've been trying to isolate when the failure started 
happening and it appears to be one of the changes made on Aug. 20 in one of the 
repositories (still trying to narrow that down).


Repository:
  rL LLVM

https://reviews.llvm.org/D49980



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


[Lldb-commits] [PATCH] D49980: [PDB] Parse UDT symbols and pointers to members (combined patch)

2018-08-21 Thread Aleksandr Urakov via Phabricator via lldb-commits
aleksandr.urakov added a comment.

It's also possible that the test has been failing since some commit in another 
repository (e.g. `clang`, `lld` or `llvm`). It seems that it was ok several 
days ago...


Repository:
  rL LLVM

https://reviews.llvm.org/D49980



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


[Lldb-commits] [PATCH] D49980: [PDB] Parse UDT symbols and pointers to members (combined patch)

2018-08-21 Thread Aleksandr Urakov via Phabricator via lldb-commits
aleksandr.urakov added a comment.

Yes, I've seen today that this test is failing now, but if I'm not mistaken 
this have happened some later than this patch was committed. But I'm not sure, 
I'll look at this more precisely tomorrow, thanks!


Repository:
  rL LLVM

https://reviews.llvm.org/D49980



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


[Lldb-commits] [PATCH] D49980: [PDB] Parse UDT symbols and pointers to members (combined patch)

2018-08-21 Thread Stella Stamenova via Phabricator via lldb-commits
stella.stamenova added a comment.

I am not 100% sure yet, but this change is the most likely culprit for one of 
the SymbolFile tests to start failing, namely: 
SymbolFile/DWARF/dwarf5-index-is-used.cpp. This fails for us on both Windows 
and Linux with the following:

  FAIL: lldb :: SymbolFile/DWARF/dwarf5-index-is-used.cpp (42214 of 43490)
   TEST 'lldb :: SymbolFile/DWARF/dwarf5-index-is-used.cpp' 
FAILED 
  Script:
  --
  : 'RUN: at line 5';   clang 
/vstsdrive/_work/11/s/llvm/tools/lldb/lit/SymbolFile/DWARF/dwarf5-index-is-used.cpp
 -g -c -o 
/vstsdrive/_work/11/b/LLVMBuild/tools/lldb/lit/SymbolFile/DWARF/Output/dwarf5-index-is-used.cpp.tmp.o
 --target=x86_64-pc-linux -mllvm -accel-tables=Dwarf
  : 'RUN: at line 6';   ld.lld 
/vstsdrive/_work/11/b/LLVMBuild/tools/lldb/lit/SymbolFile/DWARF/Output/dwarf5-index-is-used.cpp.tmp.o
 -o 
/vstsdrive/_work/11/b/LLVMBuild/tools/lldb/lit/SymbolFile/DWARF/Output/dwarf5-index-is-used.cpp.tmp
  : 'RUN: at line 7';   /vstsdrive/_work/11/b/LLVMBuild/bin/lldb-test symbols 
/vstsdrive/_work/11/b/LLVMBuild/tools/lldb/lit/SymbolFile/DWARF/Output/dwarf5-index-is-used.cpp.tmp
 | /vstsdrive/_work/11/b/LLVMBuild/bin/FileCheck 
/vstsdrive/_work/11/s/llvm/tools/lldb/lit/SymbolFile/DWARF/dwarf5-index-is-used.cpp
  --
  Exit Code: 1
  
  Command Output (stderr):
  --
  
/vstsdrive/_work/11/s/llvm/tools/lldb/lit/SymbolFile/DWARF/dwarf5-index-is-used.cpp:9:11:
 error: CHECK: expected string not found in input
  // CHECK: Name Index
^
  :1:1: note: scanning from here
  Module: 
/vstsdrive/_work/11/b/LLVMBuild/tools/lldb/lit/SymbolFile/DWARF/Output/dwarf5-index-is-used.cpp.tmp
  ^
  :36:5: note: possible intended match here
  IDX name type flags addr offset size link info addralgn entsize Name
  ^
  
  --
  
  

I tried looking through the results from the bots to see if the test is passing 
online and as far as I can tell, none of the bots that are currently online 
have ran it or if they did, the results got lost because of the failures from 
the new vscode tests.


Repository:
  rL LLVM

https://reviews.llvm.org/D49980



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


[Lldb-commits] [PATCH] D49980: [PDB] Parse UDT symbols and pointers to members (combined patch)

2018-08-17 Thread Aleksandr Urakov via Phabricator via lldb-commits
aleksandr.urakov added a comment.

Thanks for catching that!

I didn't know about that warnings because I build LLVM with MSVC compiler, but 
it doesn't introduce such warnings. Now I've fixed the problem with 
https://reviews.llvm.org/rLLDB339994.


Repository:
  rL LLVM

https://reviews.llvm.org/D49980



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


[Lldb-commits] [PATCH] D49980: [PDB] Parse UDT symbols and pointers to members (combined patch)

2018-08-16 Thread Raphael Isemann via Phabricator via lldb-commits
teemperor added a comment.

@aleksandr.urakov Not sure if you are already working of this, but just FYI 
this patch introduced a few compiler warnings:

  
/Users/teemperor/llvm/sidestuff/llvm/tools/lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp:55:3:
 warning: default label in switch which covers all enumeration values 
[-Wcovered-switch-default]   
default:

 
^   

 
  
/Users/teemperor/llvm/sidestuff/llvm/tools/lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp:216:3:
 warning: default label in switch which covers all enumeration values 
[-Wcovered-switch-default]  
default:

 
^   

 
  
/Users/teemperor/llvm/sidestuff/llvm/tools/lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp:229:3:
 warning: default label in switch which covers all enumeration values 
[-Wcovered-switch-default]  
default:

 
^   

 
  3 warnings generated.
   20% [125/610] Building CXX object 
tools/lldb/source/Symbol/CMakeFiles/lldbSymbol.dir/ClangASTContext.cpp.o

   
  
/Users/teemperor/llvm/sidestuff/llvm/tools/lldb/source/Symbol/ClangASTContext.cpp:6547:45:
 warning: comparison of integers of different signs: 'const int32_t' (aka 
'const int') and 'unsigned int' [-Wsig$-compare]
  if (base_offset != UINT32_MAX) {
  ~~~ ^  ~~
  
/Users/teemperor/llvm/sidestuff/llvm/tools/lldb/source/Symbol/ClangASTContext.cpp:6760:34:
 warning: comparison of integers of different signs: 'int32_t' (aka 'int') and 
'unsigned int' [-Wsign-compare]  
if (bit_offset == UINT32_MAX)
~~ ^  ~~
  2 warnings generated.


Repository:
  rL LLVM

https://reviews.llvm.org/D49980



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


[Lldb-commits] [PATCH] D49980: [PDB] Parse UDT symbols and pointers to members (combined patch)

2018-08-14 Thread Aleksandr Urakov via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL339649: [PDB] Parse UDT symbols and pointers to members 
(combined patch) (authored by aleksandr.urakov, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D49980?vs=158194=160521#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D49980

Files:
  lldb/trunk/include/lldb/Symbol/ClangASTContext.h
  lldb/trunk/lit/SymbolFile/PDB/Inputs/ClassLayoutTest.cpp
  lldb/trunk/lit/SymbolFile/PDB/Inputs/PointerTypeTest.cpp
  lldb/trunk/lit/SymbolFile/PDB/Inputs/UdtLayoutTest.cpp
  lldb/trunk/lit/SymbolFile/PDB/Inputs/UdtLayoutTest.script
  lldb/trunk/lit/SymbolFile/PDB/class-layout.test
  lldb/trunk/lit/SymbolFile/PDB/pointers.test
  lldb/trunk/lit/SymbolFile/PDB/udt-layout.test
  lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/trunk/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
  lldb/trunk/source/Plugins/SymbolFile/PDB/PDBASTParser.h
  lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
  lldb/trunk/source/Symbol/ClangASTContext.cpp

Index: lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
===
--- lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
+++ lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
@@ -45,7 +45,7 @@
 #include "llvm/DebugInfo/PDB/PDBSymbolTypeTypedef.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolTypeUDT.h"
 
-#include "Plugins/Language/CPlusPlus/CPlusPlusLanguage.h"
+#include "Plugins/Language/CPlusPlus/CPlusPlusLanguage.h" // For IsCPPMangledName
 #include "Plugins/SymbolFile/PDB/PDBASTParser.h"
 #include "Plugins/SymbolFile/PDB/PDBLocationToDWARFExpression.h"
 
@@ -459,10 +459,13 @@
 
 // This should cause the type to get cached and stored in the `m_types`
 // lookup.
-if (!ResolveTypeUID(symbol->getSymIndexId()))
-  continue;
-
-++num_added;
+if (auto type = ResolveTypeUID(symbol->getSymIndexId())) {
+  // Resolve the type completely to avoid a completion
+  // (and so a list change, which causes an iterators invalidation)
+  // during a TypeList dumping
+  type->GetFullCompilerType();
+  ++num_added;
+}
   }
 }
   };
@@ -568,8 +571,20 @@
 }
 
 bool SymbolFilePDB::CompleteType(lldb_private::CompilerType _type) {
-  // TODO: Implement this
-  return false;
+  std::lock_guard guard(
+  GetObjectFile()->GetModule()->GetMutex());
+
+  ClangASTContext *clang_ast_ctx = llvm::dyn_cast_or_null(
+  GetTypeSystemForLanguage(lldb::eLanguageTypeC_plus_plus));
+  if (!clang_ast_ctx)
+return false;
+
+  PDBASTParser *pdb =
+  llvm::dyn_cast(clang_ast_ctx->GetPDBParser());
+  if (!pdb)
+return false;
+
+  return pdb->CompleteTypeFromPDB(compiler_type);
 }
 
 lldb_private::CompilerDecl SymbolFilePDB::GetDeclForUID(lldb::user_id_t uid) {
Index: lldb/trunk/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
===
--- lldb/trunk/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
+++ lldb/trunk/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
@@ -9,11 +9,15 @@
 
 #include "PDBASTParser.h"
 
+#include "SymbolFilePDB.h"
+
 #include "clang/AST/CharUnits.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclCXX.h"
 
+#include "lldb/Core/Module.h"
 #include "lldb/Symbol/ClangASTContext.h"
+#include "lldb/Symbol/ClangExternalASTSourceCommon.h"
 #include "lldb/Symbol/ClangUtil.h"
 #include "lldb/Symbol/Declaration.h"
 #include "lldb/Symbol/SymbolFile.h"
@@ -48,8 +52,9 @@
 return clang::TTK_Union;
   case PDB_UdtType::Interface:
 return clang::TTK_Interface;
+  default:
+llvm_unreachable("unsuported PDB UDT type");
   }
-  return -1;
 }
 
 lldb::Encoding TranslateBuiltinEncoding(PDB_BuiltinType type) {
@@ -199,6 +204,68 @@
   decl.SetLine(first_line_up->getLineNumber());
   return true;
 }
+
+AccessType TranslateMemberAccess(PDB_MemberAccess access) {
+  switch (access) {
+  case PDB_MemberAccess::Private:
+return eAccessPrivate;
+  case PDB_MemberAccess::Protected:
+return eAccessProtected;
+  case PDB_MemberAccess::Public:
+return eAccessPublic;
+  default:
+return eAccessNone;
+  }
+}
+
+AccessType GetDefaultAccessibilityForUdtKind(PDB_UdtType udt_kind) {
+  switch (udt_kind) {
+  case PDB_UdtType::Struct:
+  case PDB_UdtType::Union:
+return eAccessPublic;
+  case PDB_UdtType::Class:
+  case PDB_UdtType::Interface:
+return eAccessPrivate;
+  default:
+llvm_unreachable("unsupported PDB UDT type");
+  }
+}
+
+AccessType GetAccessibilityForUdt(const PDBSymbolTypeUDT ) {
+  AccessType access = TranslateMemberAccess(udt.getAccess());
+  if (access != lldb::eAccessNone || !udt.isNested())
+return access;
+
+  auto parent = udt.getClassParent();
+  if (!parent)
+return lldb::eAccessNone;
+
+  auto parent_udt = 

[Lldb-commits] [PATCH] D49980: [PDB] Parse UDT symbols and pointers to members (combined patch)

2018-08-14 Thread Aleksandr Urakov via Phabricator via lldb-commits
aleksandr.urakov added a comment.

Thanks a lot!


https://reviews.llvm.org/D49980



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


[Lldb-commits] [PATCH] D49980: [PDB] Parse UDT symbols and pointers to members (combined patch)

2018-08-13 Thread Zachary Turner via Phabricator via lldb-commits
zturner added a comment.

Ok I’ll take a look later today then when i get in


https://reviews.llvm.org/D49980



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


Re: [Lldb-commits] [PATCH] D49980: [PDB] Parse UDT symbols and pointers to members (combined patch)

2018-08-13 Thread Zachary Turner via lldb-commits
Ok I’ll take a look later today then when i get in
On Mon, Aug 13, 2018 at 2:13 AM Aleksandr Urakov via Phabricator <
revi...@reviews.llvm.org> wrote:

> aleksandr.urakov added a comment.
>
> Unfortunately, there was no people yet, who can review this :)
>
> Ping! Can anyone review this, please?
>
>
> https://reviews.llvm.org/D49980
>
>
>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D49980: [PDB] Parse UDT symbols and pointers to members (combined patch)

2018-08-13 Thread Aleksandr Urakov via Phabricator via lldb-commits
aleksandr.urakov added a comment.

Unfortunately, there was no people yet, who can review this :)

Ping! Can anyone review this, please?


https://reviews.llvm.org/D49980



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


Re: [Lldb-commits] [PATCH] D49980: [PDB] Parse UDT symbols and pointers to members (combined patch)

2018-08-06 Thread Zachary Turner via lldb-commits
I am OOO this week and only have access to mobile, so hopefully someone
else can review it
On Mon, Aug 6, 2018 at 11:05 AM Aleksandr Urakov via Phabricator <
revi...@reviews.llvm.org> wrote:

> aleksandr.urakov added a comment.
>
> Ping!
>
> Can you review this, please?
>
>
> https://reviews.llvm.org/D49980
>
>
>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D49980: [PDB] Parse UDT symbols and pointers to members (combined patch)

2018-08-06 Thread Zachary Turner via Phabricator via lldb-commits
zturner added a subscriber: aleksandr.urakov.
zturner added a comment.

I am OOO this week and only have access to mobile, so hopefully someone
else can review it


https://reviews.llvm.org/D49980



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


[Lldb-commits] [PATCH] D49980: [PDB] Parse UDT symbols and pointers to members (combined patch)

2018-08-06 Thread Aleksandr Urakov via Phabricator via lldb-commits
aleksandr.urakov added a comment.

Ping!

Can you review this, please?


https://reviews.llvm.org/D49980



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


[Lldb-commits] [PATCH] D49980: [PDB] Parse UDT symbols and pointers to members (combined patch)

2018-07-31 Thread Aleksandr Urakov via Phabricator via lldb-commits
aleksandr.urakov updated this revision to Diff 158194.
aleksandr.urakov added a comment.

Added support of a MSInheritance attribute.


https://reviews.llvm.org/D49980

Files:
  include/lldb/Symbol/ClangASTContext.h
  lit/SymbolFile/PDB/Inputs/ClassLayoutTest.cpp
  lit/SymbolFile/PDB/Inputs/PointerTypeTest.cpp
  lit/SymbolFile/PDB/Inputs/UdtLayoutTest.cpp
  lit/SymbolFile/PDB/Inputs/UdtLayoutTest.script
  lit/SymbolFile/PDB/class-layout.test
  lit/SymbolFile/PDB/pointers.test
  lit/SymbolFile/PDB/udt-layout.test
  source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
  source/Plugins/SymbolFile/PDB/PDBASTParser.h
  source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
  source/Symbol/ClangASTContext.cpp

Index: source/Symbol/ClangASTContext.cpp
===
--- source/Symbol/ClangASTContext.cpp
+++ source/Symbol/ClangASTContext.cpp
@@ -124,6 +124,84 @@
  // Use Clang for D until there is a proper language plugin for it
  language == eLanguageTypeD;
 }
+
+// Checks whether m1 is an overload of m2 (as opposed to an override). This is
+// called by addOverridesForMethod to distinguish overrides (which share a
+// vtable entry) from overloads (which require distinct entries).
+bool isOverload(clang::CXXMethodDecl *m1, clang::CXXMethodDecl *m2) {
+  // FIXME: This should detect covariant return types, but currently doesn't.
+  lldbassert(>getASTContext() == >getASTContext() &&
+ "Methods should have the same AST context");
+  clang::ASTContext  = m1->getASTContext();
+
+  const auto *m1Type = llvm::cast(
+  context.getCanonicalType(m1->getType()));
+
+  const auto *m2Type = llvm::cast(
+  context.getCanonicalType(m2->getType()));
+
+  auto compareArgTypes = [](const clang::QualType ,
+const clang::QualType ) {
+return context.hasSameType(m1p.getUnqualifiedType(),
+   m2p.getUnqualifiedType());
+  };
+
+  // FIXME: In C++14 and later, we can just pass m2Type->param_type_end()
+  //as a fourth parameter to std::equal().
+  return (m1->getNumParams() != m2->getNumParams()) ||
+ !std::equal(m1Type->param_type_begin(), m1Type->param_type_end(),
+ m2Type->param_type_begin(), compareArgTypes);
+}
+
+// If decl is a virtual method, walk the base classes looking for methods that
+// decl overrides. This table of overridden methods is used by IRGen to
+// determine the vtable layout for decl's parent class.
+void addOverridesForMethod(clang::CXXMethodDecl *decl) {
+  if (!decl->isVirtual())
+return;
+
+  clang::CXXBasePaths paths;
+
+  auto find_overridden_methods =
+  [decl](const clang::CXXBaseSpecifier *specifier,
+ clang::CXXBasePath ) {
+if (auto *base_record = llvm::dyn_cast(
+specifier->getType()->getAs()->getDecl())) {
+
+  clang::DeclarationName name = decl->getDeclName();
+
+  // If this is a destructor, check whether the base class destructor is
+  // virtual.
+  if (name.getNameKind() == clang::DeclarationName::CXXDestructorName)
+if (auto *baseDtorDecl = base_record->getDestructor()) {
+  if (baseDtorDecl->isVirtual()) {
+path.Decls = baseDtorDecl;
+return true;
+  } else
+return false;
+}
+
+  // Otherwise, search for name in the base class.
+  for (path.Decls = base_record->lookup(name); !path.Decls.empty();
+   path.Decls = path.Decls.slice(1)) {
+if (auto *method_decl =
+llvm::dyn_cast(path.Decls.front()))
+  if (method_decl->isVirtual() && !isOverload(decl, method_decl)) {
+path.Decls = method_decl;
+return true;
+  }
+  }
+}
+
+return false;
+  };
+
+  if (decl->getParent()->lookupInBases(find_overridden_methods, paths)) {
+for (auto *overridden_decl : paths.found_decls())
+  decl->addOverriddenMethod(
+  llvm::cast(overridden_decl));
+  }
+}
 }
 
 typedef lldb_private::ThreadSafeDenseMap
@@ -6373,7 +6451,7 @@
   language_flags = 0;
 
   const bool idx_is_valid = idx < GetNumChildren(type, omit_empty_base_classes);
-  uint32_t bit_offset;
+  int32_t bit_offset;
   switch (parent_type_class) {
   case clang::Type::Builtin:
 if (idx_is_valid) {
@@ -6465,8 +6543,8 @@
 cxx_record_decl, base_class_decl);
 const lldb::addr_t base_offset_addr =
 vbtable_ptr + vbtable_index * 4;
-const uint32_t base_offset =
-process->ReadUnsignedIntegerFromMemory(
+const int32_t base_offset =
+process->ReadSignedIntegerFromMemory(
  

[Lldb-commits] [PATCH] D49980: [PDB] Parse UDT symbols and pointers to members (combined patch)

2018-07-30 Thread Aleksandr Urakov via Phabricator via lldb-commits
aleksandr.urakov added inline comments.



Comment at: lit/SymbolFile/PDB/udt-layout.test:1-51
+REQUIRES: windows
+RUN: clang-cl /Zi %S/Inputs/UdtLayoutTest.cpp /o %t.exe
+RUN: %lldb -b -s %S/Inputs/UdtLayoutTest.script -- %t.exe | FileCheck %s
+
+CHECK:(int) int C::abc = 123
+CHECK:(List [16]) ls = {
+CHECK:  [15] = {

I've preserved this Windows-only test (but also have included other 
non-execution tests, which may become cross-platform some later, as Pavel has 
mentioned at D49410), but if you'll find it unnecessary I'll remove it.


https://reviews.llvm.org/D49980



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


[Lldb-commits] [PATCH] D49980: [PDB] Parse UDT symbols and pointers to members (combined patch)

2018-07-30 Thread Aleksandr Urakov via Phabricator via lldb-commits
aleksandr.urakov created this revision.
aleksandr.urakov added reviewers: asmith, zturner, rnk, labath, clayborg, 
lldb-commits.
Herald added a subscriber: JDevlieghere.

In this patch I've tried to combine the best ideas from 
https://reviews.llvm.org/D49368 and https://reviews.llvm.org/D49410, so it 
implements following:

- Completion of UDTs from a PDB with a filling of a layout info;
- Pointers to members;
- Fixes the bug relating to a virtual base offset reading from `vbtable`. The 
offset was treated as an unsigned, but it can be a negative sometimes.


https://reviews.llvm.org/D49980

Files:
  include/lldb/Symbol/ClangASTContext.h
  lit/SymbolFile/PDB/Inputs/ClassLayoutTest.cpp
  lit/SymbolFile/PDB/Inputs/PointerTypeTest.cpp
  lit/SymbolFile/PDB/Inputs/UdtLayoutTest.cpp
  lit/SymbolFile/PDB/Inputs/UdtLayoutTest.script
  lit/SymbolFile/PDB/class-layout.test
  lit/SymbolFile/PDB/pointers.test
  lit/SymbolFile/PDB/udt-layout.test
  source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
  source/Plugins/SymbolFile/PDB/PDBASTParser.h
  source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
  source/Symbol/ClangASTContext.cpp

Index: source/Symbol/ClangASTContext.cpp
===
--- source/Symbol/ClangASTContext.cpp
+++ source/Symbol/ClangASTContext.cpp
@@ -124,6 +124,84 @@
  // Use Clang for D until there is a proper language plugin for it
  language == eLanguageTypeD;
 }
+
+// Checks whether m1 is an overload of m2 (as opposed to an override). This is
+// called by addOverridesForMethod to distinguish overrides (which share a
+// vtable entry) from overloads (which require distinct entries).
+bool isOverload(clang::CXXMethodDecl *m1, clang::CXXMethodDecl *m2) {
+  // FIXME: This should detect covariant return types, but currently doesn't.
+  lldbassert(>getASTContext() == >getASTContext() &&
+ "Methods should have the same AST context");
+  clang::ASTContext  = m1->getASTContext();
+
+  const auto *m1Type = llvm::cast(
+  context.getCanonicalType(m1->getType()));
+
+  const auto *m2Type = llvm::cast(
+  context.getCanonicalType(m2->getType()));
+
+  auto compareArgTypes = [](const clang::QualType ,
+const clang::QualType ) {
+return context.hasSameType(m1p.getUnqualifiedType(),
+   m2p.getUnqualifiedType());
+  };
+
+  // FIXME: In C++14 and later, we can just pass m2Type->param_type_end()
+  //as a fourth parameter to std::equal().
+  return (m1->getNumParams() != m2->getNumParams()) ||
+ !std::equal(m1Type->param_type_begin(), m1Type->param_type_end(),
+ m2Type->param_type_begin(), compareArgTypes);
+}
+
+// If decl is a virtual method, walk the base classes looking for methods that
+// decl overrides. This table of overridden methods is used by IRGen to
+// determine the vtable layout for decl's parent class.
+void addOverridesForMethod(clang::CXXMethodDecl *decl) {
+  if (!decl->isVirtual())
+return;
+
+  clang::CXXBasePaths paths;
+
+  auto find_overridden_methods =
+  [decl](const clang::CXXBaseSpecifier *specifier,
+ clang::CXXBasePath ) {
+if (auto *base_record = llvm::dyn_cast(
+specifier->getType()->getAs()->getDecl())) {
+
+  clang::DeclarationName name = decl->getDeclName();
+
+  // If this is a destructor, check whether the base class destructor is
+  // virtual.
+  if (name.getNameKind() == clang::DeclarationName::CXXDestructorName)
+if (auto *baseDtorDecl = base_record->getDestructor()) {
+  if (baseDtorDecl->isVirtual()) {
+path.Decls = baseDtorDecl;
+return true;
+  } else
+return false;
+}
+
+  // Otherwise, search for name in the base class.
+  for (path.Decls = base_record->lookup(name); !path.Decls.empty();
+   path.Decls = path.Decls.slice(1)) {
+if (auto *method_decl =
+llvm::dyn_cast(path.Decls.front()))
+  if (method_decl->isVirtual() && !isOverload(decl, method_decl)) {
+path.Decls = method_decl;
+return true;
+  }
+  }
+}
+
+return false;
+  };
+
+  if (decl->getParent()->lookupInBases(find_overridden_methods, paths)) {
+for (auto *overridden_decl : paths.found_decls())
+  decl->addOverriddenMethod(
+  llvm::cast(overridden_decl));
+  }
+}
 }
 
 typedef lldb_private::ThreadSafeDenseMap
@@ -6373,7 +6451,7 @@
   language_flags = 0;
 
   const bool idx_is_valid = idx < GetNumChildren(type, omit_empty_base_classes);
-  uint32_t bit_offset;
+  int32_t bit_offset;
   switch (parent_type_class) {
   case clang::Type::Builtin:
 if (idx_is_valid) {
@@ -6465,8 +6543,8 @@