[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();
+  }
----------------
asmith wrote:
> 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) {
> ```
Our experience on Windows is that when lldb is built with exceptions disabled, std::regex segfaults on an invalid regex causing lldb to terminate.

The test case will reproduce the failure or an example from the lldb console (if you had the corresponding changes required to do the symbol lookup):

image lookup -n function_name


Repository:
  rL LLVM

https://reviews.llvm.org/D41086