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)
On Wed, Dec 13, 2017 at 10:27 AM Aaron Smith via Phabricator <
revi...@reviews.llvm.org> wrote:
> 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
> ®ex,
> > 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
>
>
>
>
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
