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

[Aaron Smith via Phabricator via lldb-commits]

asmith added inline comments.


================
Comment at: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp:392-399
+  bool is_regex = false;
+  if (name_str.find_first_of("[]?*.-+\\") != std::string::npos) {
+    // Trying to compile an invalid regex could throw an exception.
+    // Only search by regex when it's valid.
+    lldb_private::RegularExpression name_regex(name_str);
+    is_regex = name_regex.IsValid();
+  }
----------------
zturner wrote:
> I can see two possible problems here.
> 
> 1. Now if it is a valid regex, it's going to compile it twice, hurting performance (not sure by how much, it could be negligible)
> 
> 2. Whether or not it's valid is determined by asking LLDB's regex engine, but it's then compiled with C++ STL's engine.  It's possible they won't agree on whether or not a regex is valid.
> 
> You mentioned that it throws an exception on an invalid regex.  What actually happens though?  Because we compile with exception support disabled.  Does it terminate the program?
> 
> At a minimum, the validity check and the find function should agree on the regex engine.  If the only way to make that work is to change from `std::regex` to `lldb_private::RegularExpression` as the matching engine, then I guess we have to do that.
How about this?

```
  lldb_private::RegularExpression name_regex(name_str);
  if (name_regex.IsValid())
    FindTypesByRegex(name_regex, max_matches, types); // pass regex here
  else
    FindTypesByName(name_str, max_matches, types);
  return types.GetSize();
}

void SymbolFilePDB::FindTypesByRegex(lldb_private::RegularExpression &regex,
                                     uint32_t max_matches,
                                     lldb_private::TypeMap &types) {
```


Repository:
  rL LLVM

https://reviews.llvm.org/D41086