From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 14322 invoked by alias); 2 Dec 2013 23:15:08 -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 14297 invoked by uid 89); 2 Dec 2013 23:15:07 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.9 required=5.0 tests=AWL,BAYES_50,RDNS_NONE,SPF_PASS,URIBL_BLOCKED autolearn=no version=3.3.2 X-HELO: mail-oa0-f74.google.com Received: from Unknown (HELO mail-oa0-f74.google.com) (209.85.219.74) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Mon, 02 Dec 2013 23:15:05 +0000 Received: by mail-oa0-f74.google.com with SMTP id o6so2671541oag.1 for ; Mon, 02 Dec 2013 15:14:58 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:subject:date:message-id:mime-version :content-type; bh=BEkOhcNoIDXzc4WCKoZcpCIIDf8JMLM5iwsL7EYosOY=; b=CmYTDQWM8KtneqojppJAjt4wzbGnr76liW28nZN5CxZ75BiVXhyOW22dHPBc44cidi m7OSlx+b7VzNxEb7rookvNFAW3jesB5dWHoaAoJFG8MuFqIr60jcBcxaQ2qf9ak4l7fn vssYXelHdhcHNcfxG9jOE7F6imOWU6SrJO6I8XCFedJcnd15TErqw1JuppdIdratCOmr mN3EToHkzyh41hI3aATsMxOI/t6gEMXKSqlayQws4CjBrSfYlSziNYG3edDM6S/EFs4s q+dQxqbQqhWzjR/NOzmMgTTIEUh/mWq5gtOzwZ6hlQteYJXPOP6h2E95uAaK9f6SlRnH sgog== X-Gm-Message-State: ALoCoQnFc+8VXjp28W3t5pUfYBeZ1TewxAYzThKcaagjPdGCSyGKKpby1gypYA+vVIGtPOpffCYJ0YY38VmWuRAsIa+Yi5aS1aBqnkHwuj3zeLW5P78o3ks8XUt4GW90nsP7Cr/0xUS/Wg/3TwJP5Kmj2aLmki65Hm7/AAO5ksCrAHB3VVLVmHO5TBKVERQFxjKl0KkA6SfLgshsJd/t62kgykZm0WxD7g== X-Received: by 10.42.131.129 with SMTP id z1mr6093034ics.25.1386026098144; Mon, 02 Dec 2013 15:14:58 -0800 (PST) Received: from corp2gmr1-2.hot.corp.google.com (corp2gmr1-2.hot.corp.google.com [172.24.189.93]) by gmr-mx.google.com with ESMTPS id d9si134457yhl.2.2013.12.02.15.14.58 for (version=TLSv1.1 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Mon, 02 Dec 2013 15:14:58 -0800 (PST) Received: from ruffy.mtv.corp.google.com (ruffy.mtv.corp.google.com [172.17.128.44]) by corp2gmr1-2.hot.corp.google.com (Postfix) with ESMTP id 702FB5A4202; Mon, 2 Dec 2013 15:14:57 -0800 (PST) From: Doug Evans To: gdb-patches@sourceware.org, brobecker@adacore.com, saugustine@google.com Subject: [PATCH] PR 16286: Reading python value as string beyond declared size Date: Mon, 02 Dec 2013 23:15:00 -0000 Message-ID: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-IsSubscribed: yes X-SW-Source: 2013-12/txt/msg00052.txt.bz2 Hi. The patch for 16196 inadvertently broke 16286. https://sourceware.org/bugzilla/show_bug.cgi?id=16196 https://sourceware.org/bugzilla/show_bug.cgi?id=16286 I think the fix to 16196 is correct: Why ignore fetchlimit if length > 0? However, it's not the behaviour c_get_string is expecting. This patch fixes things by removing fetchlimit when length > 0 when calling read_string in c_get_string. One could want c_get_string to, say, only ignore the declared size if it's 1, since that's how the variable-length-array-at-end-of-struct idiom is commonly used. I don't have a strong opinion, I just went for preserving the previous behaviour. c_get_string is the LA_GET_STRING method, which is only called from py-value.c, so I think this patch is reasonable and safe. We can augment and/or add a new method should the need ever arise. I will check this in if there are no objections. I think this is a blocker for 7.7. 2013-12-02 Doug Evans PR 16286 * c-lang.c (c_get_string): Ignore the declared size of the object if a specific length is requested. testsuite/ * gdb.python/py-value.c: #include stdlib.h, string.h. (str): New struct. (main): New local xstr. * gdb.python/py-value.exp (test_value_in_inferior): Add test to fetch a value as a string with a length beyond the declared length of the array. diff --git a/gdb/c-lang.c b/gdb/c-lang.c index eecc76d..8d25893 100644 --- a/gdb/c-lang.c +++ b/gdb/c-lang.c @@ -227,9 +227,13 @@ c_printstr (struct ui_file *stream, struct type *type, until a null character of the appropriate width is found, otherwise the string is read to the length of characters specified. The size of a character is determined by the length of the target type of - the pointer or array. If VALUE is an array with a known length, - the function will not read past the end of the array. On - completion, *LENGTH will be set to the size of the string read in + the pointer or array. + + If VALUE is an array with a known length, and *LENGTH is -1, + the function will not read past the end of the array. However, any + declared size of the array is ignored if *LENGTH > 0. + + On completion, *LENGTH will be set to the size of the string read in characters. (If a length of -1 is specified, the length returned will not include the null character). CHARSET is always set to the target charset. */ @@ -309,6 +313,21 @@ c_get_string (struct value *value, gdb_byte **buffer, { CORE_ADDR 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: + The caller may want to fetch 100 bytes from a variable length array + implemented using the common idiom of having an array of length 1 at + the end of a struct. In this case we want to ignore the declared + size of the array. However, it's counterintuitive to implement that + behaviour in read_string: what does fetchlimit otherwise mean if + length > 0. Therefore we implement the behaviour we want here: + If *length > 0, don't specify a fetchlimit. This preserves the + previous behaviour. We could move this check above where we know + whether the array is declared with a fixed size, but we only want + to apply this behaviour when calling read_string. PR 16286. */ + if (*length > 0) + fetchlimit = UINT_MAX; + err = read_string (addr, *length, width, fetchlimit, byte_order, buffer, length); if (err) diff --git a/gdb/testsuite/gdb.python/py-value.c b/gdb/testsuite/gdb.python/py-value.c index 0c94e64..4f8c27b 100644 --- a/gdb/testsuite/gdb.python/py-value.c +++ b/gdb/testsuite/gdb.python/py-value.c @@ -16,6 +16,8 @@ along with this program. If not, see . */ #include +#include +#include struct s { @@ -39,6 +41,13 @@ typedef struct s *PTR; enum e evalue = TWO; +struct str +{ + int length; + /* Variable length. */ + char text[1]; +}; + #ifdef __cplusplus struct Base { @@ -86,6 +95,8 @@ main (int argc, char *argv[]) int i = 2; int *ptr_i = &i; const char *sn = 0; + struct str *xstr; + s.a = 3; s.b = 5; u.a = 7; @@ -96,6 +107,12 @@ main (int argc, char *argv[]) ptr_ref(ptr_i); #endif +#define STR_LENGTH 100 + xstr = (struct str *) malloc (sizeof (*xstr) + STR_LENGTH); + xstr->length = STR_LENGTH; + memset (xstr->text, 'x', STR_LENGTH); +#undef STR_LENGTH + save_argv = argv; /* break to inspect struct and union */ return 0; } diff --git a/gdb/testsuite/gdb.python/py-value.exp b/gdb/testsuite/gdb.python/py-value.exp index 43de063..a052104 100644 --- a/gdb/testsuite/gdb.python/py-value.exp +++ b/gdb/testsuite/gdb.python/py-value.exp @@ -291,6 +291,12 @@ proc test_value_in_inferior {} { # For the purposes of this test, use repr() gdb_py_test_silent_cmd "python nullst = nullst.string (length = 9)" "get string beyond null" 1 gdb_test "python print (repr(nullst))" "u?'divide\\\\x00et'" + + # Test fetching a string longer than its declared (in C) size. + # PR 16286 + 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" } proc test_lazy_strings {} {