Re: [Lldb-commits] [lldb] r321120 - Fix a couple of warnings (NFC)

2017-12-20 Thread Davide Italiano via lldb-commits
On Tue, Dec 19, 2017 at 11:54 PM, Adrian Prantl via lldb-commits
 wrote:
> Author: adrian
> Date: Tue Dec 19 14:54:37 2017
> New Revision: 321120
>
> URL: http://llvm.org/viewvc/llvm-project?rev=321120&view=rev
> Log:
> Fix a couple of warnings (NFC)
>

Thanks for doing this!

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


[Lldb-commits] [PATCH] D41352: debugserver: Propagate environment in launch-mode (pr35671)

2017-12-20 Thread Pavel Labath via Phabricator via lldb-commits
labath updated this revision to Diff 127681.
labath added a comment.
Herald added a subscriber: mgorny.

New version: Make sure we respect variables set by --env and that they are not
overridden by --forward-env (the last part relies on the fact that in the
presence of multiply-defined environment variables, getenv() will pick the
first one).


https://reviews.llvm.org/D41352

Files:
  tools/debugserver/source/debugserver.cpp
  unittests/tools/lldb-server/CMakeLists.txt
  unittests/tools/lldb-server/tests/LLGSTest.cpp
  unittests/tools/lldb-server/tests/TestClient.cpp
  unittests/tools/lldb-server/tests/TestClient.h

Index: unittests/tools/lldb-server/tests/TestClient.h
===
--- unittests/tools/lldb-server/tests/TestClient.h
+++ unittests/tools/lldb-server/tests/TestClient.h
@@ -21,12 +21,20 @@
 #include 
 #include 
 
+#if LLDB_SERVER_IS_DEBUGSERVER
+#define LLGS_TEST(x) DISABLED_ ## x
+#define DS_TEST(x) x
+#else
+#define LLGS_TEST(x) x
+#define DS_TEST(x) DISABLED_ ## x
+#endif
+
 namespace llgs_tests {
 class TestClient
 : public lldb_private::process_gdb_remote::GDBRemoteCommunicationClient {
 public:
-  static bool IsDebugServer();
-  static bool IsLldbServer();
+  static bool IsDebugServer() { return LLDB_SERVER_IS_DEBUGSERVER; }
+  static bool IsLldbServer() { return !IsDebugServer(); }
 
   /// Launches the server, connects it to the client and returns the client. If
   /// Log is non-empty, the server will write it's log to this file.
@@ -37,6 +45,13 @@
   static llvm::Expected>
   launch(llvm::StringRef Log, llvm::ArrayRef InferiorArgs);
 
+  /// Allows user to pass additional arguments to the server. Be careful when
+  /// using this for generic tests, as the two stubs have different
+  /// command-line interfaces.
+  static llvm::Expected>
+  launchCustom(llvm::StringRef Log, llvm::ArrayRef ServerArgs, llvm::ArrayRef InferiorArgs);
+
+
   ~TestClient() override;
   llvm::Error SetInferior(llvm::ArrayRef inferior_args);
   llvm::Error ListThreadsInStopReply();
Index: unittests/tools/lldb-server/tests/TestClient.cpp
===
--- unittests/tools/lldb-server/tests/TestClient.cpp
+++ unittests/tools/lldb-server/tests/TestClient.cpp
@@ -27,11 +27,6 @@
 using namespace llvm;
 
 namespace llgs_tests {
-bool TestClient::IsDebugServer() {
-  return sys::path::filename(LLDB_SERVER).contains("debugserver");
-}
-
-bool TestClient::IsLldbServer() { return !IsDebugServer(); }
 
 TestClient::TestClient(std::unique_ptr Conn) {
   SetConnection(Conn.release());
@@ -56,6 +51,10 @@
 }
 
 Expected> TestClient::launch(StringRef Log, ArrayRef InferiorArgs) {
+  return launchCustom(Log, {}, InferiorArgs);
+}
+
+Expected> TestClient::launchCustom(StringRef Log, ArrayRef ServerArgs, ArrayRef InferiorArgs) {
   const ArchSpec &arch_spec = HostInfo::GetArchitecture();
   Args args;
   args.AppendArgument(LLDB_SERVER);
@@ -80,6 +79,9 @@
   args.AppendArgument(
   ("localhost:" + Twine(listen_socket.GetLocalPortNumber())).str());
 
+  for (StringRef arg : ServerArgs)
+args.AppendArgument(arg);
+
   if (!InferiorArgs.empty()) {
 args.AppendArgument("--");
 for (StringRef arg : InferiorArgs)
Index: unittests/tools/lldb-server/tests/LLGSTest.cpp
===
--- unittests/tools/lldb-server/tests/LLGSTest.cpp
+++ unittests/tools/lldb-server/tests/LLGSTest.cpp
@@ -16,11 +16,6 @@
 using namespace llvm;
 
 TEST_F(TestBase, LaunchModePreservesEnvironment) {
-  if (TestClient::IsDebugServer()) {
-GTEST_LOG_(WARNING) << "Test fails with debugserver: llvm.org/pr35671";
-return;
-  }
-
   putenv(const_cast("LLDB_TEST_MAGIC_VARIABLE=LLDB_TEST_MAGIC_VALUE"));
 
   auto ClientOr = TestClient::launch(getLogFileName(),
@@ -34,3 +29,20 @@
   HasValue(testing::Property(&StopReply::getKind,
  WaitStatus{WaitStatus::Exit, 0})));
 }
+
+TEST_F(TestBase, DS_TEST(DebugserverEnv)) {
+  // Test that --env takes precedence over inherited environment variables.
+  putenv(const_cast("LLDB_TEST_MAGIC_VARIABLE=foobar"));
+
+  auto ClientOr = TestClient::launchCustom(getLogFileName(),
+  { "--env", "LLDB_TEST_MAGIC_VARIABLE=LLDB_TEST_MAGIC_VALUE" },
+ {getInferiorPath("environment_check")});
+  ASSERT_THAT_EXPECTED(ClientOr, Succeeded());
+  auto &Client = **ClientOr;
+
+  ASSERT_THAT_ERROR(Client.ContinueAll(), Succeeded());
+  ASSERT_THAT_EXPECTED(
+  Client.GetLatestStopReplyAs(),
+  HasValue(testing::Property(&StopReply::getKind,
+ WaitStatus{WaitStatus::Exit, 0})));
+}
Index: unittests/tools/lldb-server/CMakeLists.txt
===
--- unittests/tools/lldb-server/CMakeLists.txt
+++ unittests/tools/lldb-server/CMakeLists.txt
@@ -13,9 +13,9 @@
 add_lldb_test_executable(environment_chec

[Lldb-commits] [PATCH] D40079: Make sure DataBufferLLVM contents are writable

2017-12-20 Thread Pavel Labath via Phabricator via lldb-commits
labath updated this revision to Diff 127687.
labath added a comment.

Add the suggested MapFileData function


https://reviews.llvm.org/D40079

Files:
  include/lldb/Interpreter/OptionValueFileSpec.h
  include/lldb/Symbol/ObjectFile.h
  include/lldb/Target/Target.h
  include/lldb/Utility/DataBufferLLVM.h
  source/Interpreter/OptionValueFileSpec.cpp
  source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
  source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
  source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
  source/Symbol/ObjectFile.cpp
  source/Target/Target.cpp
  source/Utility/DataBufferLLVM.cpp

Index: source/Utility/DataBufferLLVM.cpp
===
--- source/Utility/DataBufferLLVM.cpp
+++ source/Utility/DataBufferLLVM.cpp
@@ -18,7 +18,8 @@
 
 using namespace lldb_private;
 
-DataBufferLLVM::DataBufferLLVM(std::unique_ptr MemBuffer)
+DataBufferLLVM::DataBufferLLVM(
+std::unique_ptr MemBuffer)
 : Buffer(std::move(MemBuffer)) {
   assert(Buffer != nullptr &&
  "Cannot construct a DataBufferLLVM with a null buffer");
@@ -28,43 +29,40 @@
 
 std::shared_ptr
 DataBufferLLVM::CreateSliceFromPath(const llvm::Twine &Path, uint64_t Size,
-   uint64_t Offset, bool Private) {
+uint64_t Offset) {
   // If the file resides non-locally, pass the volatile flag so that we don't
   // mmap it.
-  if (!Private)
-Private = !llvm::sys::fs::is_local(Path);
+  bool IsVolatile = !llvm::sys::fs::is_local(Path);
 
-  auto Buffer = llvm::MemoryBuffer::getFileSlice(Path, Size, Offset, Private);
+  auto Buffer =
+  llvm::WritableMemoryBuffer::getFileSlice(Path, Size, Offset, IsVolatile);
   if (!Buffer)
 return nullptr;
   return std::shared_ptr(
   new DataBufferLLVM(std::move(*Buffer)));
 }
 
 std::shared_ptr
-DataBufferLLVM::CreateFromPath(const llvm::Twine &Path, bool NullTerminate, bool Private) {
+DataBufferLLVM::CreateFromPath(const llvm::Twine &Path) {
   // If the file resides non-locally, pass the volatile flag so that we don't
   // mmap it.
-  if (!Private)
-Private = !llvm::sys::fs::is_local(Path);
+  bool IsVolatile = !llvm::sys::fs::is_local(Path);
 
-  auto Buffer = llvm::MemoryBuffer::getFile(Path, -1, NullTerminate, Private);
+  auto Buffer = llvm::WritableMemoryBuffer::getFile(Path, -1, IsVolatile);
   if (!Buffer)
 return nullptr;
   return std::shared_ptr(
   new DataBufferLLVM(std::move(*Buffer)));
 }
 
 uint8_t *DataBufferLLVM::GetBytes() {
-  return const_cast(GetBuffer());
+  return reinterpret_cast(Buffer->getBufferStart());
 }
 
-const uint8_t *DataBufferLLVM::GetBytes() const { return GetBuffer(); }
+const uint8_t *DataBufferLLVM::GetBytes() const {
+  return reinterpret_cast(Buffer->getBufferStart());
+}
 
 lldb::offset_t DataBufferLLVM::GetByteSize() const {
   return Buffer->getBufferSize();
 }
-
-const uint8_t *DataBufferLLVM::GetBuffer() const {
-  return reinterpret_cast(Buffer->getBufferStart());
-}
Index: source/Target/Target.cpp
===
--- source/Target/Target.cpp
+++ source/Target/Target.cpp
@@ -2313,7 +2313,7 @@
 result_valobj_sp = persistent_var_sp->GetValueObject();
 execution_results = eExpressionCompleted;
   } else {
-const char *prefix = GetExpressionPrefixContentsAsCString();
+llvm::StringRef prefix = GetExpressionPrefixContents();
 Status error;
 execution_results = UserExpression::Evaluate(exe_ctx, options, expr, prefix,
  result_valobj_sp, error,
@@ -4033,18 +4033,19 @@
   return LanguageType();
 }
 
-const char *TargetProperties::GetExpressionPrefixContentsAsCString() {
+llvm::StringRef TargetProperties::GetExpressionPrefixContents() {
   const uint32_t idx = ePropertyExprPrefix;
   OptionValueFileSpec *file =
   m_collection_sp->GetPropertyAtIndexAsOptionValueFileSpec(nullptr, false,
idx);
   if (file) {
-const bool null_terminate = true;
-DataBufferSP data_sp(file->GetFileContents(null_terminate));
+DataBufferSP data_sp(file->GetFileContents());
 if (data_sp)
-  return (const char *)data_sp->GetBytes();
+  return llvm::StringRef(
+  reinterpret_cast(data_sp->GetBytes()),
+  data_sp->GetByteSize());
   }
-  return nullptr;
+  return "";
 }
 
 bool TargetProperties::GetBreakpointsConsultPlatformAvoidList() {
Index: source/Symbol/ObjectFile.cpp
===
--- source/Symbol/ObjectFile.cpp
+++ source/Symbol/ObjectFile.cpp
@@ -688,3 +688,8 @@
 void ObjectFile::RelocateSection(lldb_private::Section *section)
 {
 }
+
+DataBufferSP ObjectFile::MapFileData(const FileSpec &file, uint64_t Size,
+ uint64_t Offset) {
+  return DataBufferLLVM::CreateSliceF

[Lldb-commits] [PATCH] D41086: [lldb] Check that a regex is valid before searching by regex for a symbol in a pdb.

2017-12-20 Thread Zachary Turner via Phabricator via lldb-commits
zturner added inline comments.



Comment at: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp:394
+  if (name_str.find_first_of("[]?*.-+\\") != std::string::npos)
+FindTypesByRegex(RegularExpression(name_str), max_matches, types);
   else

clayborg wrote:
> You should make the RegularExpression object first and then check for errors. 
> If there are errors, the type lookup should happen by normal name. This is 
> again why I don't like us sniffing for a regular expression. If there is an 
> error, like when trying to lookup "char*" as mentioned by Aaron, would you 
> expect to see an error message saying "char*" isn't a valid regex? Then the 
> user thinks that the type lookup always takes a regular expression which is 
> not the case, just something the PDB plugin added for some reason. I do 
> believe part of this patch should be adding the 
> lldb_private::SymbolFile::FindTypesByRegex(...) to the base class and fixing 
> this issue by removing the regex sniffing code since it is fragile at best 
> (how are we doing to lookup a template type without triggering the regex 
> issues?).
The reason I don't think it's appropriate for this patch is because nothing 
currently calls that method.  So something much higher level would then have to 
be updated to call this new method, which might even entail adding a new 
command line option.  For now, just fixing a crash is sufficient.


Repository:
  rL LLVM

https://reviews.llvm.org/D41086



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


[Lldb-commits] [PATCH] D41092: Enable more abilities in SymbolFilePDB

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

This looks better.  Technically it's possible to break this with some weird 
PDBs but I don't think it's possible to do better without using the native 
reader.


Repository:
  rL LLVM

https://reviews.llvm.org/D41092



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


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

2017-12-20 Thread Zachary Turner via Phabricator via lldb-commits
zturner added a comment.

Since it seems like you're going to be doing some work in here, it would be 
great if you could update `lldb-test` to dump PDB symbol information.  This 
would allow us to easily test all sorts of things in here.  For example, you 
could find a PDB that returned an empty source file name, and then have the 
tool dump something like:

Compiland: {foo}, Source File: {bar}

then we could just grep this output against FileCheck.




Comment at: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp:367-370
+  // For some reason, source file name could be empty, so we will walk through
+  // all source files of this compiland, and determine the right source file
+  // if any that is used to generate this compiland based on language
+  // indicated in compilanddetails language field.

Is it DIA that is returning the empty name?



Comment at: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp:371
+  // indicated in compilanddetails language field.
+  if (source_file_name.empty()) {
+auto details_up = pdb_compiland->findOneChild();

Can you do an early return here?



Comment at: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp:382-389
+(ConstString::Compare(file_extension, ConstString("CPP"),
+  false) == 0 ||
+ ConstString::Compare(file_extension, ConstString("C"),
+  false) == 0 ||
+ ConstString::Compare(file_extension, ConstString("CC"),
+  false) == 0 ||
+ ConstString::Compare(file_extension, ConstString("CXX"),

This would probably be shorter as:

```
llvm::is_contained({"cpp", "c", "cc", "cxx"}, 
file_extension.GetStringRef().lower());
```



Comment at: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp:683
+
+  if (cu_sp) {
+m_comp_units.insert(std::make_pair(id, cu_sp));

Early return here.



Comment at: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp:763
   auto prologue = func->findOneChild();
-  is_prologue = (addr == prologue->getVirtualAddress());
+  if (prologue.get())
+is_prologue = (addr == prologue->getVirtualAddress());

nit: `.get()` shouldn't be necessary



Comment at: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp:767
   auto epilogue = func->findOneChild();
-  is_epilogue = (addr == epilogue->getVirtualAddress());
+  if (epilogue.get())
+is_epilogue = (addr == epilogue->getVirtualAddress());

Same here.



Comment at: source/Plugins/SymbolFile/PDB/SymbolFilePDB.h:197
   std::unique_ptr m_session_up;
+  std::unique_ptr m_global_scope_up;
   uint32_t m_cached_compile_unit_count;

Is this a performance win or just a convenience?  I assumed the session itself 
would cache the global scope, but maybe that assumption was wrong.


Repository:
  rL LLVM

https://reviews.llvm.org/D41428



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


[Lldb-commits] [PATCH] D41427: [lldb] Fix crash when parsing the type of a function without any arguments

2017-12-20 Thread Zachary Turner via Phabricator via lldb-commits
zturner added a comment.

This is another example where we could test it easily if `lldb-test` could dump 
this.  Are you willing to take a stab at this?


Repository:
  rL LLVM

https://reviews.llvm.org/D41427



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


[Lldb-commits] [PATCH] D41352: debugserver: Propagate environment in launch-mode (pr35671)

2017-12-20 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added inline comments.



Comment at: tools/debugserver/source/debugserver.cpp:1426
+for (i = 0; (env_entry = host_env[i]) != NULL; ++i)
+  remote->Context().PushEnvironment(env_entry);
+  }

We need to check if the env var is already in the environment in 
remote->Context() first before pushing the new version. I would assume that if 
you exec a program with an env like:
```
FOO=bar
USER=me
FOO=baz
```

That FOO=baz will end up being the value that is used. If the user specifies 
things with --env, we will just overwrite it. We might add a 
PushEnvironmentIfNeeded() function to the Context() class that will make sure 
it isn't in the list first and push it only if needed.


https://reviews.llvm.org/D41352



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


[Lldb-commits] [PATCH] D40079: Make sure DataBufferLLVM contents are writable

2017-12-20 Thread Greg Clayton via Phabricator via lldb-commits
clayborg accepted this revision.
clayborg added a comment.
This revision is now accepted and ready to land.

Looks good, thanks for the making the changes. This will make future tweaks to 
reading of file data in object files just a single change in ObjectFile.cpp.


https://reviews.llvm.org/D40079



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


[Lldb-commits] [PATCH] D41086: [lldb] Check that a regex is valid before searching by regex for a symbol in a pdb.

2017-12-20 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment.

If you load a exe file that has a PDB file, people can currently run:

  (lldb) type lookup "char*"

If no testing is using the regular expression stuff, then just pull it out.  Do 
we have unit tests that depend on this working? If not, lets just pull it out 
from the SymbolFilePDB::FindTypes() and we can leave the fix in that uses the 
RegularExpression class so it won't crash in the future?


Repository:
  rL LLVM

https://reviews.llvm.org/D41086



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


Re: [Lldb-commits] [PATCH] D41086: [lldb] Check that a regex is valid before searching by regex for a symbol in a pdb.

2017-12-20 Thread Zachary Turner via lldb-commits
That seems reasonable. Seems Aaron ran into this not because he was trying
to do a regex search, but because he *wasnt* trying to do a regex search.
So if he doesn’t have immediate need for a regex search, and if it’s not
tested anyway, removing it seems fine
On Wed, Dec 20, 2017 at 10:49 AM Greg Clayton via Phabricator <
revi...@reviews.llvm.org> wrote:

> clayborg added a comment.
>
> If you load a exe file that has a PDB file, people can currently run:
>
>   (lldb) type lookup "char*"
>
> If no testing is using the regular expression stuff, then just pull it
> out.  Do we have unit tests that depend on this working? If not, lets just
> pull it out from the SymbolFilePDB::FindTypes() and we can leave the fix in
> that uses the RegularExpression class so it won't crash in the future?
>
>
> Repository:
>   rL LLVM
>
> https://reviews.llvm.org/D41086
>
>
>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D41086: [lldb] Stop searching for a symbol in a pdb by regex

2017-12-20 Thread Aaron Smith via Phabricator via lldb-commits
asmith updated this revision to Diff 127803.
asmith retitled this revision from "[lldb] Check that a regex is valid before 
searching by regex for a symbol in a pdb." to "[lldb] Stop searching for a 
symbol in a pdb by regex".
asmith edited the summary of this revision.

Repository:
  rL LLVM

https://reviews.llvm.org/D41086

Files:
  source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
  source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
  source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
  unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp

Index: unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp
===
--- unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp
+++ unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp
@@ -520,6 +520,13 @@
 false, 0, searched_files, results);
   EXPECT_GT(num_results, 1u);
   EXPECT_EQ(num_results, results.GetSize());
+
+  // We expect no exception thrown if the given regex can't be compiled
+  results.Clear();
+  num_results = symfile->FindTypes(sc, ConstString("**"), nullptr,
+   false, 0, searched_files, results);
+  EXPECT_EQ(num_results, 0u);
+  EXPECT_EQ(num_results, results.GetSize());
 }
 
 TEST_F(SymbolFilePDBTests, TestMaxMatches) {
Index: source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
===
--- source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
+++ source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
@@ -172,7 +172,8 @@
   const llvm::pdb::PDBSymbolCompiland &cu,
   llvm::DenseMap &index_map) const;
 
-  void FindTypesByRegex(const std::string ®ex, uint32_t max_matches,
+  void FindTypesByRegex(const lldb_private::RegularExpression ®ex,
+uint32_t max_matches,
 lldb_private::TypeMap &types);
 
   void FindTypesByName(const std::string &name, uint32_t max_matches,
Index: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
===
--- source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
+++ source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
@@ -19,6 +19,7 @@
 #include "lldb/Symbol/ObjectFile.h"
 #include "lldb/Symbol/SymbolContext.h"
 #include "lldb/Symbol/TypeMap.h"
+#include "lldb/Utility/RegularExpression.h"
 
 #include "llvm/DebugInfo/PDB/GenericError.h"
 #include "llvm/DebugInfo/PDB/IPDBEnumChildren.h"
@@ -250,7 +251,8 @@
 return nullptr;
 
   lldb::TypeSP result = pdb->CreateLLDBTypeFromPDBType(*pdb_type);
-  m_types.insert(std::make_pair(type_uid, result));
+  if (result.get())
+m_types.insert(std::make_pair(type_uid, result));
   return result.get();
 }
 
@@ -385,19 +387,16 @@
 
   std::string name_str = name.AsCString();
 
-  // If this might be a regex, we have to return EVERY symbol and process them
-  // one by one, which is going to destroy performance on large PDB files.  So
-  // try really hard not to use a regex match.
-  if (name_str.find_first_of("[]?*.-+\\") != std::string::npos)
-FindTypesByRegex(name_str, max_matches, types);
-  else
-FindTypesByName(name_str, max_matches, types);
+  // There is an assumption 'name' is not a regex
+  FindTypesByName(name_str, max_matches, types);
+   
   return types.GetSize();
 }
 
-void SymbolFilePDB::FindTypesByRegex(const std::string ®ex,
- uint32_t max_matches,
- lldb_private::TypeMap &types) {
+void
+SymbolFilePDB::FindTypesByRegex(const lldb_private::RegularExpression ®ex,
+uint32_t max_matches,
+lldb_private::TypeMap &types) {
   // When searching by regex, we need to go out of our way to limit the search
   // space as much as possible since this searches EVERYTHING in the PDB,
   // manually doing regex comparisons.  PDB library isn't optimized for regex
@@ -409,8 +408,6 @@
   auto global = m_session_up->getGlobalScope();
   std::unique_ptr results;
 
-  std::regex re(regex);
-
   uint32_t matches = 0;
 
   for (auto tag : tags_to_search) {
@@ -433,7 +430,7 @@
 continue;
   }
 
-  if (!std::regex_match(type_name, re))
+  if (!regex.Execute(type_name))
 continue;
 
   // This should cause the type to get cached and stored in the `m_types`
Index: source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
===
--- source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
+++ source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
@@ -124,6 +124,8 @@
   } else if (auto type_def = llvm::dyn_cast(&type)) {
 lldb_private::Type *target_type =
 m_ast.GetSymbolFile()->ResolveTypeUID(type_def->getTypeId());
+if (!target_type)
+  return nullptr;
 std::string name = type_def->getName();
 uint64_t bytes = type_def->getLength();
 if (!target_type)
@@ -179,6 +181,8 @@
 
 lldb_private::Type *element_type =