Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Checking for conflicts before registering synthetic child providers/filters doesn't work correctly with regex #57947

Closed
slackito opened this issue Sep 23, 2022 · 3 comments
Assignees
Labels

Comments

@slackito
Copy link
Collaborator

CommandObjectTypeSynthAdd::AddSynth has some code to check for conflicting filters in the same category (there's a symmetric check for conflicting synth providers when adding a new filter, but let's focus on only one case). The check looks like this:

  if (category->AnyMatches(type_name, eFormatCategoryItemFilter, false)) {
    if (error)
      error->SetErrorStringWithFormat("cannot add synthetic for type %s when "
                                      "filter is defined in same category!",
                                      type_name.AsCString());
    return false;
  }

The important detail here is that type_name can actually be either a type name or a regex (if using type synthetic add -x). However, category->AnyMatches always treats type_name as a type name, and will try to match a regex string against other regex strings, which is not correct.

For example, this is lldb working correctly and rejecting a conflicting formatter.

(lldb) script
Python Interactive Interpreter. To exit, type 'quit()', 'exit()' or Ctrl-D.
>>> class my_child_provider:
...   pass
... 
>>> ^D
now exiting InteractiveConsole...
(lldb) type synthetic add -l my_child_provider whatever
(lldb) type filter add --child a whatever
error: cannot add filter for type whatever when synthetic is defined in same category!

However, if we use regex it will happily accept conflicting formatters:

(lldb) type synthetic add -l my_child_provider -x "^whatever$"
(lldb) type filter add --child a -x "^whatever$"
(lldb) 

because the string "^whatever$" is not matched by the regex "^whatever$".

We can also make it fail the other way around, and make it incorrectly reject non-conflicting formatters. For example:

(lldb) type synthetic add -l my_child_provider -x "^MyType\[[0-9]+]$"
(lldb) type filter add --child a -x "MyType[12]"
error: cannot add filter for type MyType[12] when synthetic is defined in same category!

In this case, "^MyType\[[0-9]+]$" would accept types that look like an array of MyType, and "MyType[12]" looks like an array, but it would actually match MyType1 and MyType2 so there's no conflict.

I think the right thing to do is to just skip this check when adding a regex-based filter. I'll prepare a patch.

@slackito slackito added the lldb label Sep 23, 2022
@llvmbot
Copy link
Collaborator

llvmbot commented Sep 23, 2022

@llvm/issue-subscribers-lldb

@slackito slackito self-assigned this Sep 23, 2022
@slackito
Copy link
Collaborator Author

Sent https://reviews.llvm.org/D134570 to skip the check if registering a new filter/synth using a regex.

slackito added a commit that referenced this issue Oct 6, 2022
When adding a new synthetic child provider, we check for an existing
conflicting filter in the same category (and vice versa). This is done
by trying to match the new type name against registered formatters.

However, the new type name we're registered can also be a regex
(`type synth add -x`), and in this case the conflict check is just
wrong: it will try to match the new regex as if it was a type name,
against previously registered regexes.

See #57947 for a longer
explanation with concrete examples of incorrect behavior.

Differential Revision: https://reviews.llvm.org/D134570
@slackito
Copy link
Collaborator Author

slackito commented Oct 6, 2022

Fixed in 69c661a by restricting the check to "exact match" formatters only.

@slackito slackito closed this as completed Oct 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants