From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 110474 invoked by alias); 2 Sep 2015 21:23:23 -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 110463 invoked by uid 89); 2 Sep 2015 21:23:23 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=0.2 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mailsec110.isp.belgacom.be Received: from mailsec110.isp.belgacom.be (HELO mailsec110.isp.belgacom.be) (195.238.20.106) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 02 Sep 2015 21:23:20 +0000 X-Cloudmark-SP-Filtered: true X-Cloudmark-SP-Result: v=1.1 cv=eWtaKQUTUbzTvYESxgzcVvJXO5dQmjbJ/tTWSlQWV0k= c=1 sm=2 a=Lc8uJ_0_oTxs4W8ewFYA:9 a=QEXdDO2ut3YA:10 a=36Z83KbKXHdfPhZN:21 a=Phg-UcMk6wqhJoEM:21 X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: A2CPBgChZ+dV/+vP81FdgxuBPYMjujSHcgKBOjsSAQEBAQEBAYEKhCMBAQEDASMzFQ4QCAMYAgImAgI5HgYTiCYMAbYKlHsBAQEHAiAZgQmFUYR7gT0Bg00HgmmBQwEEjHOIVox2jnGLfiaCEA0PgVY8M4JNAQEB Received: from 235.207-243-81.adsl-dyn.isp.belgacom.be (HELO soleil) ([81.243.207.235]) by relay.skynet.be with ESMTP/TLS/AES128-SHA; 02 Sep 2015 23:23:18 +0200 Message-ID: <1441229015.13071.23.camel@skynet.be> Subject: Re: [PATCH] PR varobj/18564 regression in showing __thread so extern variable From: Philippe Waroquiers To: Yao Qi Cc: gdb-patches@sourceware.org Date: Wed, 02 Sep 2015 21:23:00 -0000 In-Reply-To: <86wpw9x9au.fsf@gmail.com> References: <1440934487.22248.9.camel@skynet.be> <86wpw9x9au.fsf@gmail.com> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2015-09/txt/msg00046.txt.bz2 On Wed, 2015-09-02 at 14:03 +0100, Yao Qi wrote: > Philippe Waroquiers writes: > > Hi Philippe, > I am not an expert on symbol stuff, but I can only review test case. > Some one else may review the rest of the patch later. Thanks for the review comments, find feedback below. I have handled all comments (except one for which I have a question). I will repost a V2 patch after review of the rest of the patch (i.e. the bug fixing part). > > diff --git a/gdb/testsuite/gdb.threads/tls-so_extern.c b/gdb/testsuite/gdb.threads/tls-so_extern.c > We need a copy right header. copyright header added (note that several existing files in gdb.threads have no copyright header, e.g. tls.c, tls-main.c, ...) > > > diff --git a/gdb/testsuite/gdb.threads/tls-so_extern.exp b/gdb/testsuite/gdb.threads/tls-so_extern.exp > Either 2014-2015 or 2015. Changed to 2003-2015, as I started from tls-shared.exp > > +remote_exec build "rm -f ${binfile}" > Don't need to remove binfile. I removed the remove :). Note that this line originates from tls-shared.exp. > > diff --git a/gdb/testsuite/gdb.threads/tls-so_extern_main.c b/gdb/testsuite/gdb.threads/tls-so_extern_main.c > Copy right header is needed. Copyright added. > > > +#include > > +#include > > + > > +extern __thread void *so_extern; > > + > > +static void *tls_ptr(void *p) > > The code should comply to GNU coding standard. Reformatted the code (e.g. space before (, function name starting at begin of line, ...) > > > +{ > > + so_extern = &so_extern; > > + printf("address is %p\n", &so_extern); /* break here to check result */ > > Don't have to call printf and include stdio.h. Not clear by what to replace this printf (or why it harms). I need to put a break after the assignment, as the .exp will compare so_extern value with address of so_extern. (note: I changed the .exp, so as to also check so_extern value in main thread). The printf line allows to put a break after the assignment. If it is really better to remove the printf/stdio.h, any suggestion about what to replace it with ? (we need to avoid the compiler to optimise away this code to be sure we can put a break). Thanks for the review/detailed comments, Philippe