[gdb/testsuite] Fix compare-sections.exp FAIL
Commit Message
Hi Pedro,
Thanks for the review. Please find an updated patch attached. Updated ChangeLog
entry and description are as follow:
compare-sections.exp has two cases that are not handled appropriately:
1) value read from read-only section is negative
2) error while patching that section
This patch adapts the regular expression to allow a minus sign to deal
with 1) and test for the error message to not set written if read-only
section cannot be written to so as to solve 2).
ChangeLog entry is as follows:
*** gdb/testsuite/ChangeLog ***
2017-09-11 Thomas Preud'homme <thomas.preudhomme@arm.com>
Pedro Alves <palves@redhat.com>
* gdb.base/compare-sections.exp (get value of read-only section): Read
as unsigned value.
(corrupt read-only section): Don't set written if patching failed.
Without this patch I get a Tcl error on the first test while it now
PASSes and the second one is marked UNSUPPORTED.
Is this ok for master?
Best regards,
Thomas
On 06/09/17 18:13, Pedro Alves wrote:
> On 09/06/2017 05:58 PM, Thomas Preudhomme wrote:
>
>>
>> diff --git a/gdb/testsuite/gdb.base/compare-sections.exp b/gdb/testsuite/gdb.base/compare-sections.exp
>> index e4b99216408f86745a890f80175e2f26c39303b5..8a0cf89a4c7c51299c59f59175865b5eb211f213 100644
>> --- a/gdb/testsuite/gdb.base/compare-sections.exp
>> +++ b/gdb/testsuite/gdb.base/compare-sections.exp
>> @@ -118,7 +118,7 @@ with_test_prefix "read-only" {
>>
>> set test "get value of read-only section"
>> gdb_test_multiple "print /d *(unsigned char *) $ro_address" "$test" {
>> - -re " = (\[0-9\]*).*$gdb_prompt $" {
>> + -re " = (-?\[0-9\]+).*$gdb_prompt $" {
>
> This is reading bytes off of memory, as we can infer from
> use of "unsigned char". The test is most probably using
> explicit /d to avoid printing the number as a character.
> I think it'd be clearer to change the "/d" to "/u", here
> and below too.
>
> Also, I think treating as negative integer would actually
> be buggy because if the value happens to be 0xff, then you
> get back -1, which would incorrectly match here:
>
> if {$orig == -1} {
> untested "couldn't read address of read-only section"
> return -1
> }
>> set orig $expect_out(1,string)
>> pass "$test"
>> }
>> @@ -136,6 +136,8 @@ with_test_prefix "read-only" {
>> set written -1
>> set test "corrupt read-only section"
>> gdb_test_multiple "print /d *(unsigned char *) $ro_address = $patch" "$test" {
>> + -re " = .*Cannot access memory at address $ro_address.*$gdb_prompt $" {
>
> Add:
> pass "$test (cannot write)"
>
>> + }
>> -re " = (\[0-9\]*).*$gdb_prompt $" {
>> set written $expect_out(1,string)
>> pass "$test"
>>
>
> Thanks,
> Pedro Alves
>
Comments
Hi Thomas,
On 09/12/2017 04:24 PM, Thomas Preudhomme wrote:
> Hi Pedro,
>
> Thanks for the review. Please find an updated patch attached. Updated
> ChangeLog entry and description are as follow:
>
> compare-sections.exp has two cases that are not handled appropriately:
> 1) value read from read-only section is negative
> 2) error while patching that section
>
> This patch adapts the regular expression to allow a minus sign to deal
> with 1)
The above is stale. It should now talk about /d vs /u.
Could you please update?
> Is this ok for master?
OK with commit log fixed, and ...
On 09/12/2017 04:24 PM, Thomas Preudhomme wrote:
> @@ -136,6 +136,9 @@ with_test_prefix "read-only" {
> set written -1
> set test "corrupt read-only section"
> gdb_test_multiple "print /d *(unsigned char *) $ro_address = $patch" "$test" {
... this should be switched to "/u" too.
> + -re " = .*Cannot access memory at address $ro_address.*$gdb_prompt $" {
> + pass "$test (cannot write)"
Thanks,
Pedro Alves
@@ -117,7 +117,7 @@ with_test_prefix "read-only" {
set orig -1
set test "get value of read-only section"
- gdb_test_multiple "print /d *(unsigned char *) $ro_address" "$test" {
+ gdb_test_multiple "print /u *(unsigned char *) $ro_address" "$test" {
-re " = (\[0-9\]*).*$gdb_prompt $" {
set orig $expect_out(1,string)
pass "$test"
@@ -136,6 +136,9 @@ with_test_prefix "read-only" {
set written -1
set test "corrupt read-only section"
gdb_test_multiple "print /d *(unsigned char *) $ro_address = $patch" "$test" {
+ -re " = .*Cannot access memory at address $ro_address.*$gdb_prompt $" {
+ pass "$test (cannot write)"
+ }
-re " = (\[0-9\]*).*$gdb_prompt $" {
set written $expect_out(1,string)
pass "$test"