From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 31187 invoked by alias); 16 Nov 2006 23:40:09 -0000 Received: (qmail 31030 invoked from network); 16 Nov 2006 23:39:56 -0000 Received: from unknown (195.23.133.213) by sourceware.org with QMTP; 16 Nov 2006 23:39:56 -0000 Received: (qmail 15065 invoked from network); 16 Nov 2006 23:39:55 -0000 Received: from unknown (HELO mailfrt10.isp.novis.pt) ([195.23.133.202]) (envelope-sender ) by mailrly03.isp.novis.pt with compressed SMTP; 16 Nov 2006 23:39:55 -0000 Received: (qmail 17842 invoked from network); 16 Nov 2006 23:39:55 -0000 Received: from unknown (HELO [192.168.0.35]) ([195.23.225.143]) (envelope-sender ) by mailfrt10.isp.novis.pt with SMTP; 16 Nov 2006 23:39:55 -0000 Message-ID: <455CF6BA.2030306@portugalmail.pt> Date: Thu, 16 Nov 2006 23:40:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.8) Gecko/20061025 Thunderbird/1.5.0.8 Mnenhy/0.7.4.0 MIME-Version: 1.0 To: gdb-patches@sourceware.org Subject: Re: Crash in write_exp_msymbol for coff targets. References: <455CCFAD.6060407@portugalmail.pt> <20061116210236.GA25020@nevyn.them.org> In-Reply-To: <20061116210236.GA25020@nevyn.them.org> Content-Type: multipart/mixed; boundary="------------070503000708030002070907" X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2006-11/txt/msg00155.txt.bz2 This is a multi-part message in MIME format. --------------070503000708030002070907 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Content-length: 1987 Daniel Jacobowitz wrote: > On Thu, Nov 16, 2006 at 08:53:01PM +0000, Pedro Alves wrote: >> Hi all, >> >> The TLS without debugging info support introduced a bug for coff based >> targets. >> While printing for example a global symbol's value I am getting a >> segfault in parse.c:write_exp_msymbol, >> at: >> if (SYMBOL_BFD_SECTION (msymbol)->flags & SEC_THREAD_LOCAL) >> >> The problem is that minimal symbols may not have a bfd section set. >> >> The attached patch fixes it, but is it correct? >> I see in coffread.c, that prim_record_minimal_symbol_and_info is always >> called with a NULL >> bfd section, whilst in elfread.c, is is not. Is this a limitation of the >> coff format? Should coffread.c >> be fixed instead? > > Honestly, I'm not quite sure. You've got the section index, so maybe > in prim_record_minimal_symbol_and_info it could fill in a NULL > bfd_section from the objfile sections table? > Like in the attached patch1.diff? Or, it isn't safe to index the objfile->sections by section index, and we have to look them up linearly? That is what patch2.diff does. In that version, I've repeated the search on coffread.c, caching the last section looked up. Only slightly tested, but I got around around 50% cache hit on a few exes. (Premature optimization?) If v1 is the way to go, do we still need both 'int section' and 'asection *bfd_section' passed in to prim_record_minimal_symbol_and_info? Both versions also fix the segfault that started this thread. Cheers, Pedro Alves --- patch v1 2006-11-16 Pedro Alves * minsyms.c (prim_record_minimal_symbol_and_info): If bfd_section is NULL, get it from the objfile sections table. --- patch v2 2006-11-16 Pedro Alves * minsyms.c (prim_record_minimal_symbol_and_info): If bfd_section is NULL, get it from the objfile sections table. * coffread.c (coff_symtab_read): Get the bfd_section from the objfile sections table, caching the result. --------------070503000708030002070907 Content-Type: text/plain; name="patch1.diff" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="patch1.diff" Content-length: 588 Index: minsyms.c =================================================================== RCS file: /cvs/src/src/gdb/minsyms.c,v retrieving revision 1.47 diff -u -p -r1.47 minsyms.c --- minsyms.c 17 Oct 2006 20:17:44 -0000 1.47 +++ minsyms.c 16 Nov 2006 23:29:53 -0000 @@ -679,6 +679,8 @@ prim_record_minimal_symbol_and_info (con SYMBOL_VALUE_ADDRESS (msymbol) = address; SYMBOL_SECTION (msymbol) = section; + if (bfd_section == NULL) + bfd_section = objfile->sections[section].the_bfd_section; SYMBOL_BFD_SECTION (msymbol) = bfd_section; MSYMBOL_TYPE (msymbol) = ms_type; --------------070503000708030002070907 Content-Type: text/plain; name="patch2.diff" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="patch2.diff" Content-length: 2169 Index: minsyms.c =================================================================== RCS file: /cvs/src/src/gdb/minsyms.c,v retrieving revision 1.47 diff -u -p -r1.47 minsyms.c --- minsyms.c 17 Oct 2006 20:17:44 -0000 1.47 +++ minsyms.c 16 Nov 2006 23:16:22 -0000 @@ -679,6 +679,18 @@ prim_record_minimal_symbol_and_info (con SYMBOL_VALUE_ADDRESS (msymbol) = address; SYMBOL_SECTION (msymbol) = section; + if (bfd_section == NULL) + { + struct obj_section *osec; + ALL_OBJFILE_OSECTIONS (objfile, osec) + { + if (osec->the_bfd_section->index == section) + { + bfd_section = osec->the_bfd_section; + break; + } + } + } SYMBOL_BFD_SECTION (msymbol) = bfd_section; MSYMBOL_TYPE (msymbol) = ms_type; Index: coffread.c =================================================================== RCS file: /cvs/src/src/gdb/coffread.c,v retrieving revision 1.63 diff -u -p -r1.63 coffread.c --- coffread.c 17 Dec 2005 22:33:59 -0000 1.63 +++ coffread.c 16 Nov 2006 23:16:25 -0000 @@ -699,6 +699,9 @@ coff_symtab_read (long symtab_offset, un long fcn_line_ptr = 0; int val; CORE_ADDR tmpaddr; + /* Avoid unnecessary lookups. */ + int cached_sec = -1; + struct bfd_section* cached_bfd_section = NULL; /* Work around a stdio bug in SunOS4.1.1 (this makes me nervous.... it's hard to know I've really worked around it. The fix should be @@ -926,9 +929,22 @@ coff_symtab_read (long symtab_offset, un if (cs->c_name[0] != '@' /* Skip tdesc symbols */ ) { struct minimal_symbol *msym; + if (cached_sec != sec) + { + struct obj_section *osec; + cached_sec = sec; + ALL_OBJFILE_OSECTIONS (objfile, osec) + { + if (osec->the_bfd_section->index == sec) + { + cached_bfd_section = osec->the_bfd_section; + break; + } + } + } msym = prim_record_minimal_symbol_and_info (cs->c_name, tmpaddr, ms_type, NULL, - sec, NULL, objfile); + sec, cached_bfd_section, objfile); if (msym) COFF_MAKE_MSYMBOL_SPECIAL (cs->c_sclass, msym); } --------------070503000708030002070907--