<div dir="ltr">Can we try using lldb_private::RegularExpression for everything?  (Long term, adding a new base class method seems like a better approach, but at least this quick fix is an immediate fix and should be strictly better than crashing)</div><br><div class="gmail_quote"><div dir="ltr">On Wed, Dec 13, 2017 at 10:27 AM Aaron Smith via Phabricator <<a href="mailto:reviews@reviews.llvm.org">reviews@reviews.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">asmith added inline comments.<br>
<br>
<br>
================<br>
Comment at: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp:392-399<br>
+  bool is_regex = false;<br>
+  if (name_str.find_first_of("[]?*.-+\\") != std::string::npos) {<br>
+    // Trying to compile an invalid regex could throw an exception.<br>
+    // Only search by regex when it's valid.<br>
+    lldb_private::RegularExpression name_regex(name_str);<br>
+    is_regex = name_regex.IsValid();<br>
+  }<br>
----------------<br>
asmith wrote:<br>
> zturner wrote:<br>
> > I can see two possible problems here.<br>
> ><br>
> > 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)<br>
> ><br>
> > 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.<br>
> ><br>
> > 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?<br>
> ><br>
> > 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.<br>
> How about this?<br>
><br>
> ```<br>
>   lldb_private::RegularExpression name_regex(name_str);<br>
>   if (name_regex.IsValid())<br>
>     FindTypesByRegex(name_regex, max_matches, types); // pass regex here<br>
>   else<br>
>     FindTypesByName(name_str, max_matches, types);<br>
>   return types.GetSize();<br>
> }<br>
><br>
> void SymbolFilePDB::FindTypesByRegex(lldb_private::RegularExpression &regex,<br>
>                                      uint32_t max_matches,<br>
>                                      lldb_private::TypeMap &types) {<br>
> ```<br>
Our experience on Windows is that when lldb is built with exceptions disabled, std::regex segfaults on an invalid regex causing lldb to terminate.<br>
<br>
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):<br>
<br>
image lookup -n function_name<br>
<br>
<br>
Repository:<br>
  rL LLVM<br>
<br>
<a href="https://reviews.llvm.org/D41086" rel="noreferrer" target="_blank">https://reviews.llvm.org/D41086</a><br>
<br>
<br>
<br>
</blockquote></div>