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

[Yao Qi <qiyaoltc@gmail.com>]

On Thu, Oct 6, 2016 at 5:27 PM, Luis Machado <lgustavo@codesourcery.com> wrote:
> 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 there. So we need to make sure
> argv is sane before accessing argv[0].
>
> After fixing that, i noticed we were assuming a value of 1 for argc, which is
> also not true, as i see 0 in my tests.

If I understand C standard
http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1570.pdf correctly,
"argv" can't be NULL.

5.1.2.2.1 Program startup
...
 The value of argc shall be nonnegative.
 argv[argc] shall be a null pointer.

The first one implies that argc can be zero, and the second one implies
argv can't be NULL.  In this case, argc is zero, so argv[0] can be
dereferenced.

> @@ -99,6 +99,10 @@ main (int argc, char *argv[])
>    const char *sn = 0;
>    struct str *xstr;
>
> +  /* Prevent gcc from optimizing argv[] out.  */
> +  if (argv != NULL)
> +    cp = argv[0];
> +

As I mentioned above, argv shouldn't be NULL.  Is it your C runtime
problem?

>    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 57a9ba1..bb802bf 100644
> --- a/gdb/testsuite/gdb.python/py-value.exp
> +++ b/gdb/testsuite/gdb.python/py-value.exp
> @@ -274,10 +274,19 @@ proc test_value_in_inferior {} {
>    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"
> +
> +  # If argv is not available, then argc is likely not available as well, thus
> +  # we expect it to be 0.

The comment isn't accurate.  Can you change it to mention "argc can
be nonnegative."?

> +  set argc_value 0
> +  if { $has_argv0 } {
> +    set argc_value 1
> +  }
> +
> +  gdb_test "print argc" " = $argc_value" "sanity check argc"
> +

Instead of setting argc_value manually, we can use get_integer_valueof
to get value of "argc", and save it in argc_value.  The purpose of
tests here is that after changing argc, argc_notlazy still equals to the
old argc while argc_lazy equals to the changed argc.  It doesn't matter
the old argc is 1 or 0.

>    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"
>

Changes in gdb.python/py-value.exp are reasonable to me, but need
some minor changes as I suggested above.  Changes in
gdb.python/py-value.c are not, and you need to fix your C runtime, IMO.

-- 
Yao (齐尧)