[PDB] Support PDB-backed expressions evaluation (+ fix stuck test)

Summary:
This patch contains several small fixes, which makes it possible to evaluate
expressions on Windows using information from PDB. The changes are:
- several sanitize checks;
- make IRExecutionUnit::MemoryManager::getSymbolAddress to not return a magic
  value on a failure, because callers wait 0 in this case;
- entry point required to be a file address, not RVA, in the ObjectFilePECOFF;
- do not crash on a debuggee second chance exception - it may be an expression
  evaluation crash. Also fix detection of "crushed" threads in tests;
- create parameter declarations for functions in AST to make it possible to call
  debugee functions from expressions;
- relax name searching rules for variables, functions, namespaces and types. Now
  it works just like in the DWARF plugin;
- fix endless recursion in SymbolFilePDB::ParseCompileUnitFunctionForPDBFunc.

Reviewers: zturner, asmith, stella.stamenova

Reviewed By: stella.stamenova, asmith

Tags: #lldb

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

12 files changed, 136 insertions, 33 deletions

diff --git a/lldb/lit/SymbolFile/PDB/Inputs/ExpressionsTest.cpp b/lldb/lit/SymbolFile/PDB/Inputs/ExpressionsTest.cpp
new file mode 100644
index 00000000000..3785cd3c64c
--- /dev/null
+++ b/lldb/lit/SymbolFile/PDB/Inputs/ExpressionsTest.cpp
@@ -0,0 +1,20 @@
+namespace N0 {
+namespace N1 {
+
+char *buf0 = nullptr;
+char buf1[] = {0, 1, 2, 3, 4, 5, 6, 7};
+
+char sum(char *buf, int size) {
+  char result = 0;
+  for (int i = 0; i < size; i++)
+    result += buf[i];
+  return result;
+}
+
+} // namespace N1
+} // namespace N0
+
+int main() {
+  char result = N0::N1::sum(N0::N1::buf1, sizeof(N0::N1::buf1));
+  return 0;
+}
diff --git a/lldb/lit/SymbolFile/PDB/Inputs/ExpressionsTest0.script b/lldb/lit/SymbolFile/PDB/Inputs/ExpressionsTest0.script
new file mode 100644
index 00000000000..d31a2abb68a
--- /dev/null
+++ b/lldb/lit/SymbolFile/PDB/Inputs/ExpressionsTest0.script
@@ -0,0 +1,7 @@
+breakpoint set --file ExpressionsTest.cpp --line 19
+run
+print result
+print N0::N1::sum(N0::N1::buf1, sizeof(N0::N1::buf1))
+print N1::sum(N1::buf1, sizeof(N1::buf1))
+print sum(buf1, sizeof(buf1))
+print sum(buf1, 1000000000)
diff --git a/lldb/lit/SymbolFile/PDB/Inputs/ExpressionsTest1.script b/lldb/lit/SymbolFile/PDB/Inputs/ExpressionsTest1.script
new file mode 100644
index 00000000000..dac887faa5b
--- /dev/null
+++ b/lldb/lit/SymbolFile/PDB/Inputs/ExpressionsTest1.script
@@ -0,0 +1 @@
+print sum(buf0, 1)
diff --git a/lldb/lit/SymbolFile/PDB/Inputs/ExpressionsTest2.script b/lldb/lit/SymbolFile/PDB/Inputs/ExpressionsTest2.script
new file mode 100644
index 00000000000..b19240baf99
--- /dev/null
+++ b/lldb/lit/SymbolFile/PDB/Inputs/ExpressionsTest2.script
@@ -0,0 +1,2 @@
+print sum(buf0, result - 28)
+print sum(buf1 + 3, 3)
diff --git a/lldb/lit/SymbolFile/PDB/expressions.test b/lldb/lit/SymbolFile/PDB/expressions.test
new file mode 100644
index 00000000000..76cb334e1ae
--- /dev/null
+++ b/lldb/lit/SymbolFile/PDB/expressions.test
@@ -0,0 +1,36 @@
+REQUIRES: system-windows, msvc
+RUN: %msvc_cl /Zi /GS- /c %S/Inputs/ExpressionsTest.cpp /Fo%t.obj
+RUN: %msvc_link /debug:full /nodefaultlib /entry:main %t.obj /out:%t.exe
+RUN: %lldb -b -s %S/Inputs/ExpressionsTest0.script -s %S/Inputs/ExpressionsTest1.script -s %S/Inputs/ExpressionsTest2.script -- %t.exe 2>&1 | FileCheck %s
+
+// Check the variable value through `print`
+CHECK: (lldb) print result
+CHECK: (char) $0 = '\x1c'
+
+// Call the function just like in the code
+CHECK: (lldb) print N0::N1::sum(N0::N1::buf1, sizeof(N0::N1::buf1))
+CHECK: (char) $1 = '\x1c'
+
+// Try the relaxed namespaces search
+CHECK: (lldb) print N1::sum(N1::buf1, sizeof(N1::buf1))
+CHECK: (char) $2 = '\x1c'
+
+// Try the relaxed variables and functions search
+CHECK: (lldb) print sum(buf1, sizeof(buf1))
+CHECK: (char) $3 = '\x1c'
+
+// Make a crash during expression calculation
+CHECK: (lldb) print sum(buf1, 1000000000)
+CHECK: The process has been returned to the state before expression evaluation.
+
+// Make one more crash
+CHECK: (lldb) print sum(buf0, 1)
+CHECK: The process has been returned to the state before expression evaluation.
+
+// Check if the process state was restored succesfully
+CHECK: (lldb) print sum(buf0, result - 28)
+CHECK: (char) $4 = '\0'
+
+// Call the function with arbitrary parameters
+CHECK: (lldb) print sum(buf1 + 3, 3)
+CHECK: (char) $5 = '\f'
diff --git a/lldb/packages/Python/lldbsuite/test/lldbutil.py b/lldb/packages/Python/lldbsuite/test/lldbutil.py
index d8dd67653e1..16950d7b5cd 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbutil.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbutil.py
@@ -736,6 +736,8 @@ def is_thread_crashed(test, thread):
     elif test.getPlatform() == "linux":
         return thread.GetStopReason() == lldb.eStopReasonSignal and thread.GetStopReasonDataAtIndex(
             0) == thread.GetProcess().GetUnixSignals().GetSignalNumberFromName("SIGSEGV")
+    elif test.getPlatform() == "windows":
+        return "Exception 0xc0000005" in thread.GetStopDescription(100)
     else:
         return "invalid address" in thread.GetStopDescription(100)
 
diff --git a/lldb/source/Expression/IRExecutionUnit.cpp b/lldb/source/Expression/IRExecutionUnit.cpp
index efa149737ee..68f9add1181 100644
--- a/lldb/source/Expression/IRExecutionUnit.cpp
+++ b/lldb/source/Expression/IRExecutionUnit.cpp
@@ -1013,7 +1013,7 @@ IRExecutionUnit::MemoryManager::getSymbolAddress(const std::string &Name) {
           Name.c_str());
 
     m_parent.ReportSymbolLookupError(name_cs);
-    return 0xbad0bad0;
+    return 0;
   } else {
     if (log)
       log->Printf("IRExecutionUnit::getSymbolAddress(Name=\"%s\") = %" PRIx64,
diff --git a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
index 78d03e27d8f..e792165734f 100644
--- a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -817,12 +817,12 @@ lldb_private::Address ObjectFilePECOFF::GetEntryPointAddress() {
     return m_entry_point_address;
 
   SectionList *section_list = GetSectionList();
-  addr_t offset = m_coff_header_opt.entry;
+  addr_t file_addr = m_coff_header_opt.entry + m_coff_header_opt.image_base;
 
   if (!section_list)
-    m_entry_point_address.SetOffset(offset);
+    m_entry_point_address.SetOffset(file_addr);
   else
-    m_entry_point_address.ResolveAddressUsingFileSections(offset, section_list);
+    m_entry_point_address.ResolveAddressUsingFileSections(file_addr, section_list);
   return m_entry_point_address;
 }
 
diff --git a/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp b/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
index 06ef6b76582..48855b9ce0b 100644
--- a/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
+++ b/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
@@ -957,8 +957,9 @@ ProcessWindows::OnDebugException(bool first_chance,
   }
 
   if (!first_chance) {
-    // Any second chance exception is an application crash by definition.
-    SetPrivateState(eStateCrashed);
+    // Not any second chance exception is an application crash by definition.
+    // It may be an expression evaluation crash.
+    SetPrivateState(eStateStopped);
   }
 
   ExceptionResult result = ExceptionResult::SendToApplication;
diff --git a/lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp b/lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
index 145587ac5be..2c7a1a045d5 100644
--- a/lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
+++ b/lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
@@ -921,6 +921,26 @@ PDBASTParser::GetDeclForSymbol(const llvm::pdb::PDBSymbol &symbol) {
         decl_context, name.c_str(), type->GetForwardCompilerType(), storage,
         func->hasInlineAttribute());
 
+    std::vector<clang::ParmVarDecl *> params;
+    if (std::unique_ptr<PDBSymbolTypeFunctionSig> sig = func->getSignature()) {
+      if (std::unique_ptr<ConcreteSymbolEnumerator<PDBSymbolTypeFunctionArg>>
+              arg_enum = sig->findAllChildren<PDBSymbolTypeFunctionArg>()) {
+        while (std::unique_ptr<PDBSymbolTypeFunctionArg> arg =
+                   arg_enum->getNext()) {
+          Type *arg_type = symbol_file->ResolveTypeUID(arg->getTypeId());
+          if (!arg_type)
+            continue;
+
+          clang::ParmVarDecl *param = m_ast.CreateParameterDeclaration(
+              nullptr, arg_type->GetForwardCompilerType(), clang::SC_None);
+          if (param)
+            params.push_back(param);
+        }
+      }
+    }
+    if (params.size())
+      m_ast.SetFunctionParameters(decl, params.data(), params.size());
+
     m_uid_to_decl[sym_id] = decl;
 
     return decl;
@@ -1030,6 +1050,7 @@ clang::DeclContext *PDBASTParser::GetDeclContextContainingSymbol(
                                               curr_context);
 
       m_parent_to_namespaces[curr_context].insert(namespace_decl);
+      m_namespaces.insert(namespace_decl);
 
       curr_context = namespace_decl;
     }
@@ -1065,18 +1086,23 @@ void PDBASTParser::ParseDeclsForDeclContext(
 clang::NamespaceDecl *
 PDBASTParser::FindNamespaceDecl(const clang::DeclContext *parent,
                                 llvm::StringRef name) {
-  if (!parent)
-    parent = m_ast.GetTranslationUnitDecl();
+  NamespacesSet *set;
+  if (parent) {
+    auto pit = m_parent_to_namespaces.find(parent);
+    if (pit == m_parent_to_namespaces.end())
+      return nullptr;
 
-  auto it = m_parent_to_namespaces.find(parent);
-  if (it == m_parent_to_namespaces.end())
-    return nullptr;
+    set = &pit->second;
+  } else {
+    set = &m_namespaces;
+  }
+  assert(set);
 
-  for (auto namespace_decl : it->second)
+  for (clang::NamespaceDecl *namespace_decl : *set)
     if (namespace_decl->getName().equals(name))
       return namespace_decl;
 
-  for (auto namespace_decl : it->second)
+  for (clang::NamespaceDecl *namespace_decl : *set)
     if (namespace_decl->isAnonymousNamespace())
       return FindNamespaceDecl(namespace_decl, name);
 
diff --git a/lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.h b/lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.h
index 28de07fd8cd..02353870ab6 100644
--- a/lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.h
+++ b/lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.h
@@ -69,7 +69,8 @@ private:
   typedef llvm::DenseMap<clang::CXXRecordDecl *, lldb::user_id_t>
       CXXRecordDeclToUidMap;
   typedef llvm::DenseMap<lldb::user_id_t, clang::Decl *> UidToDeclMap;
-  typedef llvm::DenseMap<clang::DeclContext *, std::set<clang::NamespaceDecl *>>
+  typedef std::set<clang::NamespaceDecl *> NamespacesSet;
+  typedef llvm::DenseMap<clang::DeclContext *, NamespacesSet>
       ParentToNamespacesMap;
   typedef llvm::DenseMap<clang::DeclContext *, lldb::user_id_t>
       DeclContextToUidMap;
@@ -109,6 +110,7 @@ private:
   CXXRecordDeclToUidMap m_forward_decl_to_uid;
   UidToDeclMap m_uid_to_decl;
   ParentToNamespacesMap m_parent_to_namespaces;
+  NamespacesSet m_namespaces;
   DeclContextToUidMap m_decl_context_to_uid;
 };
 
diff --git a/lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp b/lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
index da878a9c0d1..bdb5436d50d 100644
--- a/lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
+++ b/lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
@@ -286,6 +286,10 @@ lldb_private::Function *SymbolFilePDB::ParseCompileUnitFunctionForPDBFunc(
     const PDBSymbolFunc &pdb_func, const lldb_private::SymbolContext &sc) {
   lldbassert(sc.comp_unit && sc.module_sp.get());
 
+  if (FunctionSP result =
+          sc.comp_unit->FindFunctionByUID(pdb_func.getSymIndexId()))
+    return result.get();
+
   auto file_vm_addr = pdb_func.getVirtualAddress();
   if (file_vm_addr == LLDB_INVALID_ADDRESS || file_vm_addr == 0)
     return nullptr;
@@ -1047,8 +1051,6 @@ uint32_t SymbolFilePDB::FindGlobalVariables(
     const lldb_private::ConstString &name,
     const lldb_private::CompilerDeclContext *parent_decl_ctx,
     uint32_t max_matches, lldb_private::VariableList &variables) {
-  if (!parent_decl_ctx)
-    parent_decl_ctx = m_tu_decl_ctx_up.get();
   if (!DeclContextMatchesThisSymbolFile(parent_decl_ctx))
     return 0;
   if (name.IsEmpty())
@@ -1078,9 +1080,8 @@ uint32_t SymbolFilePDB::FindGlobalVariables(
     if (sc.comp_unit == nullptr)
       continue;
 
-    auto actual_parent_decl_ctx =
-        GetDeclContextContainingUID(result->getSymIndexId());
-    if (actual_parent_decl_ctx != *parent_decl_ctx)
+    if (parent_decl_ctx && GetDeclContextContainingUID(
+                               result->getSymIndexId()) != *parent_decl_ctx)
       continue;
 
     ParseVariables(sc, *pdb_data, &variables);
@@ -1269,20 +1270,28 @@ uint32_t SymbolFilePDB::FindFunctions(
     CacheFunctionNames();
 
     std::set<uint32_t> resolved_ids;
-    auto ResolveFn = [include_inlines, &name, &sc_list, &resolved_ids,
-                      this](UniqueCStringMap<uint32_t> &Names) {
+    auto ResolveFn = [this, &name, parent_decl_ctx, include_inlines, &sc_list,
+                      &resolved_ids](UniqueCStringMap<uint32_t> &Names) {
       std::vector<uint32_t> ids;
-      if (Names.GetValues(name, ids)) {
-        for (auto id : ids) {
-          if (resolved_ids.find(id) == resolved_ids.end()) {
-            if (ResolveFunction(id, include_inlines, sc_list))
-              resolved_ids.insert(id);
-          }
-        }
+      if (!Names.GetValues(name, ids))
+        return;
+
+      for (uint32_t id : ids) {
+        if (resolved_ids.find(id) != resolved_ids.end())
+          continue;
+
+        if (parent_decl_ctx &&
+            GetDeclContextContainingUID(id) != *parent_decl_ctx)
+          continue;
+
+        if (ResolveFunction(id, include_inlines, sc_list))
+          resolved_ids.insert(id);
       }
     };
     if (name_type_mask & eFunctionNameTypeFull) {
       ResolveFn(m_func_full_names);
+      ResolveFn(m_func_base_names);
+      ResolveFn(m_func_method_names);
     }
     if (name_type_mask & eFunctionNameTypeBase) {
       ResolveFn(m_func_base_names);
@@ -1470,8 +1479,6 @@ void SymbolFilePDB::FindTypesByName(
     llvm::StringRef name,
     const lldb_private::CompilerDeclContext *parent_decl_ctx,
     uint32_t max_matches, lldb_private::TypeMap &types) {
-  if (!parent_decl_ctx)
-    parent_decl_ctx = m_tu_decl_ctx_up.get();
   std::unique_ptr<IPDBEnumSymbols> results;
   if (name.empty())
     return;
@@ -1505,9 +1512,8 @@ void SymbolFilePDB::FindTypesByName(
     if (!ResolveTypeUID(result->getSymIndexId()))
       continue;
 
-    auto actual_parent_decl_ctx =
-        GetDeclContextContainingUID(result->getSymIndexId());
-    if (actual_parent_decl_ctx != *parent_decl_ctx)
+    if (parent_decl_ctx && GetDeclContextContainingUID(
+                               result->getSymIndexId()) != *parent_decl_ctx)
       continue;
 
     auto iter = m_types.find(result->getSymIndexId());