From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 2650 invoked by alias); 31 Jan 2003 20:06:48 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 2641 invoked from network); 31 Jan 2003 20:06:46 -0000 Received: from unknown (HELO zenia.red-bean.com) (66.244.67.22) by 172.16.49.205 with SMTP; 31 Jan 2003 20:06:46 -0000 Received: from zenia.red-bean.com (localhost.localdomain [127.0.0.1]) by zenia.red-bean.com (8.12.5/8.12.5) with ESMTP id h0VK1Q8A020146; Fri, 31 Jan 2003 15:01:27 -0500 Received: (from jimb@localhost) by zenia.red-bean.com (8.12.5/8.12.5/Submit) id h0VK1P4F020139; Fri, 31 Jan 2003 15:01:25 -0500 To: David Carlton Cc: gdb-patches@sources.redhat.com Subject: Re: [rfa] ALL_OBJFILE_MSYMBOLS References: From: Jim Blandy Date: Fri, 31 Jan 2003 20:06:00 -0000 In-Reply-To: Message-ID: User-Agent: Gnus/5.09 (Gnus v5.9.0) Emacs/21.2.92 MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-SW-Source: 2003-01/txt/msg00845.txt.bz2 David Carlton writes: > When playing around with anonymous objfiles on a branch, I noticed > another problem with them: the macro ALL_OBJFILE_MSYMBOLS assumes that > objfile->msymbols is always non-NULL, which isn't the case if you have > an artificial objfile floating around. > > Looking further, I noticed that the macro ALL_MSYMBOLS already has an > explicit test that objfile->msymbols is non-NULL; it seems to me that > the easiest thing to do is is to require users of ALL_OBJFILE_MSYMBOLS > to first check that objfile->msymbols is non-NULL. > > Here's a patch that does that, along with adding some comments to > objfiles.h explaining the situation. It turns out that > ALL_OBJFILE_MSYMBOLS is only used in two places; in two of those > places, it's immediately within ALL_OBJFILES, so the intended effect > is to look at all the msymbols; I thus replaced those two occurrences > by uses of ALL_MSYMBOLS. The third occurrence isn't surrounded by a > use of ALL_OBJFILES, so I just enclosed it in a test that > objfile->msymbols is non-NULL. > > Like my earlier allocate_objfile(NULL, 0) patch, I don't know of any > concrete bugs that this fixes on mainline GDB, though I could imagine > that Java code could run into this problem since Java does allocate an > anonymous objfile. But the patch is so straightforward that I don't > see how applying it could hurt. (It looks a little more complicated > than it is because it changes the indentation levels.) > > Tested on i686-pc-linux-gnu/GCC3.1/DWARF-2; I also verified that 'make > arm-linux-tdep.o' compiles without errors, though I haven't run that > code directly. (The code in question in arm-linux-tdep.c is > essentially identical to the code in i386-linux-tdep.c.) > > David Carlton > carlton@math.stanford.edu > > 2003-01-27 David Carlton > > * objfiles.h: Add comments about objfile->msymbols being NULL. > * objfiles.c (objfile_relocate): Enclose ALL_OBJFILE_MSYMBOLS in > guard. > * i386-linux-tdep.c (find_minsym_and_objfile): Call ALL_MSYMBOLS > instead of ALL_OBJFILES and ALL_OBJFILE_MSYMBOLS. > * arm-linux-tdep.c (find_minsym_and_objfile): Ditto. I think I'd prefer the patch below. Could you try it out and see if it works as well? This was messier than I had expected. My first impulse was to make ALL_OBJFILE_MSYMBOLS just do the 'if' itself. But it's a macro, and there's no way to prevent the macro from grabbing any 'else' that happens to follow the loop body. (Which is a great example of one reason C macros suck and Lisp macros don't --- in case that's an emotionally charged issue for anyone else out there like it is for me. :) ) But it would be better anyway for an empty minsym table to have a single, consistent representation. So I thought, "Hey, why don't we just forget about the terminating entry, and always use the count?" But there's no way I could see to reliably find all the code that expects it. Not every loop uses ALL_OBJFILE_MSYMBOLS. And since not every loop need necessarily refer to objfile->msymbols, you can't grep for that either. And grepping for uses of 'struct msymbol' yields hundreds of hits. So here's a patch which simply ensures that every objfile's minsym table has a terminating entry, and makes some appropriate accompanying changes. The tests are still running, but I haven't noticed any regressions yet. 2003-01-31 Jim Blandy Use a single, consistent representation for an empty minimal symbol table in an objfile. * objfiles.c (terminate_minimal_symbol_table): New function. (allocate_objfile): Call it. * objfiles.h (terminate_minimal_symbol_table): New declaration. (ALL_MSYMBOLS): No need to test whether (objfile)->msymbols is non-NULL. * minsyms.c (lookup_minimal_symbol_by_pc_section): To see whether objfile has minimal symbols, compare minimal_symbol_count to zero, instead of comparing msymbols with NULL. * objfiles.c (have_minimal_symbols): Same. * solib-sunos.c (solib_add_common_symbols): Call terminate_minimal_symbol_table. * symfile.c (reread_symbols): Same. Index: gdb/minsyms.c =================================================================== RCS file: /cvs/src/src/gdb/minsyms.c,v retrieving revision 1.23 diff -c -r1.23 minsyms.c *** gdb/minsyms.c 8 Jan 2003 18:38:47 -0000 1.23 --- gdb/minsyms.c 31 Jan 2003 19:52:29 -0000 *************** *** 411,418 **** "null symbol". If there are no real symbols, then there is no minimal symbol table at all. */ ! if ((msymbol = objfile->msymbols) != NULL) { lo = 0; hi = objfile->minimal_symbol_count - 1; --- 411,419 ---- "null symbol". If there are no real symbols, then there is no minimal symbol table at all. */ ! if (objfile->minimal_symbol_count > 0) { + msymbol = objfile->msymbols; lo = 0; hi = objfile->minimal_symbol_count - 1; Index: gdb/objfiles.c =================================================================== RCS file: /cvs/src/src/gdb/objfiles.c,v retrieving revision 1.24 diff -c -r1.24 objfiles.c *** gdb/objfiles.c 23 Jan 2003 23:03:31 -0000 1.24 --- gdb/objfiles.c 31 Jan 2003 19:52:29 -0000 *************** *** 281,286 **** --- 281,288 ---- obstack_specify_allocation (&objfile->type_obstack, 0, 0, xmalloc, xfree); flags &= ~OBJF_MAPPED; + + terminate_minimal_symbol_table (objfile); } /* Update the per-objfile information that comes from the bfd, ensuring *************** *** 333,338 **** --- 335,367 ---- return (objfile); } + + /* Create the terminating entry of OBJFILE's minimal symbol table. + If OBJFILE->msymbols is zero, allocate a single entry from + OBJFILE->symbol_obstack; otherwise, just initialize + OBJFILE->msymbols[OBJFILE->minimal_symbol_count]. */ + void + terminate_minimal_symbol_table (struct objfile *objfile) + { + if (! objfile->msymbols) + objfile->msymbols = ((struct minimal_symbol *) + obstack_alloc (&objfile->symbol_obstack, + sizeof (objfile->msymbols[0]))); + + { + struct minimal_symbol *m + = &objfile->msymbols[objfile->minimal_symbol_count]; + + memset (m, 0, sizeof (*m)); + SYMBOL_NAME (m) = NULL; + SYMBOL_VALUE_ADDRESS (m) = 0; + MSYMBOL_INFO (m) = NULL; + MSYMBOL_TYPE (m) = mst_unknown; + SYMBOL_INIT_LANGUAGE_SPECIFIC (m, language_unknown); + } + } + + /* Put one object file before a specified on in the global list. This can be used to make sure an object file is destroyed before another when using ALL_OBJFILES_SAFE to free all objfiles. */ *************** *** 810,816 **** ALL_OBJFILES (ofp) { ! if (ofp->msymbols != NULL) { return 1; } --- 839,845 ---- ALL_OBJFILES (ofp) { ! if (ofp->minimal_symbol_count > 0) { return 1; } Index: gdb/objfiles.h =================================================================== RCS file: /cvs/src/src/gdb/objfiles.h,v retrieving revision 1.18 diff -c -r1.18 objfiles.h *** gdb/objfiles.h 29 Jan 2003 23:46:39 -0000 1.18 --- gdb/objfiles.h 31 Jan 2003 19:52:30 -0000 *************** *** 515,520 **** --- 515,522 ---- extern int build_objfile_section_table (struct objfile *); + extern void terminate_minimal_symbol_table (struct objfile *objfile); + extern void put_objfile_before (struct objfile *, struct objfile *); extern void objfile_to_front (struct objfile *); *************** *** 595,602 **** #define ALL_MSYMBOLS(objfile, m) \ ALL_OBJFILES (objfile) \ ! if ((objfile)->msymbols) \ ! ALL_OBJFILE_MSYMBOLS (objfile, m) #define ALL_OBJFILE_OSECTIONS(objfile, osect) \ for (osect = objfile->sections; osect < objfile->sections_end; osect++) --- 597,603 ---- #define ALL_MSYMBOLS(objfile, m) \ ALL_OBJFILES (objfile) \ ! ALL_OBJFILE_MSYMBOLS (objfile, m) #define ALL_OBJFILE_OSECTIONS(objfile, osect) \ for (osect = objfile->sections; osect < objfile->sections_end; osect++) Index: gdb/solib-sunos.c =================================================================== RCS file: /cvs/src/src/gdb/solib-sunos.c,v retrieving revision 1.6 diff -c -r1.6 solib-sunos.c *** gdb/solib-sunos.c 20 Oct 2002 14:38:26 -0000 1.6 --- gdb/solib-sunos.c 31 Jan 2003 19:52:30 -0000 *************** *** 184,189 **** --- 184,190 ---- xmalloc, xfree); rt_common_objfile->minimal_symbol_count = 0; rt_common_objfile->msymbols = NULL; + terminate_minimal_symbol_table (rt_common_objfile); } init_minimal_symbol_collection (); Index: gdb/symfile.c =================================================================== RCS file: /cvs/src/src/gdb/symfile.c,v retrieving revision 1.83 diff -c -r1.83 symfile.c *** gdb/symfile.c 30 Jan 2003 21:45:07 -0000 1.83 --- gdb/symfile.c 31 Jan 2003 19:52:33 -0000 *************** *** 2018,2023 **** --- 2018,2024 ---- error ("Can't find the file sections in `%s': %s", objfile->name, bfd_errmsg (bfd_get_error ())); } + terminate_minimal_symbol_table (objfile); /* We use the same section offsets as from last time. I'm not sure whether that is always correct for shared libraries. */