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

Add enable&disable breakpoints by rg-expression. #230

Merged
merged 7 commits into from
Mar 7, 2018

Conversation

sunbohong
Copy link
Contributor

No description provided.

Copy link
Contributor

@kastiglione kastiglione left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution, this is a cool command.


br_list = listStr.splitlines()
expression_pattern = re.compile(r'{}'.format(expression),re.I)
id_pattern = re.compile(r'^\s*([1-9]\d*)(\.\d+)?',re.I)
Copy link
Contributor

@kastiglione kastiglione Feb 26, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about using the Python API, instead of parsing the output of breakpoint list?

For example, it would roughly look like:

for breakpoint in target.breakpoint_iter():
  for location in breakpoint:
    if pattern.search(location.GetAddress().symbol.name):
      location.SetEnabled(False)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  target = lldb.debugger.GetSelectedTarget()
  for breakpoint in target.breakpoint_iter():
    for location in breakpoint:
      if expression_pattern.search('{}'.format(location)):
        location.SetEnabled(on)

What do you think about using location?
They have some different.

location	2.1: where = SunLigth`__66+[SUNNetService httpAsyncPostWithDictionary:completionBlock:]_block_invoke + 1984 at SUNNetService.m:37, address = 0x0000000104514dfc, unresolved, hit count = 0  Options: disabled 
address		SunLigth`__66+[SUNNetService httpAsyncPostWithDictionary:completionBlock:]_block_invoke + 1984 at SUNNetService.m:37
symbol		id = {0x00006db1}, range = [0x00000001001e063c-0x00000001001e0ec8), mangled="__66+[SUNNetService httpAsyncPostWithDictionary:completionBlock:]_block_invoke"
name 		__66+[SUNNetService httpAsyncPostWithDictionary:completionBlock:]_block_invoke

So that,we can use benable disabled to switch all breakpoints to enable,and also as follow


#use `benable disabled` to switch all breakpoints to `enable`
benable disabled

* benable ***address***
benable 0x0000000104514dfc

* benable ***filename***
benable SUNNetService.m 

* benable ***module***
benable UIKit
benable SunLigth 

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Enabling/disabling via filename and module definitely seem useful!

return 'benable'

def description(self):
return "Enable a set of breakpoints for a relative expression"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo: should be "regular", not "relative"


class FBMethodBreakpointEnableCommand(fb.FBCommand):
def name(self):
return 'benable'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lldb has an alias command named rb for regex breakpoint. I think it makes sense to call this rbenable and rbdisable, what do you think?

@@ -189,6 +191,86 @@ def run(self, arguments, options):
fb.evaluateEffect('[[UIApplication sharedApplication] performSelector:@selector(_performMemoryWarning)]')


def switchBreakpointState(expression,on):
ci = lldb.debugger.GetCommandInterpreter()
res = lldb.SBCommandReturnObject()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These two lines are no longer used.

ci = lldb.debugger.GetCommandInterpreter()
res = lldb.SBCommandReturnObject()

expression_pattern = re.compile(r'{}'.format(expression),re.I)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be re.compile(expression, re.I)

target = lldb.debugger.GetSelectedTarget()
for breakpoint in target.breakpoint_iter():
for location in breakpoint:
if expression_pattern.search('{}'.format(location)):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This also doesn't need the format, I think it can be just .search(str(location)).

for breakpoint in target.breakpoint_iter():
for location in breakpoint:
if expression_pattern.search('{}'.format(location)):
print location
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice addition 👍


#use `rbenable ***module(AppName)***` to switch all breakpoints in this module to `enable`
benable UIKit
benable Foundation
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This documentation block uses benable where it should be rbenable.

Examples:

#use `rbenable disabled` to switch all breakpoints to `enable`
benable disabled
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't appear to be implemented. I don't think it's necessary to implement, since br dis (breakpoint disable) does this already.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since br dis (breakpoint disable)has already this.
Should we use location.GetAddress() to search?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

location	57.1: where = libsystem_kernel.dylib`mach_msg_trap + 4, address = 0x00000001832ef564, resolved, hit count = 0 
address		libsystem_kernel.dylib`mach_msg_trap + 4
symbol		id = {0x0000024e}, range = [0x000000018095b560-0x000000018095b56c), name="mach_msg_trap"
name		mach_msg_trap

when i set breakpoint at this address(0x0000024e),only symbol contain 0x0000024e.
So i want search address and symbol.How do you think?

Copy link
Contributor

@kastiglione kastiglione Mar 1, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does id = {0x0000024e} represent? The address of the breakpoint is in the location: address = 0x00000001832ef564.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i am sorry.i mean that i want search 0x1832ef564 ,not 0x00000001832ef564

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean you want rbdisable 0x1832ef564 to disable the breakpoint whose address is 0x00000001832ef564?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes

Copy link
Contributor

@kastiglione kastiglione Mar 2, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add special cased input handling. If the user's input expression matches 0x[[:xdigit:]], then when building the regex, add a 0* after the 0x. So 0x1832ef564 would become 0x0*1832ef564.

@kastiglione
Copy link
Contributor

@sunbohong I have just a few more small comments. Once those are resolved, I will merge this pull request. Thanks again.

@kastiglione kastiglione self-assigned this Mar 2, 2018
@kastiglione
Copy link
Contributor

@sunbohong after thinking about it a little bit, I think benable and bdisable are good names for these commands. Disabling by address isn't really a regular expression, and I have an idea or two for extensions to this command, which are also not for regular expressions. One command I've built for myself in the past is to disable the current breakpoint when stopped, and I think bdisable is a good place to add that functionality.

Examples:

#use `benable disabled` to switch all breakpoints to `enable`
benable disabled
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this should be documented. A builtin command already does this: breakpoint enable (or br en for short).

Examples:

#use `bdisable disabled` to switch all breakpoints to `disable`
bdisable disabled
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Likewise, this is builtin.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok,i will remove it

for breakpoint in target.breakpoint_iter():
if breakpoint.IsEnabled() != on and (expression_pattern.search(str(breakpoint))):
print str(breakpoint)
breakpoint.SetEnabled(on)
Copy link
Contributor

@kastiglione kastiglione Mar 6, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you find this was necessary in your experience? I'm curious what are some example cases where these commands need to apply to the breakpoint itself instead of working only on the locations.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we use br disable 41 to off 41,and benable 41 only enable locations's state.
The 41 41.1 will be ignore.

@kastiglione kastiglione merged commit 89297aa into facebook:master Mar 7, 2018
@kastiglione
Copy link
Contributor

Thanks @sunbohong!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants