From patchwork Mon Apr 29 21:02:11 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tom Tromey X-Patchwork-Id: 32466 Received: (qmail 41814 invoked by alias); 29 Apr 2019 21:02:16 -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 41797 invoked by uid 89); 29 Apr 2019 21:02:16 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-22.0 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=xstr, hack, Pedro, pedro 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; Mon, 29 Apr 2019 21:02:14 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id DB8B611717C; Mon, 29 Apr 2019 17:02:12 -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 6ZwhCNUILH11; Mon, 29 Apr 2019 17:02:12 -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 62E18116573; Mon, 29 Apr 2019 17:02:12 -0400 (EDT) From: Tom Tromey To: Pedro Alves Cc: Tom Tromey , 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> <877ebh7sve.fsf@tromey.com> <458ac0e4-8ed7-60e5-0af9-e2af6312283b@redhat.com> <87y33w679y.fsf@tromey.com> <93c0526d-396b-5ca6-ee8a-45a39be78e7b@redhat.com> Date: Mon, 29 Apr 2019 15:02:11 -0600 In-Reply-To: <93c0526d-396b-5ca6-ee8a-45a39be78e7b@redhat.com> (Pedro Alves's message of "Fri, 26 Apr 2019 20:44:54 +0100") Message-ID: <87bm0o4jak.fsf@tromey.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) MIME-Version: 1.0 >>>>> "Pedro" == Pedro Alves writes: Pedro> Try this: Pedro> (gdb) python print (gdb.parse_and_eval("\"ab\"").string (length = 10)) Pedro> This will go to the "in GDB's memory" branch, but it will Pedro> read beyond the value's contents buffer. Ouch. I've fixed this in the new version. Pedro> Note: I found this comment confusing. [...] I rewrote it. Pedro> value_coerce_array, if it were reached, has this: Pedro> if (VALUE_LVAL (arg1) != lval_memory) Pedro> error (_("Attempt to take address of value not located in memory.")); Pedro> So shouldn't we have such a check here too? Yes, I've added it. Here's the new patch, which I think addresses all the issues. Let me know what you think. Tom commit 2d41e1a3d8acbad856cdfda054f61b127ba97a65 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-29 Tom Tromey * c-lang.c (c_get_string): Handle non-C-style arrays. gdb/testsuite/ChangeLog 2019-04-29 Tom Tromey * gdb.python/py-value.exp (test_value_in_inferior): Add Ada test. diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 4f595922f6a..1b717110ed1 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,7 @@ +2019-04-29 Tom Tromey + + * c-lang.c (c_get_string): Handle non-C-style arrays. + 2019-04-27 Philippe Waroquiers Support style in 'frame|thread apply' diff --git a/gdb/c-lang.c b/gdb/c-lang.c index 33506f1d1ed..d25c7c6f296 100644 --- a/gdb/c-lang.c +++ b/gdb/c-lang.c @@ -279,10 +279,21 @@ 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 if either no length was + specified, or the length is within the existing bounds. This + avoids running off the end of the value's contents. */ if ((VALUE_LVAL (value) == not_lval - || VALUE_LVAL (value) == lval_internalvar) - && fetchlimit != UINT_MAX) + || VALUE_LVAL (value) == lval_internalvar + || TYPE_CODE (type) == TYPE_CODE_ARRAY) + && fetchlimit != UINT_MAX + && (*length < 0 || *length <= fetchlimit)) { int i; const gdb_byte *contents = value_contents (value); @@ -306,7 +317,19 @@ c_get_string (struct value *value, gdb::unique_xmalloc_ptr *buffer, } else { - CORE_ADDR addr = value_as_address (value); + /* value_as_address does not return an address for an array when + c_style_arrays is false, so we handle that specially + here. */ + CORE_ADDR addr; + if (TYPE_CODE (type) == TYPE_CODE_ARRAY) + { + if (VALUE_LVAL (value) != lval_memory) + error (_("Attempt to take address of value " + "not located in memory.")); + 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 b6c3618a9c6..0116d26ea90 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,7 @@ +2019-04-29 Tom Tromey + + * gdb.python/py-value.exp (test_value_in_inferior): Add Ada test. + 2019-04-29 Tom de Vries * lib/opencl.exp (skip_opencl_tests): Add missing "with" in regexp. diff --git a/gdb/testsuite/gdb.python/py-value.exp b/gdb/testsuite/gdb.python/py-value.exp index b3d90b52272..51edfa30958 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\".*" @@ -330,6 +337,12 @@ proc test_value_in_inferior {} { gdb_py_test_silent_cmd "python xstr = gdb.parse_and_eval('xstr')" "get xstr" 1 gdb_test "python print(xstr\['text'\].string (length = xstr\['length'\]))" "x{100}" \ "read string beyond declared size" + + # However it shouldn't be possible to fetch past the end of a + # non-memory value. + gdb_py_test_silent_cmd "python str = '\"str\"'" "set up str variable" 1 + gdb_test "python print (gdb.parse_and_eval (str).string (length = 10))" \ + "gdb.error: Attempt to take address of value not located in memory.\r\nError while executing Python code." } proc test_inferior_function_call {} {