From patchwork Thu Apr 25 20:05:41 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tom Tromey X-Patchwork-Id: 32421 Received: (qmail 104607 invoked by alias); 25 Apr 2019 20:05:46 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 104599 invoked by uid 89); 25 Apr 2019 20:05:46 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-22.3 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.1 spammy=lives, 2799, fewer X-HELO: rock.gnat.com Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 25 Apr 2019 20:05:44 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id 113D51170F2; Thu, 25 Apr 2019 16:05:43 -0400 (EDT) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id GHeYXXv8s7MD; Thu, 25 Apr 2019 16:05:43 -0400 (EDT) Received: from murgatroyd (97-122-168-123.hlrn.qwest.net [97.122.168.123]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by rock.gnat.com (Postfix) with ESMTPSA id 8EDFB1170D5; Thu, 25 Apr 2019 16:05:42 -0400 (EDT) From: Tom Tromey To: Tom Tromey Cc: Pedro Alves , gdb-patches@sourceware.org Subject: Re: [PATCH] Force array coercion in c_get_string References: <20190425181740.18062-1-tromey@adacore.com> <6d0c9915-cb2d-b7c2-1336-9347c90d22e6@redhat.com> <87v9z19auk.fsf@tromey.com> Date: Thu, 25 Apr 2019 14:05:41 -0600 In-Reply-To: <87v9z19auk.fsf@tromey.com> (Tom Tromey's message of "Thu, 25 Apr 2019 12:52:03 -0600") Message-ID: <877ebh7sve.fsf@tromey.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) MIME-Version: 1.0 Pedro> So with the patch, we coerce the array to inferior memory, in order Pedro> to read its contents back from the inferior? Is that right? Pedro> Why not skip the detour and save the memory allocation in the inferior? Tom> Yeah, that's weird, isn't it. Plan B is a bit more complicated, but may be nicer because in some cases it will require fewer memory reads, and also doesn't force coercion. Let me know what you think. Tom commit 9acd65e278c569f47e73f9d455127b3379792c34 Author: Tom Tromey Date: Thu Apr 25 12:14:58 2019 -0600 Correctly handle non-C-style arrays in c_get_string A user here noticed that the Python Value.string method did not work for Ada arrays. I tracked this down to an oddity in value_as_address -- namely, it calls coerce_array, but that function will not force array coercion when the language has c_style_arrays=false, as Ada does. This patch fixes the problem by changing c_get_string so that arrays take the "in GDB's memory" branch. The actual patch is somewhat more complicated than you might think, because the caller can request more array elements than the type allows. This is normal when the type is using the C struct hack. Tested on x86-64 Fedora 29. gdb/ChangeLog 2019-04-25 Tom Tromey * c-lang.c (c_get_string): Handle non-C-style arrays. gdb/testsuite/ChangeLog 2019-04-25 Tom Tromey * gdb.python/py-value.exp (test_value_in_inferior): Add Ada test. diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 17e10e869b8..b8e018a97e5 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,7 @@ +2019-04-25 Tom Tromey + + * c-lang.c (c_get_string): Handle non-C-style arrays. + 2019-04-25 Tom Tromey PR gdb/24475: diff --git a/gdb/c-lang.c b/gdb/c-lang.c index 33506f1d1ed..0c189aae606 100644 --- a/gdb/c-lang.c +++ b/gdb/c-lang.c @@ -279,9 +279,18 @@ c_get_string (struct value *value, gdb::unique_xmalloc_ptr *buffer, /* If the string lives in GDB's memory instead of the inferior's, then we just need to copy it to BUFFER. Also, since such strings are arrays with known size, FETCHLIMIT will hold the size of the - array. */ + array. + + An array is assumed to live in GDB's memory, so we take this path + here. However, it's possible for the caller to request more + array elements than apparently exist -- this can happen when + using the C struct hack. So, only do this for an array if either + no length was specified, or the length is within the existing + bounds. */ if ((VALUE_LVAL (value) == not_lval - || VALUE_LVAL (value) == lval_internalvar) + || VALUE_LVAL (value) == lval_internalvar + || (TYPE_CODE (type) == TYPE_CODE_ARRAY + && (*length < 0 || *length <= fetchlimit))) && fetchlimit != UINT_MAX) { int i; @@ -306,7 +315,14 @@ c_get_string (struct value *value, gdb::unique_xmalloc_ptr *buffer, } else { - CORE_ADDR addr = value_as_address (value); + /* value_as_address calls coerce_array, but that won't actually + coerce the array if c_style_arrays is false. In this case, + get the address of the array directly. */ + CORE_ADDR addr; + if (TYPE_CODE (type) == TYPE_CODE_ARRAY) + addr = value_address (value); + else + addr = value_as_address (value); /* Prior to the fix for PR 16196 read_string would ignore fetchlimit if length > 0. The old "broken" behaviour is the behaviour we want: diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index c81d841e93a..7127cbdc75f 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,7 @@ +2019-04-25 Tom Tromey + + * gdb.python/py-value.exp (test_value_in_inferior): Add Ada test. + 2019-04-25 Sergio Durigan Junior PR corefiles/11608 diff --git a/gdb/testsuite/gdb.python/py-value.exp b/gdb/testsuite/gdb.python/py-value.exp index b3d90b52272..7f42d31572a 100644 --- a/gdb/testsuite/gdb.python/py-value.exp +++ b/gdb/testsuite/gdb.python/py-value.exp @@ -315,6 +315,13 @@ proc test_value_in_inferior {} { gdb_test "python print (\"---\"+st.string (length = 0)+\"---\")" "------" "test string (length = 0) is empty" gdb_test "python print (len(st.string (length = 0)))" "0" "test length is 0" + # We choose Ada here to test a language where c_style_arrays is + # false. + gdb_test "set lang ada" \ + "Warning: the current language does not match this frame." + gdb_test "python print (st.string ())" "divide et impera" \ + "Test string with no length in ada" + gdb_test_no_output "set lang auto" # Fetch a string that has embedded nulls. gdb_test "print nullst" "\"divide\\\\000et\\\\000impera\".*"