From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 32643 invoked by alias); 2 Sep 2013 18:04:17 -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 32634 invoked by uid 89); 2 Sep 2013 18:04:17 -0000 Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 02 Sep 2013 18:04:17 +0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-5.8 required=5.0 tests=AWL,BAYES_00,KHOP_THREADED,RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r82I4Ctx024495 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Mon, 2 Sep 2013 14:04:12 -0400 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx01.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id r82I4AsH005723; Mon, 2 Sep 2013 14:04:11 -0400 Message-ID: <5224D319.1000704@redhat.com> Date: Mon, 02 Sep 2013 18:04:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130625 Thunderbird/17.0.7 MIME-Version: 1.0 To: Ricard Wanderlof CC: gdb-patches@sourceware.org Subject: Re: [PATCH][CRISv32] Add support for threaded debugging References: In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2013-09/txt/msg00040.txt.bz2 (fixing subject) On 08/30/2013 02:29 PM, Ricard Wanderlof wrote: > > This patch adds support for threaded debugging on the CRISv32 > architecture. It's been floating around for several years now in our local > repository so it's way overdue pushing upstream. > > Patch included inline for review and as attachement for use. > > > Suggested-by: Edgar Iglisias > Signed-off-by: Ricard Wanderlof > > > 2013-08-30 Ricard Wanderlof > > * cris-tdep.c: Fix typedef for elf_greg_t. > (cris_gdbarch_init): Add call to > set_gdbarch_fetch_tls_load_module_address. Indentation doesn't look right. Should be indented with a single tab. > Suggested-by: Edgar Iglisias (Note: surname/address don't match) > Signed-off-by: Ricard Wanderlof > > > 2013-08-30 Ricard Wanderlof > > * cris-tdep.c: Fix typedef for elf_greg_t. This seems to be unrelated to threaded debugging support. It just looks like a host-dependency fix. Please keep logically unrelated patches separate, and always send them as separate threads each with their own rationale. (Note that "fix" is not "what" changed, but "why" you changed it, so it's not the proper ChangeLog description of the change.) In this case, the fix should just go the extra mile and remove the reliance on host alignment from this type, that is representing an external structure. IOW, that typedef, if any, would better be: typedef gdb_byte cris_elf_greg_t[4]; See frv_linux_supply_gregset for example. > (cris_gdbarch_init): Add call to > set_gdbarch_fetch_tls_load_module_address. This part looks OK, though it did raise some eyebrows to have GNU/Linux-specific code in cris-tdep.c, rather than in a cris-linux-tdep.c file. It seems there's no real support for cris bare-metal debugging? > > gdbserver > > * linux-crisv32-low.c (ps_get_thread_area): New function. This part is OK, though should mention also the addition of PTRACE_GET_THREAD_AREA, and, > + *base = (void *) ((char *)*base - idx); missing space after '(char *) '. -- Pedro Alves