Re: [python] Allow explicit locations in breakpoints.

[Phil Muldoon <pmuldoon at redhat dot com>]

On 23/11/17 22:17, Simon Marchi wrote:
> On 2017-11-17 06:02 AM, Phil Muldoon wrote:

> 
> I would suggest putting the argument validation code in its own function (e.g.
> bppy_init_validate_args), to keep bppy_init of a reasonnable size.

Okay, will do.
 
> For convenience, I think it would be nice to accept integers for line.  So perhaps
> something like this:
> 
> From d0e3122bca8b4fcb2d6f303bdc66f68929bdc14f Mon Sep 17 00:00:00 2001
> From: Simon Marchi <simon.marchi@ericsson.com>
> Date: Thu, 23 Nov 2017 16:58:35 -0500
> Subject: [PATCH] suggestion

Yeah good idea. I remember we talked about this, but I must have
forgotten about it. I'll add your patch in.

>> diff --git a/gdb/testsuite/gdb.python/py-breakpoint.exp b/gdb/testsuite/gdb.python/py-breakpoint.exp
>> index bd138ac3d2..d91e5deb50 100644
>> --- a/gdb/testsuite/gdb.python/py-breakpoint.exp
>> +++ b/gdb/testsuite/gdb.python/py-breakpoint.exp
>> @@ -547,6 +547,74 @@ proc test_bkpt_events {} {
>>      check_last_event breakpoint_deleted
>>  }
>>  
>> +proc test_bkpt_explicit_loc {} {
>> +    global srcfile testfile
>> +
>> +    with_test_prefix test_bkpt_invisible {
>> +	# Start with a fresh gdb.
>> +	clean_restart ${testfile}
>> +
>> +	if ![runto_main] then {
>> +	    fail "cannot run to main."
>> +	    return 0
>> +	}
>> +
>> +	delete_breakpoints
>> +
>> +	set bp_location1 [gdb_get_line_number "Break at multiply."]
>> +	set bp_location2 [gdb_get_line_number "Break at add."]
>> +
>> +	gdb_py_test_silent_cmd "python bp1 = gdb.Breakpoint (line=\"$bp_location1\")" \
>> +	    "set explicit breakpoint by line" 0
>> +	gdb_continue_to_breakpoint "Break at multiply" \
>> +	    ".*Break at multiply.*"
>> +
>> +	gdb_py_test_silent_cmd "python bp1 = gdb.Breakpoint (line=\"+1\")" \
>> +	    "set explicit breakpoint by relative line" 0
>> +	gdb_continue_to_breakpoint "Break at add" \
>> +	    ".*Break at add.*"
>> +
>> +	delete_breakpoints
>> +	gdb_py_test_silent_cmd "python bp1 = gdb.Breakpoint (line=\"-1\")" \
>> +	    "set explicit breakpoint by relative negative line" 0
>> +	gdb_continue_to_breakpoint "Break at multiply" \
>> +	    ".*Break at multiply.*"
>> +
>> +	delete_breakpoints
>> +	gdb_py_test_silent_cmd "python bp1 = gdb.Breakpoint (function=\"add\")" \
>> +	    "set explicit breakpoint by function" 0
>> +	gdb_continue_to_breakpoint "Break at function add" \
>> +	    ".*Break at function add.*"
>> +
>> +	delete_breakpoints
>> +	gdb_py_test_silent_cmd "python bp1 = gdb.Breakpoint (source=\"$srcfile\", function=\"add\")" \
>> +	    "set explicit breakpoint by source file and function" 0
>> +	gdb_continue_to_breakpoint "Break at function add" \
>> +	    ".*Break at function add.*"
>> +
>> +	delete_breakpoints
>> +	gdb_py_test_silent_cmd "python bp1 = gdb.Breakpoint (source=\"$srcfile\", line=\"$bp_location2\")" \
>> +	    "set explicit breakpoint by source file and line number" 0
>> +	gdb_continue_to_breakpoint "Break at add" \
>> +	    ".*Break at add.*"
>> +
>> +	delete_breakpoints
>> +	gdb_test "python bp1 = gdb.Breakpoint (source=\"$srcfile\")" \
>> +	    "RuntimeError: Specifying a source must also include a line, label or function.*" \
>> +	    "set invalid explicit breakpoint by source only"
>> +
>> +	gdb_test "python bp1 = gdb.Breakpoint (source=\"foo.c\", line=\"5\")" \
>> +	    "No source file named foo.*" \
>> +	    "set invalid explicit breakpoint by missing source and line"
>> +	gdb_test "python bp1 = gdb.Breakpoint (source=\"$srcfile\", line=\"900\")" \
>> +	    "No line 900 in file \"$srcfile\".*" \
>> +	    "set invalid explicit breakpoint by source and invalid line"
>> +	gdb_test "python bp1 = gdb.Breakpoint (function=\"blah\")" \
>> +	    "Function \"blah\" not defined.*" \
>> +	    "set invalid explicit breakpoint by missing function"
>> +    }
> 
> The test names should start with a lower case letter:
> 
> https://sourceware.org/gdb/wiki/GDBTestcaseCookbook#Follow_the_test_name_convention
> 
> Also make sure that test names are not duplicated.  You can use this command to quickly
> find duplicated names in gdb.sum:
> 
> $ cat testsuite/gdb.sum | grep -e PASS -e FAIL | sort | uniq -c | sort -n

I thought they all were. Regardless, I'll check again before resubmitting.
 
> Would it be good to test that explicit breakpoint locations through the spec
> argument work as well?  Things like
> 
>   gdb.Breakpoint('-line 5')

Yeah, good idea. They all originally were but I converted them to the
keywords without thinking of the spec case

Cheers

Phil