Skip to content

[lldb] Remove SymbolFilePDB and make the native one the default #113647

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

JDevlieghere
Copy link
Member

Remove SymbolFilePDB in favor of always using SymbolFileNativePDB. This effectively makes LLDB_USE_NATIVE_PDB_READER the default. The non-native (DIA based) PDB symbol file implementation was unmaintained and known to have issues.

Remove lldbPluginSymbolFilePDB in favor of always using
lldbPluginSymbolFileNativePDB and make LLDB_USE_NATIVE_PDB_READER the
default. The non-native (DIA based) PDB library was unmaintained and
known to have issues.
@llvmbot
Copy link
Member

llvmbot commented Oct 25, 2024

@llvm/pr-subscribers-lldb

Author: Jonas Devlieghere (JDevlieghere)

Changes

Remove SymbolFilePDB in favor of always using SymbolFileNativePDB. This effectively makes LLDB_USE_NATIVE_PDB_READER the default. The non-native (DIA based) PDB symbol file implementation was unmaintained and known to have issues.


Patch is 212.52 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/113647.diff

66 Files Affected:

  • (modified) lldb/source/Plugins/SymbolFile/CMakeLists.txt (-1)
  • (removed) lldb/source/Plugins/SymbolFile/PDB/CMakeLists.txt (-18)
  • (removed) lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp (-1455)
  • (removed) lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.h (-116)
  • (removed) lldb/source/Plugins/SymbolFile/PDB/PDBLocationToDWARFExpression.cpp (-182)
  • (removed) lldb/source/Plugins/SymbolFile/PDB/PDBLocationToDWARFExpression.h (-47)
  • (removed) lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp (-2053)
  • (removed) lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h (-252)
  • (modified) lldb/source/Plugins/TypeSystem/Clang/CMakeLists.txt (+1-1)
  • (modified) lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp (-9)
  • (modified) lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h (-3)
  • (modified) lldb/test/Shell/Minidump/Windows/Sigsegv/sigsegv.test (+1-1)
  • (modified) lldb/test/Shell/Process/Windows/exception_access_violation.cpp (+2-2)
  • (modified) lldb/test/Shell/Process/Windows/process_load.cpp (+1-1)
  • (modified) lldb/test/Shell/SymbolFile/NativePDB/ast-functions-msvc.cpp (+1-1)
  • (modified) lldb/test/Shell/SymbolFile/NativePDB/ast-functions.cpp (+1-1)
  • (modified) lldb/test/Shell/SymbolFile/NativePDB/ast-methods.cpp (+2-2)
  • (modified) lldb/test/Shell/SymbolFile/NativePDB/ast-types.cpp (+1-1)
  • (modified) lldb/test/Shell/SymbolFile/NativePDB/bitfields.cpp (+1-1)
  • (modified) lldb/test/Shell/SymbolFile/NativePDB/blocks.s (+1-1)
  • (modified) lldb/test/Shell/SymbolFile/NativePDB/break-by-function.cpp (+1-1)
  • (modified) lldb/test/Shell/SymbolFile/NativePDB/break-by-line.cpp (+1-1)
  • (modified) lldb/test/Shell/SymbolFile/NativePDB/class_layout.cpp (+1-1)
  • (modified) lldb/test/Shell/SymbolFile/NativePDB/disassembly.cpp (+1-1)
  • (modified) lldb/test/Shell/SymbolFile/NativePDB/function-types-builtins.cpp (+1-1)
  • (modified) lldb/test/Shell/SymbolFile/NativePDB/function-types-calling-conv.cpp (+1-1)
  • (modified) lldb/test/Shell/SymbolFile/NativePDB/function-types-classes.cpp (+1-1)
  • (modified) lldb/test/Shell/SymbolFile/NativePDB/global-classes.cpp (+1-1)
  • (modified) lldb/test/Shell/SymbolFile/NativePDB/global-ctor-dtor.cpp (+1-1)
  • (modified) lldb/test/Shell/SymbolFile/NativePDB/globals-bss.cpp (+1-1)
  • (modified) lldb/test/Shell/SymbolFile/NativePDB/globals-fundamental.cpp (+1-1)
  • (modified) lldb/test/Shell/SymbolFile/NativePDB/icf.cpp (+1-1)
  • (modified) lldb/test/Shell/SymbolFile/NativePDB/incomplete-tag-type.cpp (+1-1)
  • (modified) lldb/test/Shell/SymbolFile/NativePDB/inline_sites.test (+1-1)
  • (modified) lldb/test/Shell/SymbolFile/NativePDB/inline_sites_live.cpp (+1-1)
  • (modified) lldb/test/Shell/SymbolFile/NativePDB/load-pdb.cpp (+1-1)
  • (modified) lldb/test/Shell/SymbolFile/NativePDB/local-variables-registers.s (+1-1)
  • (modified) lldb/test/Shell/SymbolFile/NativePDB/local-variables.cpp (+1-1)
  • (modified) lldb/test/Shell/SymbolFile/NativePDB/locate-pdb.cpp (+2-2)
  • (modified) lldb/test/Shell/SymbolFile/NativePDB/lookup-by-address.cpp (+1-1)
  • (modified) lldb/test/Shell/SymbolFile/NativePDB/lookup-by-types.cpp (+1-1)
  • (modified) lldb/test/Shell/SymbolFile/NativePDB/nested-blocks-same-address.s (+1-1)
  • (modified) lldb/test/Shell/SymbolFile/NativePDB/nested-types.cpp (+1-1)
  • (modified) lldb/test/Shell/SymbolFile/NativePDB/s_constant.cpp (+1-1)
  • (modified) lldb/test/Shell/SymbolFile/NativePDB/source-list.cpp (+1-1)
  • (modified) lldb/test/Shell/SymbolFile/NativePDB/stack_unwinding01.cpp (+1-1)
  • (modified) lldb/test/Shell/SymbolFile/NativePDB/tag-types.cpp (+1-1)
  • (modified) lldb/test/Shell/SymbolFile/NativePDB/typedefs.cpp (+1-1)
  • (modified) lldb/test/Shell/SymbolFile/PDB/ast-restore.test (+2-4)
  • (modified) lldb/test/Shell/SymbolFile/PDB/compilands.test (+1-2)
  • (modified) lldb/test/Shell/SymbolFile/PDB/function-level-linking.test (+1-2)
  • (modified) lldb/test/Shell/SymbolFile/PDB/variables-locations.test (+1-2)
  • (modified) lldb/unittests/SymbolFile/CMakeLists.txt (-3)
  • (modified) lldb/unittests/SymbolFile/DWARF/CMakeLists.txt (-1)
  • (modified) lldb/unittests/SymbolFile/DWARF/SymbolFileDWARFTests.cpp (+1-4)
  • (removed) lldb/unittests/SymbolFile/PDB/CMakeLists.txt (-25)
  • (removed) lldb/unittests/SymbolFile/PDB/Inputs/test-pdb-alt.cpp (-7)
  • (removed) lldb/unittests/SymbolFile/PDB/Inputs/test-pdb-nested.h (-6)
  • (removed) lldb/unittests/SymbolFile/PDB/Inputs/test-pdb-types.cpp (-77)
  • (removed) lldb/unittests/SymbolFile/PDB/Inputs/test-pdb-types.exe ()
  • (removed) lldb/unittests/SymbolFile/PDB/Inputs/test-pdb-types.pdb ()
  • (removed) lldb/unittests/SymbolFile/PDB/Inputs/test-pdb.cpp (-9)
  • (removed) lldb/unittests/SymbolFile/PDB/Inputs/test-pdb.exe ()
  • (removed) lldb/unittests/SymbolFile/PDB/Inputs/test-pdb.h (-10)
  • (removed) lldb/unittests/SymbolFile/PDB/Inputs/test-pdb.pdb ()
  • (removed) lldb/unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp (-628)
diff --git a/lldb/source/Plugins/SymbolFile/CMakeLists.txt b/lldb/source/Plugins/SymbolFile/CMakeLists.txt
index 106387b45ec1a9..6b0a3901cc4b9e 100644
--- a/lldb/source/Plugins/SymbolFile/CMakeLists.txt
+++ b/lldb/source/Plugins/SymbolFile/CMakeLists.txt
@@ -3,5 +3,4 @@ add_subdirectory(CTF)
 add_subdirectory(DWARF)
 add_subdirectory(JSON)
 add_subdirectory(NativePDB)
-add_subdirectory(PDB)
 add_subdirectory(Symtab)
diff --git a/lldb/source/Plugins/SymbolFile/PDB/CMakeLists.txt b/lldb/source/Plugins/SymbolFile/PDB/CMakeLists.txt
deleted file mode 100644
index ceeb173a99e1d6..00000000000000
--- a/lldb/source/Plugins/SymbolFile/PDB/CMakeLists.txt
+++ /dev/null
@@ -1,18 +0,0 @@
-add_lldb_library(lldbPluginSymbolFilePDB PLUGIN
-  PDBASTParser.cpp
-  PDBLocationToDWARFExpression.cpp
-  SymbolFilePDB.cpp
-
-  LINK_LIBS
-    lldbCore
-    lldbPluginSymbolFileNativePDB
-    lldbSymbol
-    lldbUtility
-    lldbPluginTypeSystemClang
-  CLANG_LIBS
-    clangAST
-    clangLex
-  LINK_COMPONENTS
-    DebugInfoPDB
-    Support
-  )
diff --git a/lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp b/lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
deleted file mode 100644
index fa3530a0c22ff3..00000000000000
--- a/lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
+++ /dev/null
@@ -1,1455 +0,0 @@
-//===-- PDBASTParser.cpp --------------------------------------------------===//
-//
-// 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
-//
-//===----------------------------------------------------------------------===//
-
-#include "PDBASTParser.h"
-
-#include "SymbolFilePDB.h"
-
-#include "clang/AST/CharUnits.h"
-#include "clang/AST/Decl.h"
-#include "clang/AST/DeclCXX.h"
-
-#include "Plugins/ExpressionParser/Clang/ClangASTMetadata.h"
-#include "Plugins/ExpressionParser/Clang/ClangUtil.h"
-#include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
-#include "lldb/Core/Declaration.h"
-#include "lldb/Core/Module.h"
-#include "lldb/Symbol/SymbolFile.h"
-#include "lldb/Symbol/TypeMap.h"
-#include "lldb/Symbol/TypeSystem.h"
-#include "lldb/Utility/LLDBLog.h"
-#include "llvm/DebugInfo/PDB/ConcreteSymbolEnumerator.h"
-#include "llvm/DebugInfo/PDB/IPDBLineNumber.h"
-#include "llvm/DebugInfo/PDB/IPDBSourceFile.h"
-#include "llvm/DebugInfo/PDB/PDBSymbol.h"
-#include "llvm/DebugInfo/PDB/PDBSymbolData.h"
-#include "llvm/DebugInfo/PDB/PDBSymbolFunc.h"
-#include "llvm/DebugInfo/PDB/PDBSymbolTypeArray.h"
-#include "llvm/DebugInfo/PDB/PDBSymbolTypeBaseClass.h"
-#include "llvm/DebugInfo/PDB/PDBSymbolTypeBuiltin.h"
-#include "llvm/DebugInfo/PDB/PDBSymbolTypeEnum.h"
-#include "llvm/DebugInfo/PDB/PDBSymbolTypeFunctionArg.h"
-#include "llvm/DebugInfo/PDB/PDBSymbolTypeFunctionSig.h"
-#include "llvm/DebugInfo/PDB/PDBSymbolTypePointer.h"
-#include "llvm/DebugInfo/PDB/PDBSymbolTypeTypedef.h"
-#include "llvm/DebugInfo/PDB/PDBSymbolTypeUDT.h"
-
-#include "Plugins/Language/CPlusPlus/MSVCUndecoratedNameParser.h"
-#include <optional>
-
-using namespace lldb;
-using namespace lldb_private;
-using namespace llvm::pdb;
-
-static int TranslateUdtKind(PDB_UdtType pdb_kind) {
-  switch (pdb_kind) {
-  case PDB_UdtType::Class:
-    return llvm::to_underlying(clang::TagTypeKind::Class);
-  case PDB_UdtType::Struct:
-    return llvm::to_underlying(clang::TagTypeKind::Struct);
-  case PDB_UdtType::Union:
-    return llvm::to_underlying(clang::TagTypeKind::Union);
-  case PDB_UdtType::Interface:
-    return llvm::to_underlying(clang::TagTypeKind::Interface);
-  }
-  llvm_unreachable("unsuported PDB UDT type");
-}
-
-static lldb::Encoding TranslateBuiltinEncoding(PDB_BuiltinType type) {
-  switch (type) {
-  case PDB_BuiltinType::Float:
-    return lldb::eEncodingIEEE754;
-  case PDB_BuiltinType::Int:
-  case PDB_BuiltinType::Long:
-  case PDB_BuiltinType::Char:
-    return lldb::eEncodingSint;
-  case PDB_BuiltinType::Bool:
-  case PDB_BuiltinType::Char16:
-  case PDB_BuiltinType::Char32:
-  case PDB_BuiltinType::UInt:
-  case PDB_BuiltinType::ULong:
-  case PDB_BuiltinType::HResult:
-  case PDB_BuiltinType::WCharT:
-    return lldb::eEncodingUint;
-  default:
-    return lldb::eEncodingInvalid;
-  }
-}
-
-static lldb::Encoding TranslateEnumEncoding(PDB_VariantType type) {
-  switch (type) {
-  case PDB_VariantType::Int8:
-  case PDB_VariantType::Int16:
-  case PDB_VariantType::Int32:
-  case PDB_VariantType::Int64:
-    return lldb::eEncodingSint;
-
-  case PDB_VariantType::UInt8:
-  case PDB_VariantType::UInt16:
-  case PDB_VariantType::UInt32:
-  case PDB_VariantType::UInt64:
-    return lldb::eEncodingUint;
-
-  default:
-    break;
-  }
-
-  return lldb::eEncodingSint;
-}
-
-static CompilerType
-GetBuiltinTypeForPDBEncodingAndBitSize(TypeSystemClang &clang_ast,
-                                       const PDBSymbolTypeBuiltin &pdb_type,
-                                       Encoding encoding, uint32_t width) {
-  clang::ASTContext &ast = clang_ast.getASTContext();
-
-  switch (pdb_type.getBuiltinType()) {
-  default:
-    break;
-  case PDB_BuiltinType::None:
-    return CompilerType();
-  case PDB_BuiltinType::Void:
-    return clang_ast.GetBasicType(eBasicTypeVoid);
-  case PDB_BuiltinType::Char:
-    return clang_ast.GetBasicType(eBasicTypeChar);
-  case PDB_BuiltinType::Bool:
-    return clang_ast.GetBasicType(eBasicTypeBool);
-  case PDB_BuiltinType::Long:
-    if (width == ast.getTypeSize(ast.LongTy))
-      return CompilerType(clang_ast.weak_from_this(),
-                          ast.LongTy.getAsOpaquePtr());
-    if (width == ast.getTypeSize(ast.LongLongTy))
-      return CompilerType(clang_ast.weak_from_this(),
-                          ast.LongLongTy.getAsOpaquePtr());
-    break;
-  case PDB_BuiltinType::ULong:
-    if (width == ast.getTypeSize(ast.UnsignedLongTy))
-      return CompilerType(clang_ast.weak_from_this(),
-                          ast.UnsignedLongTy.getAsOpaquePtr());
-    if (width == ast.getTypeSize(ast.UnsignedLongLongTy))
-      return CompilerType(clang_ast.weak_from_this(),
-                          ast.UnsignedLongLongTy.getAsOpaquePtr());
-    break;
-  case PDB_BuiltinType::WCharT:
-    if (width == ast.getTypeSize(ast.WCharTy))
-      return CompilerType(clang_ast.weak_from_this(),
-                          ast.WCharTy.getAsOpaquePtr());
-    break;
-  case PDB_BuiltinType::Char16:
-    return CompilerType(clang_ast.weak_from_this(),
-                        ast.Char16Ty.getAsOpaquePtr());
-  case PDB_BuiltinType::Char32:
-    return CompilerType(clang_ast.weak_from_this(),
-                        ast.Char32Ty.getAsOpaquePtr());
-  case PDB_BuiltinType::Float:
-    // Note: types `long double` and `double` have same bit size in MSVC and
-    // there is no information in the PDB to distinguish them. So when falling
-    // back to default search, the compiler type of `long double` will be
-    // represented by the one generated for `double`.
-    break;
-  }
-  // If there is no match on PDB_BuiltinType, fall back to default search by
-  // encoding and width only
-  return clang_ast.GetBuiltinTypeForEncodingAndBitSize(encoding, width);
-}
-
-static ConstString GetPDBBuiltinTypeName(const PDBSymbolTypeBuiltin &pdb_type,
-                                         CompilerType &compiler_type) {
-  PDB_BuiltinType kind = pdb_type.getBuiltinType();
-  switch (kind) {
-  default:
-    break;
-  case PDB_BuiltinType::Currency:
-    return ConstString("CURRENCY");
-  case PDB_BuiltinType::Date:
-    return ConstString("DATE");
-  case PDB_BuiltinType::Variant:
-    return ConstString("VARIANT");
-  case PDB_BuiltinType::Complex:
-    return ConstString("complex");
-  case PDB_BuiltinType::Bitfield:
-    return ConstString("bitfield");
-  case PDB_BuiltinType::BSTR:
-    return ConstString("BSTR");
-  case PDB_BuiltinType::HResult:
-    return ConstString("HRESULT");
-  case PDB_BuiltinType::BCD:
-    return ConstString("BCD");
-  case PDB_BuiltinType::Char16:
-    return ConstString("char16_t");
-  case PDB_BuiltinType::Char32:
-    return ConstString("char32_t");
-  case PDB_BuiltinType::None:
-    return ConstString("...");
-  }
-  return compiler_type.GetTypeName();
-}
-
-static bool AddSourceInfoToDecl(const PDBSymbol &symbol, Declaration &decl) {
-  auto &raw_sym = symbol.getRawSymbol();
-  auto first_line_up = raw_sym.getSrcLineOnTypeDefn();
-
-  if (!first_line_up) {
-    auto lines_up = symbol.getSession().findLineNumbersByAddress(
-        raw_sym.getVirtualAddress(), raw_sym.getLength());
-    if (!lines_up)
-      return false;
-    first_line_up = lines_up->getNext();
-    if (!first_line_up)
-      return false;
-  }
-  uint32_t src_file_id = first_line_up->getSourceFileId();
-  auto src_file_up = symbol.getSession().getSourceFileById(src_file_id);
-  if (!src_file_up)
-    return false;
-
-  FileSpec spec(src_file_up->getFileName());
-  decl.SetFile(spec);
-  decl.SetColumn(first_line_up->getColumnNumber());
-  decl.SetLine(first_line_up->getLineNumber());
-  return true;
-}
-
-static AccessType TranslateMemberAccess(PDB_MemberAccess access) {
-  switch (access) {
-  case PDB_MemberAccess::Private:
-    return eAccessPrivate;
-  case PDB_MemberAccess::Protected:
-    return eAccessProtected;
-  case PDB_MemberAccess::Public:
-    return eAccessPublic;
-  }
-  return eAccessNone;
-}
-
-static AccessType GetDefaultAccessibilityForUdtKind(PDB_UdtType udt_kind) {
-  switch (udt_kind) {
-  case PDB_UdtType::Struct:
-  case PDB_UdtType::Union:
-    return eAccessPublic;
-  case PDB_UdtType::Class:
-  case PDB_UdtType::Interface:
-    return eAccessPrivate;
-  }
-  llvm_unreachable("unsupported PDB UDT type");
-}
-
-static AccessType GetAccessibilityForUdt(const PDBSymbolTypeUDT &udt) {
-  AccessType access = TranslateMemberAccess(udt.getAccess());
-  if (access != lldb::eAccessNone || !udt.isNested())
-    return access;
-
-  auto parent = udt.getClassParent();
-  if (!parent)
-    return lldb::eAccessNone;
-
-  auto parent_udt = llvm::dyn_cast<PDBSymbolTypeUDT>(parent.get());
-  if (!parent_udt)
-    return lldb::eAccessNone;
-
-  return GetDefaultAccessibilityForUdtKind(parent_udt->getUdtKind());
-}
-
-static clang::MSInheritanceAttr::Spelling
-GetMSInheritance(const PDBSymbolTypeUDT &udt) {
-  int base_count = 0;
-  bool has_virtual = false;
-
-  auto bases_enum = udt.findAllChildren<PDBSymbolTypeBaseClass>();
-  if (bases_enum) {
-    while (auto base = bases_enum->getNext()) {
-      base_count++;
-      has_virtual |= base->isVirtualBaseClass();
-    }
-  }
-
-  if (has_virtual)
-    return clang::MSInheritanceAttr::Keyword_virtual_inheritance;
-  if (base_count > 1)
-    return clang::MSInheritanceAttr::Keyword_multiple_inheritance;
-  return clang::MSInheritanceAttr::Keyword_single_inheritance;
-}
-
-static std::unique_ptr<llvm::pdb::PDBSymbol>
-GetClassOrFunctionParent(const llvm::pdb::PDBSymbol &symbol) {
-  const IPDBSession &session = symbol.getSession();
-  const IPDBRawSymbol &raw = symbol.getRawSymbol();
-  auto tag = symbol.getSymTag();
-
-  // For items that are nested inside of a class, return the class that it is
-  // nested inside of.
-  // Note that only certain items can be nested inside of classes.
-  switch (tag) {
-  case PDB_SymType::Function:
-  case PDB_SymType::Data:
-  case PDB_SymType::UDT:
-  case PDB_SymType::Enum:
-  case PDB_SymType::FunctionSig:
-  case PDB_SymType::Typedef:
-  case PDB_SymType::BaseClass:
-  case PDB_SymType::VTable: {
-    auto class_parent_id = raw.getClassParentId();
-    if (auto class_parent = session.getSymbolById(class_parent_id))
-      return class_parent;
-    break;
-  }
-  default:
-    break;
-  }
-
-  // Otherwise, if it is nested inside of a function, return the function.
-  // Note that only certain items can be nested inside of functions.
-  switch (tag) {
-  case PDB_SymType::Block:
-  case PDB_SymType::Data: {
-    auto lexical_parent_id = raw.getLexicalParentId();
-    auto lexical_parent = session.getSymbolById(lexical_parent_id);
-    if (!lexical_parent)
-      return nullptr;
-
-    auto lexical_parent_tag = lexical_parent->getSymTag();
-    if (lexical_parent_tag == PDB_SymType::Function)
-      return lexical_parent;
-    if (lexical_parent_tag == PDB_SymType::Exe)
-      return nullptr;
-
-    return GetClassOrFunctionParent(*lexical_parent);
-  }
-  default:
-    return nullptr;
-  }
-}
-
-static clang::NamedDecl *
-GetDeclFromContextByName(const clang::ASTContext &ast,
-                         const clang::DeclContext &decl_context,
-                         llvm::StringRef name) {
-  clang::IdentifierInfo &ident = ast.Idents.get(name);
-  clang::DeclarationName decl_name = ast.DeclarationNames.getIdentifier(&ident);
-  clang::DeclContext::lookup_result result = decl_context.lookup(decl_name);
-  if (result.empty())
-    return nullptr;
-
-  return *result.begin();
-}
-
-static bool IsAnonymousNamespaceName(llvm::StringRef name) {
-  return name == "`anonymous namespace'" || name == "`anonymous-namespace'";
-}
-
-static clang::CallingConv TranslateCallingConvention(PDB_CallingConv pdb_cc) {
-  switch (pdb_cc) {
-  case llvm::codeview::CallingConvention::NearC:
-    return clang::CC_C;
-  case llvm::codeview::CallingConvention::NearStdCall:
-    return clang::CC_X86StdCall;
-  case llvm::codeview::CallingConvention::NearFast:
-    return clang::CC_X86FastCall;
-  case llvm::codeview::CallingConvention::ThisCall:
-    return clang::CC_X86ThisCall;
-  case llvm::codeview::CallingConvention::NearVector:
-    return clang::CC_X86VectorCall;
-  case llvm::codeview::CallingConvention::NearPascal:
-    return clang::CC_X86Pascal;
-  default:
-    assert(false && "Unknown calling convention");
-    return clang::CC_C;
-  }
-}
-
-PDBASTParser::PDBASTParser(lldb_private::TypeSystemClang &ast) : m_ast(ast) {}
-
-PDBASTParser::~PDBASTParser() = default;
-
-// DebugInfoASTParser interface
-
-lldb::TypeSP PDBASTParser::CreateLLDBTypeFromPDBType(const PDBSymbol &type) {
-  Declaration decl;
-  switch (type.getSymTag()) {
-  case PDB_SymType::BaseClass: {
-    auto symbol_file = m_ast.GetSymbolFile();
-    if (!symbol_file)
-      return nullptr;
-
-    auto ty = symbol_file->ResolveTypeUID(type.getRawSymbol().getTypeId());
-    return ty ? ty->shared_from_this() : nullptr;
-  } break;
-  case PDB_SymType::UDT: {
-    auto udt = llvm::dyn_cast<PDBSymbolTypeUDT>(&type);
-    assert(udt);
-
-    // Note that, unnamed UDT being typedef-ed is generated as a UDT symbol
-    // other than a Typedef symbol in PDB. For example,
-    //    typedef union { short Row; short Col; } Union;
-    // is generated as a named UDT in PDB:
-    //    union Union { short Row; short Col; }
-    // Such symbols will be handled here.
-
-    // Some UDT with trival ctor has zero length. Just ignore.
-    if (udt->getLength() == 0)
-      return nullptr;
-
-    // Ignore unnamed-tag UDTs.
-    std::string name =
-        std::string(MSVCUndecoratedNameParser::DropScope(udt->getName()));
-    if (name.empty())
-      return nullptr;
-
-    auto decl_context = GetDeclContextContainingSymbol(type);
-
-    // Check if such an UDT already exists in the current context.
-    // This may occur with const or volatile types. There are separate type
-    // symbols in PDB for types with const or volatile modifiers, but we need
-    // to create only one declaration for them all.
-    Type::ResolveState type_resolve_state;
-    CompilerType clang_type =
-        m_ast.GetTypeForIdentifier<clang::CXXRecordDecl>(name, decl_context);
-    if (!clang_type.IsValid()) {
-      auto access = GetAccessibilityForUdt(*udt);
-
-      auto tag_type_kind = TranslateUdtKind(udt->getUdtKind());
-
-      ClangASTMetadata metadata;
-      metadata.SetUserID(type.getSymIndexId());
-      metadata.SetIsDynamicCXXType(false);
-
-      clang_type = m_ast.CreateRecordType(
-          decl_context, OptionalClangModuleID(), access, name, tag_type_kind,
-          lldb::eLanguageTypeC_plus_plus, metadata);
-      assert(clang_type.IsValid());
-
-      auto record_decl =
-          m_ast.GetAsCXXRecordDecl(clang_type.GetOpaqueQualType());
-      assert(record_decl);
-      m_uid_to_decl[type.getSymIndexId()] = record_decl;
-
-      auto inheritance_attr = clang::MSInheritanceAttr::CreateImplicit(
-          m_ast.getASTContext(), GetMSInheritance(*udt));
-      record_decl->addAttr(inheritance_attr);
-
-      TypeSystemClang::StartTagDeclarationDefinition(clang_type);
-
-      auto children = udt->findAllChildren();
-      if (!children || children->getChildCount() == 0) {
-        // PDB does not have symbol of forwarder. We assume we get an udt w/o
-        // any fields. Just complete it at this point.
-        TypeSystemClang::CompleteTagDeclarationDefinition(clang_type);
-
-        TypeSystemClang::SetHasExternalStorage(clang_type.GetOpaqueQualType(),
-                                               false);
-
-        type_resolve_state = Type::ResolveState::Full;
-      } else {
-        // Add the type to the forward declarations. It will help us to avoid
-        // an endless recursion in CompleteTypeFromUdt function.
-        m_forward_decl_to_uid[record_decl] = type.getSymIndexId();
-
-        TypeSystemClang::SetHasExternalStorage(clang_type.GetOpaqueQualType(),
-                                               true);
-
-        type_resolve_state = Type::ResolveState::Forward;
-      }
-    } else
-      type_resolve_state = Type::ResolveState::Forward;
-
-    if (udt->isConstType())
-      clang_type = clang_type.AddConstModifier();
-
-    if (udt->isVolatileType())
-      clang_type = clang_type.AddVolatileModifier();
-
-    AddSourceInfoToDecl(type, decl);
-    return m_ast.GetSymbolFile()->MakeType(
-        type.getSymIndexId(), ConstString(name), udt->getLength(), nullptr,
-        LLDB_INVALID_UID, lldb_private::Type::eEncodingIsUID, decl, clang_type,
-        type_resolve_state);
-  } break;
-  case PDB_SymType::Enum: {
-    auto enum_type = llvm::dyn_cast<PDBSymbolTypeEnum>(&type);
-    assert(enum_type);
-
-    std::string name =
-        std::string(MSVCUndecoratedNameParser::DropScope(enum_type->getName()));
-    auto decl_context = GetDeclContextContainingSymbol(type);
-    uint64_t bytes = enum_type->getLength();
-
-    // Check if such an enum already exists in the current context
-    CompilerType ast_enum =
-        m_ast.GetTypeForIdentifier<clang::EnumDecl>(name, decl_context);
-    if (!ast_enum.IsValid()) {
-      auto underlying_type_up = enum_type->getUnderlyingType();
-      if (!underlying_type_up)
-        return nullptr;
-
-      lldb::Encoding encoding =
-          TranslateBuiltinEncoding(underlying_type_up->getBuiltinType());
-      // FIXME: Type of underlying builtin is always `Int`. We correct it with
-      // the very first enumerator's encoding if any.
-      auto first_child = enum_type->findOneChild<PDBSymbolData>();
-      if (first_child)
-        encoding = TranslateEnumEncoding(first_child->getValue().Type);
-
-      CompilerType builtin_type;
-      if (bytes > 0)
-        builtin_type = GetBuiltinTypeForPDBEncodingAndBitSize(
-            m_ast, *underlying_type_up, encoding, bytes * 8);
-      else
-        builtin_type = m_ast.GetBasicType(eBasicTypeInt);
-
-      // FIXME: PDB does not have information about scoped enumeration (Enum
-      // Class). Set it false for now.
-      bool isScoped = false;
-
-      ast_enum = m_ast.CreateEnumerationType(name, decl_context,
-                                             OptionalClangModuleID(), decl,
-                                             builtin_type, isScoped);
-
-      auto enum_decl = TypeSystemClang::GetAsEnumDecl(ast_enum);
-      assert(enum_decl);
-      m_uid_to_decl[type.getSymIndexId()] = enum_decl;
-
-      auto enum_values = enum_type->findAllChildren<PDBSymbolData>();
-      if (enum_values) {
-        while (auto enum_value = enum_values->getNext()) {
-          if (enum_value->getDataKind() != PDB_DataKind::Constant)
-            continue;
-          AddEnumValue(ast_enum, *enum_value);
-        }
-      }
-
-      if (TypeSystemClang::StartTagDeclarationDefinition(ast_enum))
-        TypeSystemClang::CompleteTagDeclarationDefinition(ast_enum);
-    }
-
-    if (enum_type->isConstType())
-      ast_enum = ast_enum.AddConstModifier();
-
-    if (enum_type->isVolatileType())
-      ast_enum = ast_enum.AddVolatileModifie...
[truncated]

@@ -1,11 +1,9 @@
REQUIRES: system-windows, msvc
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tests in SymbolFile/PDB should probably move to SymbolFile/NativePDB for consistency

Copy link

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:
git-clang-format --diff a31ce36f568723ffa5b4155d0ba009403ccd0d9e c11f8f87156e630f9fed1e5bc615217353bba56d --extensions h,cpp -- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h lldb/test/Shell/Process/Windows/exception_access_violation.cpp lldb/test/Shell/Process/Windows/process_load.cpp lldb/test/Shell/SymbolFile/NativePDB/ast-functions-msvc.cpp lldb/test/Shell/SymbolFile/NativePDB/ast-functions.cpp lldb/test/Shell/SymbolFile/NativePDB/ast-methods.cpp lldb/test/Shell/SymbolFile/NativePDB/ast-types.cpp lldb/test/Shell/SymbolFile/NativePDB/bitfields.cpp lldb/test/Shell/SymbolFile/NativePDB/break-by-function.cpp lldb/test/Shell/SymbolFile/NativePDB/break-by-line.cpp lldb/test/Shell/SymbolFile/NativePDB/class_layout.cpp lldb/test/Shell/SymbolFile/NativePDB/disassembly.cpp lldb/test/Shell/SymbolFile/NativePDB/function-types-builtins.cpp lldb/test/Shell/SymbolFile/NativePDB/function-types-calling-conv.cpp lldb/test/Shell/SymbolFile/NativePDB/function-types-classes.cpp lldb/test/Shell/SymbolFile/NativePDB/global-classes.cpp lldb/test/Shell/SymbolFile/NativePDB/global-ctor-dtor.cpp lldb/test/Shell/SymbolFile/NativePDB/globals-bss.cpp lldb/test/Shell/SymbolFile/NativePDB/globals-fundamental.cpp lldb/test/Shell/SymbolFile/NativePDB/icf.cpp lldb/test/Shell/SymbolFile/NativePDB/incomplete-tag-type.cpp lldb/test/Shell/SymbolFile/NativePDB/inline_sites_live.cpp lldb/test/Shell/SymbolFile/NativePDB/load-pdb.cpp lldb/test/Shell/SymbolFile/NativePDB/local-variables.cpp lldb/test/Shell/SymbolFile/NativePDB/locate-pdb.cpp lldb/test/Shell/SymbolFile/NativePDB/lookup-by-address.cpp lldb/test/Shell/SymbolFile/NativePDB/lookup-by-types.cpp lldb/test/Shell/SymbolFile/NativePDB/nested-types.cpp lldb/test/Shell/SymbolFile/NativePDB/s_constant.cpp lldb/test/Shell/SymbolFile/NativePDB/source-list.cpp lldb/test/Shell/SymbolFile/NativePDB/stack_unwinding01.cpp lldb/test/Shell/SymbolFile/NativePDB/tag-types.cpp lldb/test/Shell/SymbolFile/NativePDB/typedefs.cpp lldb/unittests/SymbolFile/DWARF/SymbolFileDWARFTests.cpp lldb/test/Shell/SymbolFile/NativePDB/Inputs/AstRestoreTest.cpp lldb/test/Shell/SymbolFile/NativePDB/Inputs/CallingConventionsTest.cpp lldb/test/Shell/SymbolFile/NativePDB/Inputs/ClassLayoutTest.cpp lldb/test/Shell/SymbolFile/NativePDB/Inputs/CompilandsTest.cpp lldb/test/Shell/SymbolFile/NativePDB/Inputs/ExpressionsTest.cpp lldb/test/Shell/SymbolFile/NativePDB/Inputs/FuncSymbols.cpp lldb/test/Shell/SymbolFile/NativePDB/Inputs/FuncSymbolsTestMain.cpp lldb/test/Shell/SymbolFile/NativePDB/Inputs/FunctionLevelLinkingTest.cpp lldb/test/Shell/SymbolFile/NativePDB/Inputs/FunctionLevelLinkingTest.h lldb/test/Shell/SymbolFile/NativePDB/Inputs/FunctionNestedBlockTest.cpp lldb/test/Shell/SymbolFile/NativePDB/Inputs/PointerTypeTest.cpp lldb/test/Shell/SymbolFile/NativePDB/Inputs/SimpleTypesTest.cpp lldb/test/Shell/SymbolFile/NativePDB/Inputs/TypeQualsTest.cpp lldb/test/Shell/SymbolFile/NativePDB/Inputs/UdtLayoutTest.cpp lldb/test/Shell/SymbolFile/NativePDB/Inputs/VBases.cpp lldb/test/Shell/SymbolFile/NativePDB/Inputs/VariablesLocationsTest.cpp lldb/test/Shell/SymbolFile/NativePDB/Inputs/VariablesTest.cpp
View the diff from clang-format here.
diff --git a/lldb/test/Shell/SymbolFile/NativePDB/Inputs/AstRestoreTest.cpp b/lldb/test/Shell/SymbolFile/NativePDB/Inputs/AstRestoreTest.cpp
index fb20c9d734..05a4d872b0 100644
--- a/lldb/test/Shell/SymbolFile/NativePDB/Inputs/AstRestoreTest.cpp
+++ b/lldb/test/Shell/SymbolFile/NativePDB/Inputs/AstRestoreTest.cpp
@@ -4,7 +4,7 @@ namespace N1 {
 namespace {
 enum Enum { Enum_0 = 1, Enum_1 = 2, Enum_2 = 4, Enum_3 = 8 };
 enum class ScopedEnum { Enum_0 = 1, Enum_1 = 2, Enum_2 = 4, Enum_3 = 8 };
-}
+} // namespace
 
 Enum Global = Enum_3;
 
@@ -16,9 +16,7 @@ class Class : public Base {
 public:
   Class(Enum e) : m_ce(e) {}
 
-  static int StaticFunc(const Class &c) {
-    return c.PrivateFunc(c.m_inner) + Global + ClassStatic;
-  }
+  static int StaticFunc(const Class &c) { return c.PrivateFunc(c.m_inner) + Global + ClassStatic; }
 
   const Enum m_ce;
 
@@ -44,12 +42,8 @@ private:
 };
 int Class::ClassStatic = 7;
 
-template<typename T>
-struct Template {
-  template<Enum E>
-  void TemplateFunc() {
-    T::StaticFunc(T(E));
-  }
+template <typename T> struct Template {
+  template <Enum E> void TemplateFunc() { T::StaticFunc(T(E)); }
 };
 
 void foo() { Template<Class>().TemplateFunc<Enum_0>(); }
diff --git a/lldb/test/Shell/SymbolFile/NativePDB/Inputs/CallingConventionsTest.cpp b/lldb/test/Shell/SymbolFile/NativePDB/Inputs/CallingConventionsTest.cpp
index 60854c04c6..26f544b123 100644
--- a/lldb/test/Shell/SymbolFile/NativePDB/Inputs/CallingConventionsTest.cpp
+++ b/lldb/test/Shell/SymbolFile/NativePDB/Inputs/CallingConventionsTest.cpp
@@ -15,6 +15,4 @@ struct S {
 };
 auto FuncThisCallPtr = &S::FuncThisCall;
 
-int main() {
-  return 0;
-}
+int main() { return 0; }
diff --git a/lldb/test/Shell/SymbolFile/NativePDB/Inputs/ClassLayoutTest.cpp b/lldb/test/Shell/SymbolFile/NativePDB/Inputs/ClassLayoutTest.cpp
index 34a0895ea7..0c6830c959 100644
--- a/lldb/test/Shell/SymbolFile/NativePDB/Inputs/ClassLayoutTest.cpp
+++ b/lldb/test/Shell/SymbolFile/NativePDB/Inputs/ClassLayoutTest.cpp
@@ -79,9 +79,7 @@ public:
   explicit Class(int a) { m_public = a; } // Test first reference of m_public.
   ~Class() {}
 
-  static int StaticMemberFunc(int a, ...) {
-    return 1;
-  } // Test static member function.
+  static int StaticMemberFunc(int a, ...) { return 1; } // Test static member function.
   int Get() { return 1; }
   int f(Friend c) { return c.f(); }
   inline bool operator==(const Class &rhs) const // Test operator.
diff --git a/lldb/test/Shell/SymbolFile/NativePDB/Inputs/CompilandsTest.cpp b/lldb/test/Shell/SymbolFile/NativePDB/Inputs/CompilandsTest.cpp
index 4cce7f667f..76e8197013 100644
--- a/lldb/test/Shell/SymbolFile/NativePDB/Inputs/CompilandsTest.cpp
+++ b/lldb/test/Shell/SymbolFile/NativePDB/Inputs/CompilandsTest.cpp
@@ -1,3 +1 @@
-int main() {
-  return 0;
-}
+int main() { return 0; }
diff --git a/lldb/test/Shell/SymbolFile/NativePDB/Inputs/FuncSymbols.cpp b/lldb/test/Shell/SymbolFile/NativePDB/Inputs/FuncSymbols.cpp
index ccccf6ffd1..f6552960b9 100644
--- a/lldb/test/Shell/SymbolFile/NativePDB/Inputs/FuncSymbols.cpp
+++ b/lldb/test/Shell/SymbolFile/NativePDB/Inputs/FuncSymbols.cpp
@@ -1,16 +1,12 @@
 // Static function
 namespace {
-static long StaticFunction(int a)
-{
-  return 2;
-}
-}
+static long StaticFunction(int a) { return 2; }
+} // namespace
 
 // Inlined function
 static inline int InlinedFunction(long a) { return 10; }
 
-void FunctionCall()
-{
+void FunctionCall() {
   StaticFunction(1);
   InlinedFunction(1);
 }
diff --git a/lldb/test/Shell/SymbolFile/NativePDB/Inputs/FuncSymbolsTestMain.cpp b/lldb/test/Shell/SymbolFile/NativePDB/Inputs/FuncSymbolsTestMain.cpp
index 17eeab7117..c6837fbf73 100644
--- a/lldb/test/Shell/SymbolFile/NativePDB/Inputs/FuncSymbolsTestMain.cpp
+++ b/lldb/test/Shell/SymbolFile/NativePDB/Inputs/FuncSymbolsTestMain.cpp
@@ -1,4 +1,4 @@
- 
+
 // Global functions
 int Func_arg_array(int array[]) { return 1; }
 void Func_arg_void(void) { return; }
@@ -7,34 +7,26 @@ void Func_varargs(...) { return; }
 
 // Class
 namespace MemberTest {
-  class A {
-  public:
-    int Func(int a, ...) { return 1; }
-  };
-}
+class A {
+public:
+  int Func(int a, ...) { return 1; }
+};
+} // namespace MemberTest
 
 // Template
-template <int N=1, class ...T>
-void TemplateFunc(T ...Arg) {
-  return;
-}
+template <int N = 1, class... T> void TemplateFunc(T... Arg) { return; }
 
 // namespace
 namespace {
-  void Func(int a, const long b, volatile bool c, ...) { return; }
-}
+void Func(int a, const long b, volatile bool c, ...) { return; }
+} // namespace
 
 namespace NS {
-  void Func(char a, int b) {
-    return;
-  }
-}
+void Func(char a, int b) { return; }
+} // namespace NS
 
 // Static function
-static long StaticFunction(int a)
-{
-  return 2;
-}
+static long StaticFunction(int a) { return 2; }
 
 // Inlined function
 inline void InlinedFunction(long a) { return; }
@@ -43,13 +35,13 @@ extern void FunctionCall();
 
 int main() {
   MemberTest::A v1;
-  v1.Func('a',10);
+  v1.Func('a', 10);
 
   Func(1, 5, true, 10, 8);
   NS::Func('c', 2);
 
   TemplateFunc(10);
-  TemplateFunc(10,11,88);
+  TemplateFunc(10, 11, 88);
 
   StaticFunction(2);
   InlinedFunction(1);
diff --git a/lldb/test/Shell/SymbolFile/NativePDB/Inputs/FunctionLevelLinkingTest.cpp b/lldb/test/Shell/SymbolFile/NativePDB/Inputs/FunctionLevelLinkingTest.cpp
index fa0030bacb..060cfcf841 100644
--- a/lldb/test/Shell/SymbolFile/NativePDB/Inputs/FunctionLevelLinkingTest.cpp
+++ b/lldb/test/Shell/SymbolFile/NativePDB/Inputs/FunctionLevelLinkingTest.cpp
@@ -1,9 +1,5 @@
 #include "FunctionLevelLinkingTest.h"
 
-int foo() {
-  return 0;
-}
+int foo() { return 0; }
 
-int main() {
-  return foo() + bar() + baz();
-}
+int main() { return foo() + bar() + baz(); }
diff --git a/lldb/test/Shell/SymbolFile/NativePDB/Inputs/FunctionLevelLinkingTest.h b/lldb/test/Shell/SymbolFile/NativePDB/Inputs/FunctionLevelLinkingTest.h
index 0cc9d80d85..eccb7a73ab 100644
--- a/lldb/test/Shell/SymbolFile/NativePDB/Inputs/FunctionLevelLinkingTest.h
+++ b/lldb/test/Shell/SymbolFile/NativePDB/Inputs/FunctionLevelLinkingTest.h
@@ -1,12 +1,8 @@
 #ifndef FUNCTION_LEVEL_LINKING_TEST_H
 #define FUNCTION_LEVEL_LINKING_TEST_H
 
-int bar() {
-  return 0;
-}
+int bar() { return 0; }
 
-int baz() {
-  return 0;
-}
+int baz() { return 0; }
 
 #endif
diff --git a/lldb/test/Shell/SymbolFile/NativePDB/Inputs/SimpleTypesTest.cpp b/lldb/test/Shell/SymbolFile/NativePDB/Inputs/SimpleTypesTest.cpp
index de13a5b430..a0064afde6 100644
--- a/lldb/test/Shell/SymbolFile/NativePDB/Inputs/SimpleTypesTest.cpp
+++ b/lldb/test/Shell/SymbolFile/NativePDB/Inputs/SimpleTypesTest.cpp
@@ -2,14 +2,14 @@
 typedef unsigned long ULongArrayTypedef[10];
 ULongArrayTypedef ULongArrayVar;
 
-typedef long double*& RefTypedef;
-long double* LongDoublePtrVar = 0;
+typedef long double *&RefTypedef;
+long double *LongDoublePtrVar = 0;
 RefTypedef RefVar = LongDoublePtrVar;
 
-typedef long long (*FuncPtrTypedef)(int&, unsigned char**, short[], const double, volatile bool);
+typedef long long (*FuncPtrTypedef)(int &, unsigned char **, short[], const double, volatile bool);
 FuncPtrTypedef FuncVar;
 
-typedef char (*VarArgsFuncTypedef)(void*, long, unsigned short, unsigned int, ...);
+typedef char (*VarArgsFuncTypedef)(void *, long, unsigned short, unsigned int, ...);
 VarArgsFuncTypedef VarArgsFuncVar;
 
 typedef float (*VarArgsFuncTypedefA)(...);
@@ -47,6 +47,4 @@ WChar32Typedef WC32Var;
 typedef wchar_t WCharTypedef;
 WCharTypedef WCVar;
 
-int main() {
-  return 0;
-}
+int main() { return 0; }
diff --git a/lldb/test/Shell/SymbolFile/NativePDB/Inputs/TypeQualsTest.cpp b/lldb/test/Shell/SymbolFile/NativePDB/Inputs/TypeQualsTest.cpp
index bedafcdacf..18d91fc4d5 100644
--- a/lldb/test/Shell/SymbolFile/NativePDB/Inputs/TypeQualsTest.cpp
+++ b/lldb/test/Shell/SymbolFile/NativePDB/Inputs/TypeQualsTest.cpp
@@ -1,46 +1,32 @@
 // Rank > 0 array
-typedef volatile int* RankNArray[10][100];
+typedef volatile int *RankNArray[10][100];
 RankNArray ArrayVar;
 
 typedef int __unaligned *UnalignedTypedef;
 UnalignedTypedef UnVar;
 
-typedef long* __restrict RestrictTypedef;
+typedef long *__restrict RestrictTypedef;
 RestrictTypedef RestrictVar;
 
-void Func1(const int* a, int const* b, const int ** const c, const int* const* d) {
-  return;
-}
+void Func1(const int *a, int const *b, const int **const c, const int *const *d) { return; }
 
-void Func2(volatile int* a, int volatile* b) {
- return;
-}
+void Func2(volatile int *a, int volatile *b) { return; }
 
-void Func3(int*& a, int& b, const int&c, int&& d) {
-  return;
-}
+void Func3(int *&a, int &b, const int &c, int &&d) { return; }
 
-void Func4(int* __unaligned a, __unaligned int* b) {
-  return;
-}
+void Func4(int *__unaligned a, __unaligned int *b) { return; }
 
-void Func5(int a, int* __restrict b, int& __restrict c) {
-  return;
-}
+void Func5(int a, int *__restrict b, int &__restrict c) { return; }
 
-void Func6(const volatile int* __restrict b) {
-  return;
-}
+void Func6(const volatile int *__restrict b) { return; }
 
 // LValue
-typedef int& IntRef;
+typedef int &IntRef;
 int x = 0;
 IntRef IVar = x;
 
 // RValue
-typedef int&& IIRef;
+typedef int &&IIRef;
 IIRef IIVar = int(1);
 
-int main() {
-  return 0;
-}
+int main() { return 0; }
diff --git a/lldb/test/Shell/SymbolFile/NativePDB/Inputs/UdtLayoutTest.cpp b/lldb/test/Shell/SymbolFile/NativePDB/Inputs/UdtLayoutTest.cpp
index 59a4fc585d..e495918deb 100644
--- a/lldb/test/Shell/SymbolFile/NativePDB/Inputs/UdtLayoutTest.cpp
+++ b/lldb/test/Shell/SymbolFile/NativePDB/Inputs/UdtLayoutTest.cpp
@@ -29,10 +29,7 @@ private:
 #pragma pack(push, 16)
 class C : private virtual B<0>, public virtual B<1>, private B<2>, public B<3> {
 public:
-  C(char x, char y, char z)
-      : A(x - y + z), B<0>(x, y, z), B<1>(x * 2, y * 2, z * 2),
-        B<2>(x * 3, y * 3, z * 3), B<3>(x * 4, y * 4, z * 4), _x(x * 5),
-        _y(y * 5), _z(z * 5) {}
+  C(char x, char y, char z) : A(x - y + z), B<0>(x, y, z), B<1>(x * 2, y * 2, z * 2), B<2>(x * 3, y * 3, z * 3), B<3>(x * 4, y * 4, z * 4), _x(x * 5), _y(y * 5), _z(z * 5) {}
 
   static int abc;
 
diff --git a/lldb/test/Shell/SymbolFile/NativePDB/Inputs/VariablesLocationsTest.cpp b/lldb/test/Shell/SymbolFile/NativePDB/Inputs/VariablesLocationsTest.cpp
index 7b7180a3ec..e7e58ef333 100644
--- a/lldb/test/Shell/SymbolFile/NativePDB/Inputs/VariablesLocationsTest.cpp
+++ b/lldb/test/Shell/SymbolFile/NativePDB/Inputs/VariablesLocationsTest.cpp
@@ -10,11 +10,10 @@ __declspec(align(128)) struct S {
 };
 
 void bar(int arg_0) {
- S loc_0;
- int loc_1 = 5678;
+  S loc_0;
+  int loc_1 = 5678;
 }
 
-
 int main(int argc, char *argv[]) {
   bool loc_0 = true;
   int loc_1 = 3333;
diff --git a/lldb/test/Shell/SymbolFile/NativePDB/Inputs/VariablesTest.cpp b/lldb/test/Shell/SymbolFile/NativePDB/Inputs/VariablesTest.cpp
index 304d905660..99c62e9943 100644
--- a/lldb/test/Shell/SymbolFile/NativePDB/Inputs/VariablesTest.cpp
+++ b/lldb/test/Shell/SymbolFile/NativePDB/Inputs/VariablesTest.cpp
@@ -1,26 +1,27 @@
 typedef int IntTypedef;
-IntTypedef g_IntVar;  // Testing globals.
+IntTypedef g_IntVar; // Testing globals.
 
 typedef enum Enum { // Testing constants.
   RED,
   GREEN,
   BLUE
 } EnumTypedef;
-EnumTypedef g_EnumVar;  // Testing members.
+EnumTypedef g_EnumVar; // Testing members.
 
 // FIXME: `sg_IntVar` appears both in global scope's children and compiland's
 // children but with different symbol's id.
-static int sg_IntVar = -1;  // Testing file statics.
+static int sg_IntVar = -1; // Testing file statics.
 
 // FIXME: `g_Const` appears both in global scope's children and compiland's
 // children but with different symbol's id.
-const int g_Const = 0x88;  // Testing constant data.
+const int g_Const = 0x88;       // Testing constant data.
 const int *g_pConst = &g_Const; // Avoid optimizing the const away
 
-thread_local int g_tls = 0;  // Testing thread-local storage.
+thread_local int g_tls = 0; // Testing thread-local storage.
 
 class Class {
   static int m_StaticClassMember;
+
 public:
   explicit Class(int a) {}
   void Func() {}
@@ -28,18 +29,18 @@ public:
 int Class::m_StaticClassMember = 10; // Testing static class members.
 Class ClassVar(1);
 
-int f(int var_arg1, int var_arg2) {  // Testing parameters.
+int f(int var_arg1, int var_arg2) { // Testing parameters.
   long same_name_var = -1;
   return 1;
 }
 
 int same_name_var = 100;
 int main() {
-  int same_name_var = 0;  // Testing locals.
+  int same_name_var = 0; // Testing locals.
   const char local_const = 0x1;
 
   // FIXME: 'local_CString` is not found through compiland's children.
-  const char local_CString[] = "abc";  // Testing constant string.
+  const char local_CString[] = "abc";         // Testing constant string.
   const char *local_pCString = local_CString; // Avoid optimizing the const away
 
   int a = 10;

@mstorsjo mstorsjo requested a review from dzhidzhoev October 25, 2024 07:34
@mstorsjo
Copy link
Member

Remove SymbolFilePDB in favor of always using SymbolFileNativePDB. This effectively makes LLDB_USE_NATIVE_PDB_READER the default. The non-native (DIA based) PDB symbol file implementation was unmaintained and known to have issues.

While this certainly is true, unfortunately the main working testing configuration of LLDB on Windows is with the DIA SDK enabled.

In that form, I can run check-lldb without any failures. By disabling use of the DIA SDK, I have the following test fallout:

Failed Tests (15):
  lldb-shell :: Driver/TestSingleQuote.test
  lldb-shell :: Expr/nodefaultlib.cpp
  lldb-shell :: SymbolFile/DWARF/dwo-static-data-member-access.test
  lldb-shell :: SymbolFile/PDB/ast-restore.test
  lldb-shell :: SymbolFile/PDB/calling-conventions-x86.test
  lldb-shell :: SymbolFile/PDB/class-layout.test
  lldb-shell :: SymbolFile/PDB/enums-layout.test
  lldb-shell :: SymbolFile/PDB/expressions.test
  lldb-shell :: SymbolFile/PDB/func-symbols.test
  lldb-shell :: SymbolFile/PDB/pointers.test
  lldb-shell :: SymbolFile/PDB/type-quals.test
  lldb-shell :: SymbolFile/PDB/typedefs.test
  lldb-shell :: SymbolFile/PDB/udt-layout.test
  lldb-shell :: SymbolFile/PDB/variables.test
  lldb-shell :: Unwind/windows-unaligned-x86_64.test

********************
Unexpectedly Passed Tests (1):
  lldb-shell :: Commands/command-disassemble-mixed.c

With this PR in place, we get even more failures:

Failed Tests (72):
  lldb-shell :: Breakpoint/case-insensitive.test
  lldb-shell :: Breakpoint/dummy-target.test
  lldb-shell :: Commands/command-disassemble-mixed.test
  lldb-shell :: Commands/command-process-launch-user-entry.test
  lldb-shell :: Commands/command-thread-backtrace.test
  lldb-shell :: Commands/command-thread-select.test
  lldb-shell :: Driver/TestSingleQuote.test
  lldb-shell :: Expr/TestIRMemoryMapWindows.test
  lldb-shell :: Expr/nodefaultlib.cpp
  lldb-shell :: Process/Windows/process_load.cpp
  lldb-shell :: Settings/TestFrameFormatColor.test
  lldb-shell :: Settings/TestFrameFormatNoColor.test
  lldb-shell :: Settings/TestLineMarkerColor.test
  lldb-shell :: SymbolFile/DWARF/dwo-debug-file-search-paths-dwoname-absolute-compdir.c
  lldb-shell :: SymbolFile/DWARF/dwo-debug-file-search-paths-filename-only-absolute-compdir.c
  lldb-shell :: SymbolFile/DWARF/dwo-debug-file-search-paths-filename-only-relative-compdir.c
  lldb-shell :: SymbolFile/DWARF/dwo-debug-file-search-paths-relative-compdir.c
  lldb-shell :: SymbolFile/DWARF/dwo-relative-filename-only-binary-dir.c
  lldb-shell :: SymbolFile/DWARF/dwo-static-data-member-access.test
  lldb-shell :: SymbolFile/NativePDB/ast-functions-msvc.cpp
  lldb-shell :: SymbolFile/NativePDB/ast-functions.cpp
  lldb-shell :: SymbolFile/NativePDB/ast-methods.cpp
  lldb-shell :: SymbolFile/NativePDB/ast-restore.test
  lldb-shell :: SymbolFile/NativePDB/ast-types.cpp
  lldb-shell :: SymbolFile/NativePDB/bitfields.cpp
  lldb-shell :: SymbolFile/NativePDB/blocks.s
  lldb-shell :: SymbolFile/NativePDB/break-by-function.cpp
  lldb-shell :: SymbolFile/NativePDB/break-by-line.cpp
  lldb-shell :: SymbolFile/NativePDB/calling-conventions-x86.test
  lldb-shell :: SymbolFile/NativePDB/class-layout.test
  lldb-shell :: SymbolFile/NativePDB/class_layout.cpp
  lldb-shell :: SymbolFile/NativePDB/compilands.test
  lldb-shell :: SymbolFile/NativePDB/disassembly.cpp
  lldb-shell :: SymbolFile/NativePDB/enums-layout.test
  lldb-shell :: SymbolFile/NativePDB/expressions.test
  lldb-shell :: SymbolFile/NativePDB/find-functions.cpp
  lldb-shell :: SymbolFile/NativePDB/func-symbols.test
  lldb-shell :: SymbolFile/NativePDB/function-level-linking.test
  lldb-shell :: SymbolFile/NativePDB/function-types-builtins.cpp
  lldb-shell :: SymbolFile/NativePDB/function-types-calling-conv.cpp
  lldb-shell :: SymbolFile/NativePDB/function-types-classes.cpp
  lldb-shell :: SymbolFile/NativePDB/global-classes.cpp
  lldb-shell :: SymbolFile/NativePDB/global-ctor-dtor.cpp
  lldb-shell :: SymbolFile/NativePDB/globals-bss.cpp
  lldb-shell :: SymbolFile/NativePDB/globals-fundamental.cpp
  lldb-shell :: SymbolFile/NativePDB/icf.cpp
  lldb-shell :: SymbolFile/NativePDB/incomplete-tag-type.cpp
  lldb-shell :: SymbolFile/NativePDB/inline_sites.test
  lldb-shell :: SymbolFile/NativePDB/inline_sites_live.cpp
  lldb-shell :: SymbolFile/NativePDB/load-pdb.cpp
  lldb-shell :: SymbolFile/NativePDB/local-variables-registers.s
  lldb-shell :: SymbolFile/NativePDB/local-variables.cpp
  lldb-shell :: SymbolFile/NativePDB/locate-pdb.cpp
  lldb-shell :: SymbolFile/NativePDB/lookup-by-address.cpp
  lldb-shell :: SymbolFile/NativePDB/lookup-by-types.cpp
  lldb-shell :: SymbolFile/NativePDB/missing-type.s
  lldb-shell :: SymbolFile/NativePDB/nested-blocks-same-address.s
  lldb-shell :: SymbolFile/NativePDB/nested-types.cpp
  lldb-shell :: SymbolFile/NativePDB/pointers.test
  lldb-shell :: SymbolFile/NativePDB/s_constant.cpp
  lldb-shell :: SymbolFile/NativePDB/source-list.cpp
  lldb-shell :: SymbolFile/NativePDB/stack_unwinding01.cpp
  lldb-shell :: SymbolFile/NativePDB/tag-types.cpp
  lldb-shell :: SymbolFile/NativePDB/type-quals.test
  lldb-shell :: SymbolFile/NativePDB/typedefs.cpp
  lldb-shell :: SymbolFile/NativePDB/typedefs.test
  lldb-shell :: SymbolFile/NativePDB/udt-layout.test
  lldb-shell :: SymbolFile/NativePDB/variables-locations.test
  lldb-shell :: SymbolFile/NativePDB/variables.test
  lldb-shell :: SymbolFile/NativePDB/vbases.test
  lldb-shell :: Unwind/windows-unaligned-x86_64.test
  lldb-shell :: Watchpoint/SetErrorCases.test

@DavidSpickett
Copy link
Collaborator

The non-native (DIA based) PDB symbol file implementation was unmaintained and known to have issues.

FWIW I have seen feedback from users who have no idea what this does or why there would be two things when one is always worse in their experience. Unfortunately I've never been able to adequately explain it to them because I myself don't understand the differences :)

What axis do we have here?

  • Native or non-native?
  • DIA SDK or not?

@omjavaid
Copy link
Contributor

We set DIA SDK as the default symbol provider on windows. So I don think this change is appropriate at this stage without testing the complete fallout. Although I believe we should move in that direction sooner or later.

Copy link
Contributor

@omjavaid omjavaid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will test this and may be we can move ahead in case failures are trivial

@JDevlieghere
Copy link
Member Author

Interesting. Based on a conversation at the dev meeting, I was under the impression that the DIA-based implementation was in worse shape and the native one in better shape. I also have a memory of bug reports about PDB parsing where originators were asked to check with LLDB_USE_NATIVE_PDB_READER instead.

FWIW I don't have a vested interest in removing this code. This came up in a conversation about understanding what works and doesn't work on Windows. I feel like we're in limbo here and I'd love to get to a point where we can set clear expectations to our users about what does and doesn't work. Having multiple configurations of things (like the symbol file an the process plugin) doesn't make that matrix easier to understand.

What axis do we have here?

  • Native or non-native?
  • DIA SDK or not?

My understanding of the situation is that SymbolFilePDB uses the Microsoft Debug Interface Access (DIA) SDK, which of course is only available on Windows and that SymbolFileNativePDB uses LLVM's "native" PDB parser, which should be available on all platforms (similar to how you can parse DWARF on Windows). In other words, if you're on Windows, you can use both. If you're on another platform, you can only use the latter.

@JDevlieghere JDevlieghere requested a review from mstorsjo October 25, 2024 14:35
@mstorsjo
Copy link
Member

Interesting. Based on a conversation at the dev meeting, I was under the impression that the DIA-based implementation was in worse shape and the native one in better shape. I also have a memory of bug reports about PDB parsing where originators were asked to check with LLDB_USE_NATIVE_PDB_READER instead.

I don't really know it too closely either, but my understanding was that the DIA SDK based one was more complete. Both probably have their own sets of bugs anyway, so "check with the other implementation" always is a reasonable assumption.

I also agree that getting rid of the DIA SDK based one would be the way to go - that also gets us equal functionality across all platforms.

In any case - using the DIA SDK is the canonical configuration where tests do pass AFAIK; bringing configurations without it up to the same level would at least be a first step towards resolving this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants