Harden gdb.arch/aarch64-pauth.exp and fix a failure

Message ID 20200221210628.24922-1-luis.machado@linaro.org
State New
Headers show
Series
  • Harden gdb.arch/aarch64-pauth.exp and fix a failure
Related show

Commit Message

Luis Machado Feb. 21, 2020, 9:06 p.m.
When running this testcase against a QEMU with PAC support, i noticed we
were failing to recognize the additional [PAC] that is emitted in the
backtrace, resulting in this failure:

FAIL: gdb.arch/aarch64-pauth.exp: backtrace

I've made the test use multi_line to make the pattern more clear.

Tested against aarch64-linux-gnu with and without PAC support.

gdb/testsuite/ChangeLog:

2020-02-21  Luis Machado  <luis.machado@linaro.org>

	* gdb.arch/aarch64-pauth.exp: Recognize optional PAC output.

Signed-off-by: Luis Machado <luis.machado@linaro.org>

---
 gdb/testsuite/gdb.arch/aarch64-pauth.exp | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

-- 
2.17.1

Comments

Kevin Buettner Feb. 22, 2020, 4:20 a.m. | #1
On Fri, 21 Feb 2020 18:06:28 -0300
Luis Machado <luis.machado@linaro.org> wrote:

> When running this testcase against a QEMU with PAC support, i noticed we

> were failing to recognize the additional [PAC] that is emitted in the

> backtrace, resulting in this failure:

> 

> FAIL: gdb.arch/aarch64-pauth.exp: backtrace

> 

> I've made the test use multi_line to make the pattern more clear.

> 

> Tested against aarch64-linux-gnu with and without PAC support.

> 

> gdb/testsuite/ChangeLog:

> 

> 2020-02-21  Luis Machado  <luis.machado@linaro.org>

> 

> 	* gdb.arch/aarch64-pauth.exp: Recognize optional PAC output.

> 

> Signed-off-by: Luis Machado <luis.machado@linaro.org>

> ---

>  gdb/testsuite/gdb.arch/aarch64-pauth.exp | 7 ++++++-

>  1 file changed, 6 insertions(+), 1 deletion(-)

> 

> diff --git a/gdb/testsuite/gdb.arch/aarch64-pauth.exp b/gdb/testsuite/gdb.arch/aarch64-pauth.exp

> index 816e58be44..e6d25c5d97 100644

> --- a/gdb/testsuite/gdb.arch/aarch64-pauth.exp

> +++ b/gdb/testsuite/gdb.arch/aarch64-pauth.exp

> @@ -40,4 +40,9 @@ gdb_breakpoint [ gdb_get_line_number "break here" ]

>  gdb_continue_to_breakpoint "break here" ".*break here.*"

>  

>  # Ensure we can get a full backtrace, despite the address signing.

> -gdb_test "bt" "^bt\r\n#0 +bar *\\(b=9\\) +at.*\r\n#1 +0x\[0-9a-f\]* +in +foo \\(a=5\\).*\r\n#2 +0x\[0-9a-f\]* +in +main \\(\\).*" "backtrace"

> +gdb_test "bt" \

> +    [multi_line \

> +	"#0\[ \t\]*bar \\(b=9\\) at \[^\r\n\]+" \

> +	"#1\[ \t\]*$hex (\\\[PAC\\\] )?in foo \\(a=5\\) at \[^\r\n\]+" \

> +	"#2\[ \t\]*$hex (\\\[PAC\\\] )?in main \\(\\) at \[^\r\n\]+" ] \

> +    "backtrace"


LGTM except for the \[ \t\]* parts of it.  I think we actually want +
instead of * to ensure that there's whitespace separating the frame
number from the hex value.

Kevin
Luis Machado Feb. 22, 2020, 11:42 a.m. | #2
On 2/22/20 1:20 AM, Kevin Buettner wrote:
> On Fri, 21 Feb 2020 18:06:28 -0300

> Luis Machado <luis.machado@linaro.org> wrote:

> 

>> When running this testcase against a QEMU with PAC support, i noticed we

>> were failing to recognize the additional [PAC] that is emitted in the

>> backtrace, resulting in this failure:

>>

>> FAIL: gdb.arch/aarch64-pauth.exp: backtrace

>>

>> I've made the test use multi_line to make the pattern more clear.

>>

>> Tested against aarch64-linux-gnu with and without PAC support.

>>

>> gdb/testsuite/ChangeLog:

>>

>> 2020-02-21  Luis Machado  <luis.machado@linaro.org>

>>

>> 	* gdb.arch/aarch64-pauth.exp: Recognize optional PAC output.

>>

>> Signed-off-by: Luis Machado <luis.machado@linaro.org>

>> ---

>>   gdb/testsuite/gdb.arch/aarch64-pauth.exp | 7 ++++++-

>>   1 file changed, 6 insertions(+), 1 deletion(-)

>>

>> diff --git a/gdb/testsuite/gdb.arch/aarch64-pauth.exp b/gdb/testsuite/gdb.arch/aarch64-pauth.exp

>> index 816e58be44..e6d25c5d97 100644

>> --- a/gdb/testsuite/gdb.arch/aarch64-pauth.exp

>> +++ b/gdb/testsuite/gdb.arch/aarch64-pauth.exp

>> @@ -40,4 +40,9 @@ gdb_breakpoint [ gdb_get_line_number "break here" ]

>>   gdb_continue_to_breakpoint "break here" ".*break here.*"

>>   

>>   # Ensure we can get a full backtrace, despite the address signing.

>> -gdb_test "bt" "^bt\r\n#0 +bar *\\(b=9\\) +at.*\r\n#1 +0x\[0-9a-f\]* +in +foo \\(a=5\\).*\r\n#2 +0x\[0-9a-f\]* +in +main \\(\\).*" "backtrace"

>> +gdb_test "bt" \

>> +    [multi_line \

>> +	"#0\[ \t\]*bar \\(b=9\\) at \[^\r\n\]+" \

>> +	"#1\[ \t\]*$hex (\\\[PAC\\\] )?in foo \\(a=5\\) at \[^\r\n\]+" \

>> +	"#2\[ \t\]*$hex (\\\[PAC\\\] )?in main \\(\\) at \[^\r\n\]+" ] \

>> +    "backtrace"

> 

> LGTM except for the \[ \t\]* parts of it.  I think we actually want +

> instead of * to ensure that there's whitespace separating the frame

> number from the hex value.

> 

> Kevin

> 


Indeed. Fixed now.

Thanks for the review.
Luis Machado Feb. 28, 2020, 10:30 a.m. | #3
On 2/22/20 1:20 AM, Kevin Buettner wrote:
> On Fri, 21 Feb 2020 18:06:28 -0300

> Luis Machado <luis.machado@linaro.org> wrote:

> 

>> When running this testcase against a QEMU with PAC support, i noticed we

>> were failing to recognize the additional [PAC] that is emitted in the

>> backtrace, resulting in this failure:

>>

>> FAIL: gdb.arch/aarch64-pauth.exp: backtrace

>>

>> I've made the test use multi_line to make the pattern more clear.

>>

>> Tested against aarch64-linux-gnu with and without PAC support.

>>

>> gdb/testsuite/ChangeLog:

>>

>> 2020-02-21  Luis Machado  <luis.machado@linaro.org>

>>

>> 	* gdb.arch/aarch64-pauth.exp: Recognize optional PAC output.

>>

>> Signed-off-by: Luis Machado <luis.machado@linaro.org>

>> ---

>>   gdb/testsuite/gdb.arch/aarch64-pauth.exp | 7 ++++++-

>>   1 file changed, 6 insertions(+), 1 deletion(-)

>>

>> diff --git a/gdb/testsuite/gdb.arch/aarch64-pauth.exp b/gdb/testsuite/gdb.arch/aarch64-pauth.exp

>> index 816e58be44..e6d25c5d97 100644

>> --- a/gdb/testsuite/gdb.arch/aarch64-pauth.exp

>> +++ b/gdb/testsuite/gdb.arch/aarch64-pauth.exp

>> @@ -40,4 +40,9 @@ gdb_breakpoint [ gdb_get_line_number "break here" ]

>>   gdb_continue_to_breakpoint "break here" ".*break here.*"

>>   

>>   # Ensure we can get a full backtrace, despite the address signing.

>> -gdb_test "bt" "^bt\r\n#0 +bar *\\(b=9\\) +at.*\r\n#1 +0x\[0-9a-f\]* +in +foo \\(a=5\\).*\r\n#2 +0x\[0-9a-f\]* +in +main \\(\\).*" "backtrace"

>> +gdb_test "bt" \

>> +    [multi_line \

>> +	"#0\[ \t\]*bar \\(b=9\\) at \[^\r\n\]+" \

>> +	"#1\[ \t\]*$hex (\\\[PAC\\\] )?in foo \\(a=5\\) at \[^\r\n\]+" \

>> +	"#2\[ \t\]*$hex (\\\[PAC\\\] )?in main \\(\\) at \[^\r\n\]+" ] \

>> +    "backtrace"

> 

> LGTM except for the \[ \t\]* parts of it.  I think we actually want +

> instead of * to ensure that there's whitespace separating the frame

> number from the hex value.

> 

> Kevin

> 


Fixed and pushed. Thanks!

I'll also push a fix to gdb.base/backtrace.exp, since it also uses * 
instead of + for the following tests:

# Backtrace with the default options.
gdb_test "bt" \
     [multi_line \
          "#0\[ \t\]*baz \\(\\) at \[^\r\n\]+" \
          "#1\[ \t\]*$hex in bar \\(\\) at \[^\r\n\]+" \
          "#2\[ \t\]*$hex in foo \\(\\) at \[^\r\n\]+" \
          "#3\[ \t\]*$hex in main \\(\\) at \[^\r\n\]+" ]

# Backtrace with 'set disassemble-next-line on'.  This shouldn't make
# any difference to the backtrace.
gdb_test "with disassemble-next-line on -- bt" \
     [multi_line \
          "#0\[ \t\]*baz \\(\\) at \[^\r\n\]+" \
          "#1\[ \t\]*$hex in bar \\(\\) at \[^\r\n\]+" \
          "#2\[ \t\]*$hex in foo \\(\\) at \[^\r\n\]+" \
          "#3\[ \t\]*$hex in main \\(\\) at \[^\r\n\]+" ]

Patch

diff --git a/gdb/testsuite/gdb.arch/aarch64-pauth.exp b/gdb/testsuite/gdb.arch/aarch64-pauth.exp
index 816e58be44..e6d25c5d97 100644
--- a/gdb/testsuite/gdb.arch/aarch64-pauth.exp
+++ b/gdb/testsuite/gdb.arch/aarch64-pauth.exp
@@ -40,4 +40,9 @@  gdb_breakpoint [ gdb_get_line_number "break here" ]
 gdb_continue_to_breakpoint "break here" ".*break here.*"
 
 # Ensure we can get a full backtrace, despite the address signing.
-gdb_test "bt" "^bt\r\n#0 +bar *\\(b=9\\) +at.*\r\n#1 +0x\[0-9a-f\]* +in +foo \\(a=5\\).*\r\n#2 +0x\[0-9a-f\]* +in +main \\(\\).*" "backtrace"
+gdb_test "bt" \
+    [multi_line \
+	"#0\[ \t\]*bar \\(b=9\\) at \[^\r\n\]+" \
+	"#1\[ \t\]*$hex (\\\[PAC\\\] )?in foo \\(a=5\\) at \[^\r\n\]+" \
+	"#2\[ \t\]*$hex (\\\[PAC\\\] )?in main \\(\\) at \[^\r\n\]+" ] \
+    "backtrace"