Harden gdb.base/bp-permanent.exp

  On 04/10/2015 07:04 AM, Pedro Alves wrote:
> On 04/09/2015 06:10 PM, Luis Machado wrote:
>
>> diff --git a/gdb/testsuite/gdb.base/bp-permanent.exp b/gdb/testsuite/gdb.base/bp-permanent.exp
>> index 81a5293..9193db8 100644
>> --- a/gdb/testsuite/gdb.base/bp-permanent.exp
>> +++ b/gdb/testsuite/gdb.base/bp-permanent.exp
>> @@ -104,7 +104,18 @@ proc test {always_inserted sw_watchpoint} {
>>   	# to memory manually.
>>   	set count [expr $address_after_bp - $address_bp]
>>   	for {set i 0} {$i < $count} {incr i} {
>> -	    gdb_test "p /x addr_bp\[$i\] = buffer\[$i\]" " = .*"
>> +	    gdb_test_multiple "p /x addr_bp\[$i\] = buffer\[$i\]" $test {
>> +		-re "Cannot access memory at address $hex.*$gdb_prompt $" {
>> +		    # Some targets (QEMU for one) do not allow writes to the
>> +		    # .text section.  It is no use continuing with the test
>> +		    # at this point. Just return.
>
> Double space after period.
>
>> +		    unsupported $test
>
> Something like:
>
> 	    unsupported "Cannot access memory"
>

Did you mean untested here also?

I went with "Cannot modify memory" to make the obstacle more explicit. I 
also modified the comment a bit to make it clear what we are dealing 
with. I'm sure older QEMU's worked in this regard, but more recent ones 
have stack protection, for example, that will lead to these failures.

> OK with those changes.
>

Attached is what i plan to push later (pending the unsupported/untested 
nit above).

> I'm thinking it'd be good to adjust the test to hardcode the
> breakpoint instruction (on an arch by arch basis, leaving the
> current generic code in place), as it'd be good to test
> stepping past permanent/program trap instructions
> on QEMU/Valgrind, etc. too.

Originally i had modified the testcase so it would write the breakpoint 
on its own based on what memcpy read before. We could still use this 
mechanism so we don't need to hardcode per-arch breakpoint patterns. 
What is your idea?

Luis
  

On 04/10/2015 03:02 PM, Luis Machado wrote:
> On 04/10/2015 07:04 AM, Pedro Alves wrote:
>> On 04/09/2015 06:10 PM, Luis Machado wrote:

>> 	    unsupported "Cannot access memory"
>>
> 

> Did you mean untested here also?

No.

>> I'm thinking it'd be good to adjust the test to hardcode the
>> breakpoint instruction (on an arch by arch basis, leaving the
>> current generic code in place), as it'd be good to test
>> stepping past permanent/program trap instructions
>> on QEMU/Valgrind, etc. too.
> 
> Originally i had modified the testcase so it would write the breakpoint 
> on its own based on what memcpy read before. We could still use this 
> mechanism so we don't need to hardcode per-arch breakpoint patterns. 
> What is your idea?

Then we'd have somehow made .text writable, or copy the instructions
somewhere else we can write, and execute them there.  Either way
sounds like we'd lose portability.

My idea was just around something like:

 #ifdef __i386__
 #  define BP int3
 #else if __aarch64__
 #  define BP ...
 #else
 #  define BP NOP
 #endif

and then in .exp file, if the breakpoint address
still has a NOP, do the probing magic, otherwise, skip it.

Thanks,
Pedro Alves
  

On 04/10/2015 11:02 AM, Luis Machado wrote:
> On 04/10/2015 07:04 AM, Pedro Alves wrote:
>> On 04/09/2015 06:10 PM, Luis Machado wrote:
>>
>>> diff --git a/gdb/testsuite/gdb.base/bp-permanent.exp
>>> b/gdb/testsuite/gdb.base/bp-permanent.exp
>>> index 81a5293..9193db8 100644
>>> --- a/gdb/testsuite/gdb.base/bp-permanent.exp
>>> +++ b/gdb/testsuite/gdb.base/bp-permanent.exp
>>> @@ -104,7 +104,18 @@ proc test {always_inserted sw_watchpoint} {
>>>       # to memory manually.
>>>       set count [expr $address_after_bp - $address_bp]
>>>       for {set i 0} {$i < $count} {incr i} {
>>> -        gdb_test "p /x addr_bp\[$i\] = buffer\[$i\]" " = .*"
>>> +        gdb_test_multiple "p /x addr_bp\[$i\] = buffer\[$i\]" $test {
>>> +        -re "Cannot access memory at address $hex.*$gdb_prompt $" {
>>> +            # Some targets (QEMU for one) do not allow writes to the
>>> +            # .text section.  It is no use continuing with the test
>>> +            # at this point. Just return.
>>
>> Double space after period.
>>
>>> +            unsupported $test
>>
>> Something like:
>>
>>         unsupported "Cannot access memory"
>>
>
> Did you mean untested here also?
>
> I went with "Cannot modify memory" to make the obstacle more explicit. I
> also modified the comment a bit to make it clear what we are dealing
> with. I'm sure older QEMU's worked in this regard, but more recent ones
> have stack protection, for example, that will lead to these failures.
>
>> OK with those changes.
>>
>
> Attached is what i plan to push later (pending the unsupported/untested
> nit above).
>

I've pushed this now.
  

Hi Luis,

On 04/13/2015 06:51 PM, Luis Machado wrote:

> I've pushed this now.

I've just noticed this:

-PASS: gdb.base/bp-permanent.exp: always_inserted=off, sw_watchpoint=0: setup: p /x addr_bp[0] = buffer[0]
-PASS: gdb.base/bp-permanent.exp: always_inserted=off, sw_watchpoint=0: setup: p /x addr_bp[1] = buffer[1]
-PASS: gdb.base/bp-permanent.exp: always_inserted=off, sw_watchpoint=0: setup: p /x addr_bp[2] = buffer[2]
-PASS: gdb.base/bp-permanent.exp: always_inserted=off, sw_watchpoint=0: setup: p /x addr_bp[3] = buffer[3]
+PASS: gdb.base/bp-permanent.exp: always_inserted=off, sw_watchpoint=0: setup: get size of instruction
+PASS: gdb.base/bp-permanent.exp: always_inserted=off, sw_watchpoint=0: setup: get size of instruction
+PASS: gdb.base/bp-permanent.exp: always_inserted=off, sw_watchpoint=0: setup: get size of instruction
+PASS: gdb.base/bp-permanent.exp: always_inserted=off, sw_watchpoint=0: setup: get size of instruction

Obviously, "get size of instruction" is the wrong test message to
use here.  Could you restore the old messages please?

Thanks,
Pedro Alves