From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 35159 invoked by alias); 2 Oct 2019 23:10:04 -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 35150 invoked by uid 89); 2 Oct 2019 23:10:04 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-10.3 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_2,SPF_HELO_PASS autolearn=ham version=3.3.1 spammy=ccp, You'd, Youd X-HELO: userp2120.oracle.com Received: from userp2120.oracle.com (HELO userp2120.oracle.com) (156.151.31.85) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 02 Oct 2019 23:10:03 +0000 Received: from pps.filterd (userp2120.oracle.com [127.0.0.1]) by userp2120.oracle.com (8.16.0.27/8.16.0.27) with SMTP id x92N8tvj144585; Wed, 2 Oct 2019 23:09:59 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=oracle.com; h=subject : to : references : from : message-id : date : mime-version : in-reply-to : content-type : content-transfer-encoding; s=corp-2019-08-05; bh=PiM6USiHiAebo+jAEYp8G7YjsRr/ZuVU/ZBThiwdRVE=; b=pdiCsGBdNd7KlmgLKBwqCAm1fGPm/gNpP/J1POOSaVfmsQD7Taj8CO+FZqqT8xtMkBmR XgUQb+jDtYcU+Zy7wfw3leRU9wJ/Qz1u+Ax6cgAvFldsgAfruQfUIdD6Exn7BjlbTe0Y Qyi5eHZOkTeBtj00w1fltVPtoRTCnOnX7ezffTR9XocthVSz0u6pbyfjPxDHphXgNaoC Q3sV1rzqYQ29E3agf+EeJRti+gqoIrz8nywoLll4JyL0Ub15z8y1+N1+ITzvSLWfnHCc YswtvTAYJABf/pD64gJajsn2Rs8FJ/IRlh4dM7Ahtv25APNXdNlJZxQH6rDMtmmwDZPc 3w== Received: from aserp3020.oracle.com (aserp3020.oracle.com [141.146.126.70]) by userp2120.oracle.com with ESMTP id 2va05s00jp-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Wed, 02 Oct 2019 23:09:58 +0000 Received: from pps.filterd (aserp3020.oracle.com [127.0.0.1]) by aserp3020.oracle.com (8.16.0.27/8.16.0.27) with SMTP id x92N3nOa144330; Wed, 2 Oct 2019 23:09:57 GMT Received: from aserv0122.oracle.com (aserv0122.oracle.com [141.146.126.236]) by aserp3020.oracle.com with ESMTP id 2vckypq8vd-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Wed, 02 Oct 2019 23:09:56 +0000 Received: from abhmp0005.oracle.com (abhmp0005.oracle.com [141.146.116.11]) by aserv0122.oracle.com (8.14.4/8.14.4) with ESMTP id x92N9qNX007777; Wed, 2 Oct 2019 23:09:53 GMT Received: from [10.159.231.222] (/10.159.231.222) by default (Oracle Beehive Gateway v4.0) with ESMTP ; Wed, 02 Oct 2019 16:09:52 -0700 Subject: Re: [PATCH v2] gdb: CTF support To: Simon Marchi , gdb-patches@sourceware.org References: <1564530195-27659-1-git-send-email-weimin.pan@oracle.com> <5377c457-52b0-583d-15b5-47024eae1f48@simark.ca> <895f47d4-3e01-4d5a-474b-43dd2dd037b4@oracle.com> <0fe82814-46b8-79c2-6a25-5f5d51b158e1@simark.ca> From: Wei-min Pan Message-ID: <1055d18f-9e5c-3344-114a-3777876c9c63@oracle.com> Date: Wed, 02 Oct 2019 23:10:00 -0000 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:60.0) Gecko/20100101 Thunderbird/60.9.0 MIME-Version: 1.0 In-Reply-To: <0fe82814-46b8-79c2-6a25-5f5d51b158e1@simark.ca> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit X-SW-Source: 2019-10/txt/msg00089.txt.bz2 On 10/1/2019 7:47 PM, Simon Marchi wrote: > On 2019-09-30 8:36 p.m., Weimin Pan wrote: >> Thank you very much for the thorough review. I have read through the >> modified patch which looks good and will be adopted. In addition, I >> have made more changes to address the issues you raised. Please see >> below and the updated patch (attached). > > Great thanks! > > I noticed at least one case of leading spaces that should be tab in the attached > patch, so maybe do: > >   grep '^        ' ctfread.c > > to catch them. Hi Simon, Thanks, I think it's now clean after fixing the one below: 1247c1247 <           sym = add_stt_func (ccp, i); --- >         sym = add_stt_func (ccp, i); > >> Yes, I've made the change in several places where the allocated copy is >> freed after >> it's being dup'ed via obstack_strdup. > > Ok thanks.  Actually, can you please use gdb::unique_xmalloc_pointer to > manage that memory?  We're trying to minimize the manual memory management > in GDB.  You'd do: > >   gdb::unique_xmalloc_ptr name (ctf_type_aname_raw (fp, tid)); > > and use `name.get ()` instead of name.  It will get free'd automatically > when exiting scope. It doesn't seem using gdb::unique_xmalloc_pointer is appropriate in these cases because we want to keep these symbol/type names around in the symbol table. > >>  > Hmm that looks fishy.  It looks like this is taking the address of a >>  > local variable for something that will be used after the current function >>  > will have returned. >> >> Sorry, but it's a "static" local variable. > > Oh ok, I did miss that. > > I think we should avoid this.  What happens, for example, in this case: > > 1. Read psymtab for objfile 1 > 2. Read psymtab for objfile 2 > 3. Expand psymtab to symtab for objfile 1 > > From what I understand, step #2 will overwrite the static variable, such that > it won't be valid anymore in step #3.  I think it would be better to allocate the > context structure dynamically and store it as the objfile, like dwarf2read does. Yes, you're right. I've made the following changes: 1322c1322 <   static ctf_context_t ccx; --- >   ctf_context_t *ccx; 1326,1328c1326,1329 <   ccx.fp = cfp; <   ccx.of = objfile; <   pst->read_symtab_private = (void *) &ccx; --- >   ccx = XOBNEW (&objfile->objfile_obstack, ctf_context_t); >   ccx->fp = cfp; >   ccx->of = objfile; >   pst->read_symtab_private = (void *) ccx; Many thanks, Weimin