From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 47684 invoked by alias); 4 Nov 2016 17:53:44 -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 47649 invoked by uid 89); 4 Nov 2016 17:53:43 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-4.1 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_NONE,RP_MATCHES_RCVD,SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mail-wm0-f47.google.com Received: from mail-wm0-f47.google.com (HELO mail-wm0-f47.google.com) (74.125.82.47) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 04 Nov 2016 17:53:33 +0000 Received: by mail-wm0-f47.google.com with SMTP id p190so66417381wmp.1 for ; Fri, 04 Nov 2016 10:53:32 -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; bh=Fz/9qwLT83JuInnyCGwxbra6CKwF1wVmdQ4dwgkR8yA=; b=UlPGjIwxZvFt0hz7WgJD/l3/ijN8+u2Dngsk4k5durVAVGnpk7dQONGaJ9IRHKdee9 UyJgy8lFzj8ClugU7awVI3pARGQ+3xGCERn8iuQQUuYre4xd2R4cptLavbSpS/r1WuL7 8dnp3SjnsAnaLjaDP3bFYJDYaTDB00JnEKILL7X5VEUhvbXnC+vL4bZwddRl2Ca+c859 BEgj3ELIyrtZ6KMFaN4Hr4XJXW0lQNBQkGtXo/pXvALJvNyiV7pOOUg2NjpxmvuB9b2U 6h7xx9NrjaZXIu6c+fPQ8ZGixtan+iKA+xIdZ5pnRq+U7hLUYZtXJwxfgGGutMkNatqA Gl/g== X-Gm-Message-State: ABUngvfGj8OsOjI20RJULt5oc1bnk4P1OWHc+po1ufdFSJEpWys8rDj2PgPadAnpXmEdoDIbiRJNakD7qDa1ilS+ X-Received: by 10.194.15.37 with SMTP id u5mr12656327wjc.117.1478282010577; Fri, 04 Nov 2016 10:53:30 -0700 (PDT) MIME-Version: 1.0 Received: by 10.28.152.70 with HTTP; Fri, 4 Nov 2016 10:52:49 -0700 (PDT) In-Reply-To: References: <94eb2c1121f421b06405406923b6@google.com> From: Doug Evans Date: Fri, 04 Nov 2016 17:53:00 -0000 Message-ID: Subject: Re: [PATCH, doc RFA] Fix lazy string type docs To: gdb-patches , Eli Zaretskii , Phil Muldoon Content-Type: text/plain; charset=UTF-8 X-IsSubscribed: yes X-SW-Source: 2016-11/txt/msg00100.txt.bz2 On Thu, Nov 3, 2016 at 5:00 PM, Doug Evans wrote: > On Thu, Nov 3, 2016 at 12:03 PM, Doug Evans wrote: >> On Thu, Nov 3, 2016 at 10:46 AM, Doug Evans wrote: >>> Hi. >>> >>> I was trying to understand a problem I was having with python lazy strings. >>> It turns out the docs are wrong, and the "type" attribute of a lazy >>> string is the character type, not a pointer to the character's type. >> >> ... Unless the thing we're making a lazy string out of is an array >> instead of a pointer. >> >> const char* foo = "Dave"; >> const char bar[] = "No, man, I'm Dave."; >> >> (gdb) py print gdb.parse_and_eval("foo").lazy_string().type >> const char >> (gdb) py print gdb.parse_and_eval("bar").lazy_string().type >> const char [19] >> >> I don't have a strong opinion on what the correct answer is, but there >> is certainly a bug here. >> >> Phil, do you remember why this code exists in valpy_lazy_string(): >> >> if (TYPE_CODE (value_type (value)) == TYPE_CODE_PTR) >> value = value_ind (value); >> >> [lazy string support went in in commit be759fcf] >> >> I kinda like the type of the lazy string version of a char array still >> being a char array. >> >> OTOH, when I look at the definition of a lazy string, recording the >> character type makes more sense >> (to me anyway: e.g., length would be redundant for the case where type >> is a char array) >> >> typedef struct { >> PyObject_HEAD >> /* Holds the address of the lazy string. */ >> CORE_ADDR address; >> >> /* Holds the encoding that will be applied to the string >> when the string is printed by GDB. If the encoding is set >> to None then GDB will select the most appropriate >> encoding when the sting is printed. */ >> char *encoding; >> >> /* Holds the length of the string in characters. If the >> length is -1, then the string will be fetched and encoded up to >> the first null of appropriate width. */ >> long length; >> >> /* This attribute holds the type that is represented by the lazy >> string's type. */ >> struct type *type; >> } lazy_string_object; >> >> To fix this bug we're going to have to break one or the other, or add >> a knob (bleah) to control the old, broken, behaviour. >> Or mark the "type" LazyString attribute as broken/deprecated and >> provide a new attribute with correct behaviour. > > Ok, an audit of all internal uses that I can find of lazy_string->type > use the type as an element of the string. > > E.g.: py-pretty-print.c:print_children has: > > gdbpy_extract_lazy_string (py_v, &addr, &type, &length, &encoding); > > local_opts.addressprint = 0; > val_print_string (type, encoding, addr, (int) length, stream, > &local_opts); > > val_print_string takes a string element type as the first arg. > > I think the right thing to do here is go with the posted patch (after > doc is ok). > And I'll post another patch to fix char[] handling (I'll file a pr for > that too, assuming my search fu can't find an existing one). > Plus I have another patch after that to fix another aspect of lazy > string handling. As more data, I found the following test in py-value.exp: gdb_test "python print (lstr.type)" "const char \*." "Test lazy-string type name equality" gdb_test "python print (sptr.type)" "const char \*." "Test string type name equality" The test is, seemingly, trying to ensure the type of the lazy string is the same as the type of the string. Not unreasonable to be sure. Alas that's not what's going on: python print (lstr.type)^M const char^M (gdb) PASS: gdb.python/py-value.exp: Test lazy-string type name equality python print (sptr.type)^M const char *^M (gdb) PASS: gdb.python/py-value.exp: Test string type name equality I'm now thinking of fixing LazyString.type for char* strings so that the above "python print (lstr.type)" prints "const char *" instead of "const char". Yeah, I think it's going to break someone, but I don't want to leave things as they are and it does feel better from a u/i perspective (even with its warts). If people want me taking a different route, please speak up. btw, I found the following four lazy string bugs. I'll try to address them all as well, if time allows. https://sourceware.org/bugzilla/show_bug.cgi?id=18779 https://sourceware.org/bugzilla/show_bug.cgi?id=18439 https://sourceware.org/bugzilla/show_bug.cgi?id=17728 https://sourceware.org/bugzilla/show_bug.cgi?id=16020