Re: [patch] Fix internal error on breaking at a multi-locations caller

[Joel Brobecker <brobecker at adacore dot com>]

Hi Jan,

> gdb/
> 2009-05-01  Joel Brobecker <brobecker@adacore.com>
> 	    Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	Fix internal error on breaking at a multi-locations caller source line.
> 	* breakpoint.c (parse_breakpoint_sals): Set EXPLICIT_PC for the `break'
> 	command with no parameters.

The code part is OK, but I was a little confused by the comment.
I think this is because you're talking about your specific scenario
whereas I'm ready to bet that the issue can appear without doing
an "up" before.  My suggestion is to keep it general, but saying
something like: "break" without arguments is equivalent to "break *PC"
where PC is the default_breakpoint_address.  So make sure to set
sal.explicit_pc to prevent GDB from trying to expand the list of
sals to include all other instances with the same symtab and line.

> +# PC should not be at the boundary of source lines to make the original bug
> +# exploitable.
> +
> +set test "p/x \$pc"
> +set pc {}
> +gdb_test_multiple $test $test {
> +    -re "\\$\[0-9\]+ = (0x\[0-9a-f\]+)\r\n$gdb_prompt $" {
> +	set pc $expect_out(1,string)
> +	pass $test
> +    }
> +}
> +
> +set test "info line"
> +set end {}
> +gdb_test_multiple $test $test {
> +    -re "Line \[0-9\]+ of .* starts at address 0x\[0-9a-f\]+.* and ends at (0x\[0-9a-f\]+).*\\.\r\n$gdb_prompt $" {
> +	set end $expect_out(1,string)
> +	pass $test
> +    }
> +}
> +
> +set test "caller line has trailing code"
> +if {$pc != $end} {
> +    pass $test
> +} else {
> +    fail $test
> +}

I don't think this is right - the fail here is conditional on the code
generated by the compiler on the given platform.  If the return address
points at the beginning of the next line of code, then our testcase
won't allow us to test the issue.  But there's no real failure
demonstrated at this point.

My feeling on this is that we should just do the testing you do after
without worrying whether the break address is at the beginning of
a line or not. So I'd just ditch the above, and keep the rest of
the testcase as is.

-- 
Joel