From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 52016 invoked by alias); 25 Jul 2016 16:00:27 -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 51923 invoked by uid 89); 25 Jul 2016 16:00:21 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.7 required=5.0 tests=BAYES_00,KAM_STOCKGEN,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=no version=3.3.2 spammy=digging, someday, exercising X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Mon, 25 Jul 2016 16:00:20 +0000 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 32AA08535A; Mon, 25 Jul 2016 16:00:19 +0000 (UTC) Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u6PG0HGR015365; Mon, 25 Jul 2016 12:00:18 -0400 Subject: Re: [RFA] PR python/20190 - compute TLS symbol without a frame To: Tom Tromey References: <1464988574-17075-1-git-send-email-tom@tromey.com> <92513300-637c-e0c8-2736-3b13c33d6f1c@redhat.com> <87eg6jm0zt.fsf@tromey.com> Cc: gdb-patches@sourceware.org From: Pedro Alves Message-ID: Date: Mon, 25 Jul 2016 16:00:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.1.1 MIME-Version: 1.0 In-Reply-To: <87eg6jm0zt.fsf@tromey.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-SW-Source: 2016-07/txt/msg00337.txt.bz2 On 07/24/2016 05:53 PM, Tom Tromey wrote: >>>>>> "Pedro" == Pedro Alves writes: > > Pedro> I think we should put a comment somewhere explaining _why_ is > Pedro> this distinction useful to have. Around here is probably a good > Pedro> place. IIUC, the reason is being able to read TLS symbols > Pedro> from within a frame unwinder, when we don't have a frame yet, > Pedro> because we're building it. > > I added this to the enum. > > Pedro> Should we write this as: > Pedro> if (nf_baton->needs < SYMBOL_NEEDS_REGISTERS) > Pedro> nf_baton-> needs = SYMBOL_NEEDS_REGISTERS; > Pedro> ? > Pedro> May make it clearer there's ordering implied? > > I don't think it matters much either way, but I went ahead and changed > it. Thanks. I should have probably explained why I thought I'd suggest it. My reasoning was that while this: if (nf_baton->needs != SYMBOL_NEEDS_FRAME) nf_baton->needs = SYMBOL_NEEDS_REGISTERS; should be read as: - only frob "nf_baton->needs" it not set to a stricter (higher) value yet and requires the reader processing that: - SYMBOL_NEEDS_FRAME is higher than SYMBOL_NEEDS_REGISTERS - SYMBOL_NEEDS_FRAME is the _only_ value that is higher than SYMBOL_NEEDS_REGISTERS (which could no longer be true someday) this: if (nf_baton->needs < SYMBOL_NEEDS_REGISTERS) nf_baton-> needs = SYMBOL_NEEDS_REGISTERS; is self-explanatory for being written in terms of a single enum value. > I completely rewrote this to: > > /* Find the "symbol_needs_kind" value for the given symbol. This > value determines whether reading the symbol needs memory (e.g., a > global variable), just registers (a thread-local), or a frame (a > local variable). */ > enum symbol_needs_kind (*get_symbol_read_needs) (struct symbol * symbol); That's great, thanks. > >>> + if {![skip_python_tests]} { >>> + gdb_test_no_output \ >>> + "python sym = gdb.lookup_symbol('a_thread_local')\[0\]" \ >>> + "${number} look up a_thread_local symbol" >>> + gdb_test "python print(sym.value())" "$expected_value" \ >>> + "${number} get symbol value without frame" > > Pedro> I'm confused on what this is testing, and on whether this is > Pedro> exercising the code changes. Is there really no frame here? > Pedro> AFAICS, this proc is always called with some thread selected, > Pedro> so there should be a frame? > > It's a bit subtle and I had to go digging again to remind myself of why > this test works. > > Basically it boils down to py-symbol.c:sympy_value: > > if (symbol_read_needs_frame (symbol) && frame_info == NULL) > error (_("symbol requires a frame to compute its value")); > > Here, frame_info comes from the caller -- and in the test we're > explicitly not passing in a frame. So, this attempt to get the symbol's > value is rejected. I see. Maybe add some small comment to the test mentioning that "symbol.value" takes an optional frame argument and we're purposely not passing one? > > However, it ought to work, because a frame isn't necessary to compute > the value. > >> With the current patch the result is nicer: >> >> (gdb) print a_thread_local >> Cannnot read `a_thread_local' without registers > > Pedro> Is this / should this be tested somewhere? > > I added a test for this. Great. Thanks, Pedro Alves