From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 88091 invoked by alias); 7 Oct 2016 10:23:22 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 87914 invoked by uid 89); 7 Oct 2016 10:23:21 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_NONE,SPF_PASS,URIBL_RED autolearn=ham version=3.3.2 spammy=Luis, luis X-HELO: mail-oi0-f45.google.com Received: from mail-oi0-f45.google.com (HELO mail-oi0-f45.google.com) (209.85.218.45) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 07 Oct 2016 10:23:11 +0000 Received: by mail-oi0-f45.google.com with SMTP id d132so52080911oib.2 for ; Fri, 07 Oct 2016 03:23:10 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=Xv2DNQ0mP2ab8loW3hJNSveFONUiYO8p310kbV6qzsc=; b=YDLb5c/z04T15h7ddWkJiAllAyUANZz5JPCpCT0/y7cdEcmamN/QUNo8HTysgZLrV7 kfUNXwj6MpSBeyVHf0YB7SinQ2orCRbpVnyLg98EcVMyOBh3nNEF8Bx9H7cTa2RYMWVa Ho9ONT09NTGRLQLyvjai53APecA3BxTZt5nIK/aFqLZf1dAvUNvcqjzyknDXKXO/rYUn 0Gdd5EddMC6+IWMVodiU8nZ700j3i8JHvDW1g7pWwVIlp6WvsGgLlnDUt8NxtXeN+d9A ZZ1ER0HZ8rRwQAMGU0qK3mN0S0Ow3LzJzjiHOTNrGQz0WGTpm9AjCIfvcucwm8H5xUVw iNMw== X-Gm-Message-State: AA6/9RnDXHRmFHrC8FNyUhiSgSs8I9cWpI1gmeJKZxS2EUOHG+mtD8BTqiEPYCKcogk0V88NiF+u+pElEoHnQw== X-Received: by 10.157.27.232 with SMTP id v37mr12108596otv.119.1475835789075; Fri, 07 Oct 2016 03:23:09 -0700 (PDT) MIME-Version: 1.0 Received: by 10.202.221.3 with HTTP; Fri, 7 Oct 2016 03:23:08 -0700 (PDT) In-Reply-To: <1475771231-1739-1-git-send-email-lgustavo@codesourcery.com> References: <1475771231-1739-1-git-send-email-lgustavo@codesourcery.com> From: Yao Qi Date: Fri, 07 Oct 2016 10:23:00 -0000 Message-ID: Subject: Re: [PATCH] Fixup gdb.python/py-value.exp for bare-metal aarch64-elf To: Luis Machado Cc: "gdb-patches@sourceware.org" Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2016-10/txt/msg00159.txt.bz2 On Thu, Oct 6, 2016 at 5:27 PM, Luis Machado wr= ote: > 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 tr= ying > 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, whic= h 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 =3D 0; > struct str *xstr; > > + /* Prevent gcc from optimizing argv[] out. */ > + if (argv !=3D NULL) > + cp =3D argv[0]; > + As I mentioned above, argv shouldn't be NULL. Is it your C runtime problem? > s.a =3D 3; > s.b =3D 5; > u.a =3D 7; > diff --git a/gdb/testsuite/gdb.python/py-value.exp b/gdb/testsuite/gdb.py= thon/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" " =3D 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" " =3D $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=3D2" > - 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. --=20 Yao (=E9=BD=90=E5=B0=A7)