[Lldb-commits] [PATCH] D41962: Fix TestYMMRegisters for older machines without AVX2

2018-01-12 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

In https://reviews.llvm.org/D41962#973827, @craig.topper wrote:

> I don't know what platforms this needs to support. But __builtin_cpu_support 
> only works when compiled with clang or gcc. And it requires compiler-rt or 
> libgcc. I don't know if that's guaranteed to exist on Windows.


I doubt this test was ever passing on windows, as our RegisterContextWindows 
does not even acknowledge the existence of sse registers. If we wanted to be 
fancy, we could do some manual cpuid parsing here (the test contains inline 
assembly anyway), but that's probably not necessary.




Comment at: 
packages/Python/lldbsuite/test/functionalities/register/intel_avx/main.c:21
+  static volatile unsigned haveAVX2;
+  haveAVX2 = __builtin_cpu_supports("avx2");
   unsigned int ymmallones = 0x;

lebedev.ri wrote:
> Note that you need to call `__builtin_cpu_init()` before calling 
> `__builtin_cpu_supports()`.
> Or maybe it is already called before this?
Gcc manual says:
```This built-in (__builtin_cpu_init) function needs to be invoked ..., only 
when used in a function that is executed before any constructors are called. ```

So calling it here should not be necessary. 

However, I am still unable to get gcc (6.3) to return 1 here. Clang (since at 
least 3.8) seems to be doing fine however, so that's probably enough for this 
test.


Repository:
  rL LLVM

https://reviews.llvm.org/D41962



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


[Lldb-commits] [PATCH] D41962: Fix TestYMMRegisters for older machines without AVX2

2018-01-12 Thread Jason Molenda via Phabricator via lldb-commits
jasonmolenda added a comment.

fwiw I'm working on upstreaming on zmm (avx512) patches that we have locally 
(there's one testsuite fail I still need to find time to fix) and the 
TestZMMRegister.py test that ChrisB wrote to test this is written as 
skip-unless-darwin, and there's a new skipUnlessFeature() method added to 
decorators.py which runs sysctl to detect hardware features (in this case, 
hw.optional.avx512f) which, I suspect, is an even more mac-specific way of 
doing this.  While Adrian's approach would be gcc/clang specific, it would def 
be better than depending on a sysctl.


Repository:
  rL LLVM

https://reviews.llvm.org/D41962



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


[Lldb-commits] [PATCH] D41962: Fix TestYMMRegisters for older machines without AVX2

2018-01-12 Thread Jason Molenda via Phabricator via lldb-commits
jasonmolenda added a comment.

I suppose a possible alternative would be to figure out the avx2 / avx512 
features manually based on the cpuid instead of letting the compiler do it for 
us.  e.g. 
https://stackoverflow.com/questions/1666093/cpuid-implementations-in-c and then 
checking the bits as e.g. described in https://en.wikipedia.org/wiki/CPUID .  
Bummer to do it so low level if we can delegate this to the compiler though.


Repository:
  rL LLVM

https://reviews.llvm.org/D41962



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


[Lldb-commits] [PATCH] D41962: Fix TestYMMRegisters for older machines without AVX2

2018-01-12 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added a comment.

In https://reviews.llvm.org/D41962#974656, @jasonmolenda wrote:

> I suppose a possible alternative would be to figure out the avx2 / avx512 
> features manually based on the cpuid instead of letting the compiler do it 
> for us.  e.g. 
> https://stackoverflow.com/questions/1666093/cpuid-implementations-in-c and 
> then checking the bits as e.g. described in 
> https://en.wikipedia.org/wiki/CPUID .  Bummer to do it so low level if we can 
> delegate this to the compiler though.


I don't know MSVC well enough and don't have access to one to test it but: This 
would also only work if there were a compiler-independent way of writing inline 
assembler. Is that possible?

Other fun facts: Clang doesn't even define `__builtin_cpu_init()`.


Repository:
  rL LLVM

https://reviews.llvm.org/D41962



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


[Lldb-commits] [PATCH] D41962: Fix TestYMMRegisters for older machines without AVX2

2018-01-12 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl updated this revision to Diff 129639.
aprantl added a comment.

Uploaded a mildly better version that is NFC on MSVC.


https://reviews.llvm.org/D41962

Files:
  
packages/Python/lldbsuite/test/functionalities/register/intel_avx/TestYMMRegister.py
  packages/Python/lldbsuite/test/functionalities/register/intel_avx/main.c


Index: packages/Python/lldbsuite/test/functionalities/register/intel_avx/main.c
===
--- packages/Python/lldbsuite/test/functionalities/register/intel_avx/main.c
+++ packages/Python/lldbsuite/test/functionalities/register/intel_avx/main.c
@@ -6,7 +6,6 @@
 // License. See LICENSE.TXT for details.
 //
 
//===--===//
-
 void func() {
   unsigned int ymmvalues[16];
   unsigned char val;
@@ -17,6 +16,14 @@
 ymmvalues[i] = (val << 24) | (val << 16) | (val << 8) | val;
   }
 
+  // Detect AVX2.
+  static volatile unsigned haveAVX2;
+#ifdef _MSC_VER
+  haveAVX2 = 1; // TODO: Implement this for MSVC.
+#else
+  haveAVX2 = __builtin_cpu_supports("avx2");
+#endif
+
   unsigned int ymmallones = 0x;
   __asm__("int3;"
   "vbroadcastss %1, %%ymm0;"
Index: 
packages/Python/lldbsuite/test/functionalities/register/intel_avx/TestYMMRegister.py
===
--- 
packages/Python/lldbsuite/test/functionalities/register/intel_avx/TestYMMRegister.py
+++ 
packages/Python/lldbsuite/test/functionalities/register/intel_avx/TestYMMRegister.py
@@ -51,6 +51,10 @@
 break
 self.assertTrue(matched, STOPPED_DUE_TO_SIGNAL)
 
+# Detect AVX2 support and early exit otherwise.
+if self.frame().FindVariable("haveAVX2").GetValue() == "0":
+  return False
+
 if self.getArchitecture() == 'x86_64':
 register_range = 16
 else:


Index: packages/Python/lldbsuite/test/functionalities/register/intel_avx/main.c
===
--- packages/Python/lldbsuite/test/functionalities/register/intel_avx/main.c
+++ packages/Python/lldbsuite/test/functionalities/register/intel_avx/main.c
@@ -6,7 +6,6 @@
 // License. See LICENSE.TXT for details.
 //
 //===--===//
-
 void func() {
   unsigned int ymmvalues[16];
   unsigned char val;
@@ -17,6 +16,14 @@
 ymmvalues[i] = (val << 24) | (val << 16) | (val << 8) | val;
   }
 
+  // Detect AVX2.
+  static volatile unsigned haveAVX2;
+#ifdef _MSC_VER
+  haveAVX2 = 1; // TODO: Implement this for MSVC.
+#else
+  haveAVX2 = __builtin_cpu_supports("avx2");
+#endif
+
   unsigned int ymmallones = 0x;
   __asm__("int3;"
   "vbroadcastss %1, %%ymm0;"
Index: packages/Python/lldbsuite/test/functionalities/register/intel_avx/TestYMMRegister.py
===
--- packages/Python/lldbsuite/test/functionalities/register/intel_avx/TestYMMRegister.py
+++ packages/Python/lldbsuite/test/functionalities/register/intel_avx/TestYMMRegister.py
@@ -51,6 +51,10 @@
 break
 self.assertTrue(matched, STOPPED_DUE_TO_SIGNAL)
 
+# Detect AVX2 support and early exit otherwise.
+if self.frame().FindVariable("haveAVX2").GetValue() == "0":
+  return False
+
 if self.getArchitecture() == 'x86_64':
 register_range = 16
 else:
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D41962: Fix TestYMMRegisters for older machines without AVX2

2018-01-12 Thread Craig Topper via Phabricator via lldb-commits
craig.topper added a comment.

__builtin_cpu_init was added to clang between 5.0 and 6.0


https://reviews.llvm.org/D41962



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


[Lldb-commits] [PATCH] D41997: Build virtual override tables in DWARFASTParserClang::CompleteTypeFromDWARF

2018-01-12 Thread Lang Hames via Phabricator via lldb-commits
lhames created this revision.
lhames added a reviewer: davide.
lhames added a project: LLDB.
Herald added subscribers: llvm-commits, JDevlieghere.

Failure to build the method override tables results in expression failures on 
the following trivial test case:

  class Base {
  public:
virtual ~Base() {}
virtual void foo() {}
  };
  
  class Derived : public Base {
  public:
virtual void foo() {}
  };
  
  int main() {
Derived d;
Base *b = &d;
return 0; // "expr b->foo()" ok. "expr d.foo()" crashes. 
  }

The reason is that without an overrides table, the definition of foo in derived 
is treated as a new method definition (rather than an override) and allocated 
its own vtable entry which does not exist in the vtable of Derived in the 
compiled program.


Repository:
  rL LLVM

https://reviews.llvm.org/D41997

Files:
  Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  Python/lldbsuite/test/expression_command/call-overridden-method/Makefile
  
Python/lldbsuite/test/expression_command/call-overridden-method/TestCallOverriddenMethod.py
  Python/lldbsuite/test/expression_command/call-overridden-method/main.cpp

Index: Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -40,6 +40,7 @@
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/StreamString.h"
 
+#include "clang/AST/CXXInheritance.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclObjC.h"
 
@@ -2099,6 +2100,84 @@
   return template_param_infos.args.size() == template_param_infos.names.size();
 }
 
+static bool isOverload(clang::CXXMethodDecl *m1, clang::CXXMethodDecl *m2) {
+  // FIXME: This should detect covariant return types, but currently doesn't.
+  assert(&m1->getASTContext() == &m2->getASTContext() &&
+ "Methods should have the same AST context");
+  clang::ASTContext &context = m1->getASTContext();
+
+  const auto *m1Type =
+llvm::cast(
+  context.getCanonicalType(m1->getType()));
+
+  const auto *m2Type =
+llvm::cast(
+  context.getCanonicalType(m2->getType()));
+
+  auto compareArgTypes =
+[&context](const clang::QualType &m1p, const clang::QualType &m2p) {
+  return context.hasSameType(m1p.getUnqualifiedType(),
+ m2p.getUnqualifiedType());
+};
+
+  return !std::equal(m1Type->param_type_begin(), m1Type->param_type_end(),
+ m2Type->param_type_begin(), compareArgTypes);
+}
+
+static void addOverridesForMethod(clang::CXXMethodDecl *decl) {
+  if (!decl->isVirtual())
+return;
+
+  clang::CXXBasePaths paths;
+
+  auto find_overridden_methods =
+[decl](const clang::CXXBaseSpecifier *specifier, clang::CXXBasePath &path) {
+  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));
+  }
+}
+
+static void addMethodOverrides(ClangASTContext &ast, CompilerType &clang_type) {
+  if (auto *record =
+  ast.GetAsCXXRecordDecl(clang_type.GetOpaqueQualType()))
+for (auto *method : record->methods())
+  addOverridesForMethod(method);
+}
+
 bool DWARFASTParserClang::CompleteTypeFromDWARF(const DWARFDIE &die,
 lldb_private::Type *type,
 CompilerType &clang_type) {
@@ -2311,6 +2390,7 @@
   }
 }
 
+addMethodOverrides(m_ast, clang_type);
 ClangASTContext::BuildIndirectFields(clang_type);
 ClangASTContext::CompleteTagDeclarationDefinition(clang_type);
 
Index: Python/lldbsuite/test/expression_command/call-overridden-method/main.cpp
===
--- /dev/null
+++ Python/lldbs

[Lldb-commits] [PATCH] D41997: Build virtual override tables in DWARFASTParserClang::CompleteTypeFromDWARF

2018-01-12 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added a comment.

Awesome!




Comment at: Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:2103
 
+static bool isOverload(clang::CXXMethodDecl *m1, clang::CXXMethodDecl *m2) {
+  // FIXME: This should detect covariant return types, but currently doesn't.

Could you add some doxygen comments explaining what the new function do and why 
doing this is necessary?



Comment at: 
Python/lldbsuite/test/expression_command/call-overridden-method/TestCallOverriddenMethod.py:14
+
+from __future__ import print_function
+

where is this used?



Comment at: 
Python/lldbsuite/test/expression_command/call-overridden-method/main.cpp:15
+  Base *b = &d;
+  return 0; // Please test these expressions while stopped at this line:
+}

the expressions are missing :-)
Perhaps convert this into an inline testcase?


Repository:
  rL LLVM

https://reviews.llvm.org/D41997



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


[Lldb-commits] [PATCH] D41997: Build virtual override tables in DWARFASTParserClang::CompleteTypeFromDWARF

2018-01-12 Thread Jim Ingham via Phabricator via lldb-commits
jingham added inline comments.



Comment at: 
Python/lldbsuite/test/expression_command/call-overridden-method/main.cpp:15
+  Base *b = &d;
+  return 0; // Please test these expressions while stopped at this line:
+}

aprantl wrote:
> the expressions are missing :-)
> Perhaps convert this into an inline testcase?
The expressions are in the test .py file.  It isn't necessary to put them here, 
I'd just fix the comment.

Please don't convert regular test cases into inline ones, however.  The benefit 
of inline test cases is mostly that they are easier to write, but they are 
harder to debug when they go wrong.  So if the are already in regular form I'd 
rather not convert them.


Repository:
  rL LLVM

https://reviews.llvm.org/D41997



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


Re: [Lldb-commits] [PATCH] D41962: Fix TestYMMRegisters for older machines without AVX2

2018-01-12 Thread Greg Clayton via lldb-commits
Why not just look for the AVX registers by name that are only available if they 
are correctly detected by the native lldb-server or debugserver? Then we can 
avoid all of this. If we don't execute any instructions that crash the program, 
we can stop before any specialized AVX instructions are executed and kill the 
program is we don't see a register by name? 

> On Jan 12, 2018, at 8:44 AM, Jason Molenda via Phabricator via lldb-commits 
>  wrote:
> 
> jasonmolenda added a comment.
> 
> I suppose a possible alternative would be to figure out the avx2 / avx512 
> features manually based on the cpuid instead of letting the compiler do it 
> for us.  e.g. 
> https://stackoverflow.com/questions/1666093/cpuid-implementations-in-c and 
> then checking the bits as e.g. described in 
> https://en.wikipedia.org/wiki/CPUID .  Bummer to do it so low level if we can 
> delegate this to the compiler though.
> 
> 
> Repository:
>  rL LLVM
> 
> https://reviews.llvm.org/D41962
> 
> 
> 
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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


[Lldb-commits] [PATCH] D41997: Build virtual override tables in DWARFASTParserClang::CompleteTypeFromDWARF

2018-01-12 Thread Greg Clayton via Phabricator via lldb-commits
clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.

Very cool and close. It would be nice to function correctly without asserts, 
see inlined comment.




Comment at: Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:2105-2106
+  // FIXME: This should detect covariant return types, but currently doesn't.
+  assert(&m1->getASTContext() == &m2->getASTContext() &&
+ "Methods should have the same AST context");
+  clang::ASTContext &context = m1->getASTContext();

Use lldb_assert and possibly return false afterwards in case the asserts are 
compiled out


Repository:
  rL LLVM

https://reviews.llvm.org/D41997



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


[Lldb-commits] [PATCH] D41997: Build virtual override tables in DWARFASTParserClang::CompleteTypeFromDWARF

2018-01-12 Thread Davide Italiano via Phabricator via lldb-commits
davide added a comment.

In https://reviews.llvm.org/D41997#974957, @clayborg wrote:

> Very cool and close. It would be nice to function correctly without asserts, 
> see inlined comment.


Out of curiosity, why does `lldb` roll its own assertion() mechanism instead of 
using the standard one?


Repository:
  rL LLVM

https://reviews.llvm.org/D41997



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


[Lldb-commits] [PATCH] D41962: Fix TestYMMRegisters for older machines without AVX2

2018-01-12 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added a comment.

> Why not just look for the AVX registers by name that are only available if 
> they are correctly detected by the native lldb-server or debugserver? Then we 
> can avoid all of this. If we don't execute any instructions that crash the 
> program, we can stop before any specialized AVX instructions are executed and 
> kill the program is we don't see a register by name?

I considered doing something like this, but I want to avoid relying on the AVX2 
support in LLDB to work in order to detect AVX2. If I use an LLDB mechanism for 
this then (exaggerating here!) someone could remove AVX support from LLDB and 
this test would still pass.


https://reviews.llvm.org/D41962



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


[Lldb-commits] [PATCH] D41962: Fix TestYMMRegisters for older machines without AVX2

2018-01-12 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added a comment.

> there's a new skipUnlessFeature() method added to decorators.py which runs 
> sysctl to detect hardware features (in this case, hw.optional.avx512f)

How does one execute a program like `sysctl` on the remote? I have seen code in 
TestLldbGdbServer.py that uses `platform get-file /proc/cpuinfo` to achieve 
something similar for Linux, but that works without executing a new process.


https://reviews.llvm.org/D41962



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


[Lldb-commits] [PATCH] D41962: Fix TestYMMRegisters for older machines without AVX2

2018-01-12 Thread Jason Molenda via Phabricator via lldb-commits
jasonmolenda added a comment.

In https://reviews.llvm.org/D41962#975168, @aprantl wrote:

> > there's a new skipUnlessFeature() method added to decorators.py which runs 
> > sysctl to detect hardware features (in this case, hw.optional.avx512f)
>
> How does one execute a program like `sysctl` on the remote? I have seen code 
> in TestLldbGdbServer.py that uses `platform get-file /proc/cpuinfo` to 
> achieve something similar for Linux, but that works without executing a new 
> process.


this skipUnlessFeature sysctl check was all performed on the system running the 
testsuite.  Checking whether the feature exists in the program (the approach 
you're taking) is more correct.  We usually do host != target testsuite runs 
for arm devices, but there's no reason why someone couldn't do a macos x 
freebsd testsuite run and the sysctl check would be invalid in that case.


https://reviews.llvm.org/D41962



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


[Lldb-commits] [lldb] r322418 - We have two sources for path remapping information that we get out

2018-01-12 Thread Jason Molenda via lldb-commits
Author: jmolenda
Date: Fri Jan 12 14:42:45 2018
New Revision: 322418

URL: http://llvm.org/viewvc/llvm-project?rev=322418&view=rev
Log:
We have two sources for path remapping information that we get out
of a dSYM per-uuid plist that may be present (dsymutil does not
create this plist, it is only added after the fact by additional
tools) -- either the DBGBuildSourcePath + DBGSourcePath pair of
k-v entries which give us the build-time and debug-time remapping,
or the newer DBGSourcePathRemapping dictionary which may give us
multiple remappings.

I'm changing the order that we process these & add them to the
list of source remappings.  If the DBGSourcePathRemapping dict
is present, it should be the first entries we will try.

 


Modified:
lldb/trunk/source/Host/macosx/Symbols.cpp
lldb/trunk/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp

Modified: lldb/trunk/source/Host/macosx/Symbols.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/macosx/Symbols.cpp?rev=322418&r1=322417&r2=322418&view=diff
==
--- lldb/trunk/source/Host/macosx/Symbols.cpp (original)
+++ lldb/trunk/source/Host/macosx/Symbols.cpp Fri Jan 12 14:42:45 2018
@@ -333,28 +333,9 @@ static bool GetModuleSpecInfoFromUUIDDic
 std::string DBGBuildSourcePath;
 std::string DBGSourcePath;
 
-cf_str = (CFStringRef)CFDictionaryGetValue((CFDictionaryRef)uuid_dict,
-   CFSTR("DBGBuildSourcePath"));
-if (cf_str && CFGetTypeID(cf_str) == CFStringGetTypeID()) {
-  CFCString::FileSystemRepresentation(cf_str, DBGBuildSourcePath);
-}
-
-cf_str = (CFStringRef)CFDictionaryGetValue((CFDictionaryRef)uuid_dict,
-   CFSTR("DBGSourcePath"));
-if (cf_str && CFGetTypeID(cf_str) == CFStringGetTypeID()) {
-  CFCString::FileSystemRepresentation(cf_str, DBGSourcePath);
-}
-
-if (!DBGBuildSourcePath.empty() && !DBGSourcePath.empty()) {
-  if (DBGSourcePath[0] == '~') {
-FileSpec resolved_source_path(DBGSourcePath.c_str(), true);
-DBGSourcePath = resolved_source_path.GetPath();
-  }
-  module_spec.GetSourceMappingList().Append(
-  ConstString(DBGBuildSourcePath.c_str()),
-  ConstString(DBGSourcePath.c_str()), true);
-}
-
+// If DBGVersion value 2 or higher, look for
+// DBGSourcePathRemapping dictionary and append the key-value pairs
+// to our remappings.
 cf_dict = (CFDictionaryRef)CFDictionaryGetValue(
 (CFDictionaryRef)uuid_dict, CFSTR("DBGSourcePathRemapping"));
 if (cf_dict && CFGetTypeID(cf_dict) == CFDictionaryGetTypeID()) {
@@ -439,6 +420,32 @@ static bool GetModuleSpecInfoFromUUIDDic
   free(values);
   }
 }
+
+
+// If we have a DBGBuildSourcePath + DBGSourcePath pair,
+// append them to the source remappings list.
+
+cf_str = (CFStringRef)CFDictionaryGetValue((CFDictionaryRef)uuid_dict,
+   CFSTR("DBGBuildSourcePath"));
+if (cf_str && CFGetTypeID(cf_str) == CFStringGetTypeID()) {
+  CFCString::FileSystemRepresentation(cf_str, DBGBuildSourcePath);
+}
+
+cf_str = (CFStringRef)CFDictionaryGetValue((CFDictionaryRef)uuid_dict,
+   CFSTR("DBGSourcePath"));
+if (cf_str && CFGetTypeID(cf_str) == CFStringGetTypeID()) {
+  CFCString::FileSystemRepresentation(cf_str, DBGSourcePath);
+}
+
+if (!DBGBuildSourcePath.empty() && !DBGSourcePath.empty()) {
+  if (DBGSourcePath[0] == '~') {
+FileSpec resolved_source_path(DBGSourcePath.c_str(), true);
+DBGSourcePath = resolved_source_path.GetPath();
+  }
+  module_spec.GetSourceMappingList().Append(
+  ConstString(DBGBuildSourcePath.c_str()),
+  ConstString(DBGSourcePath.c_str()), true);
+}
   }
   return success;
 }

Modified: lldb/trunk/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp?rev=322418&r1=322417&r2=322418&view=diff
==
--- lldb/trunk/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp 
(original)
+++ lldb/trunk/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp Fri 
Jan 12 14:42:45 2018
@@ -179,21 +179,6 @@ SymbolVendorMacOSX::CreateInstance(const
   std::string DBGBuildSourcePath;
   std::string DBGSourcePath;
 
-  plist.GetValueAsString("DBGBuildSourcePath",
- DBGBuildSourcePath);
-  plist.GetValueAsString("DBGSourcePath", DBGSourcePath);
-  if (!DBGBuildSourcePath.empty() &&
-  !DBGSourcePath.empty()) {
-if (DBGSourcePath[0]

[Lldb-commits] [PATCH] D41997: Build virtual override tables in DWARFASTParserClang::CompleteTypeFromDWARF

2018-01-12 Thread Lang Hames via Phabricator via lldb-commits
lhames added inline comments.



Comment at: Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:2103
 
+static bool isOverload(clang::CXXMethodDecl *m1, clang::CXXMethodDecl *m2) {
+  // FIXME: This should detect covariant return types, but currently doesn't.

aprantl wrote:
> Could you add some doxygen comments explaining what the new function do and 
> why doing this is necessary?
Will do.



Comment at: Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:2105-2106
+  // FIXME: This should detect covariant return types, but currently doesn't.
+  assert(&m1->getASTContext() == &m2->getASTContext() &&
+ "Methods should have the same AST context");
+  clang::ASTContext &context = m1->getASTContext();

clayborg wrote:
> Use lldb_assert and possibly return false afterwards in case the asserts are 
> compiled out
We can switch to lldb_assert to give us more control (at compile time) about 
when we turn it on or off, but I don't think we should bail out: this is an 
invariant, rather than a potential error case.



Comment at: 
Python/lldbsuite/test/expression_command/call-overridden-method/main.cpp:15
+  Base *b = &d;
+  return 0; // Please test these expressions while stopped at this line:
+}

jingham wrote:
> aprantl wrote:
> > the expressions are missing :-)
> > Perhaps convert this into an inline testcase?
> The expressions are in the test .py file.  It isn't necessary to put them 
> here, I'd just fix the comment.
> 
> Please don't convert regular test cases into inline ones, however.  The 
> benefit of inline test cases is mostly that they are easier to write, but 
> they are harder to debug when they go wrong.  So if the are already in 
> regular form I'd rather not convert them.
The text of the comment was just cribbed from one of the other expression 
tests. Is there a preferred phraseology?

  return 0; // run expressions here

?


Repository:
  rL LLVM

https://reviews.llvm.org/D41997



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


[Lldb-commits] [PATCH] D41428: [lldb] This commit adds support to cache a PDB's global scope and fixes a bug in getting the source file name for a compiland

2018-01-12 Thread Aaron Smith via Phabricator via lldb-commits
asmith updated this revision to Diff 129747.
asmith added a comment.
Herald added a subscriber: llvm-commits.

Added a lit unit test


Repository:
  rL LLVM

https://reviews.llvm.org/D41428

Files:
  lit/SymbolFile/PDB/Inputs/CompilandsTest.cpp
  lit/SymbolFile/PDB/compilands.test
  lit/SymbolFile/PDB/lit.local.cfg
  source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
  source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
  source/Plugins/SymbolFile/PDB/SymbolFilePDB.h

Index: source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
===
--- source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
+++ source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
@@ -16,6 +16,7 @@
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/DebugInfo/PDB/IPDBSession.h"
 #include "llvm/DebugInfo/PDB/PDB.h"
+#include "llvm/DebugInfo/PDB/PDBSymbolExe.h"
 
 class SymbolFilePDB : public lldb_private::SymbolFile {
 public:
@@ -167,23 +168,34 @@
   const llvm::pdb::IPDBSession &GetPDBSession() const;
 
 private:
-  lldb::CompUnitSP ParseCompileUnitForSymIndex(uint32_t id);
+  lldb::CompUnitSP
+  ParseCompileUnitForUID(uint32_t id, uint32_t index = UINT32_MAX);
 
   bool ParseCompileUnitLineTable(const lldb_private::SymbolContext &sc,
  uint32_t match_line);
 
   void BuildSupportFileIdToSupportFileIndexMap(
-  const llvm::pdb::PDBSymbolCompiland &cu,
+  const llvm::pdb::PDBSymbolCompiland &pdb_compiland,
   llvm::DenseMap &index_map) const;
 
   void FindTypesByName(const std::string &name, uint32_t max_matches,
lldb_private::TypeMap &types);
 
+  void GetCompileUnitIndex(const llvm::pdb::PDBSymbolCompiland *pdb_compiland,
+   uint32_t &index);
+
+  std::string GetSourceFileNameForPDBCompiland(
+  const llvm::pdb::PDBSymbolCompiland *pdb_compiland);
+
+  std::unique_ptr
+  GetPDBCompilandByUID(uint32_t uid);
+
   llvm::DenseMap m_comp_units;
   llvm::DenseMap m_types;
 
   std::vector m_builtin_types;
   std::unique_ptr m_session_up;
+  std::unique_ptr m_global_scope_up;
   uint32_t m_cached_compile_unit_count;
   std::unique_ptr m_tu_decl_ctx_up;
 };
Index: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
===
--- source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
+++ source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
@@ -18,6 +18,7 @@
 #include "lldb/Symbol/LineTable.h"
 #include "lldb/Symbol/ObjectFile.h"
 #include "lldb/Symbol/SymbolContext.h"
+#include "lldb/Symbol/SymbolVendor.h"
 #include "lldb/Symbol/TypeMap.h"
 #include "lldb/Utility/RegularExpression.h"
 
@@ -43,6 +44,7 @@
 
 #include 
 
+using namespace lldb;
 using namespace lldb_private;
 using namespace llvm::pdb;
 
@@ -92,7 +94,8 @@
 }
 
 SymbolFilePDB::SymbolFilePDB(lldb_private::ObjectFile *object_file)
-: SymbolFile(object_file), m_cached_compile_unit_count(0) {}
+: SymbolFile(object_file), m_session_up(), m_global_scope_up(),
+  m_cached_compile_unit_count(0), m_tu_decl_ctx_up() {}
 
 SymbolFilePDB::~SymbolFilePDB() {}
 
@@ -152,41 +155,95 @@
 
 void SymbolFilePDB::InitializeObject() {
   lldb::addr_t obj_load_address = m_obj_file->GetFileOffset();
+  lldbassert(obj_load_address &&
+ obj_load_address != LLDB_INVALID_ADDRESS);
   m_session_up->setLoadAddress(obj_load_address);
+  if (!m_global_scope_up)
+m_global_scope_up = m_session_up->getGlobalScope();
+  lldbassert(m_global_scope_up.get());
 
   TypeSystem *type_system =
   GetTypeSystemForLanguage(lldb::eLanguageTypeC_plus_plus);
   ClangASTContext *clang_type_system =
   llvm::dyn_cast_or_null(type_system);
+  lldbassert(clang_type_system);
   m_tu_decl_ctx_up = llvm::make_unique(
   type_system, clang_type_system->GetTranslationUnitDecl());
 }
 
 uint32_t SymbolFilePDB::GetNumCompileUnits() {
   if (m_cached_compile_unit_count == 0) {
-auto global = m_session_up->getGlobalScope();
-auto compilands = global->findAllChildren();
+auto compilands = m_global_scope_up->findAllChildren();
+if (!compilands)
+  return 0;
+
+// The linker could link *.dll (compiland language = LINK), or import
+// *.dll. For example, a compiland with name `Import:KERNEL32.dll`
+// could be found as a child of the global scope (PDB executable).
+// Usually, such compilands contain `thunk` symbols in which we are not
+// interested for now. However we still count them in the compiland list
+// so making access to them by index could be more easier.
+// The other reason is that if we perform any compiland related activity,
+// like finding symbols through llvm::pdb::IPDBSession methods,
+// such compilands will be all searched automatically no matter whether
+// we count them in or not.
 m_cached_compile_unit_count = compilands->getChildCount();
 
 // The linker can inject an additional "dummy" compilation unit into the
 // PDB. Ignore this special compile u

[Lldb-commits] [PATCH] D41428: [lldb] This commit adds support to cache a PDB's global scope and fixes a bug in getting the source file name for a compiland

2018-01-12 Thread Zachary Turner via Phabricator via lldb-commits
zturner accepted this revision.
zturner added a comment.
This revision is now accepted and ready to land.

It's nice that this turned out so easy.  Didn't even require modifying 
`lldb-test`.




Comment at: lit/SymbolFile/PDB/compilands.test:9
+CHECK: {{^[0-9A-F]+}}: SymbolVendor ([[CU]])
+CHECK-DAG: {{^[0-9A-F]+}}:   CompileUnit{{[{]0x[0-9a-f]+[}]}}, language = 
"c++", file = '{{.*}}\CompilandsTest.cpp'

If there's only one line then `CHECK-DAG` is equivalent to `CHECK`.  



Repository:
  rL LLVM

https://reviews.llvm.org/D41428



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


[Lldb-commits] [PATCH] D41428: [lldb] Add support to cache a PDB's global scope and fix a bug in getting the source file name for a compiland

2018-01-12 Thread Aaron Smith via Phabricator via lldb-commits
asmith updated this revision to Diff 129750.
asmith retitled this revision from "[lldb] This commit adds support to cache a 
PDB's global scope and fixes a bug in getting the source file name for a 
compiland" to "[lldb] Add support to cache a PDB's global scope and fix a bug 
in getting the source file name for a compiland".
asmith edited the summary of this revision.
asmith added a comment.

Cleaned up some comments before commit


Repository:
  rL LLVM

https://reviews.llvm.org/D41428

Files:
  lit/SymbolFile/PDB/Inputs/CompilandsTest.cpp
  lit/SymbolFile/PDB/compilands.test
  lit/SymbolFile/PDB/lit.local.cfg
  source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
  source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
  source/Plugins/SymbolFile/PDB/SymbolFilePDB.h

Index: source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
===
--- source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
+++ source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
@@ -16,6 +16,7 @@
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/DebugInfo/PDB/IPDBSession.h"
 #include "llvm/DebugInfo/PDB/PDB.h"
+#include "llvm/DebugInfo/PDB/PDBSymbolExe.h"
 
 class SymbolFilePDB : public lldb_private::SymbolFile {
 public:
@@ -167,23 +168,34 @@
   const llvm::pdb::IPDBSession &GetPDBSession() const;
 
 private:
-  lldb::CompUnitSP ParseCompileUnitForSymIndex(uint32_t id);
+  lldb::CompUnitSP
+  ParseCompileUnitForUID(uint32_t id, uint32_t index = UINT32_MAX);
 
   bool ParseCompileUnitLineTable(const lldb_private::SymbolContext &sc,
  uint32_t match_line);
 
   void BuildSupportFileIdToSupportFileIndexMap(
-  const llvm::pdb::PDBSymbolCompiland &cu,
+  const llvm::pdb::PDBSymbolCompiland &pdb_compiland,
   llvm::DenseMap &index_map) const;
 
   void FindTypesByName(const std::string &name, uint32_t max_matches,
lldb_private::TypeMap &types);
 
+  void GetCompileUnitIndex(const llvm::pdb::PDBSymbolCompiland *pdb_compiland,
+   uint32_t &index);
+
+  std::string GetSourceFileNameForPDBCompiland(
+  const llvm::pdb::PDBSymbolCompiland *pdb_compiland);
+
+  std::unique_ptr
+  GetPDBCompilandByUID(uint32_t uid);
+
   llvm::DenseMap m_comp_units;
   llvm::DenseMap m_types;
 
   std::vector m_builtin_types;
   std::unique_ptr m_session_up;
+  std::unique_ptr m_global_scope_up;
   uint32_t m_cached_compile_unit_count;
   std::unique_ptr m_tu_decl_ctx_up;
 };
Index: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
===
--- source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
+++ source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
@@ -18,6 +18,7 @@
 #include "lldb/Symbol/LineTable.h"
 #include "lldb/Symbol/ObjectFile.h"
 #include "lldb/Symbol/SymbolContext.h"
+#include "lldb/Symbol/SymbolVendor.h"
 #include "lldb/Symbol/TypeMap.h"
 #include "lldb/Utility/RegularExpression.h"
 
@@ -43,6 +44,7 @@
 
 #include 
 
+using namespace lldb;
 using namespace lldb_private;
 using namespace llvm::pdb;
 
@@ -92,7 +94,8 @@
 }
 
 SymbolFilePDB::SymbolFilePDB(lldb_private::ObjectFile *object_file)
-: SymbolFile(object_file), m_cached_compile_unit_count(0) {}
+: SymbolFile(object_file), m_session_up(), m_global_scope_up(),
+  m_cached_compile_unit_count(0), m_tu_decl_ctx_up() {}
 
 SymbolFilePDB::~SymbolFilePDB() {}
 
@@ -152,41 +155,93 @@
 
 void SymbolFilePDB::InitializeObject() {
   lldb::addr_t obj_load_address = m_obj_file->GetFileOffset();
+  lldbassert(obj_load_address &&
+ obj_load_address != LLDB_INVALID_ADDRESS);
   m_session_up->setLoadAddress(obj_load_address);
+  if (!m_global_scope_up)
+m_global_scope_up = m_session_up->getGlobalScope();
+  lldbassert(m_global_scope_up.get());
 
   TypeSystem *type_system =
   GetTypeSystemForLanguage(lldb::eLanguageTypeC_plus_plus);
   ClangASTContext *clang_type_system =
   llvm::dyn_cast_or_null(type_system);
+  lldbassert(clang_type_system);
   m_tu_decl_ctx_up = llvm::make_unique(
   type_system, clang_type_system->GetTranslationUnitDecl());
 }
 
 uint32_t SymbolFilePDB::GetNumCompileUnits() {
   if (m_cached_compile_unit_count == 0) {
-auto global = m_session_up->getGlobalScope();
-auto compilands = global->findAllChildren();
+auto compilands = m_global_scope_up->findAllChildren();
+if (!compilands)
+  return 0;
+
+// The linker could link *.dll (compiland language = LINK), or import
+// *.dll. For example, a compiland with name `Import:KERNEL32.dll`
+// could be found as a child of the global scope (PDB executable).
+// Usually, such compilands contain `thunk` symbols in which we are not
+// interested for now. However we still count them in the compiland list.
+// If we perform any compiland related activity, like finding symbols
+// through llvm::pdb::IPDBSession methods, such compilands will all be
+// searched automatically no matter whether we i