From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 13763 invoked by alias); 18 Jan 2016 12:27:25 -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 13739 invoked by uid 89); 18 Jan 2016 12:27:25 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=0.8 required=5.0 tests=BAYES_50,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=auxv, gross, FreeBSDs, FreeBSD's X-Spam-User: qpsmtpd, 2 recipients 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, 18 Jan 2016 12:27:24 +0000 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) by mx1.redhat.com (Postfix) with ESMTPS id 12F618CF4D; Mon, 18 Jan 2016 12:27:23 +0000 (UTC) Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u0ICRLeX021245; Mon, 18 Jan 2016 07:27:22 -0500 Message-ID: <569CDA29.3050000@redhat.com> Date: Mon, 18 Jan 2016 12:27:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 MIME-Version: 1.0 To: John Baldwin CC: gdb-patches@sourceware.org, binutils@sourceware.org Subject: Re: [PATCH v2 3/6] Display per-thread information for threads in FreeBSD cores. References: <1452721551-657-1-git-send-email-jhb@FreeBSD.org> <1452721551-657-4-git-send-email-jhb@FreeBSD.org> <5697B8D6.9010301@redhat.com> <2320320.x23qbZCVbY@ralph.baldwin.cx> In-Reply-To: <2320320.x23qbZCVbY@ralph.baldwin.cx> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-SW-Source: 2016-01/txt/msg00372.txt.bz2 On 01/15/2016 08:13 PM, John Baldwin wrote: > On Thursday, January 14, 2016 03:03:50 PM Pedro Alves wrote: >> On 01/13/2016 09:45 PM, John Baldwin wrote: >>> >>> >>> +/* This is how we want PTIDs from core files to be printed. */ >>> + >>> +static char * >>> +fbsd_core_pid_to_str (struct gdbarch *gdbarch, ptid_t ptid) >>> +{ >>> + static char buf[80], name[64]; >> >> This static "name" buffer appears unused, and then masked >> by the "name" pointer below. > > Yes, it was from an earlier rev of the patch before I switched to > alloca(). Removed. >> >>> + struct bfd_section *section; >>> + bfd_size_type size; >>> + char sectionstr[32]; >>> + >>> + if (ptid_get_lwp (ptid) != 0) >>> + { >>> + snprintf (sectionstr, sizeof sectionstr, ".thrmisc/%ld", >>> + ptid_get_lwp (ptid)); >> >> xsnprintf. > > Fixed here and throughout. > >>> + section = bfd_get_section_by_name (core_bfd, sectionstr); >>> + if (section != NULL) >>> + { >>> + char *name; >>> + >>> + size = bfd_section_size (core_bfd, section); >>> + name = alloca (size + 1); >>> + if (bfd_get_section_contents (core_bfd, section, name, (file_ptr) 0, >>> + size) && name[0] != '\0') >> >> This indentation / line break reads unusual to me. I think breaking >> before the && would be clearer: >> >> if (bfd_get_section_contents (core_bfd, section, name, >> (file_ptr) 0, size) >> && name[0] != '\0') >> >> Guess this should check size > 0 as well, otherwise name[0] contains >> garbage. > > Ok. I decided to check the size in the earlier if statement to avoid > the alloca(), etc. when the size is zero. > >>> + { >>> + name[size] = '\0'; >>> + if (strcmp(name, elf_tdata (core_bfd)->core->program) != 0) >> >> Missing space after strcmp. > > Fixed. > >> Is this ".thrmisc" section documented somewhere? Could you add a >> small comment on what this entry you're skipping means? > > It is not documented aside from the code unfortunately. :( If I had my > way we would be dumping the full ptrace_lwpinfo structure that contains > per-LWP info used by the 'nat' target instead (it has other potentially > useful information such as the siginfo_t for each thread). At the moment > I don't generate a NT_THRMISC note in 'gcore' because 'struct thrmisc' is > currently only present in FreeBSD's headers. I think just having a comment mentioning that this note contains the "struct thrmisc" structure, which contains the thread name, would be helpful. But, if this is about the thread name, is there a reason you're hacking it through fbsd_core_pid_to_str, rather than hooking this up through "thread name" ? > > A somewhat related question: would it be reasonable to dump additional > notes in 'gcore' only when native? That's OK, as long as you go through the target vector, through some target method or by reading some target object. If there was a FreeBSD gdbserver port I'd push you into defining whatever packets are missing to make it work in gdbserver too, though... > For example, FreeBSD kernels dump > several additional notes including things like auxv, open files, etc. See the TARGET_OBJECT_AUXV dumping in linux-tdep.c. When native, that goes to linux-nat.c. When cross/remote, it goes through remote.c/qXfer:auxv:read. > The "gross" way I can think of for doing that is to simply place that > additional logic under #ifdef __FreeBSD__ in fbsd-tdep.c, but that seems > quite hackish given the otherwise clean split between tdep.c and nat.c. Yeah, I'd rather not. Best go through the target vector instead. Thanks, Pedro Alves