Skip to content

Commit

Permalink
Instantiate 'std' templates explicitly in the expression evaluator
Browse files Browse the repository at this point in the history
Summary:
This patch is a follow-up for D58125. It implements the manual instantiation and merging of 'std' templates like
`std::vector` and `std::shared_ptr` with information from the debug info AST. This (finally) allows using these classes
in the expression evaluator like every other class (i.e. things like `vec.size()` and shared_ptr debugging now works, yay!).

The main logic is the `CxxModuleHandler` which intercept the ASTImporter import process and replaces any `std` decls
by decls from the C++ module. The decls from the C++ module are "imported" by just deserializing them directly in
the expression evaluation context. This is mostly because we don't want to rely on the ASTImporter to correctly import
these declarations, but in the future we should also move to the ASTImporter for that.

This patch doesn't contain the automatic desugaring for result variables. This means that if you call for example
`size` of `std::vector` you maybe get some very verbose typedef'd type as the variable type, e.g.
`std::vector<int, std::allocator<int>>::value_type`.

This is not only unreadable, it also means that our ASTImporter has to import all these types and associated
decls into the persisent variable context. This currently usually leads to some assertion getting triggered
in Clang when the ASTImporter either makes a mistake during importing or our debug info AST is inconsitent.
The current workaround I use in the tests is to just cast the result to it's actual type (e.g. `size_t` or `int`) to prevent
the ASTImporter from having to handle all these complicated decls.

The automatic desugaring will be a future patch because I'm not happy yet with the current code for that and because
I anticipate that this will be a controversial patch.

Reviewers: aprantl, shafik, jingham, martong, serge-sans-paille

Reviewed By: martong

Subscribers: balazske, rnkovacs, mgorny, mgrang, abidh, jdoerfert, lldb-commits

Tags: #c_modules_in_lldb, #lldb

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

llvm-svn: 359538
  • Loading branch information
Teemperor committed Apr 30, 2019
1 parent 180f1ae commit f74a4c1
Show file tree
Hide file tree
Showing 58 changed files with 1,244 additions and 1 deletion.
7 changes: 7 additions & 0 deletions lldb/include/lldb/Symbol/ClangASTContext.h
Expand Up @@ -104,6 +104,9 @@ class ClangASTContext : public TypeSystem {

clang::TargetInfo *getTargetInfo();

void setSema(clang::Sema *s);
clang::Sema *getSema() { return m_sema; }

void Clear();

const char *GetTargetTriple();
Expand Down Expand Up @@ -997,6 +1000,10 @@ class ClangASTContext : public TypeSystem {
uint32_t m_pointer_byte_size;
bool m_ast_owned;
bool m_can_evaluate_expressions;
/// The sema associated that is currently used to build this ASTContext.
/// May be null if we are already done parsing this ASTContext or the
/// ASTContext wasn't created by parsing source code.
clang::Sema * m_sema = nullptr;
// clang-format on
private:
// For ClangASTContext only
Expand Down
42 changes: 42 additions & 0 deletions lldb/include/lldb/Symbol/ClangASTImporter.h
Expand Up @@ -23,6 +23,7 @@

#include "lldb/Host/FileSystem.h"
#include "lldb/Symbol/CompilerDeclContext.h"
#include "lldb/Symbol/CxxModuleHandler.h"
#include "lldb/lldb-types.h"

#include "llvm/ADT/DenseMap.h"
Expand Down Expand Up @@ -243,6 +244,41 @@ class ClangASTImporter {
m_decls_to_deport(nullptr), m_decls_already_deported(nullptr),
m_master(master), m_source_ctx(source_ctx) {}

/// Scope guard that attaches a CxxModuleHandler to a Minion and deattaches
/// it at the end of the scope. Supports being used multiple times on the
/// same Minion instance in nested scopes.
class CxxModuleScope {
/// The handler we attach to the Minion.
CxxModuleHandler m_handler;
/// The Minion we are supposed to attach the handler to.
Minion &m_minion;
/// True iff we attached the handler to the Minion.
bool m_valid = false;

public:
CxxModuleScope(Minion &minion, clang::ASTContext *dst_ctx)
: m_minion(minion) {
// If the minion doesn't have a CxxModuleHandler yet, create one
// and attach it.
if (!minion.m_std_handler) {
m_handler = CxxModuleHandler(minion, dst_ctx);
m_valid = true;
minion.m_std_handler = &m_handler;
}
}
~CxxModuleScope() {
if (m_valid) {
// Make sure no one messed with the handler we placed.
assert(m_minion.m_std_handler == &m_handler);
m_minion.m_std_handler = nullptr;
}
}
};

protected:
llvm::Expected<clang::Decl *> ImportImpl(clang::Decl *From) override;

public:
// A call to "InitDeportWorkQueues" puts the minion into deport mode.
// In deport mode, every copied Decl that could require completion is
// recorded and placed into the decls_to_deport set.
Expand All @@ -266,10 +302,16 @@ class ClangASTImporter {

clang::Decl *GetOriginalDecl(clang::Decl *To) override;

/// Decls we should ignore when mapping decls back to their original
/// ASTContext. Used by the CxxModuleHandler to mark declarations that
/// were created from the 'std' C++ module to prevent that the Importer
/// tries to sync them with the broken equivalent in the debug info AST.
std::set<clang::Decl *> m_decls_to_ignore;
std::set<clang::NamedDecl *> *m_decls_to_deport;
std::set<clang::NamedDecl *> *m_decls_already_deported;
ClangASTImporter &m_master;
clang::ASTContext *m_source_ctx;
CxxModuleHandler *m_std_handler = nullptr;
};

typedef std::shared_ptr<Minion> MinionSP;
Expand Down
65 changes: 65 additions & 0 deletions lldb/include/lldb/Symbol/CxxModuleHandler.h
@@ -0,0 +1,65 @@
//===-- CxxModuleHandler.h --------------------------------------*- C++ -*-===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//

#ifndef liblldb_CxxModuleHandler_h_
#define liblldb_CxxModuleHandler_h_

#include "clang/AST/ASTImporter.h"
#include "clang/Sema/Sema.h"
#include "llvm/ADT/StringSet.h"

namespace lldb_private {

/// Handles importing decls into an ASTContext with an attached C++ module.
///
/// This class searches a C++ module (which must be attached to the target
/// ASTContext) for an equivalent decl to the one that should be imported.
/// If the decl that is found in the module is a suitable replacement
/// for the decl that should be imported, the module decl will be treated as
/// the result of the import process.
///
/// If the Decl that should be imported is a template specialization
/// that doesn't exist yet in the target ASTContext (e.g. `std::vector<int>`),
/// then this class tries to create the template specialization in the target
/// ASTContext. This is only possible if the CxxModuleHandler can determine
/// that instantiating this template is safe to do, e.g. because the target
/// decl is a container class from the STL.
class CxxModuleHandler {
/// The ASTImporter that should be used to import any Decls which aren't
/// directly handled by this class itself.
clang::ASTImporter *m_importer = nullptr;

/// The Sema instance of the target ASTContext.
clang::Sema *m_sema = nullptr;

/// List of template names this class currently supports. These are the
/// template names inside the 'std' namespace such as 'vector' or 'list'.
llvm::StringSet<> m_supported_templates;

/// Tries to manually instantiate the given foreign template in the target
/// context (designated by m_sema).
llvm::Optional<clang::Decl *> tryInstantiateStdTemplate(clang::Decl *d);

public:
CxxModuleHandler() = default;
CxxModuleHandler(clang::ASTImporter &importer, clang::ASTContext *target);

/// Attempts to import the given decl into the target ASTContext by
/// deserializing it from the 'std' module. This function returns a Decl if a
/// Decl has been deserialized from the 'std' module. Otherwise this function
/// returns nothing.
llvm::Optional<clang::Decl *> Import(clang::Decl *d);

/// Returns true iff this instance is capable of importing any declarations
/// in the target ASTContext.
bool isValid() const { return m_sema != nullptr; }
};

} // namespace lldb_private

#endif // liblldb_CxxModuleHandler_h_
@@ -0,0 +1,5 @@
LEVEL = ../../../make
USE_LIBCPP := 1
CXXFLAGS += $(MANDATORY_CXXMODULE_BUILD_CFLAGS)
CXX_SOURCES := main.cpp
include $(LEVEL)/Makefile.rules
@@ -0,0 +1,41 @@
"""
Test basic std::list functionality.
"""

from lldbsuite.test.decorators import *
from lldbsuite.test.lldbtest import *
from lldbsuite.test import lldbutil

class TestBasicDeque(TestBase):

mydir = TestBase.compute_mydir(__file__)

# FIXME: This should work on more setups, so remove these
# skipIf's in the future.
@add_test_categories(["libc++"])
@skipIf(compiler=no_match("clang"))
@skipIf(oslist=no_match(["linux"]))
@skipIf(debug_info=no_match(["dwarf"]))
def test(self):
self.build()

lldbutil.run_to_source_breakpoint(self,
"// Set break point at this line.", lldb.SBFileSpec("main.cpp"))

self.runCmd("settings set target.import-std-module true")

self.expect("expr (size_t)a.size()", substrs=['(size_t) $0 = 3'])
self.expect("expr (int)a.front()", substrs=['(int) $1 = 3'])
self.expect("expr (int)a.back()", substrs=['(int) $2 = 2'])

self.expect("expr std::sort(a.begin(), a.end())")
self.expect("expr (int)a.front()", substrs=['(int) $3 = 1'])
self.expect("expr (int)a.back()", substrs=['(int) $4 = 3'])

self.expect("expr std::reverse(a.begin(), a.end())")
self.expect("expr (int)a.front()", substrs=['(int) $5 = 3'])
self.expect("expr (int)a.back()", substrs=['(int) $6 = 1'])

self.expect("expr (int)(*a.begin())", substrs=['(int) $7 = 3'])
self.expect("expr (int)(*a.rbegin())", substrs=['(int) $8 = 1'])

@@ -0,0 +1,6 @@
#include <deque>

int main(int argc, char **argv) {
std::deque<int> a = {3, 1, 2};
return 0; // Set break point at this line.
}
@@ -0,0 +1,5 @@
LEVEL = ../../../make
USE_LIBCPP := 1
CXXFLAGS += $(MANDATORY_CXXMODULE_BUILD_CFLAGS)
CXX_SOURCES := main.cpp
include $(LEVEL)/Makefile.rules
@@ -0,0 +1,37 @@
"""
Test std::deque functionality with a decl from dbg info as content.
"""

from lldbsuite.test.decorators import *
from lldbsuite.test.lldbtest import *
from lldbsuite.test import lldbutil

class TestDbgInfoContentDeque(TestBase):

mydir = TestBase.compute_mydir(__file__)

# FIXME: This should work on more setups, so remove these
# skipIf's in the future.
@add_test_categories(["libc++"])
@skipIf(compiler=no_match("clang"))
@skipIf(oslist=no_match(["linux"]))
@skipIf(debug_info=no_match(["dwarf"]))
def test(self):
self.build()

lldbutil.run_to_source_breakpoint(self,
"// Set break point at this line.", lldb.SBFileSpec("main.cpp"))

self.runCmd("settings set target.import-std-module true")

self.expect("expr (size_t)a.size()", substrs=['(size_t) $0 = 3'])
self.expect("expr (int)a.front()->a", substrs=['(int) $1 = 3'])
self.expect("expr (int)a.back()->a", substrs=['(int) $2 = 2'])

self.expect("expr std::reverse(a.begin(), a.end())")
self.expect("expr (int)a.front()->a", substrs=['(int) $3 = 2'])
self.expect("expr (int)a.back()->a", substrs=['(int) $4 = 3'])

self.expect("expr (int)(a.begin()->a)", substrs=['(int) $5 = 2'])
self.expect("expr (int)(a.rbegin()->a)", substrs=['(int) $6 = 3'])

@@ -0,0 +1,10 @@
#include <deque>

struct Foo {
int a;
};

int main(int argc, char **argv) {
std::deque<Foo> a = {{3}, {1}, {2}};
return 0; // Set break point at this line.
}
@@ -0,0 +1,5 @@
LEVEL = ../../../make
USE_LIBCPP := 1
CXXFLAGS += $(MANDATORY_CXXMODULE_BUILD_CFLAGS)
CXX_SOURCES := main.cpp
include $(LEVEL)/Makefile.rules
@@ -0,0 +1,34 @@
"""
Test basic std::forward_list functionality.
"""

from lldbsuite.test.decorators import *
from lldbsuite.test.lldbtest import *
from lldbsuite.test import lldbutil

class TestBasicList(TestBase):

mydir = TestBase.compute_mydir(__file__)

# FIXME: This should work on more setups, so remove these
# skipIf's in the future.
@add_test_categories(["libc++"])
@skipIf(compiler=no_match("clang"))
@skipIf(oslist=no_match(["linux"]))
@skipIf(debug_info=no_match(["dwarf"]))
def test(self):
self.build()

lldbutil.run_to_source_breakpoint(self,
"// Set break point at this line.", lldb.SBFileSpec("main.cpp"))

self.runCmd("settings set target.import-std-module true")

self.expect("expr (size_t)std::distance(a.begin(), a.end())", substrs=['(size_t) $0 = 3'])
self.expect("expr (int)a.front()", substrs=['(int) $1 = 3'])

self.expect("expr a.sort()")
self.expect("expr (int)a.front()", substrs=['(int) $2 = 1'])

self.expect("expr (int)(*a.begin())", substrs=['(int) $3 = 1'])

@@ -0,0 +1,6 @@
#include <forward_list>

int main(int argc, char **argv) {
std::forward_list<int> a = {3, 1, 2};
return 0; // Set break point at this line.
}
@@ -0,0 +1,5 @@
LEVEL = ../../../make
USE_LIBCPP := 1
CXXFLAGS += $(MANDATORY_CXXMODULE_BUILD_CFLAGS)
CXX_SOURCES := main.cpp
include $(LEVEL)/Makefile.rules
@@ -0,0 +1,31 @@
"""
Test std::forward_list functionality with a decl from debug info as content.
"""

from lldbsuite.test.decorators import *
from lldbsuite.test.lldbtest import *
from lldbsuite.test import lldbutil

class TestDbgInfoContentForwardList(TestBase):

mydir = TestBase.compute_mydir(__file__)

# FIXME: This should work on more setups, so remove these
# skipIf's in the future.
@add_test_categories(["libc++"])
@skipIf(compiler=no_match("clang"))
@skipIf(oslist=no_match(["linux"]))
@skipIf(debug_info=no_match(["dwarf"]))
def test(self):
self.build()

lldbutil.run_to_source_breakpoint(self,
"// Set break point at this line.", lldb.SBFileSpec("main.cpp"))

self.runCmd("settings set target.import-std-module true")

self.expect("expr (size_t)std::distance(a.begin(), a.end())", substrs=['(size_t) $0 = 3'])
self.expect("expr (int)a.front()->a", substrs=['(int) $1 = 3'])

self.expect("expr (int)(a.begin()->a)", substrs=['(int) $2 = 3'])

@@ -0,0 +1,10 @@
#include <forward_list>

struct Foo {
int a;
};

int main(int argc, char **argv) {
std::forward_list<Foo> a = {{3}, {1}, {2}};
return 0; // Set break point at this line.
}
@@ -0,0 +1,5 @@
LEVEL = ../../../make
USE_LIBCPP := 1
CXXFLAGS += $(MANDATORY_CXXMODULE_BUILD_CFLAGS)
CXX_SOURCES := main.cpp
include $(LEVEL)/Makefile.rules

0 comments on commit f74a4c1

Please sign in to comment.