[Lldb-commits] [PATCH] D38153: Inhibit global lookups for symbols in the IR dynamic checks

2017-09-26 Thread Sean Callanan via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL314225: [Expression Parser] Inhibit global lookups for 
symbols in the IR dynamic checks (authored by spyffe).

Changed prior to commit:
  https://reviews.llvm.org/D38153?vs=116444=116727#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D38153

Files:
  lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
  lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangASTSource.h
  lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
  
lldb/trunk/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp

Index: lldb/trunk/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
===
--- lldb/trunk/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
+++ lldb/trunk/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
@@ -811,85 +811,43 @@
 
   int len = 0;
   if (m_has_object_getClass) {
-len = ::snprintf(check_function_code, sizeof(check_function_code),
- "extern \"C\" void *gdb_object_getClass(void *);  "
- "\n"
- "extern \"C\"  int printf(const char *format, ...);   "
- "\n"
- "extern \"C\" void"
- "\n"
- "%s(void *$__lldb_arg_obj, void *$__lldb_arg_selector)"
- "\n"
- "{"
- "\n"
- "   if ($__lldb_arg_obj == (void *)0) "
- "\n"
- "   return; // nil is ok  "
- "\n"
- "   if (!gdb_object_getClass($__lldb_arg_obj))"
- "\n"
- "   *((volatile int *)0) = 'ocgc';"
- "\n"
- "   else if ($__lldb_arg_selector != (void *)0)   "
- "\n"
- "   { "
- "\n"
- "signed char responds = (signed char) [(id) "
- "$__lldb_arg_obj   \n"
- ""
- "respondsToSelector:  \n"
- "   (struct "
- "objc_selector *) $__lldb_arg_selector];   \n"
- "   if (responds == (signed char) 0)  "
- "\n"
- "   *((volatile int *)0) = 'ocgc';"
- "\n"
- "   } "
- "\n"
- "}"
- "\n",
- name);
+len = ::snprintf(check_function_code, sizeof(check_function_code), R"(
+ extern "C" void *gdb_object_getClass(void *);
+ extern "C" int printf(const char *format, ...);
+ extern "C" void
+ %s(void *$__lldb_arg_obj, void *$__lldb_arg_selector) {
+   if ($__lldb_arg_obj == (void *)0)
+ return; // nil is ok
+   if (!gdb_object_getClass($__lldb_arg_obj)) {
+ *((volatile int *)0) = 'ocgc';
+   } else if ($__lldb_arg_selector != (void *)0) {
+ signed char $responds = (signed char)
+ [(id)$__lldb_arg_obj respondsToSelector:
+ (void *) $__lldb_arg_selector];
+ if ($responds == (signed char) 0)
+   *((volatile int *)0) = 'ocgc';
+   }
+ })", name);
   } else {
-len = ::snprintf(check_function_code, sizeof(check_function_code),
- "extern \"C\" void *gdb_class_getClass(void *);   "
- "\n"
- "extern \"C\"  int 

[Lldb-commits] [PATCH] D38153: Inhibit global lookups for symbols in the IR dynamic checks

2017-09-26 Thread Sean Callanan via Phabricator via lldb-commits
spyffe abandoned this revision.
spyffe added a comment.

Committed as LLDB r314225


Repository:
  rL LLVM

https://reviews.llvm.org/D38153



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


[Lldb-commits] [PATCH] D38153: Inhibit global lookups for symbols in the IR dynamic checks

2017-09-21 Thread Sean Callanan via Phabricator via lldb-commits
spyffe created this revision.
spyffe added a project: LLDB.
Herald added subscribers: arichardson, aprantl, sdardis.

The IR dynamic checks are self-contained functions whose job is to

- verify that pointers referenced in an expression are valid at runtime; and
- verify that selectors sent to Objective-C objects by an expression are 
actually supported by that object.

These dynamic checks forward-declare all the functions they use and should not 
require any external debug information.  The way they ensure this is by marking 
all the names they use with a dollar sign (`$`).  The expression parser 
recognizes such symbols and perform no lookups for them.

This patch fixes three issues surrounding the use of the dollar sign:

- to fix a MIPS issue, the name of the pointer checker was changed from 
starting with `$` to starting with `_$`, but this was not properly ignored; and
- the Objective-C object checker used a temporary variable that did not start 
with `$`.
- the Objective-C object checker used an externally-defined struct (`struct 
objc_selector`) but didn't need to.

The patch also reformats the string containing the Objective-C object checker, 
which was mangled horribly when the code was transformed to a uniform width of 
80 columns.


Repository:
  rL LLVM

https://reviews.llvm.org/D38153

Files:
  source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
  source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
  source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp

Index: source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
===
--- source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
+++ source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
@@ -812,84 +812,44 @@
   int len = 0;
   if (m_has_object_getClass) {
 len = ::snprintf(check_function_code, sizeof(check_function_code),
- "extern \"C\" void *gdb_object_getClass(void *);  "
- "\n"
- "extern \"C\"  int printf(const char *format, ...);   "
- "\n"
- "extern \"C\" void"
- "\n"
- "%s(void *$__lldb_arg_obj, void *$__lldb_arg_selector)"
- "\n"
- "{"
- "\n"
- "   if ($__lldb_arg_obj == (void *)0) "
- "\n"
- "   return; // nil is ok  "
- "\n"
- "   if (!gdb_object_getClass($__lldb_arg_obj))"
- "\n"
- "   *((volatile int *)0) = 'ocgc';"
- "\n"
- "   else if ($__lldb_arg_selector != (void *)0)   "
- "\n"
- "   { "
- "\n"
- "signed char responds = (signed char) [(id) "
- "$__lldb_arg_obj   \n"
- ""
- "respondsToSelector:  \n"
- "   (struct "
- "objc_selector *) $__lldb_arg_selector];   \n"
- "   if (responds == (signed char) 0)  "
- "\n"
- "   *((volatile int *)0) = 'ocgc';"
- "\n"
- "   } "
- "\n"
- "}"
- "\n",
- name);
+ "extern \"C\" void *gdb_object_getClass(void *);\n"
+ "extern \"C\"  int printf(const char *format, ...); \n"
+ "extern \"C\" void  \n"
+ "%s(void *$__lldb_arg_obj, void *$__lldb_arg_selector) {\n"
+ "  if ($__lldb_arg_obj == (void *)0)\n"
+ "

[Lldb-commits] [PATCH] D35083: [TypeSystem] Guard the global `ASTSourceMap` with a mutex

2017-07-25 Thread Sean Callanan via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL308993: [TypeSystem] Guard the global `ASTSourceMap` with a 
mutex (authored by spyffe).

Changed prior to commit:
  https://reviews.llvm.org/D35083?vs=106087=108116#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D35083

Files:
  lldb/trunk/source/Symbol/ClangExternalASTSourceCommon.cpp


Index: lldb/trunk/source/Symbol/ClangExternalASTSourceCommon.cpp
===
--- lldb/trunk/source/Symbol/ClangExternalASTSourceCommon.cpp
+++ lldb/trunk/source/Symbol/ClangExternalASTSourceCommon.cpp
@@ -10,23 +10,29 @@
 #include "lldb/Symbol/ClangExternalASTSourceCommon.h"
 #include "lldb/Utility/Stream.h"
 
+#include 
+
 using namespace lldb_private;
 
 uint64_t g_TotalSizeOfMetadata = 0;
 
 typedef llvm::DenseMap
 ASTSourceMap;
 
-static ASTSourceMap () {
+static ASTSourceMap (std::unique_lock ) {
   // Intentionally leaked to avoid problems with global destructors.
   static ASTSourceMap *s_source_map = new ASTSourceMap;
+  static std::mutex s_mutex;
+  std::unique_lock locked_guard(s_mutex);
+  guard.swap(locked_guard);
   return *s_source_map;
 }
 
 ClangExternalASTSourceCommon *
 ClangExternalASTSourceCommon::Lookup(clang::ExternalASTSource *source) {
-  ASTSourceMap _map = GetSourceMap();
+  std::unique_lock guard;
+  ASTSourceMap _map = GetSourceMap(guard);
 
   ASTSourceMap::iterator iter = source_map.find(source);
 
@@ -40,11 +46,13 @@
 ClangExternalASTSourceCommon::ClangExternalASTSourceCommon()
 : clang::ExternalASTSource() {
   g_TotalSizeOfMetadata += m_metadata.size();
-  GetSourceMap()[this] = this;
+  std::unique_lock guard;
+  GetSourceMap(guard)[this] = this;
 }
 
 ClangExternalASTSourceCommon::~ClangExternalASTSourceCommon() {
-  GetSourceMap().erase(this);
+  std::unique_lock guard;
+  GetSourceMap(guard).erase(this);
   g_TotalSizeOfMetadata -= m_metadata.size();
 }
 


Index: lldb/trunk/source/Symbol/ClangExternalASTSourceCommon.cpp
===
--- lldb/trunk/source/Symbol/ClangExternalASTSourceCommon.cpp
+++ lldb/trunk/source/Symbol/ClangExternalASTSourceCommon.cpp
@@ -10,23 +10,29 @@
 #include "lldb/Symbol/ClangExternalASTSourceCommon.h"
 #include "lldb/Utility/Stream.h"
 
+#include 
+
 using namespace lldb_private;
 
 uint64_t g_TotalSizeOfMetadata = 0;
 
 typedef llvm::DenseMap
 ASTSourceMap;
 
-static ASTSourceMap () {
+static ASTSourceMap (std::unique_lock ) {
   // Intentionally leaked to avoid problems with global destructors.
   static ASTSourceMap *s_source_map = new ASTSourceMap;
+  static std::mutex s_mutex;
+  std::unique_lock locked_guard(s_mutex);
+  guard.swap(locked_guard);
   return *s_source_map;
 }
 
 ClangExternalASTSourceCommon *
 ClangExternalASTSourceCommon::Lookup(clang::ExternalASTSource *source) {
-  ASTSourceMap _map = GetSourceMap();
+  std::unique_lock guard;
+  ASTSourceMap _map = GetSourceMap(guard);
 
   ASTSourceMap::iterator iter = source_map.find(source);
 
@@ -40,11 +46,13 @@
 ClangExternalASTSourceCommon::ClangExternalASTSourceCommon()
 : clang::ExternalASTSource() {
   g_TotalSizeOfMetadata += m_metadata.size();
-  GetSourceMap()[this] = this;
+  std::unique_lock guard;
+  GetSourceMap(guard)[this] = this;
 }
 
 ClangExternalASTSourceCommon::~ClangExternalASTSourceCommon() {
-  GetSourceMap().erase(this);
+  std::unique_lock guard;
+  GetSourceMap(guard).erase(this);
   g_TotalSizeOfMetadata -= m_metadata.size();
 }
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D35083: [TypeSystem] Guard the global `ASTSourceMap` with a mutex

2017-07-11 Thread Sean Callanan via Phabricator via lldb-commits
spyffe updated this revision to Diff 106087.
spyffe added a comment.

Upon reflection, it's not worth coming up with a new pattern if

- that pattern does not considerably reduce the incidence of bugs in the source 
file I'm applying it in (and indeed that seems quite unlikely), and
- that pattern is unlikely to be adopted elsewhere.

I'm going to simply pass in a `std::unique_lock` and swap into that.  This has 
the pleasant side effect of guarding the `g_totalSizeOfMetadata` variable 
against at least some overrides, although to be honest I'd accept a separate 
patch that removes this variable.


Repository:
  rL LLVM

https://reviews.llvm.org/D35083

Files:
  source/Symbol/ClangExternalASTSourceCommon.cpp


Index: source/Symbol/ClangExternalASTSourceCommon.cpp
===
--- source/Symbol/ClangExternalASTSourceCommon.cpp
+++ source/Symbol/ClangExternalASTSourceCommon.cpp
@@ -10,23 +10,29 @@
 #include "lldb/Symbol/ClangExternalASTSourceCommon.h"
 #include "lldb/Utility/Stream.h"
 
+#include 
+
 using namespace lldb_private;
 
 uint64_t g_TotalSizeOfMetadata = 0;
 
 typedef llvm::DenseMap
 ASTSourceMap;
 
-static ASTSourceMap () {
+static ASTSourceMap (std::unique_lock ) {
   // Intentionally leaked to avoid problems with global destructors.
   static ASTSourceMap *s_source_map = new ASTSourceMap;
+  static std::mutex s_mutex;
+  std::unique_lock locked_guard(s_mutex);
+  guard.swap(locked_guard);
   return *s_source_map;
 }
 
 ClangExternalASTSourceCommon *
 ClangExternalASTSourceCommon::Lookup(clang::ExternalASTSource *source) {
-  ASTSourceMap _map = GetSourceMap();
+  std::unique_lock guard;
+  ASTSourceMap _map = GetSourceMap(guard);
 
   ASTSourceMap::iterator iter = source_map.find(source);
 
@@ -40,11 +46,13 @@
 ClangExternalASTSourceCommon::ClangExternalASTSourceCommon()
 : clang::ExternalASTSource() {
   g_TotalSizeOfMetadata += m_metadata.size();
-  GetSourceMap()[this] = this;
+  std::unique_lock guard;
+  GetSourceMap(guard)[this] = this;
 }
 
 ClangExternalASTSourceCommon::~ClangExternalASTSourceCommon() {
-  GetSourceMap().erase(this);
+  std::unique_lock guard;
+  GetSourceMap(guard).erase(this);
   g_TotalSizeOfMetadata -= m_metadata.size();
 }
 


Index: source/Symbol/ClangExternalASTSourceCommon.cpp
===
--- source/Symbol/ClangExternalASTSourceCommon.cpp
+++ source/Symbol/ClangExternalASTSourceCommon.cpp
@@ -10,23 +10,29 @@
 #include "lldb/Symbol/ClangExternalASTSourceCommon.h"
 #include "lldb/Utility/Stream.h"
 
+#include 
+
 using namespace lldb_private;
 
 uint64_t g_TotalSizeOfMetadata = 0;
 
 typedef llvm::DenseMap
 ASTSourceMap;
 
-static ASTSourceMap () {
+static ASTSourceMap (std::unique_lock ) {
   // Intentionally leaked to avoid problems with global destructors.
   static ASTSourceMap *s_source_map = new ASTSourceMap;
+  static std::mutex s_mutex;
+  std::unique_lock locked_guard(s_mutex);
+  guard.swap(locked_guard);
   return *s_source_map;
 }
 
 ClangExternalASTSourceCommon *
 ClangExternalASTSourceCommon::Lookup(clang::ExternalASTSource *source) {
-  ASTSourceMap _map = GetSourceMap();
+  std::unique_lock guard;
+  ASTSourceMap _map = GetSourceMap(guard);
 
   ASTSourceMap::iterator iter = source_map.find(source);
 
@@ -40,11 +46,13 @@
 ClangExternalASTSourceCommon::ClangExternalASTSourceCommon()
 : clang::ExternalASTSource() {
   g_TotalSizeOfMetadata += m_metadata.size();
-  GetSourceMap()[this] = this;
+  std::unique_lock guard;
+  GetSourceMap(guard)[this] = this;
 }
 
 ClangExternalASTSourceCommon::~ClangExternalASTSourceCommon() {
-  GetSourceMap().erase(this);
+  std::unique_lock guard;
+  GetSourceMap(guard).erase(this);
   g_TotalSizeOfMetadata -= m_metadata.size();
 }
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D35083: [TypeSystem] Guard the global `ASTSourceMap` with a mutex

2017-07-09 Thread Sean Callanan via Phabricator via lldb-commits
spyffe added a comment.

Responded to Lang's comments inline.

**Jim**: you say this patch "doesn't actually enforce that you take the lock to 
get the SourceMap."  How do you get the source map otherwise?  It's static 
inside the function so nothing can see it outside.  Do you mean that it's still 
possible to have the lambda make an alias to the reference and return?

**Pavel**: if I have `GetSourceMap()` take a `unique_lock&` and leave the 
return value as it is, then clients get an unguarded alias to `s_source_map` to 
play with regardless of whether they're holding the lock or not.  We have to 
trust that they hold on to the lock as long as they need to – or use 
`GUARDED_BY`, adding more complexity for something that 
`WithExclusiveSourceMap()` enforced by default.  (Creating an escaping alias is 
still an issue.)

I'm going to hold off for Lang's opinion on this.  I feel like we have a few 
workable solutions, and since this isn't API for anything even outside the 
`.cpp` file picking one and going with it is fine.




Comment at: source/Symbol/ClangExternalASTSourceCommon.cpp:24
+template 
+static decltype(std::declval()(std::declval()))
+WithExclusiveSourceMap(FnType fn) {

lhames wrote:
> Does std::result_of::type work as the return type?
> 
> http://en.cppreference.com/w/cpp/types/result_of
No, it doesn't.  If I change the declaration to
```
template 
static typename std::result_of::type
WithExclusiveSourceMap(FnType fn) {
```
then the compiler can no longer resolve calls:
```
…/lldb/source/Symbol/ClangExternalASTSourceCommon.cpp:25:1: Candidate template 
ignored: substitution failure [with FnType = (lambda at 
…/lldb/source/Symbol/ClangExternalASTSourceCommon.cpp:39:7)]: implicit 
instantiation of undefined template 'std::__1::result_of<(lambda at 
…/lldb/source/Symbol/ClangExternalASTSourceCommon.cpp:39:7)>'
```



Comment at: source/Symbol/ClangExternalASTSourceCommon.cpp:28
   static ASTSourceMap *s_source_map = new ASTSourceMap;
-  return *s_source_map;
+  static std::mutex s_source_map_mutex;
+  

lhames wrote:
> Should this be on a context object of some kind (ASTContext?).
No, it shouldn't.  This is intended to be global.


Repository:
  rL LLVM

https://reviews.llvm.org/D35083



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


[Lldb-commits] [PATCH] D35083: [TypeSystem] Guard the global `ASTSourceMap` with a mutex

2017-07-06 Thread Sean Callanan via Phabricator via lldb-commits
spyffe created this revision.

`s_source_map` in `ClangExternalASTSourceCommon.cpp` is unguarded and therefore 
can break in multithreaded conditions.  This can cause crashes in particular if 
multiple targets are being set up at once.

This patch wraps `s_source_map` in a function that ensures exclusivity, and 
makes every user of it use that function instead.


Repository:
  rL LLVM

https://reviews.llvm.org/D35083

Files:
  source/Symbol/ClangExternalASTSourceCommon.cpp


Index: source/Symbol/ClangExternalASTSourceCommon.cpp
===
--- source/Symbol/ClangExternalASTSourceCommon.cpp
+++ source/Symbol/ClangExternalASTSourceCommon.cpp
@@ -10,41 +10,55 @@
 #include "lldb/Symbol/ClangExternalASTSourceCommon.h"
 #include "lldb/Utility/Stream.h"
 
+#include 
+
 using namespace lldb_private;
 
 uint64_t g_TotalSizeOfMetadata = 0;
 
 typedef llvm::DenseMap
 ASTSourceMap;
 
-static ASTSourceMap () {
+template 
+static decltype(std::declval()(std::declval()))
+WithExclusiveSourceMap(FnType fn) {
   // Intentionally leaked to avoid problems with global destructors.
   static ASTSourceMap *s_source_map = new ASTSourceMap;
-  return *s_source_map;
+  static std::mutex s_source_map_mutex;
+  
+  {
+std::lock_guard source_map_locker(s_source_map_mutex);
+return fn(*s_source_map);
+  }
 }
 
 ClangExternalASTSourceCommon *
 ClangExternalASTSourceCommon::Lookup(clang::ExternalASTSource *source) {
-  ASTSourceMap _map = GetSourceMap();
-
-  ASTSourceMap::iterator iter = source_map.find(source);
-
-  if (iter != source_map.end()) {
-return iter->second;
-  } else {
-return nullptr;
-  }
+  return WithExclusiveSourceMap(
+  [source](ASTSourceMap _map) -> ClangExternalASTSourceCommon * {
+ASTSourceMap::iterator iter = source_map.find(source);
+
+if (iter != source_map.end()) {
+  return iter->second;
+} else {
+  return nullptr;
+}
+  });
 }
 
 ClangExternalASTSourceCommon::ClangExternalASTSourceCommon()
 : clang::ExternalASTSource() {
+  WithExclusiveSourceMap([this](ASTSourceMap _map) {
+source_map[this] = this;
+  });
   g_TotalSizeOfMetadata += m_metadata.size();
-  GetSourceMap()[this] = this;
 }
 
 ClangExternalASTSourceCommon::~ClangExternalASTSourceCommon() {
-  GetSourceMap().erase(this);
+  WithExclusiveSourceMap([this](ASTSourceMap _map) {
+source_map.erase(this);
+  });
   g_TotalSizeOfMetadata -= m_metadata.size();
 }
 


Index: source/Symbol/ClangExternalASTSourceCommon.cpp
===
--- source/Symbol/ClangExternalASTSourceCommon.cpp
+++ source/Symbol/ClangExternalASTSourceCommon.cpp
@@ -10,41 +10,55 @@
 #include "lldb/Symbol/ClangExternalASTSourceCommon.h"
 #include "lldb/Utility/Stream.h"
 
+#include 
+
 using namespace lldb_private;
 
 uint64_t g_TotalSizeOfMetadata = 0;
 
 typedef llvm::DenseMap
 ASTSourceMap;
 
-static ASTSourceMap () {
+template 
+static decltype(std::declval()(std::declval()))
+WithExclusiveSourceMap(FnType fn) {
   // Intentionally leaked to avoid problems with global destructors.
   static ASTSourceMap *s_source_map = new ASTSourceMap;
-  return *s_source_map;
+  static std::mutex s_source_map_mutex;
+  
+  {
+std::lock_guard source_map_locker(s_source_map_mutex);
+return fn(*s_source_map);
+  }
 }
 
 ClangExternalASTSourceCommon *
 ClangExternalASTSourceCommon::Lookup(clang::ExternalASTSource *source) {
-  ASTSourceMap _map = GetSourceMap();
-
-  ASTSourceMap::iterator iter = source_map.find(source);
-
-  if (iter != source_map.end()) {
-return iter->second;
-  } else {
-return nullptr;
-  }
+  return WithExclusiveSourceMap(
+  [source](ASTSourceMap _map) -> ClangExternalASTSourceCommon * {
+ASTSourceMap::iterator iter = source_map.find(source);
+
+if (iter != source_map.end()) {
+  return iter->second;
+} else {
+  return nullptr;
+}
+  });
 }
 
 ClangExternalASTSourceCommon::ClangExternalASTSourceCommon()
 : clang::ExternalASTSource() {
+  WithExclusiveSourceMap([this](ASTSourceMap _map) {
+source_map[this] = this;
+  });
   g_TotalSizeOfMetadata += m_metadata.size();
-  GetSourceMap()[this] = this;
 }
 
 ClangExternalASTSourceCommon::~ClangExternalASTSourceCommon() {
-  GetSourceMap().erase(this);
+  WithExclusiveSourceMap([this](ASTSourceMap _map) {
+source_map.erase(this);
+  });
   g_TotalSizeOfMetadata -= m_metadata.size();
 }
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D33812: [TypeSystem] Handle Clang AttributedTypes

2017-06-01 Thread Sean Callanan via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL304510: [TypeSystem] Handle Clang AttributedTypes (authored 
by spyffe).

Changed prior to commit:
  https://reviews.llvm.org/D33812?vs=101140=101158#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D33812

Files:
  
lldb/trunk/packages/Python/lldbsuite/test/lang/objc/objc-new-syntax/TestObjCNewSyntax.py
  lldb/trunk/source/Symbol/ClangASTContext.cpp

Index: lldb/trunk/packages/Python/lldbsuite/test/lang/objc/objc-new-syntax/TestObjCNewSyntax.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/lang/objc/objc-new-syntax/TestObjCNewSyntax.py
+++ lldb/trunk/packages/Python/lldbsuite/test/lang/objc/objc-new-syntax/TestObjCNewSyntax.py
@@ -26,16 +26,7 @@
 # Find the line number to break inside main().
 self.line = line_number('main.m', '// Set breakpoint 0 here.')
 
-@skipUnlessDarwin
-@expectedFailureAll(
-oslist=['macosx'],
-compiler='clang',
-compiler_version=[
-'<',
-'7.0.0'])
-@skipIf(macos_version=["<", "10.12"])
-@expectedFailureAll(archs=["i[3-6]86"])
-def test_expr(self):
+def runToBreakpoint(self):
 self.build()
 exe = os.path.join(os.getcwd(), "a.out")
 self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
@@ -55,6 +46,18 @@
 self.expect("breakpoint list -f", BREAKPOINT_HIT_ONCE,
 substrs=[' resolved, hit count = 1'])
 
+@skipUnlessDarwin
+@expectedFailureAll(
+oslist=['macosx'],
+compiler='clang',
+compiler_version=[
+'<',
+'7.0.0'])
+@skipIf(macos_version=["<", "10.12"])
+@expectedFailureAll(archs=["i[3-6]86"])
+def test_read_array(self):
+self.runToBreakpoint()
+
 self.expect(
 "expr --object-description -- immutable_array[0]",
 VARIABLES_DISPLAYED_CORRECTLY,
@@ -65,6 +68,18 @@
 VARIABLES_DISPLAYED_CORRECTLY,
 substrs=["foo"])
 
+@skipUnlessDarwin
+@expectedFailureAll(
+oslist=['macosx'],
+compiler='clang',
+compiler_version=[
+'<',
+'7.0.0'])
+@skipIf(macos_version=["<", "10.12"])
+@expectedFailureAll(archs=["i[3-6]86"])
+def test_update_array(self):
+self.runToBreakpoint()
+
 self.expect(
 "expr --object-description -- mutable_array[0] = @\"bar\"",
 VARIABLES_DISPLAYED_CORRECTLY,
@@ -75,6 +90,18 @@
 VARIABLES_DISPLAYED_CORRECTLY,
 substrs=["bar"])
 
+@skipUnlessDarwin
+@expectedFailureAll(
+oslist=['macosx'],
+compiler='clang',
+compiler_version=[
+'<',
+'7.0.0'])
+@skipIf(macos_version=["<", "10.12"])
+@expectedFailureAll(archs=["i[3-6]86"])
+def test_read_dictionary(self):
+self.runToBreakpoint()
+
 self.expect(
 "expr --object-description -- immutable_dictionary[@\"key\"]",
 VARIABLES_DISPLAYED_CORRECTLY,
@@ -85,6 +112,18 @@
 VARIABLES_DISPLAYED_CORRECTLY,
 substrs=["value"])
 
+@skipUnlessDarwin
+@expectedFailureAll(
+oslist=['macosx'],
+compiler='clang',
+compiler_version=[
+'<',
+'7.0.0'])
+@skipIf(macos_version=["<", "10.12"])
+@expectedFailureAll(archs=["i[3-6]86"])
+def test_update_dictionary(self):
+self.runToBreakpoint()
+
 self.expect(
 "expr --object-description -- mutable_dictionary[@\"key\"] = @\"object\"",
 VARIABLES_DISPLAYED_CORRECTLY,
@@ -95,24 +134,72 @@
 VARIABLES_DISPLAYED_CORRECTLY,
 substrs=["object"])
 
+@skipUnlessDarwin
+@expectedFailureAll(
+oslist=['macosx'],
+compiler='clang',
+compiler_version=[
+'<',
+'7.0.0'])
+@skipIf(macos_version=["<", "10.12"])
+@expectedFailureAll(archs=["i[3-6]86"])
+def test_array_literal(self):
+self.runToBreakpoint()
+
 self.expect(
 "expr --object-description -- @[ @\"foo\", @\"bar\" ]",
 VARIABLES_DISPLAYED_CORRECTLY,
 substrs=[
 "NSArray",
 "foo",
 "bar"])
 
+@skipUnlessDarwin
+@expectedFailureAll(
+oslist=['macosx'],
+compiler='clang',
+compiler_version=[
+'<',
+'7.0.0'])
+@skipIf(macos_version=["<", "10.12"])
+@expectedFailureAll(archs=["i[3-6]86"])
+def test_dictionary_literal(self):
+self.runToBreakpoint()
+
 self.expect(
 "expr --object-description -- @{ @\"key\" : @\"object\" }",
 VARIABLES_DISPLAYED_CORRECTLY,
 substrs=[
 "key",
 "object"])
 
+@skipUnlessDarwin
+@expectedFailureAll(
+

[Lldb-commits] [PATCH] D33812: [TypeSystem] Handle Clang AttributedTypes

2017-06-01 Thread Sean Callanan via Phabricator via lldb-commits
spyffe added a comment.

`ModifiedType` gets assigned during construction and `getModifiedType()` is 
just an accessor, so if that is `nullptr` something very bad is happening.  We 
should not expect that except in case of a parser bug of some sort.

`AttributedType` checks that a `Type*` is an `AttributedType` in the following 
way (Type.h):

  static bool classof(const Type *T) {
return T->getTypeClass() == Attributed;
  }

So our use of the type class is canonical.


Repository:
  rL LLVM

https://reviews.llvm.org/D33812



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


[Lldb-commits] [PATCH] D33812: [TypeSystem] Handle Clang AttributedTypes

2017-06-01 Thread Sean Callanan via Phabricator via lldb-commits
spyffe created this revision.

When parsing types originating in modules, it is possible to encounter 
`AttributedType`s (such as the type generated for `NSString *_Nonnull`).  Some 
of LLDB's `ClangASTContext` methods deal with them; others do not.  In 
particular, one function that did not was `GetTypeInfo`, causing 
`TestObjCNewSyntax` to fail.

This fixes that, treating `AttributedType` as essentially transparent and 
getting the information for the modified type.

In addition, however, `TestObjCNewSyntax` is a monolithic test that verifies a 
bunch of different things, all of which can break independently of one another. 
 I broke it apart into smaller tests so that we get more precise failures when 
something (like this) breaks.


Repository:
  rL LLVM

https://reviews.llvm.org/D33812

Files:
  packages/Python/lldbsuite/test/lang/objc/objc-new-syntax/TestObjCNewSyntax.py
  source/Symbol/ClangASTContext.cpp

Index: source/Symbol/ClangASTContext.cpp
===
--- source/Symbol/ClangASTContext.cpp
+++ source/Symbol/ClangASTContext.cpp
@@ -3938,6 +3938,11 @@
 
   const clang::Type::TypeClass type_class = qual_type->getTypeClass();
   switch (type_class) {
+  case clang::Type::Attributed:
+return GetTypeInfo(
+qual_type->getAs()
+->getModifiedType().getAsOpaquePtr(),
+pointee_or_element_clang_type);
   case clang::Type::Builtin: {
 const clang::BuiltinType *builtin_type = llvm::dyn_cast(
 qual_type->getCanonicalTypeInternal());
Index: packages/Python/lldbsuite/test/lang/objc/objc-new-syntax/TestObjCNewSyntax.py
===
--- packages/Python/lldbsuite/test/lang/objc/objc-new-syntax/TestObjCNewSyntax.py
+++ packages/Python/lldbsuite/test/lang/objc/objc-new-syntax/TestObjCNewSyntax.py
@@ -26,16 +26,7 @@
 # Find the line number to break inside main().
 self.line = line_number('main.m', '// Set breakpoint 0 here.')
 
-@skipUnlessDarwin
-@expectedFailureAll(
-oslist=['macosx'],
-compiler='clang',
-compiler_version=[
-'<',
-'7.0.0'])
-@skipIf(macos_version=["<", "10.12"])
-@expectedFailureAll(archs=["i[3-6]86"])
-def test_expr(self):
+def runToBreakpoint(self):
 self.build()
 exe = os.path.join(os.getcwd(), "a.out")
 self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
@@ -55,6 +46,18 @@
 self.expect("breakpoint list -f", BREAKPOINT_HIT_ONCE,
 substrs=[' resolved, hit count = 1'])
 
+@skipUnlessDarwin
+@expectedFailureAll(
+oslist=['macosx'],
+compiler='clang',
+compiler_version=[
+'<',
+'7.0.0'])
+@skipIf(macos_version=["<", "10.12"])
+@expectedFailureAll(archs=["i[3-6]86"])
+def test_read_array(self):
+self.runToBreakpoint()
+
 self.expect(
 "expr --object-description -- immutable_array[0]",
 VARIABLES_DISPLAYED_CORRECTLY,
@@ -65,6 +68,18 @@
 VARIABLES_DISPLAYED_CORRECTLY,
 substrs=["foo"])
 
+@skipUnlessDarwin
+@expectedFailureAll(
+oslist=['macosx'],
+compiler='clang',
+compiler_version=[
+'<',
+'7.0.0'])
+@skipIf(macos_version=["<", "10.12"])
+@expectedFailureAll(archs=["i[3-6]86"])
+def test_update_array(self):
+self.runToBreakpoint()
+
 self.expect(
 "expr --object-description -- mutable_array[0] = @\"bar\"",
 VARIABLES_DISPLAYED_CORRECTLY,
@@ -75,6 +90,18 @@
 VARIABLES_DISPLAYED_CORRECTLY,
 substrs=["bar"])
 
+@skipUnlessDarwin
+@expectedFailureAll(
+oslist=['macosx'],
+compiler='clang',
+compiler_version=[
+'<',
+'7.0.0'])
+@skipIf(macos_version=["<", "10.12"])
+@expectedFailureAll(archs=["i[3-6]86"])
+def test_read_dictionary(self):
+self.runToBreakpoint()
+
 self.expect(
 "expr --object-description -- immutable_dictionary[@\"key\"]",
 VARIABLES_DISPLAYED_CORRECTLY,
@@ -85,6 +112,18 @@
 VARIABLES_DISPLAYED_CORRECTLY,
 substrs=["value"])
 
+@skipUnlessDarwin
+@expectedFailureAll(
+oslist=['macosx'],
+compiler='clang',
+compiler_version=[
+'<',
+'7.0.0'])
+@skipIf(macos_version=["<", "10.12"])
+@expectedFailureAll(archs=["i[3-6]86"])
+def test_update_dictionary(self):
+self.runToBreakpoint()
+
 self.expect(
 "expr --object-description -- mutable_dictionary[@\"key\"] = @\"object\"",
 VARIABLES_DISPLAYED_CORRECTLY,
@@ -95,24 +134,72 @@
 VARIABLES_DISPLAYED_CORRECTLY,
 substrs=["object"])
 
+@skipUnlessDarwin
+@expectedFailureAll(
+oslist=['macosx'],
+compiler='clang',
+   

[Lldb-commits] [PATCH] D33083: [Expression parser] Look up module symbols before hunting globally

2017-05-16 Thread Sean Callanan via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL303223: [Expression parser] Look up module symbols before 
hunting globally (authored by spyffe).

Changed prior to commit:
  https://reviews.llvm.org/D33083?vs=99211=99227#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D33083

Files:
  lldb/trunk/include/lldb/Symbol/SymbolContext.h
  lldb/trunk/packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Makefile
  lldb/trunk/packages/Python/lldbsuite/test/lang/c/conflicting-symbol/One.mk
  lldb/trunk/packages/Python/lldbsuite/test/lang/c/conflicting-symbol/One/One.c
  lldb/trunk/packages/Python/lldbsuite/test/lang/c/conflicting-symbol/One/One.h
  
lldb/trunk/packages/Python/lldbsuite/test/lang/c/conflicting-symbol/One/OneConstant.c
  
lldb/trunk/packages/Python/lldbsuite/test/lang/c/conflicting-symbol/TestConflictingSymbol.py
  lldb/trunk/packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two.mk
  lldb/trunk/packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/Two.c
  lldb/trunk/packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/Two.h
  
lldb/trunk/packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/TwoConstant.c
  lldb/trunk/packages/Python/lldbsuite/test/lang/c/conflicting-symbol/main.c
  lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
  lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.h
  lldb/trunk/source/Symbol/SymbolContext.cpp

Index: lldb/trunk/packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/Two.h
===
--- lldb/trunk/packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/Two.h
+++ lldb/trunk/packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/Two.h
@@ -0,0 +1,4 @@
+#ifndef TWO_H
+#define TWO_H
+void two();
+#endif
Index: lldb/trunk/packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/Two.c
===
--- lldb/trunk/packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/Two.c
+++ lldb/trunk/packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/Two.c
@@ -0,0 +1,6 @@
+#include "Two.h"
+#include 
+
+void two() {
+  printf("Two\n"); // break here
+}
Index: lldb/trunk/packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/TwoConstant.c
===
--- lldb/trunk/packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/TwoConstant.c
+++ lldb/trunk/packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/TwoConstant.c
@@ -0,0 +1 @@
+int __attribute__ ((visibility("hidden"))) conflicting_symbol = 2;
Index: lldb/trunk/packages/Python/lldbsuite/test/lang/c/conflicting-symbol/One.mk
===
--- lldb/trunk/packages/Python/lldbsuite/test/lang/c/conflicting-symbol/One.mk
+++ lldb/trunk/packages/Python/lldbsuite/test/lang/c/conflicting-symbol/One.mk
@@ -0,0 +1,12 @@
+LEVEL := ../../../make
+
+DYLIB_NAME := One
+DYLIB_C_SOURCES := One/One.c One/OneConstant.c
+DYLIB_ONLY := YES
+
+include $(LEVEL)/Makefile.rules
+
+CFLAGS_EXTRAS += -fPIC
+
+One/OneConstant.o: One/OneConstant.c
+	$(CC) $(CFLAGS_NO_DEBUG) -c $< -o $@
Index: lldb/trunk/packages/Python/lldbsuite/test/lang/c/conflicting-symbol/main.c
===
--- lldb/trunk/packages/Python/lldbsuite/test/lang/c/conflicting-symbol/main.c
+++ lldb/trunk/packages/Python/lldbsuite/test/lang/c/conflicting-symbol/main.c
@@ -0,0 +1,11 @@
+#include "One/One.h"
+#include "Two/Two.h"
+
+#include 
+
+int main() {
+  one();
+  two();
+  printf("main\n"); // break here
+  return(0); 
+}
Index: lldb/trunk/packages/Python/lldbsuite/test/lang/c/conflicting-symbol/One/One.h
===
--- lldb/trunk/packages/Python/lldbsuite/test/lang/c/conflicting-symbol/One/One.h
+++ lldb/trunk/packages/Python/lldbsuite/test/lang/c/conflicting-symbol/One/One.h
@@ -0,0 +1,4 @@
+#ifndef ONE_H
+#define ONE_H
+void one();
+#endif
Index: lldb/trunk/packages/Python/lldbsuite/test/lang/c/conflicting-symbol/One/One.c
===
--- lldb/trunk/packages/Python/lldbsuite/test/lang/c/conflicting-symbol/One/One.c
+++ lldb/trunk/packages/Python/lldbsuite/test/lang/c/conflicting-symbol/One/One.c
@@ -0,0 +1,6 @@
+#include "One.h"
+#include 
+
+void one() {
+  printf("One\n"); // break here
+}
Index: lldb/trunk/packages/Python/lldbsuite/test/lang/c/conflicting-symbol/One/OneConstant.c
===
--- lldb/trunk/packages/Python/lldbsuite/test/lang/c/conflicting-symbol/One/OneConstant.c
+++ lldb/trunk/packages/Python/lldbsuite/test/lang/c/conflicting-symbol/One/OneConstant.c
@@ -0,0 +1 @@
+int __attribute__ ((visibility("hidden"))) conflicting_symbol = 1;
Index: 

[Lldb-commits] [PATCH] D33083: [Expression parser] Look up module symbols before hunting globally

2017-05-16 Thread Sean Callanan via Phabricator via lldb-commits
spyffe updated this revision to Diff 99211.
spyffe added a comment.

Used `CFLAGS_EXTRAS` instead of `CFLAGS` at Greg Clayton's suggestion.


https://reviews.llvm.org/D33083

Files:
  include/lldb/Symbol/SymbolContext.h
  packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Makefile
  packages/Python/lldbsuite/test/lang/c/conflicting-symbol/One.mk
  packages/Python/lldbsuite/test/lang/c/conflicting-symbol/One/One.c
  packages/Python/lldbsuite/test/lang/c/conflicting-symbol/One/One.h
  packages/Python/lldbsuite/test/lang/c/conflicting-symbol/One/OneConstant.c
  
packages/Python/lldbsuite/test/lang/c/conflicting-symbol/TestConflictingSymbol.py
  packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two.mk
  packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/Two.c
  packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/Two.h
  packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/TwoConstant.c
  packages/Python/lldbsuite/test/lang/c/conflicting-symbol/main.c
  source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
  source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.h
  source/Symbol/SymbolContext.cpp

Index: source/Symbol/SymbolContext.cpp
===
--- source/Symbol/SymbolContext.cpp
+++ source/Symbol/SymbolContext.cpp
@@ -799,6 +799,163 @@
   return true;
 }
 
+const Symbol *
+SymbolContext::FindBestGlobalDataSymbol(const ConstString , Status ) {
+  error.Clear();
+  
+  if (!target_sp) {
+return nullptr;
+  }
+  
+  Target  = *target_sp;
+  Module *module = module_sp.get();
+  
+  auto ProcessMatches = [this, , , module]
+  (SymbolContextList _list, Status ) -> const Symbol* {
+llvm::SmallVector external_symbols;
+llvm::SmallVector internal_symbols;
+const uint32_t matches = sc_list.GetSize();
+for (uint32_t i = 0; i < matches; ++i) {
+  SymbolContext sym_ctx;
+  sc_list.GetContextAtIndex(i, sym_ctx);
+  if (sym_ctx.symbol) {
+const Symbol *symbol = sym_ctx.symbol;
+const Address sym_address = symbol->GetAddress();
+
+if (sym_address.IsValid()) {
+  switch (symbol->GetType()) {
+case eSymbolTypeData:
+case eSymbolTypeRuntime:
+case eSymbolTypeAbsolute:
+case eSymbolTypeObjCClass:
+case eSymbolTypeObjCMetaClass:
+case eSymbolTypeObjCIVar:
+  if (symbol->GetDemangledNameIsSynthesized()) {
+// If the demangled name was synthesized, then don't use it
+// for expressions. Only let the symbol match if the mangled
+// named matches for these symbols.
+if (symbol->GetMangled().GetMangledName() != name)
+  break;
+  }
+  if (symbol->IsExternal()) {
+external_symbols.push_back(symbol);
+  } else {
+internal_symbols.push_back(symbol);
+  }
+  break;
+case eSymbolTypeReExported: {
+  ConstString reexport_name = symbol->GetReExportedSymbolName();
+  if (reexport_name) {
+ModuleSP reexport_module_sp;
+ModuleSpec reexport_module_spec;
+reexport_module_spec.GetPlatformFileSpec() =
+symbol->GetReExportedSymbolSharedLibrary();
+if (reexport_module_spec.GetPlatformFileSpec()) {
+  reexport_module_sp =
+  target.GetImages().FindFirstModule(reexport_module_spec);
+  if (!reexport_module_sp) {
+reexport_module_spec.GetPlatformFileSpec()
+.GetDirectory()
+.Clear();
+reexport_module_sp =
+target.GetImages().FindFirstModule(reexport_module_spec);
+  }
+}
+// Don't allow us to try and resolve a re-exported symbol if it is
+// the same as the current symbol
+if (name == symbol->GetReExportedSymbolName() &&
+module == reexport_module_sp.get())
+  return nullptr;
+
+return FindBestGlobalDataSymbol(
+symbol->GetReExportedSymbolName(), error);
+  }
+} break;
+  
+case eSymbolTypeCode: // We already lookup functions elsewhere
+case eSymbolTypeVariable:
+case eSymbolTypeLocal:
+case eSymbolTypeParam:
+case eSymbolTypeTrampoline:
+case eSymbolTypeInvalid:
+case eSymbolTypeException:
+case eSymbolTypeSourceFile:
+case eSymbolTypeHeaderFile:
+case eSymbolTypeObjectFile:
+case eSymbolTypeCommonBlock:
+case eSymbolTypeBlock:
+case eSymbolTypeVariableType:
+case eSymbolTypeLineEntry:
+  

[Lldb-commits] [PATCH] D33083: [Expression parser] Look up module symbols before hunting globally

2017-05-16 Thread Sean Callanan via Phabricator via lldb-commits
spyffe updated this revision to Diff 99186.
spyffe added a comment.

Updated to reflect Pavel Labath's suggestions:

- Used `CFLAGS_NO_DEBUG` where appropriate
- Marked the testcase as having no debug info variants


https://reviews.llvm.org/D33083

Files:
  include/lldb/Symbol/SymbolContext.h
  packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Makefile
  packages/Python/lldbsuite/test/lang/c/conflicting-symbol/One.mk
  packages/Python/lldbsuite/test/lang/c/conflicting-symbol/One/One.c
  packages/Python/lldbsuite/test/lang/c/conflicting-symbol/One/One.h
  packages/Python/lldbsuite/test/lang/c/conflicting-symbol/One/OneConstant.c
  
packages/Python/lldbsuite/test/lang/c/conflicting-symbol/TestConflictingSymbol.py
  packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two.mk
  packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/Two.c
  packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/Two.h
  packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/TwoConstant.c
  packages/Python/lldbsuite/test/lang/c/conflicting-symbol/main.c
  source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
  source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.h
  source/Symbol/SymbolContext.cpp

Index: source/Symbol/SymbolContext.cpp
===
--- source/Symbol/SymbolContext.cpp
+++ source/Symbol/SymbolContext.cpp
@@ -799,6 +799,163 @@
   return true;
 }
 
+const Symbol *
+SymbolContext::FindBestGlobalDataSymbol(const ConstString , Status ) {
+  error.Clear();
+  
+  if (!target_sp) {
+return nullptr;
+  }
+  
+  Target  = *target_sp;
+  Module *module = module_sp.get();
+  
+  auto ProcessMatches = [this, , , module]
+  (SymbolContextList _list, Status ) -> const Symbol* {
+llvm::SmallVector external_symbols;
+llvm::SmallVector internal_symbols;
+const uint32_t matches = sc_list.GetSize();
+for (uint32_t i = 0; i < matches; ++i) {
+  SymbolContext sym_ctx;
+  sc_list.GetContextAtIndex(i, sym_ctx);
+  if (sym_ctx.symbol) {
+const Symbol *symbol = sym_ctx.symbol;
+const Address sym_address = symbol->GetAddress();
+
+if (sym_address.IsValid()) {
+  switch (symbol->GetType()) {
+case eSymbolTypeData:
+case eSymbolTypeRuntime:
+case eSymbolTypeAbsolute:
+case eSymbolTypeObjCClass:
+case eSymbolTypeObjCMetaClass:
+case eSymbolTypeObjCIVar:
+  if (symbol->GetDemangledNameIsSynthesized()) {
+// If the demangled name was synthesized, then don't use it
+// for expressions. Only let the symbol match if the mangled
+// named matches for these symbols.
+if (symbol->GetMangled().GetMangledName() != name)
+  break;
+  }
+  if (symbol->IsExternal()) {
+external_symbols.push_back(symbol);
+  } else {
+internal_symbols.push_back(symbol);
+  }
+  break;
+case eSymbolTypeReExported: {
+  ConstString reexport_name = symbol->GetReExportedSymbolName();
+  if (reexport_name) {
+ModuleSP reexport_module_sp;
+ModuleSpec reexport_module_spec;
+reexport_module_spec.GetPlatformFileSpec() =
+symbol->GetReExportedSymbolSharedLibrary();
+if (reexport_module_spec.GetPlatformFileSpec()) {
+  reexport_module_sp =
+  target.GetImages().FindFirstModule(reexport_module_spec);
+  if (!reexport_module_sp) {
+reexport_module_spec.GetPlatformFileSpec()
+.GetDirectory()
+.Clear();
+reexport_module_sp =
+target.GetImages().FindFirstModule(reexport_module_spec);
+  }
+}
+// Don't allow us to try and resolve a re-exported symbol if it is
+// the same as the current symbol
+if (name == symbol->GetReExportedSymbolName() &&
+module == reexport_module_sp.get())
+  return nullptr;
+
+return FindBestGlobalDataSymbol(
+symbol->GetReExportedSymbolName(), error);
+  }
+} break;
+  
+case eSymbolTypeCode: // We already lookup functions elsewhere
+case eSymbolTypeVariable:
+case eSymbolTypeLocal:
+case eSymbolTypeParam:
+case eSymbolTypeTrampoline:
+case eSymbolTypeInvalid:
+case eSymbolTypeException:
+case eSymbolTypeSourceFile:
+case eSymbolTypeHeaderFile:
+case eSymbolTypeObjectFile:
+case eSymbolTypeCommonBlock:
+case eSymbolTypeBlock:
+

[Lldb-commits] [PATCH] D33083: [Expression parser] Look up module symbols before hunting globally

2017-05-15 Thread Sean Callanan via Phabricator via lldb-commits
spyffe updated this revision to Diff 99084.
spyffe added a comment.

Two changes after Greg and Pavel's suggestions:

- Moved the symbol lookup function into the global context; and
- Rewrote the test case's build system to be more generic.


https://reviews.llvm.org/D33083

Files:
  include/lldb/Symbol/SymbolContext.h
  packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Makefile
  packages/Python/lldbsuite/test/lang/c/conflicting-symbol/One.mk
  packages/Python/lldbsuite/test/lang/c/conflicting-symbol/One/One.c
  packages/Python/lldbsuite/test/lang/c/conflicting-symbol/One/One.h
  packages/Python/lldbsuite/test/lang/c/conflicting-symbol/One/OneConstant.c
  
packages/Python/lldbsuite/test/lang/c/conflicting-symbol/TestConflictingSymbol.py
  packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two.mk
  packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/Two.c
  packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/Two.h
  packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/TwoConstant.c
  packages/Python/lldbsuite/test/lang/c/conflicting-symbol/main.c
  source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
  source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.h
  source/Symbol/SymbolContext.cpp

Index: source/Symbol/SymbolContext.cpp
===
--- source/Symbol/SymbolContext.cpp
+++ source/Symbol/SymbolContext.cpp
@@ -799,6 +799,163 @@
   return true;
 }
 
+const Symbol *
+SymbolContext::FindBestGlobalDataSymbol(const ConstString , Status ) {
+  error.Clear();
+  
+  if (!target_sp) {
+return nullptr;
+  }
+  
+  Target  = *target_sp;
+  Module *module = module_sp.get();
+  
+  auto ProcessMatches = [this, , , module]
+  (SymbolContextList _list, Status ) -> const Symbol* {
+llvm::SmallVector external_symbols;
+llvm::SmallVector internal_symbols;
+const uint32_t matches = sc_list.GetSize();
+for (uint32_t i = 0; i < matches; ++i) {
+  SymbolContext sym_ctx;
+  sc_list.GetContextAtIndex(i, sym_ctx);
+  if (sym_ctx.symbol) {
+const Symbol *symbol = sym_ctx.symbol;
+const Address sym_address = symbol->GetAddress();
+
+if (sym_address.IsValid()) {
+  switch (symbol->GetType()) {
+case eSymbolTypeData:
+case eSymbolTypeRuntime:
+case eSymbolTypeAbsolute:
+case eSymbolTypeObjCClass:
+case eSymbolTypeObjCMetaClass:
+case eSymbolTypeObjCIVar:
+  if (symbol->GetDemangledNameIsSynthesized()) {
+// If the demangled name was synthesized, then don't use it
+// for expressions. Only let the symbol match if the mangled
+// named matches for these symbols.
+if (symbol->GetMangled().GetMangledName() != name)
+  break;
+  }
+  if (symbol->IsExternal()) {
+external_symbols.push_back(symbol);
+  } else {
+internal_symbols.push_back(symbol);
+  }
+  break;
+case eSymbolTypeReExported: {
+  ConstString reexport_name = symbol->GetReExportedSymbolName();
+  if (reexport_name) {
+ModuleSP reexport_module_sp;
+ModuleSpec reexport_module_spec;
+reexport_module_spec.GetPlatformFileSpec() =
+symbol->GetReExportedSymbolSharedLibrary();
+if (reexport_module_spec.GetPlatformFileSpec()) {
+  reexport_module_sp =
+  target.GetImages().FindFirstModule(reexport_module_spec);
+  if (!reexport_module_sp) {
+reexport_module_spec.GetPlatformFileSpec()
+.GetDirectory()
+.Clear();
+reexport_module_sp =
+target.GetImages().FindFirstModule(reexport_module_spec);
+  }
+}
+// Don't allow us to try and resolve a re-exported symbol if it is
+// the same as the current symbol
+if (name == symbol->GetReExportedSymbolName() &&
+module == reexport_module_sp.get())
+  return nullptr;
+
+return FindBestGlobalDataSymbol(
+symbol->GetReExportedSymbolName(), error);
+  }
+} break;
+  
+case eSymbolTypeCode: // We already lookup functions elsewhere
+case eSymbolTypeVariable:
+case eSymbolTypeLocal:
+case eSymbolTypeParam:
+case eSymbolTypeTrampoline:
+case eSymbolTypeInvalid:
+case eSymbolTypeException:
+case eSymbolTypeSourceFile:
+case eSymbolTypeHeaderFile:
+case eSymbolTypeObjectFile:
+case eSymbolTypeCommonBlock:
+case 

[Lldb-commits] [PATCH] D33077: [TypeSystem] Fix inspection of Objective-C object types

2017-05-15 Thread Sean Callanan via Phabricator via lldb-commits
spyffe closed this revision.
spyffe added a comment.

Committed r303110.


https://reviews.llvm.org/D33077



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


[Lldb-commits] [PATCH] D33077: [TypeSystem] Fix inspection of Objective-C object types

2017-05-15 Thread Sean Callanan via Phabricator via lldb-commits
spyffe updated this revision to Diff 99052.
spyffe marked an inline comment as done.
spyffe added a comment.

Updated the Makefile to fix a problem caught by Pavel Labath.
Also relocated the new test to lang/objc.


https://reviews.llvm.org/D33077

Files:
  packages/Python/lldbsuite/test/lang/objc/ptr_refs/Makefile
  packages/Python/lldbsuite/test/lang/objc/ptr_refs/TestPtrRefsObjC.py
  packages/Python/lldbsuite/test/lang/objc/ptr_refs/main.m
  source/Symbol/ClangASTContext.cpp

Index: source/Symbol/ClangASTContext.cpp
===
--- source/Symbol/ClangASTContext.cpp
+++ source/Symbol/ClangASTContext.cpp
@@ -4493,7 +4493,7 @@
 
 case clang::Type::ObjCObjectPointer: {
   const clang::ObjCObjectPointerType *objc_class_type =
-  qual_type->getAsObjCInterfacePointerType();
+  qual_type->getAs();
   const clang::ObjCInterfaceType *objc_interface_type =
   objc_class_type->getInterfaceType();
   if (objc_interface_type &&
@@ -4602,7 +4602,7 @@
 
 case clang::Type::ObjCObjectPointer: {
   const clang::ObjCObjectPointerType *objc_class_type =
-  qual_type->getAsObjCInterfacePointerType();
+  qual_type->getAs();
   const clang::ObjCInterfaceType *objc_interface_type =
   objc_class_type->getInterfaceType();
   if (objc_interface_type &&
@@ -5671,7 +5671,7 @@
 
   case clang::Type::ObjCObjectPointer: {
 const clang::ObjCObjectPointerType *objc_class_type =
-qual_type->getAsObjCInterfacePointerType();
+qual_type->getAs();
 const clang::ObjCInterfaceType *objc_interface_type =
 objc_class_type->getInterfaceType();
 if (objc_interface_type &&
@@ -5819,7 +5819,7 @@
 
   case clang::Type::ObjCObjectPointer: {
 const clang::ObjCObjectPointerType *objc_class_type =
-qual_type->getAsObjCInterfacePointerType();
+qual_type->getAs();
 const clang::ObjCInterfaceType *objc_interface_type =
 objc_class_type->getInterfaceType();
 if (objc_interface_type &&
Index: packages/Python/lldbsuite/test/lang/objc/ptr_refs/main.m
===
--- packages/Python/lldbsuite/test/lang/objc/ptr_refs/main.m
+++ packages/Python/lldbsuite/test/lang/objc/ptr_refs/main.m
@@ -0,0 +1,39 @@
+//===-- main.c --*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#import 
+
+@interface MyClass : NSObject {
+};
+-(void)test;
+@end
+
+@implementation MyClass
+-(void)test {
+printf("%p\n", self); // break here
+}
+@end
+
+@interface MyOwner : NSObject {
+  @public id ownedThing; // should be id, to test 
+};
+@end
+
+@implementation MyOwner
+@end
+
+int main (int argc, char const *argv[]) {
+@autoreleasepool {
+MyOwner *owner = [[MyOwner alloc] init];
+owner->ownedThing = [[MyClass alloc] init];
+[(MyClass*)owner->ownedThing test];
+}
+return 0;
+}
+
Index: packages/Python/lldbsuite/test/lang/objc/ptr_refs/TestPtrRefsObjC.py
===
--- packages/Python/lldbsuite/test/lang/objc/ptr_refs/TestPtrRefsObjC.py
+++ packages/Python/lldbsuite/test/lang/objc/ptr_refs/TestPtrRefsObjC.py
@@ -0,0 +1,50 @@
+"""
+Test the ptr_refs tool on Darwin with Objective-C
+"""
+
+from __future__ import print_function
+
+import os
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestPtrRefsObjC(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+@skipUnlessDarwin
+def test_ptr_refs(self):
+"""Test the ptr_refs tool on Darwin with Objective-C"""
+self.build()
+exe_name = 'a.out'
+exe = os.path.join(os.getcwd(), exe_name)
+
+target = self.dbg.CreateTarget(exe)
+self.assertTrue(target, VALID_TARGET)
+
+main_file_spec = lldb.SBFileSpec('main.m')
+breakpoint = target.BreakpointCreateBySourceRegex(
+'break', main_file_spec)
+self.assertTrue(breakpoint and
+breakpoint.GetNumLocations() == 1,
+VALID_BREAKPOINT)
+
+process = target.LaunchSimple(
+None, None, self.get_process_working_directory())
+self.assertTrue(process, PROCESS_IS_VALID)
+
+# Frame #0 should be on self.line1 and the break condition should hold.
+thread = lldbutil.get_stopped_thread(
+process, lldb.eStopReasonBreakpoint)
+self.assertTrue(
+thread.IsValid(),
+"There should be a thread stopped due to breakpoint condition")
+
+frame = thread.GetFrameAtIndex(0)
+
+  

[Lldb-commits] [PATCH] D33025: [DWARF parser] Produce correct template parameter packs

2017-05-11 Thread Sean Callanan via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL302833: [DWARF parser] Produce correct template parameter 
packs (authored by spyffe).

Changed prior to commit:
  https://reviews.llvm.org/D33025?vs=98388=98687#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D33025

Files:
  lldb/trunk/include/lldb/Symbol/ClangASTContext.h
  
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/class-template-parameter-pack/Makefile
  
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/class-template-parameter-pack/TestClassTemplateParameterPack.py
  
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/class-template-parameter-pack/main.cpp
  
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/function-template-parameter-pack/Makefile
  
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/function-template-parameter-pack/TestFunctionTemplateParameterPack.py
  
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/function-template-parameter-pack/main.cpp
  lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
  lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
  lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDefines.cpp
  lldb/trunk/source/Symbol/ClangASTContext.cpp

Index: lldb/trunk/include/lldb/Symbol/ClangASTContext.h
===
--- lldb/trunk/include/lldb/Symbol/ClangASTContext.h
+++ lldb/trunk/include/lldb/Symbol/ClangASTContext.h
@@ -275,17 +275,16 @@
 bool IsValid() const {
   if (args.empty())
 return false;
-  return args.size() == names.size();
-}
-
-size_t GetSize() const {
-  if (IsValid())
-return args.size();
-  return 0;
+  return args.size() == names.size() &&
+((bool)pack_name == (bool)packed_args) &&
+(!packed_args || !packed_args->packed_args);
 }
 
 llvm::SmallVector names;
 llvm::SmallVector args;
+
+const char * pack_name = nullptr;
+std::unique_ptr packed_args;
   };
 
   clang::FunctionTemplateDecl *
Index: lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/function-template-parameter-pack/TestFunctionTemplateParameterPack.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/function-template-parameter-pack/TestFunctionTemplateParameterPack.py
+++ lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/function-template-parameter-pack/TestFunctionTemplateParameterPack.py
@@ -0,0 +1,6 @@
+from lldbsuite.test import lldbinline
+from lldbsuite.test import decorators
+
+lldbinline.MakeInlineTest(
+__file__, globals(), [
+decorators.expectedFailureAll(bugnumber="rdar://problem/32096064")])
Index: lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/function-template-parameter-pack/main.cpp
===
--- lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/function-template-parameter-pack/main.cpp
+++ lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/function-template-parameter-pack/main.cpp
@@ -0,0 +1,24 @@
+//===-- main.cpp *- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LIDENSE.TXT for details.
+//
+//===--===//
+
+template  int staticSizeof() {
+  return sizeof(T);
+}
+
+template  int staticSizeof() {
+  return staticSizeof() + sizeof(T1);
+}
+
+int main (int argc, char const *argv[])
+{
+  int sz = staticSizeof();
+  return staticSizeof() != sz; //% self.expect("expression -- sz == staticSizeof()", "staticSizeof worked", substrs = ["true"])
+  //% self.expect("expression -- sz == staticSizeof() + sizeof(char)", "staticSizeof worked", substrs = ["true"])
+  //% self.expect("expression -- sz == staticSizeof() + sizeof(int) + sizeof(char)", "staticSizeof worked", substrs = ["true"])
+}
Index: lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/function-template-parameter-pack/Makefile
===
--- lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/function-template-parameter-pack/Makefile
+++ lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/function-template-parameter-pack/Makefile
@@ -0,0 +1,3 @@
+LEVEL = ../../../make
+CXX_SOURCES := main.cpp
+include $(LEVEL)/Makefile.rules
Index: lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/class-template-parameter-pack/Makefile
===
--- 

[Lldb-commits] [PATCH] D33083: [Expression parser] Look up module symbols before hunting globally

2017-05-11 Thread Sean Callanan via Phabricator via lldb-commits
spyffe updated this revision to Diff 98672.
spyffe added a comment.

Improved symbol lookup per Greg and Jim's suggestion, with flipped steps 2 and 
3.
Also added testing for the error case.


https://reviews.llvm.org/D33083

Files:
  packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Makefile
  packages/Python/lldbsuite/test/lang/c/conflicting-symbol/One/One.c
  packages/Python/lldbsuite/test/lang/c/conflicting-symbol/One/One.h
  packages/Python/lldbsuite/test/lang/c/conflicting-symbol/One/OneConstant.c
  
packages/Python/lldbsuite/test/lang/c/conflicting-symbol/TestConflictingSymbol.py
  packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/Two.c
  packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/Two.h
  packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/TwoConstant.c
  packages/Python/lldbsuite/test/lang/c/conflicting-symbol/main.c
  source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
  source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.h

Index: packages/Python/lldbsuite/test/lang/c/conflicting-symbol/main.c
===
--- packages/Python/lldbsuite/test/lang/c/conflicting-symbol/main.c
+++ packages/Python/lldbsuite/test/lang/c/conflicting-symbol/main.c
@@ -0,0 +1,11 @@
+#include "One/One.h"
+#include "Two/Two.h"
+
+#include 
+
+int main() {
+  one();
+  two();
+  printf("main\n"); // break here
+  return(0); 
+}
Index: packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/TwoConstant.c
===
--- packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/TwoConstant.c
+++ packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/TwoConstant.c
@@ -0,0 +1 @@
+__private_extern__ int conflicting_symbol = 2;
Index: packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/Two.h
===
--- packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/Two.h
+++ packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/Two.h
@@ -0,0 +1,4 @@
+#ifndef TWO_H
+#define TWO_H
+void two();
+#endif
Index: packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/Two.c
===
--- packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/Two.c
+++ packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/Two.c
@@ -0,0 +1,6 @@
+#include "Two.h"
+#include 
+
+void two() {
+  printf("Two\n"); // break here
+}
Index: packages/Python/lldbsuite/test/lang/c/conflicting-symbol/TestConflictingSymbol.py
===
--- packages/Python/lldbsuite/test/lang/c/conflicting-symbol/TestConflictingSymbol.py
+++ packages/Python/lldbsuite/test/lang/c/conflicting-symbol/TestConflictingSymbol.py
@@ -0,0 +1,87 @@
+"""Test that conflicting symbols in different shared libraries work correctly"""
+
+from __future__ import print_function
+
+
+import os
+import time
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestRealDefinition(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+@skipUnlessDarwin
+def test_frame_var_after_stop_at_implementation(self):
+if self.getArchitecture() == 'i386':
+self.skipTest("requires modern objc runtime")
+self.build()
+self.common_setup()
+
+One_line = line_number('One/One.c', '// break here')
+Two_line = line_number('Two/Two.c', '// break here')
+main_line = line_number('main.c', '// break here')
+lldbutil.run_break_set_by_file_and_line(
+self, 'One.c', One_line, num_expected_locations=1, loc_exact=True)
+lldbutil.run_break_set_by_file_and_line(
+self, 'Two.c', Two_line, num_expected_locations=1, loc_exact=True)
+lldbutil.run_break_set_by_file_and_line(
+self, 'main.c', main_line, num_expected_locations=1, loc_exact=True)
+
+self.runCmd("run", RUN_SUCCEEDED)
+
+# The stop reason of the thread should be breakpoint.
+self.expect("thread list", STOPPED_DUE_TO_BREAKPOINT,
+substrs=['stopped',
+ 'stop reason = breakpoint'])
+
+self.expect("breakpoint list -f", BREAKPOINT_HIT_ONCE,
+substrs=[' resolved, hit count = 1'])
+
+# This should display correctly.
+self.expect(
+"expr (unsigned long long)conflicting_symbol",
+"Symbol from One should be found",
+substrs=[
+"1"])
+
+self.runCmd("continue", RUN_SUCCEEDED)
+
+# The stop reason of the thread should be breakpoint.
+self.expect("thread list", STOPPED_DUE_TO_BREAKPOINT,
+substrs=['stopped',
+ 'stop reason = breakpoint'])
+
+ 

[Lldb-commits] [PATCH] D33083: [Expression parser] Look up module symbols before hunting globally

2017-05-11 Thread Sean Callanan via Phabricator via lldb-commits
spyffe added a comment.

I also would flip 2 and 3.  I'll give this a shot.


https://reviews.llvm.org/D33083



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


[Lldb-commits] [PATCH] D33083: [Expression parser] Look up module symbols before hunting globally

2017-05-10 Thread Sean Callanan via Phabricator via lldb-commits
spyffe created this revision.

When it resolved symbol-only variables, the expression parser currently looks 
only in the global module list.   It should prefer the current module.

I've fixed that behavior by making it search the current module first, and only 
search globally if it finds nothing.  I've also added a test case.


https://reviews.llvm.org/D33083

Files:
  packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Makefile
  packages/Python/lldbsuite/test/lang/c/conflicting-symbol/One/One.c
  packages/Python/lldbsuite/test/lang/c/conflicting-symbol/One/One.h
  packages/Python/lldbsuite/test/lang/c/conflicting-symbol/One/OneConstant.c
  
packages/Python/lldbsuite/test/lang/c/conflicting-symbol/TestConflictingSymbol.py
  packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/Two.c
  packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/Two.h
  packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/TwoConstant.c
  packages/Python/lldbsuite/test/lang/c/conflicting-symbol/main.c
  source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp

Index: source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
===
--- source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
+++ source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
@@ -1526,9 +1526,15 @@
   // We couldn't find a non-symbol variable for this.  Now we'll hunt for
   // a generic
   // data symbol, and -- if it is found -- treat it as a variable.
-
-  const Symbol *data_symbol = FindGlobalDataSymbol(*target, name);
-
+  
+  const Symbol *module_data_symbol = (m_parser_vars->m_sym_ctx.module_sp ?
+  FindGlobalDataSymbol(*target, name,
+   m_parser_vars->m_sym_ctx.module_sp.get()) :
+  nullptr);
+  
+  const Symbol *data_symbol = module_data_symbol ?
+  module_data_symbol : FindGlobalDataSymbol(*target, name);
+  
   if (data_symbol) {
 std::string warning("got name from symbols: ");
 warning.append(name.AsCString());
Index: packages/Python/lldbsuite/test/lang/c/conflicting-symbol/main.c
===
--- packages/Python/lldbsuite/test/lang/c/conflicting-symbol/main.c
+++ packages/Python/lldbsuite/test/lang/c/conflicting-symbol/main.c
@@ -0,0 +1,8 @@
+#include "One/One.h"
+#include "Two/Two.h"
+
+int main() {
+  one();
+  two();
+  return(0);
+}
Index: packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/TwoConstant.c
===
--- packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/TwoConstant.c
+++ packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/TwoConstant.c
@@ -0,0 +1 @@
+__private_extern__ int conflicting_symbol = 2;
\ No newline at end of file
Index: packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/Two.h
===
--- packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/Two.h
+++ packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/Two.h
@@ -0,0 +1,4 @@
+#ifndef TWO_H
+#define TWO_H
+void two();
+#endif
Index: packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/Two.c
===
--- packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/Two.c
+++ packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/Two.c
@@ -0,0 +1,6 @@
+#include "Two.h"
+#include 
+
+void two() {
+  printf("Two\n"); // break here
+}
Index: packages/Python/lldbsuite/test/lang/c/conflicting-symbol/TestConflictingSymbol.py
===
--- packages/Python/lldbsuite/test/lang/c/conflicting-symbol/TestConflictingSymbol.py
+++ packages/Python/lldbsuite/test/lang/c/conflicting-symbol/TestConflictingSymbol.py
@@ -0,0 +1,67 @@
+"""Test that conflicting symbols in different shared libraries work correctly"""
+
+from __future__ import print_function
+
+
+import os
+import time
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestRealDefinition(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+@skipUnlessDarwin
+def test_frame_var_after_stop_at_implementation(self):
+if self.getArchitecture() == 'i386':
+self.skipTest("requires modern objc runtime")
+self.build()
+self.common_setup()
+
+line = line_number('One/One.c', '// break here')
+line = line_number('Two/Two.c', '// break here')
+lldbutil.run_break_set_by_file_and_line(
+self, 'One.c', line, num_expected_locations=1, loc_exact=True)
+lldbutil.run_break_set_by_file_and_line(
+

[Lldb-commits] [PATCH] D33077: [TypeSystem] Fix inspection of Objective-C object types

2017-05-10 Thread Sean Callanan via Phabricator via lldb-commits
spyffe created this revision.

`ptr_refs` exposed a problem in ClangASTContext's implementation; it uses an 
accessor to convert a `QualType` into an `ObjCObjectPointerType`, but the 
accessor is not fully general.  `getAs()` is the safer way to go.

I've added a test case that uses `ptr_refs` in a way that would crash before 
the fix.


Repository:
  rL LLVM

https://reviews.llvm.org/D33077

Files:
  packages/Python/lldbsuite/test/functionalities/ptr_refs-objc/Makefile
  
packages/Python/lldbsuite/test/functionalities/ptr_refs-objc/TestPtrRefsObjC.py
  packages/Python/lldbsuite/test/functionalities/ptr_refs-objc/main.m
  source/Symbol/ClangASTContext.cpp

Index: source/Symbol/ClangASTContext.cpp
===
--- source/Symbol/ClangASTContext.cpp
+++ source/Symbol/ClangASTContext.cpp
@@ -4458,7 +4458,7 @@
 
 case clang::Type::ObjCObjectPointer: {
   const clang::ObjCObjectPointerType *objc_class_type =
-  qual_type->getAsObjCInterfacePointerType();
+  qual_type->getAs();
   const clang::ObjCInterfaceType *objc_interface_type =
   objc_class_type->getInterfaceType();
   if (objc_interface_type &&
@@ -4567,7 +4567,7 @@
 
 case clang::Type::ObjCObjectPointer: {
   const clang::ObjCObjectPointerType *objc_class_type =
-  qual_type->getAsObjCInterfacePointerType();
+  qual_type->getAs();
   const clang::ObjCInterfaceType *objc_interface_type =
   objc_class_type->getInterfaceType();
   if (objc_interface_type &&
@@ -5636,7 +5636,7 @@
 
   case clang::Type::ObjCObjectPointer: {
 const clang::ObjCObjectPointerType *objc_class_type =
-qual_type->getAsObjCInterfacePointerType();
+qual_type->getAs();
 const clang::ObjCInterfaceType *objc_interface_type =
 objc_class_type->getInterfaceType();
 if (objc_interface_type &&
@@ -5784,7 +5784,7 @@
 
   case clang::Type::ObjCObjectPointer: {
 const clang::ObjCObjectPointerType *objc_class_type =
-qual_type->getAsObjCInterfacePointerType();
+qual_type->getAs();
 const clang::ObjCInterfaceType *objc_interface_type =
 objc_class_type->getInterfaceType();
 if (objc_interface_type &&
Index: packages/Python/lldbsuite/test/functionalities/ptr_refs-objc/main.m
===
--- packages/Python/lldbsuite/test/functionalities/ptr_refs-objc/main.m
+++ packages/Python/lldbsuite/test/functionalities/ptr_refs-objc/main.m
@@ -0,0 +1,39 @@
+//===-- main.c --*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#import 
+
+@interface MyClass : NSObject {
+};
+-(void)test;
+@end
+
+@implementation MyClass
+-(void)test {
+printf("%p\n", self); // break here
+}
+@end
+
+@interface MyOwner : NSObject {
+  @public id ownedThing; // should be id, to test 
+};
+@end
+
+@implementation MyOwner
+@end
+
+int main (int argc, char const *argv[]) {
+@autoreleasepool {
+MyOwner *owner = [[MyOwner alloc] init];
+owner->ownedThing = [[MyClass alloc] init];
+[(MyClass*)owner->ownedThing test];
+}
+return 0;
+}
+
Index: packages/Python/lldbsuite/test/functionalities/ptr_refs-objc/TestPtrRefsObjC.py
===
--- packages/Python/lldbsuite/test/functionalities/ptr_refs-objc/TestPtrRefsObjC.py
+++ packages/Python/lldbsuite/test/functionalities/ptr_refs-objc/TestPtrRefsObjC.py
@@ -0,0 +1,50 @@
+"""
+Test the ptr_refs tool on Darwin with Objective-C
+"""
+
+from __future__ import print_function
+
+import os
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestPtrRefsObjC(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+@skipUnlessDarwin
+def test_ptr_refs(self):
+"""Test the ptr_refs tool on Darwin with Objective-C"""
+self.build()
+exe_name = 'a.out'
+exe = os.path.join(os.getcwd(), exe_name)
+
+target = self.dbg.CreateTarget(exe)
+self.assertTrue(target, VALID_TARGET)
+
+main_file_spec = lldb.SBFileSpec('main.m')
+breakpoint = target.BreakpointCreateBySourceRegex(
+'break', main_file_spec)
+self.assertTrue(breakpoint and
+breakpoint.GetNumLocations() == 1,
+VALID_BREAKPOINT)
+
+process = target.LaunchSimple(
+None, None, self.get_process_working_directory())
+self.assertTrue(process, PROCESS_IS_VALID)
+
+# Frame #0 should be on self.line1 and the break condition should hold.
+thread = 

[Lldb-commits] [PATCH] D33025: [DWARF parser] Produce correct template parameter packs

2017-05-09 Thread Sean Callanan via Phabricator via lldb-commits
spyffe created this revision.

Templates can end in parameter packs, like this

  template  struct MyStruct { /*...*/ };

LLDB does not currently support these parameter packs; it does not emit them 
into the template argument list at all.  This causes problems when you 
specialize, e.g.:

  template <> struct MyStruct { /*...*/ };
  template <> struct MyStruct : MyStruct { /*...*/ };

LLDB generates two template specializations, each with no template arguments, 
and then when they are imported by the `ASTImporter` into a parser's AST 
context we get a single specialization that inherits from itself, causing 
Clang's record layout mechanism to smash its stack.

This patch fixes the problem for classes and adds tests.  The tests for 
functions fail because Clang's `ASTImporter` can't import them at the moment, 
so I've xfailed that test.


Repository:
  rL LLVM

https://reviews.llvm.org/D33025

Files:
  include/lldb/Symbol/ClangASTContext.h
  packages/Python/lldbsuite/test/lang/cpp/class-template-parameter-pack/Makefile
  
packages/Python/lldbsuite/test/lang/cpp/class-template-parameter-pack/TestClassTemplateParameterPack.py
  packages/Python/lldbsuite/test/lang/cpp/class-template-parameter-pack/main.cpp
  
packages/Python/lldbsuite/test/lang/cpp/function-template-parameter-pack/Makefile
  
packages/Python/lldbsuite/test/lang/cpp/function-template-parameter-pack/TestFunctionTemplateParameterPack.py
  
packages/Python/lldbsuite/test/lang/cpp/function-template-parameter-pack/main.cpp
  source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
  source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
  source/Plugins/SymbolFile/DWARF/DWARFDefines.cpp
  source/Symbol/ClangASTContext.cpp

Index: packages/Python/lldbsuite/test/lang/cpp/function-template-parameter-pack/main.cpp
===
--- packages/Python/lldbsuite/test/lang/cpp/function-template-parameter-pack/main.cpp
+++ packages/Python/lldbsuite/test/lang/cpp/function-template-parameter-pack/main.cpp
@@ -0,0 +1,24 @@
+//===-- main.cpp *- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LIDENSE.TXT for details.
+//
+//===--===//
+
+template  int staticSizeof() {
+  return sizeof(T);
+}
+
+template  int staticSizeof() {
+  return staticSizeof() + sizeof(T1);
+}
+
+int main (int argc, char const *argv[])
+{
+  int sz = staticSizeof();
+  return staticSizeof() != sz; //% self.expect("expression -- sz == staticSizeof()", "staticSizeof worked", substrs = ["true"])
+  //% self.expect("expression -- sz == staticSizeof() + sizeof(char)", "staticSizeof worked", substrs = ["true"])
+  //% self.expect("expression -- sz == staticSizeof() + sizeof(int) + sizeof(char)", "staticSizeof worked", substrs = ["true"])
+}
Index: packages/Python/lldbsuite/test/lang/cpp/function-template-parameter-pack/TestFunctionTemplateParameterPack.py
===
--- packages/Python/lldbsuite/test/lang/cpp/function-template-parameter-pack/TestFunctionTemplateParameterPack.py
+++ packages/Python/lldbsuite/test/lang/cpp/function-template-parameter-pack/TestFunctionTemplateParameterPack.py
@@ -0,0 +1,6 @@
+from lldbsuite.test import lldbinline
+from lldbsuite.test import decorators
+
+lldbinline.MakeInlineTest(
+__file__, globals(), [
+decorators.expectedFailureAll(bugnumber="rdar://problem/32096064")])
Index: packages/Python/lldbsuite/test/lang/cpp/function-template-parameter-pack/Makefile
===
--- packages/Python/lldbsuite/test/lang/cpp/function-template-parameter-pack/Makefile
+++ packages/Python/lldbsuite/test/lang/cpp/function-template-parameter-pack/Makefile
@@ -0,0 +1,3 @@
+LEVEL = ../../../make
+CXX_SOURCES := main.cpp
+include $(LEVEL)/Makefile.rules
Index: packages/Python/lldbsuite/test/lang/cpp/class-template-parameter-pack/main.cpp
===
--- packages/Python/lldbsuite/test/lang/cpp/class-template-parameter-pack/main.cpp
+++ packages/Python/lldbsuite/test/lang/cpp/class-template-parameter-pack/main.cpp
@@ -0,0 +1,61 @@
+//===-- main.cpp *- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LIDENSE.TXT for details.
+//
+//===--===//
+
+template  

[Lldb-commits] [PATCH] D32100: [Expression parser] Return both types and variables

2017-04-24 Thread Sean Callanan via Phabricator via lldb-commits
spyffe closed this revision.
spyffe added a comment.

Closed by r301273


https://reviews.llvm.org/D32100



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


[Lldb-commits] [PATCH] D32375: [DWARF] Fix lookup in the abstract origins of inlined blocks/functions

2017-04-24 Thread Sean Callanan via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL301263: [DWARF] Fix lookup in the abstract origins of 
inlined blocks/functions (authored by spyffe).

Changed prior to commit:
  https://reviews.llvm.org/D32375?vs=96229=96477#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D32375

Files:
  lldb/trunk/packages/Python/lldbsuite/test/lang/c/inlines/main.c
  lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
  lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

Index: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -3682,7 +3682,7 @@
   break;
 
 case DW_TAG_lexical_block:
-  decl_ctx = (clang::DeclContext *)ResolveBlockDIE(die);
+  decl_ctx = GetDeclContextForBlock(die);
   try_parsing_type = false;
   break;
 
@@ -3704,6 +3704,69 @@
   return nullptr;
 }
 
+static bool IsSubroutine(const DWARFDIE ) {
+  switch (die.Tag()) {
+  case DW_TAG_subprogram:
+  case DW_TAG_inlined_subroutine:
+return true;
+  default:
+return false;
+  }
+}
+
+static DWARFDIE GetContainingFunctionWithAbstractOrigin(const DWARFDIE ) {
+  for (DWARFDIE candidate = die; candidate; candidate = candidate.GetParent()) {
+if (IsSubroutine(candidate)) {
+  if (candidate.GetReferencedDIE(DW_AT_abstract_origin)) {
+return candidate;
+  } else {
+return DWARFDIE();
+  }
+}
+  }
+  assert(!"Shouldn't call GetContainingFunctionWithAbstractOrigin on something "
+  "not in a function");
+  return DWARFDIE();
+}
+
+static DWARFDIE FindAnyChildWithAbstractOrigin(const DWARFDIE ) {
+  for (DWARFDIE candidate = context.GetFirstChild(); candidate.IsValid();
+   candidate = candidate.GetSibling()) {
+if (candidate.GetReferencedDIE(DW_AT_abstract_origin)) {
+  return candidate;
+}
+  }
+  return DWARFDIE();
+}
+
+static DWARFDIE FindFirstChildWithAbstractOrigin(const DWARFDIE ,
+ const DWARFDIE ) {
+  assert(IsSubroutine(function));
+  for (DWARFDIE context = block; context != function.GetParent();
+   context = context.GetParent()) {
+assert(!IsSubroutine(context) || context == function);
+if (DWARFDIE child = FindAnyChildWithAbstractOrigin(context)) {
+  return child;
+}
+  }
+  return DWARFDIE();
+}
+
+clang::DeclContext *
+DWARFASTParserClang::GetDeclContextForBlock(const DWARFDIE ) {
+  assert(die.Tag() == DW_TAG_lexical_block);
+  DWARFDIE containing_function_with_abstract_origin =
+  GetContainingFunctionWithAbstractOrigin(die);
+  if (!containing_function_with_abstract_origin) {
+return (clang::DeclContext *)ResolveBlockDIE(die);
+  }
+  DWARFDIE child = FindFirstChildWithAbstractOrigin(
+  die, containing_function_with_abstract_origin);
+  CompilerDeclContext decl_context =
+  GetDeclContextContainingUIDFromDWARF(child);
+  return (clang::DeclContext *)decl_context.GetOpaqueDeclContext();
+}
+
 clang::BlockDecl *DWARFASTParserClang::ResolveBlockDIE(const DWARFDIE ) {
   if (die && die.Tag() == DW_TAG_lexical_block) {
 clang::BlockDecl *decl =
Index: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
===
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
@@ -66,6 +66,8 @@
   class DelayedAddObjCClassProperty;
   typedef std::vector DelayedPropertyList;
 
+  clang::DeclContext *GetDeclContextForBlock(const DWARFDIE );
+
   clang::BlockDecl *ResolveBlockDIE(const DWARFDIE );
 
   clang::NamespaceDecl *ResolveNamespaceDIE(const DWARFDIE );
Index: lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -3051,7 +3051,13 @@
 case DW_TAG_lexical_block:
 case DW_TAG_subprogram:
   return die;
-
+case DW_TAG_inlined_subroutine: {
+  DWARFDIE abs_die = die.GetReferencedDIE(DW_AT_abstract_origin);
+  if (abs_die) {
+return abs_die;
+  }
+  break;
+}
 default:
   break;
 }
Index: lldb/trunk/packages/Python/lldbsuite/test/lang/c/inlines/main.c
===
--- lldb/trunk/packages/Python/lldbsuite/test/lang/c/inlines/main.c
+++ lldb/trunk/packages/Python/lldbsuite/test/lang/c/inlines/main.c
@@ -5,6 +5,11 @@
 
 void test2(int b) {
 printf("test2(%d)\n", b); //% 

[Lldb-commits] [PATCH] D32375: [DWARF] Fix lookup in the abstract origins of inlined blocks/functions

2017-04-24 Thread Sean Callanan via Phabricator via lldb-commits
spyffe added inline comments.



Comment at: source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:3723-3742
+static DWARFDIE FindAnyChildWithAbstractOrigin(const DWARFDIE ) {
+  for (DWARFDIE candidate = context.GetFirstChild(); candidate.IsValid();
+   candidate = candidate.GetSibling()) {
+if (candidate.GetReferencedDIE(DW_AT_abstract_origin)) {
+  return candidate;
+}
+  }

jingham wrote:
> The second of these functions seems dangerous since it doesn't check that 
> `block` is contained in `function`.  As such, I'm not sure it's worth 
> breaking it out into a separate function rather than inlining it in 
> GetDeclContextForBlock, where that relationship is already known.
I'll put in an `assert()` that `context` is a `DW_TAG_subprogram` or 
`DW_TAG_inlined_subroutine` iff it is `== function` to make sure people use it 
right.


Repository:
  rL LLVM

https://reviews.llvm.org/D32375



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


[Lldb-commits] [PATCH] D32375: [DWARF] Fix lookup in the abstract origins of inlined blocks/functions

2017-04-21 Thread Sean Callanan via Phabricator via lldb-commits
spyffe created this revision.
spyffe added a project: LLDB.

LLDB uses `clang::DeclContext`s for lookups, and variables get put into the 
`DeclContext` for their //abstract origin//.  (The abstract origin is a DWARF 
pointer that indicates the unique definition of inlined code.)  When the 
expression parser is looking for variables, it locates the `DeclContext` for 
the current context.  This needs to be done carefully, though, e.g.:

  __attribute__ ((always_inline)) void f(int a) {
{
  int b = a * 2;
}
  }
  
  void g() {
f(3);
  }

Here, if we're stopped in the inlined copy of `f`, we have to find the 
`DeclContext` corresponding to the definition of `f` – its abstract origin.  
Clang doesn't allow multiple functions with the same name and arguments to 
exist.  It also means that any variables we see must be placed in the 
appropriate `DeclContext`.

[Bug 1]: When stopped in an inline block, the function 
`GetDeclContextDIEContainingDIE` for that block doesn't properly construct a 
DeclContext for the abstract origin for inlined subroutines.  That means we get 
duplicated function `DeclContext`s, but function arguments only get put in the 
abstract origin's `DeclContext`, and as a result when we try to look for them 
in nested contexts they aren't found.

[Bug 2]: When stopped in an inline block, the DWARF (for space reasons) doesn't 
explicitly point to the abstract origin for that block.  This means that the 
function `GetClangDeclContextForDIE` returns a different `DeclContext` for each 
place the block is inlined.  However, any variables defined in the block have 
abstract origins, so they will only get placed in the `DeclContext` for their 
abstract origin.

In this fix, I've introduced a test covering both of these issues, and fixed 
them.

Bug 1 could be resolved simply by making sure we look up the abstract origin 
for inlined functions when looking up their DeclContexts on behalf of nested 
blocks.

For Bug 2, I've implemented an algorithm that makes the `DeclContext` for a 
block be the containing `DeclContext` for the closest entity we would find 
during lookup that has an abstract origin pointer.  That means that in the 
following situation:

  { // block 1
int a;
{ // block 2
  int b;
}
  }

if we looked up the `DeclContext` for block 2, we'd find the block containing 
the abstract origin of `b`, and lookup would proceed correctly because we'd see 
`b` and `a`.  However, in the situation

  { // block 1
int a;
{ // block 2
}
  }

since there isn't anything to look up in block 2, we can't determine its 
abstract origin (and there is no such pointer in the DWARF for blocks).  
However, we can walk up the parent chain and find `a`, and its abstract origin 
lives in the abstract origin of block 1.  So we simply say that the 
`DeclContext` for block 2 is the same as the `DeclContext` for block 1, which 
contains `a`.  Lookups will return the same results.


Repository:
  rL LLVM

https://reviews.llvm.org/D32375

Files:
  packages/Python/lldbsuite/test/lang/c/inlines/main.c
  source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

Index: packages/Python/lldbsuite/test/lang/c/inlines/main.c
===
--- packages/Python/lldbsuite/test/lang/c/inlines/main.c
+++ packages/Python/lldbsuite/test/lang/c/inlines/main.c
@@ -5,6 +5,11 @@
 
 void test2(int b) {
 printf("test2(%d)\n", b); //% self.expect("expression b", DATA_TYPES_DISPLAYED_CORRECTLY, substrs = ["42"])
+{
+  int c = b * 2;
+  printf("c=%d\n", c); //% self.expect("expression b", DATA_TYPES_DISPLAYED_CORRECTLY, substrs = ["42"])
+   //% self.expect("expression c", DATA_TYPES_DISPLAYED_CORRECTLY, substrs = ["84"])
+}
 }
 
 void test1(int a) {
Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -3051,7 +3051,13 @@
 case DW_TAG_lexical_block:
 case DW_TAG_subprogram:
   return die;
-
+case DW_TAG_inlined_subroutine: {
+  DWARFDIE abs_die = die.GetReferencedDIE(DW_AT_abstract_origin);
+  if (abs_die) {
+return abs_die;
+  }
+  break;
+}
 default:
   break;
 }
Index: source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
===
--- source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
+++ source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
@@ -66,6 +66,8 @@
   class DelayedAddObjCClassProperty;
   typedef std::vector DelayedPropertyList;
 
+  clang::DeclContext *GetDeclContextForBlock(const DWARFDIE );
+
   clang::BlockDecl 

[Lldb-commits] [PATCH] D32100: [Expression parser] Return both types and variables

2017-04-14 Thread Sean Callanan via Phabricator via lldb-commits
spyffe created this revision.

Many times a user wants to access a type when there's a variable of the same 
name, or a variable when there's a type of the same name.  Depending on the 
precise context, currently the expression parser can fail to resolve one or the 
other.

This is because `ClangExpressionDeclMap` has logic to limit the amount of 
information it searches, and that logic sometimes cuts down the search 
prematurely.  This patch removes some of those early exits.

In that sense, this patch trades performance (early exit is faster) for 
correctness.

I've also included two new test cases showing examples of this behavior – as 
well as modifying an existing test case that gets it wrong.


https://reviews.llvm.org/D32100

Files:
  packages/Python/lldbsuite/test/lang/cpp/llvm-style/Makefile
  packages/Python/lldbsuite/test/lang/cpp/llvm-style/TestLLVMStyle.py
  packages/Python/lldbsuite/test/lang/cpp/llvm-style/main.cc
  packages/Python/lldbsuite/test/lang/cpp/nsimport/TestCppNsImport.py
  packages/Python/lldbsuite/test/lang/cpp/symbols/Makefile
  packages/Python/lldbsuite/test/lang/cpp/symbols/TestSymbpls.py
  packages/Python/lldbsuite/test/lang/cpp/symbols/main.cc
  source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp

Index: source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
===
--- source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
+++ source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
@@ -814,9 +814,8 @@
 FindExternalVisibleDecls(context, lldb::ModuleSP(), namespace_decl,
  current_id);
   }
-
-  if (!context.m_found.variable && !context.m_found.local_vars_nsp)
-ClangASTSource::FindExternalVisibleDecls(context);
+  
+  ClangASTSource::FindExternalVisibleDecls(context);
 }
 
 void ClangExpressionDeclMap::FindExternalVisibleDecls(
@@ -1217,7 +1216,7 @@
 }
   }
 
-  if (var) {
+  if (var && !variable_found) {
 variable_found = true;
 valobj = ValueObjectVariable::Create(frame, var);
 AddOneVariable(context, var, valobj, current_id);
@@ -1248,303 +1247,297 @@
   }
 }
 
-if (!context.m_found.variable) {
-  const bool include_inlines = false;
-  const bool append = false;
+const bool include_inlines = false;
+const bool append = false;
 
-  if (namespace_decl && module_sp) {
-const bool include_symbols = false;
+if (namespace_decl && module_sp) {
+  const bool include_symbols = false;
 
-module_sp->FindFunctions(name, _decl, eFunctionNameTypeBase,
- include_symbols, include_inlines, append,
- sc_list);
-  } else if (target && !namespace_decl) {
-const bool include_symbols = true;
+  module_sp->FindFunctions(name, _decl, eFunctionNameTypeBase,
+   include_symbols, include_inlines, append,
+   sc_list);
+} else if (target && !namespace_decl) {
+  const bool include_symbols = true;
 
-// TODO Fix FindFunctions so that it doesn't return
-//   instance methods for eFunctionNameTypeBase.
+  // TODO Fix FindFunctions so that it doesn't return
+  //   instance methods for eFunctionNameTypeBase.
 
-target->GetImages().FindFunctions(name, eFunctionNameTypeFull,
-  include_symbols, include_inlines,
-  append, sc_list);
-  }
+  target->GetImages().FindFunctions(name, eFunctionNameTypeFull,
+include_symbols, include_inlines,
+append, sc_list);
+}
 
-  // If we found more than one function, see if we can use the
-  // frame's decl context to remove functions that are shadowed
-  // by other functions which match in type but are nearer in scope.
-  //
-  // AddOneFunction will not add a function whose type has already been
-  // added, so if there's another function in the list with a matching
-  // type, check to see if their decl context is a parent of the current
-  // frame's or was imported via a and using statement, and pick the
-  // best match according to lookup rules.
-  if (sc_list.GetSize() > 1) {
-// Collect some info about our frame's context.
-StackFrame *frame = m_parser_vars->m_exe_ctx.GetFramePtr();
-SymbolContext frame_sym_ctx;
-if (frame != nullptr)
-  frame_sym_ctx = frame->GetSymbolContext(lldb::eSymbolContextFunction |
-  lldb::eSymbolContextBlock);
-CompilerDeclContext frame_decl_context =
-frame_sym_ctx.block != nullptr
-? frame_sym_ctx.block->GetDeclContext()
-: CompilerDeclContext();
+

[Lldb-commits] [PATCH] D27291: Handle UTF-16 and UTF-32 constant CFStrings

2016-12-01 Thread Sean Callanan via Phabricator via lldb-commits
spyffe updated this revision to Diff 79935.
spyffe added a comment.

Updated to reflect Jim's comments.


Repository:
  rL LLVM

https://reviews.llvm.org/D27291

Files:
  packages/Python/lldbsuite/test/lang/objc/unicode-string/TestUnicodeString.py
  packages/Python/lldbsuite/test/lang/objc/unicode-string/main.m
  source/Plugins/ExpressionParser/Clang/IRForTarget.cpp

Index: source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
===
--- source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
+++ source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
@@ -498,42 +498,60 @@
   Constant *bytes_arg = cstr ? ConstantExpr::getBitCast(cstr, i8_ptr_ty)
  : Constant::getNullValue(i8_ptr_ty);
   Constant *numBytes_arg = ConstantInt::get(
-  m_intptr_ty, cstr ? string_array->getNumElements() - 1 : 0, false);
-  Constant *encoding_arg = ConstantInt::get(
-  i32_ty, 0x0600, false); /* 0x0600 is kCFStringEncodingASCII */
-  Constant *isExternal_arg =
-  ConstantInt::get(i8_ty, 0x0, false); /* 0x0 is false */
+  m_intptr_ty, cstr ? (string_array->getNumElements() - 1) * string_array->getElementByteSize() : 0, false);
+ int encoding_flags = 0;
+ switch (string_array->getElementByteSize()) {
+ case 1:
+   encoding_flags = 0x08000100; /* 0x08000100 is kCFStringEncodingUTF8 */
+   break;
+ case 2:
+   encoding_flags = 0x0100; /* 0x0100 is kCFStringEncodingUTF16 */
+   break;
+ case 4:
+   encoding_flags = 0x0c000100; /* 0x0c000100 is kCFStringEncodingUTF32 */
+   break;
+ default:
+   encoding_flags = 0x0600; /* fall back to 0x0600, kCFStringEncodingASCII */
+   if (log) {
+ log->Printf("Encountered an Objective-C constant string with unusual "
+ "element size %llu",
+ string_array->getElementByteSize());
+   }
+ }
+ Constant *encoding_arg = ConstantInt::get(i32_ty, encoding_flags, false);
+ Constant *isExternal_arg =
+ ConstantInt::get(i8_ty, 0x0, false); /* 0x0 is false */
 
-  Value *argument_array[5];
+ Value *argument_array[5];
 
-  argument_array[0] = alloc_arg;
-  argument_array[1] = bytes_arg;
-  argument_array[2] = numBytes_arg;
-  argument_array[3] = encoding_arg;
-  argument_array[4] = isExternal_arg;
+ argument_array[0] = alloc_arg;
+ argument_array[1] = bytes_arg;
+ argument_array[2] = numBytes_arg;
+ argument_array[3] = encoding_arg;
+ argument_array[4] = isExternal_arg;
 
-  ArrayRef CFSCWB_arguments(argument_array, 5);
+ ArrayRef CFSCWB_arguments(argument_array, 5);
 
-  FunctionValueCache CFSCWB_Caller(
-  [this, _arguments](llvm::Function *function) -> llvm::Value * {
-return CallInst::Create(
-m_CFStringCreateWithBytes, CFSCWB_arguments,
-"CFStringCreateWithBytes",
-llvm::cast(
-m_entry_instruction_finder.GetValue(function)));
-  });
+ FunctionValueCache CFSCWB_Caller(
+ [this, _arguments](llvm::Function *function) -> llvm::Value * {
+   return CallInst::Create(
+   m_CFStringCreateWithBytes, CFSCWB_arguments,
+   "CFStringCreateWithBytes",
+   llvm::cast(
+   m_entry_instruction_finder.GetValue(function)));
+ });
 
-  if (!UnfoldConstant(ns_str, nullptr, CFSCWB_Caller,
-  m_entry_instruction_finder, m_error_stream)) {
-if (log)
-  log->PutCString(
-  "Couldn't replace the NSString with the result of the call");
+ if (!UnfoldConstant(ns_str, nullptr, CFSCWB_Caller, m_entry_instruction_finder,
+ m_error_stream)) {
+   if (log)
+ log->PutCString(
+ "Couldn't replace the NSString with the result of the call");
 
-m_error_stream.Printf("error [IRForTarget internal]: Couldn't replace an "
-  "Objective-C constant string with a dynamic "
-  "string\n");
+   m_error_stream.Printf("error [IRForTarget internal]: Couldn't replace an "
+ "Objective-C constant string with a dynamic "
+ "string\n");
 
-return false;
+   return false;
   }
 
   ns_str->eraseFromParent();
@@ -642,31 +660,23 @@
 return false;
   }
 
-  if (nsstring_expr->getOpcode() != Instruction::GetElementPtr) {
-if (log)
-  log->Printf("NSString initializer's str element is not a "
-  "GetElementPtr expression, it's a %s",
-  nsstring_expr->getOpcodeName());
+  GlobalVariable *cstr_global = nullptr;
 
-m_error_stream.Printf("Internal error [IRForTarget]: An Objective-C "
-  "constant string's string initializer is not an "
-  "array\n");
-
-return false;
+  if (nsstring_expr->getOpcode() == Instruction::GetElementPtr) {
+Constant *nsstring_cstr = nsstring_expr->getOperand(0);
+cstr_global = dyn_cast(nsstring_cstr);
+  } else if (nsstring_expr->getOpcode() == 

[Lldb-commits] [PATCH] D27291: Handle UTF-16 and UTF-32 constant CFStrings

2016-11-30 Thread Sean Callanan via Phabricator via lldb-commits
spyffe created this revision.
spyffe added a reviewer: jingham.
spyffe added a subscriber: LLDB.
spyffe set the repository for this revision to rL LLVM.

We have a longstanding issue where the expression parser does not handle wide 
CFStrings (e.g., `@"凸凹"`) correctly, producing the useless error message

  Internal error [IRForTarget]: An Objective-C constant string's string 
initializer is not an array
  error: warning: expression result unused
  error: The expression could not be prepared to run in the target

This is just a side effect of the fact that we don't handle wide string 
constants when converting these to `CFStringCreateWithBytes`.  That function 
takes the string's encoding as an argument, so I made it work and added a 
testcase.


Repository:
  rL LLVM

https://reviews.llvm.org/D27291

Files:
  packages/Python/lldbsuite/test/lang/objc/unicode-string/TestUnicodeString.py
  packages/Python/lldbsuite/test/lang/objc/unicode-string/main.m
  source/Plugins/ExpressionParser/Clang/IRForTarget.cpp

Index: source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
===
--- source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
+++ source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
@@ -498,42 +498,55 @@
   Constant *bytes_arg = cstr ? ConstantExpr::getBitCast(cstr, i8_ptr_ty)
  : Constant::getNullValue(i8_ptr_ty);
   Constant *numBytes_arg = ConstantInt::get(
-  m_intptr_ty, cstr ? string_array->getNumElements() - 1 : 0, false);
-  Constant *encoding_arg = ConstantInt::get(
-  i32_ty, 0x0600, false); /* 0x0600 is kCFStringEncodingASCII */
-  Constant *isExternal_arg =
-  ConstantInt::get(i8_ty, 0x0, false); /* 0x0 is false */
+  m_intptr_ty, cstr ? (string_array->getNumElements() - 1) * string_array->getElementByteSize() : 0, false);
+ int encoding_flags = 0;
+ switch (string_array->getElementByteSize()) {
+ case 1:
+   encoding_flags = 0x08000100; /* 0x08000100 is kCFStringEncodingUTF8 */
+   break;
+ case 2:
+   encoding_flags = 0x0100; /* 0x0100 is kCFStringEncodingUTF16 */
+   break;
+ case 4:
+   encoding_flags = 0x0c000100; /* 0x0c000100 is kCFStringEncodingUTF32 */
+   break;
+ default:
+   encoding_flags = 0x0600; /* fall back to 0x0600, kCFStringEncodingASCII */
+ }
+ Constant *encoding_arg = ConstantInt::get(i32_ty, encoding_flags, false);
+ Constant *isExternal_arg =
+ ConstantInt::get(i8_ty, 0x0, false); /* 0x0 is false */
 
-  Value *argument_array[5];
+ Value *argument_array[5];
 
-  argument_array[0] = alloc_arg;
-  argument_array[1] = bytes_arg;
-  argument_array[2] = numBytes_arg;
-  argument_array[3] = encoding_arg;
-  argument_array[4] = isExternal_arg;
+ argument_array[0] = alloc_arg;
+ argument_array[1] = bytes_arg;
+ argument_array[2] = numBytes_arg;
+ argument_array[3] = encoding_arg;
+ argument_array[4] = isExternal_arg;
 
-  ArrayRef CFSCWB_arguments(argument_array, 5);
+ ArrayRef CFSCWB_arguments(argument_array, 5);
 
-  FunctionValueCache CFSCWB_Caller(
-  [this, _arguments](llvm::Function *function) -> llvm::Value * {
-return CallInst::Create(
-m_CFStringCreateWithBytes, CFSCWB_arguments,
-"CFStringCreateWithBytes",
-llvm::cast(
-m_entry_instruction_finder.GetValue(function)));
-  });
+ FunctionValueCache CFSCWB_Caller(
+ [this, _arguments](llvm::Function *function) -> llvm::Value * {
+   return CallInst::Create(
+   m_CFStringCreateWithBytes, CFSCWB_arguments,
+   "CFStringCreateWithBytes",
+   llvm::cast(
+   m_entry_instruction_finder.GetValue(function)));
+ });
 
-  if (!UnfoldConstant(ns_str, nullptr, CFSCWB_Caller,
-  m_entry_instruction_finder, m_error_stream)) {
-if (log)
-  log->PutCString(
-  "Couldn't replace the NSString with the result of the call");
+ if (!UnfoldConstant(ns_str, nullptr, CFSCWB_Caller, m_entry_instruction_finder,
+ m_error_stream)) {
+   if (log)
+ log->PutCString(
+ "Couldn't replace the NSString with the result of the call");
 
-m_error_stream.Printf("error [IRForTarget internal]: Couldn't replace an "
-  "Objective-C constant string with a dynamic "
-  "string\n");
+   m_error_stream.Printf("error [IRForTarget internal]: Couldn't replace an "
+ "Objective-C constant string with a dynamic "
+ "string\n");
 
-return false;
+   return false;
   }
 
   ns_str->eraseFromParent();
@@ -642,31 +655,23 @@
 return false;
   }
 
-  if (nsstring_expr->getOpcode() != Instruction::GetElementPtr) {
-if (log)
-  log->Printf("NSString initializer's str element is not a "
-  "GetElementPtr expression, it's a %s",
-  nsstring_expr->getOpcodeName());
+  GlobalVariable *cstr_global = nullptr;
 
-