diff mbox

[v2] Fixup gdb.python/py-value.exp for bare-metal aarch64-elf

Message ID 1476219321-10659-1-git-send-email-lgustavo@codesourcery.com
State New
Headers show

Commit Message

Luis Machado Oct. 11, 2016, 8:55 p.m. UTC 
I noticed that testing aarch64-elf gdb with a physical board
ran into issues with gdb.python/py-value.exp. Further investigation showed
that we were actually trying to dereference a NULL pointer (argv) when trying
to access argv[0].

Being bare-metal, argv is not guaranteed to be valid. So we need to make sure
argv is sane before accessing argv[0].

The following patch fixes up the test program to check for a NULL argv and also
improves the testcase a bit so it doesn't have to work with a hardcoded argc
value.

Regression-tested on x86-64 Ubuntu 16.04.

I've incorporated Yao's feedback in v2.

Does it look OK?

gdb/testsuite/ChangeLog:

2016-10-11  Luis Machado  <lgustavo@codesourcery.com>

	* gdb.python/py-value.c (main): Check if argv is NULL before using it.
	* gdb.python/py-value.exp (test_value_in_inferior): Don't use a
	hardcoded argc value.
---
 gdb/testsuite/ChangeLog               |  6 ++++++
 gdb/testsuite/gdb.python/py-value.c   | 10 +++++++++-
 gdb/testsuite/gdb.python/py-value.exp |  5 +++--
 3 files changed, 18 insertions(+), 3 deletions(-)

Comments

Yao Qi Oct. 12, 2016, 1:26 p.m. UTC | #1 
On Tue, Oct 11, 2016 at 9:55 PM, Luis Machado <lgustavo@codesourcery.com> wrote:
>
> Does it look OK?
>
> gdb/testsuite/ChangeLog:
>
> 2016-10-11  Luis Machado  <lgustavo@codesourcery.com>
>
>         * gdb.python/py-value.c (main): Check if argv is NULL before using it.
>         * gdb.python/py-value.exp (test_value_in_inferior): Don't use a
>         hardcoded argc value.

Yes, it is good to me.
Pedro Alves Oct. 12, 2016, 1:31 p.m. UTC | #2 
On 10/11/2016 09:55 PM, Luis Machado wrote:
> +  set argc_value [get_integer_valueof "argc" 0]
>    gdb_test "python argc_lazy = gdb.parse_and_eval('argc')"
>    gdb_test "python argc_notlazy = gdb.parse_and_eval('argc')"
>    gdb_test "python argc_notlazy.fetch_lazy()"
>    gdb_test "python print (argc_lazy.is_lazy)" "True"
>    gdb_test "python print (argc_notlazy.is_lazy)" "False"
> -  gdb_test "print argc" " = 1" "sanity check argc"
> +  gdb_test "print argc" " = $argc_value" "sanity check argc"
>    gdb_test "python print (argc_lazy.is_lazy)" "\r\nTrue"
>    gdb_test_no_output "set argc=2"

Pedantically, $argc_value could be 2, so this would be better
something like

   gdb_test_no_output "set argc=[expr $argc_value + 1]" "change argc"

> -  gdb_test "python print (argc_notlazy)" "\r\n1"
> +  gdb_test "python print (argc_notlazy)" "\r\n$argc_value"


>    gdb_test "python print (argc_lazy)" "\r\n2"

Likewise.

>    gdb_test "python print (argc_lazy.is_lazy)" "False"

Thanks,
Pedro Alves
Luis Machado Oct. 12, 2016, 3:13 p.m. UTC | #3 
On 10/12/2016 08:31 AM, Pedro Alves wrote:
> On 10/11/2016 09:55 PM, Luis Machado wrote:
>> +  set argc_value [get_integer_valueof "argc" 0]
>>    gdb_test "python argc_lazy = gdb.parse_and_eval('argc')"
>>    gdb_test "python argc_notlazy = gdb.parse_and_eval('argc')"
>>    gdb_test "python argc_notlazy.fetch_lazy()"
>>    gdb_test "python print (argc_lazy.is_lazy)" "True"
>>    gdb_test "python print (argc_notlazy.is_lazy)" "False"
>> -  gdb_test "print argc" " = 1" "sanity check argc"
>> +  gdb_test "print argc" " = $argc_value" "sanity check argc"
>>    gdb_test "python print (argc_lazy.is_lazy)" "\r\nTrue"
>>    gdb_test_no_output "set argc=2"
>
> Pedantically, $argc_value could be 2, so this would be better
> something like
>
>    gdb_test_no_output "set argc=[expr $argc_value + 1]" "change argc"
>
>> -  gdb_test "python print (argc_notlazy)" "\r\n1"
>> +  gdb_test "python print (argc_notlazy)" "\r\n$argc_value"
>
>
>>    gdb_test "python print (argc_lazy)" "\r\n2"
>
> Likewise.
>

Alright. I've addressed this and checked it in on master as 
4dac951e11030b43b17f52df8bdfa7432e4bf73c.

Thanks Yao and Pedro.
diff mbox

Patch

diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index b4ccd4a..2ed2c9c 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,9 @@ 
+2016-10-11  Luis Machado  <lgustavo@codesourcery.com>
+
+	* gdb.python/py-value.c (main): Check if argv is NULL before using it.
+	* gdb.python/py-value.exp (test_value_in_inferior): Adjust initial
+	argc value based on the existence of argv.
+
 2016-10-11  Jan Kratochvil  <jan.kratochvil@redhat.com>
 
 	* gdb.arch/powerpc-prologue.c (optimized_1): New declaration.
diff --git a/gdb/testsuite/gdb.python/py-value.c b/gdb/testsuite/gdb.python/py-value.c
index 586a0fd..8a8ace6 100644
--- a/gdb/testsuite/gdb.python/py-value.c
+++ b/gdb/testsuite/gdb.python/py-value.c
@@ -82,7 +82,7 @@  char **save_argv;
 int
 main (int argc, char *argv[])
 {
-  char *cp = argv[0]; /* Prevent gcc from optimizing argv[] out.  */
+  char *cp;
   struct s s;
   union u u;
   PTR x = &s;
@@ -99,6 +99,14 @@  main (int argc, char *argv[])
   const char *sn = 0;
   struct str *xstr;
 
+  /* Prevent gcc from optimizing argv[] out.  */
+
+  /* We also check for a NULL argv in case we are dealing with a target
+     executing in a freestanding environment, therefore there are no
+     guarantees about argc or argv.  */
+  if (argv != NULL)
+    cp = argv[0];
+
   s.a = 3;
   s.b = 5;
   u.a = 7;
diff --git a/gdb/testsuite/gdb.python/py-value.exp b/gdb/testsuite/gdb.python/py-value.exp
index 662c5b4..f31096b 100644
--- a/gdb/testsuite/gdb.python/py-value.exp
+++ b/gdb/testsuite/gdb.python/py-value.exp
@@ -274,15 +274,16 @@  proc test_value_in_inferior {} {
     gdb_test "python inval2 = inval+1" "gdb.MemoryError: Cannot access memory at address 0x0.*" $test
     gdb_test "python inval.fetch_lazy ()" "gdb.MemoryError: Cannot access memory at address 0x0.*" $test
   }
+  set argc_value [get_integer_valueof "argc" 0]
   gdb_test "python argc_lazy = gdb.parse_and_eval('argc')"
   gdb_test "python argc_notlazy = gdb.parse_and_eval('argc')"
   gdb_test "python argc_notlazy.fetch_lazy()"
   gdb_test "python print (argc_lazy.is_lazy)" "True"
   gdb_test "python print (argc_notlazy.is_lazy)" "False"
-  gdb_test "print argc" " = 1" "sanity check argc"
+  gdb_test "print argc" " = $argc_value" "sanity check argc"
   gdb_test "python print (argc_lazy.is_lazy)" "\r\nTrue"
   gdb_test_no_output "set argc=2"
-  gdb_test "python print (argc_notlazy)" "\r\n1"
+  gdb_test "python print (argc_notlazy)" "\r\n$argc_value"
   gdb_test "python print (argc_lazy)" "\r\n2"
   gdb_test "python print (argc_lazy.is_lazy)" "False"