From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 25796 invoked by alias); 15 Aug 2011 20:21:14 -0000 Received: (qmail 25786 invoked by uid 22791); 15 Aug 2011 20:21:13 -0000 X-SWARE-Spam-Status: No, hits=-6.6 required=5.0 tests=AWL,BAYES_00,KAM_STOCKGEN,RCVD_IN_DNSWL_HI,RP_MATCHES_RCVD,SPF_HELO_PASS,TW_BJ X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 15 Aug 2011 20:20:54 +0000 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id p7FKKrHK004399 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Mon, 15 Aug 2011 16:20:53 -0400 Received: from ns3.rdu.redhat.com (ns3.rdu.redhat.com [10.11.255.199]) by int-mx02.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id p7FKKqM4007052; Mon, 15 Aug 2011 16:20:53 -0400 Received: from barimba (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by ns3.rdu.redhat.com (8.13.8/8.13.8) with ESMTP id p7FKKpNd027656; Mon, 15 Aug 2011 16:20:51 -0400 From: Tom Tromey To: Sanjoy Das Cc: gdb-patches@sourceware.org Subject: Re: [PATCH 4/7] Use the loaded reader. References: <1312903509-25132-1-git-send-email-sanjoy@playingwithpointers.com> <1312903509-25132-5-git-send-email-sanjoy@playingwithpointers.com> Date: Mon, 15 Aug 2011 20:21:00 -0000 In-Reply-To: <1312903509-25132-5-git-send-email-sanjoy@playingwithpointers.com> (Sanjoy Das's message of "Tue, 9 Aug 2011 20:55:06 +0530") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.0.50 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain 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 X-SW-Source: 2011-08/txt/msg00312.txt.bz2 >>>>> "Sanjoy" == Sanjoy Das writes: Sanjoy> Invoke the loaded JIT debug info reader to parse the registered symbol Sanjoy> files. Thanks. Sanjoy> +/* Remember a mapping from entry_addr to objfile. */ Sanjoy> +static void Sanjoy> +add_objfile_entry (struct objfile *objfile, CORE_ADDR entry) Sanjoy> +{ Sanjoy> + CORE_ADDR *entry_addr_ptr; Sanjoy> + Sanjoy> + entry_addr_ptr = xmalloc (sizeof (CORE_ADDR)); Sanjoy> + *entry_addr_ptr = entry; Sanjoy> + set_objfile_data (objfile, jit_objfile_data, entry_addr_ptr); Sanjoy> +} Sanjoy> +/* Returns true if the block corrensponding to old should be placed Typo, "corresponding". Sanjoy> +static struct gdb_block * Sanjoy> +jit_block_open_impl (struct gdb_symbol_callbacks *cb, struct gdb_symtab *symtab, This line is too long. Every new function should have some kind of introductory comment. There should be a blank line between the comment and the start of the function. Sanjoy> static void Sanjoy> -jit_register_code (struct gdbarch *gdbarch, Sanjoy> - CORE_ADDR entry_addr, struct jit_code_entry *code_entry) Sanjoy> +jit_symtab_line_mapping_add_impl (struct gdb_symbol_callbacks *cb, Sanjoy> + struct gdb_symtab *stab, int nlines, Sanjoy> + struct gdb_line_mapping *map) Sanjoy> +{ Sanjoy> + int i; Sanjoy> + if (!nlines) Blank line between declarations and code. For safety this check should probably be 'if (nlines < 1)'. Sanjoy> + stab->linetable = xmalloc (sizeof (struct linetable) + (nlines - 1) * Sanjoy> + sizeof (struct linetable_entry)); Break before "*", not after it. This is a GNU style thing. Personally I'd prefer a break before the "+" instead. Sanjoy> +static void Sanjoy> +jit_symtab_close_impl (struct gdb_symbol_callbacks *cb, struct gdb_symtab *stab) Line too long. Sanjoy> + int actual_nblocks = FIRST_LOCAL_BLOCK + stab->nblocks, i, blockvector_size; Line too long. Sanjoy> + int size = (stab->linetable->nitems - 1) * Sanjoy> + sizeof (struct linetable_entry) + sizeof (struct linetable); Line breaks before operators. According to GNU style you have to parenthesize the RHS here, and further indent the second line. Sanjoy> + blockvector_size = (sizeof (struct blockvector) + Sanjoy> + (actual_nblocks - 1) * sizeof (struct block *)); ... like this :-) Sanjoy> + block_name->symtab = symtab; You can use SYMBOL_SYMTAB here. Sanjoy> + block_name->ginfo.name = obstack_alloc (&objfile->objfile_obstack, 1 + Sanjoy> + strlen (gdb_block_iter->name)); I think it needs to be strlen() + 1 -- but you can just use obsavestring instead. Sanjoy> +/* Convert OBJ to a proper objfile. */ Sanjoy> +static void Sanjoy> +jit_object_close_impl (struct gdb_symbol_callbacks *cb, struct gdb_object *obj) Line too long. Sanjoy> + objfile->msymbols = obstack_alloc (&objfile->objfile_obstack, Sanjoy> + sizeof (struct minimal_symbol)); Sanjoy> + objfile->msymbols[0].ginfo.name = NULL; Sanjoy> + objfile->msymbols[0].ginfo.value.address = 0; A little surprising maybe, but I guess it is ok. Sanjoy> + objfile->name = xstrdup ("JIT"); You have to xfree the old name first. Sanjoy> static void Sanjoy> jit_unregister_code (struct objfile *objfile) Sanjoy> { Sanjoy> + xfree (objfile_data (objfile, jit_objfile_data)); Sanjoy> free_objfile (objfile); Sanjoy> } I think a better fix for this problem would be to change the initialization of jit_objfile_data to use register_objfile_data_with_cleanup. Tom