From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 22585 invoked by alias); 13 Dec 2012 12:05:46 -0000 Received: (qmail 22574 invoked by uid 22791); 13 Dec 2012 12:05:45 -0000 X-SWARE-Spam-Status: No, hits=-1.8 required=5.0 tests=AWL,BAYES_00,RCVD_IN_HOSTKARMA_NO,TW_BJ,TW_FN,TW_YM X-Spam-Check-By: sourceware.org Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 13 Dec 2012 12:05:42 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id 712F52E23B; Thu, 13 Dec 2012 07:05:41 -0500 (EST) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id SDPSEUUtlobj; Thu, 13 Dec 2012 07:05:41 -0500 (EST) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id E6C122E237; Thu, 13 Dec 2012 07:05:40 -0500 (EST) Received: by joel.gnat.com (Postfix, from userid 1000) id 66DF9C3734; Thu, 13 Dec 2012 16:05:28 +0400 (RET) Date: Thu, 13 Dec 2012 12:05:00 -0000 From: Joel Brobecker To: Yao Qi Cc: Pedro Alves , gdb-patches@sourceware.org Subject: crash/regression with ia64 targets (was: "Re: [PATCH] use gdbarch_addr_bits_remove for entry point address") Message-ID: <20121213120528.GA19986@adacore.com> References: <1353404184-22073-1-git-send-email-yao@codesourcery.com> <50AFD573.1090601@gmail.com> <50B0ABF9.1080606@codesourcery.com> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="vkogqOf2sHV7VnPd" Content-Disposition: inline In-Reply-To: <50B0ABF9.1080606@codesourcery.com> User-Agent: Mutt/1.5.21 (2010-09-15) 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: 2012-12/txt/msg00440.txt.bz2 --vkogqOf2sHV7VnPd Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-length: 3203 > 2012-11-24 Daniel Jacobowitz > Kazu Hirata > Yao Qi > > * objfiles.c (entry_point_address_query): Move some code ... > (init_entry_point_info): ... here. > * solib-svr4.c (exec_entry_point): Likewise. > * symfile.c (generic_load): Call gdbarch_addr_bits_remove on > the entry address. Unfortunately, this patch breaks GDB on ia64 (linux and hpux) :-(. Take any program, and try loading it in GDB: (gdb) file hello Reading symbols from /[...]/hello... zsh: 9462 segmentation fault /[...]/gdb What happens is that we've now added a call to gdbarch_convert_from_func_ptr_addr inside init_entry_point_info, which is called from syms_from_objfile *before* the objfile's section_offsets array is allocated. On ia64, gdbarch_convert_from_func_ptr_addr resolves to ia64_convert_from_func_ptr_addr, which calls find_pc_section. This, in turns calls update_section_map, which does a sort on all sections, where qsort_cmp uses the obj_section_addr macro: #define obj_section_addr(s) \ (bfd_get_section_vma ((s)->objfile->obfd, s->the_bfd_section) \ + obj_section_offset (s)) ... and obj_section_offset is defined as: #define obj_section_offset(s) \ (((s)->objfile->section_offsets)->offsets[(s)->the_bfd_section->index]) ^^^^^^^^^^^^^^^^^^ Basically, to resolve whether our pointer is pointing to a descriptor or not, we need to find its associated section. But to find the section, we need the section_offsets to be defined. So, to me, it looks like the attempt at resolving the entry point is performed too early. And to make things even more fun, there are cases where we do not allocated section_offsets at all: if (objfile->sf == NULL) return; /* No symbols. */ But the other worrisome element is that most calls to init_entry_point_info are made from routines used as the "sym_init" struct sym_fns hook, and this hook is in fact called, in syms_from_objfile, before the section_offsets table is allocated. Since syms_from_objfile is calling init_entry_point_info, it seems to me that the call to init_entry_point_info in the "sym_init" hooks are redundant, and could be removed, clearing one hurdle. The other hurdle is making sure that init_entry_point_info is called *after* the section offsets have been allocated. Which means we need to make sure that we always allocate some, including in the case where no symbols are found. This must also become a documented invariant. Attached is a prototype that seems to work on ia64-linux. I've only tested it against our testsuite for now, but it will need to be tested with the official testsuite on GNU/Linux, as well as on Darwin, AiX, and maybe Windows (although, I think the changes removing the calls to init_entry_point_info should be fine). Note that there is a second call to init_entry_point_info, this time inside reread_symbols, but this one should be fine. This patch also begs the question whether we might want to move init_entry_point_info to objfiles.c and make it static. Thoughts? -- Joel --vkogqOf2sHV7VnPd Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="ia64-entry-point.diff" Content-length: 3227 diff --git a/gdb/coffread.c b/gdb/coffread.c index 3789995..186c8d2 100644 --- a/gdb/coffread.c +++ b/gdb/coffread.c @@ -468,8 +468,6 @@ coff_symfile_init (struct objfile *objfile) find this causes a significant slowdown in gdb then we could set it in the debug symbol readers only when necessary. */ objfile->flags |= OBJF_REORDERED; - - init_entry_point_info (objfile); } /* This function is called for every section; it finds the outer diff --git a/gdb/machoread.c b/gdb/machoread.c index 6a6eaa1..c0e6d90 100644 --- a/gdb/machoread.c +++ b/gdb/machoread.c @@ -79,7 +79,6 @@ static void macho_symfile_init (struct objfile *objfile) { objfile->flags |= OBJF_REORDERED; - init_entry_point_info (objfile); } /* Add a new OSO to the vector of OSO to load. */ diff --git a/gdb/symfile.c b/gdb/symfile.c index bc4f40a..f182617 100644 --- a/gdb/symfile.c +++ b/gdb/symfile.c @@ -929,8 +929,8 @@ read_symbols (struct objfile *objfile, int add_flags) an extra symbol file such as dynamically loaded code, and wether breakpoint reset should be deferred. */ -void -syms_from_objfile (struct objfile *objfile, +static void +syms_from_objfile_1 (struct objfile *objfile, struct section_addr_info *addrs, struct section_offsets *offsets, int num_offsets, @@ -943,11 +943,19 @@ syms_from_objfile (struct objfile *objfile, gdb_assert (! (addrs && offsets)); clear_ada_sym_cache (); - init_entry_point_info (objfile); objfile->sf = find_sym_fns (objfile->obfd); if (objfile->sf == NULL) - return; /* No symbols. */ + { + int num_sections = bfd_count_sections (objfile->obfd); + size_t size = SIZEOF_N_SECTION_OFFSETS (num_offsets); + + objfile->num_sections = num_sections; + objfile->section_offsets + = obstack_alloc (&objfile->objfile_obstack, size); + memset (objfile->section_offsets, 0, size); + return; /* No symbols. */ + } /* Make sure that partially constructed symbol tables will be cleaned up if an error occurs during symbol reading. */ @@ -1028,6 +1036,17 @@ syms_from_objfile (struct objfile *objfile, xfree (local_addr); } +void +syms_from_objfile (struct objfile *objfile, + struct section_addr_info *addrs, + struct section_offsets *offsets, + int num_offsets, + int add_flags) +{ + syms_from_objfile_1 (objfile, addrs, offsets, num_offsets, add_flags); + init_entry_point_info (objfile); +} + /* Perform required actions after either reading in the initial symbols for a new objfile, or mapping in the symbols from a reusable objfile. ADD_FLAGS is a bitmask of enum symfile_add_flags. */ diff --git a/gdb/xcoffread.c b/gdb/xcoffread.c index 2e562ed..e340b39 100644 --- a/gdb/xcoffread.c +++ b/gdb/xcoffread.c @@ -1927,8 +1927,6 @@ xcoff_symfile_init (struct objfile *objfile) find this causes a significant slowdown in gdb then we could set it in the debug symbol readers only when necessary. */ objfile->flags |= OBJF_REORDERED; - - init_entry_point_info (objfile); } /* Perform any local cleanups required when we are done with a particular --vkogqOf2sHV7VnPd--