Re: Crash in write_exp_msymbol for coff targets.

[Pedro Alves <pedro_alves at portugalmail dot pt>]

I realize I just dumped the previous patch without explaining it,
so maybe this will help reducing the workload from Daniel,
or maybe even finding a new reviewer (yes, that's you! :) ).

In a nutshell:
In coff based targets, there is a new segfault in
parse.c:write_exp_msymbol,
at:
 if (SYMBOL_BFD_SECTION (msymbol)->flags & SEC_THREAD_LOCAL)

Easily triggered by just issuing p.ex.:
p globalvar

The problem is that minimal symbols may have a bfd_section set to
NULL at this point. (SYMBOL_BFD_SECTION (msymbol) == NULL).

This segfault doesn't happen in elf targets, because in elfread.c,
prim_record_minimal_symbol_and_info is always called with a non-NULL bfd_section*,
effectively always creating a minimal symbol with a bfd_section set.

In coffread.c, prim_record_minimal_symbol_and_info is always called with
bfd_section == NULL.

The attached patch, (functionally equivalent to the previous one,
with just a small cleanup), makes the coff reader match
the bfd_section from the coff_symbol, using the symbols' section number
and bfd_map_over_sections. This matching was already done in the
existing code, in cs_to_section, so it should be correct,
unlike the previous versions that used objfile->sections.

A few other functions are then adjusted to be able to pass bfd_section while
preserving the rest of the existing behavior.

I don't thing that the original workaround in the beginning of this thread ...:
- if (SYMBOL_BFD_SECTION (msymbol)->flags & SEC_THREAD_LOCAL)
+ if (SYMBOL_BFD_SECTION (msymbol)
+     && (SYMBOL_BFD_SECTION (msymbol)->flags & SEC_THREAD_LOCAL))

... should still be applied. Having it segfault there for other
formats and fix them accordingly would be better, than hiding
the real bug, IMHO.


Hope this helped,

Cheers,
Pedro Alves

---

2006-11-18 Pedro Alves <pedro_alves@portugalmail.pt>

* symtab.h (prim_record_minimal_symbol_and_bfd_section): Declare.
* minsyms.c (prim_record_minimal_symbol_and_bfd_section): Rename from prim_record_minimal_symbol
and add bfd_section parameter.
(prim_record_minimal_symbol): New version; wraps prim_record_minimal_symbol_and_bfd_section.
* coffread.c (cs_to_bfd_section): New function.
(cs_to_section): Use cs_to_bfd_section.
(record_minimal_symbol): Add bfd_section parameter. Call prim_record_minimal_symbol_and_bfd_section.
(coff_symtab_read): Use cs_to_bfd_section.

Index: symtab.h
===================================================================
RCS file: /cvs/src/src/gdb/symtab.h,v
retrieving revision 1.98
diff -u -p -r1.98 symtab.h
--- symtab.h	17 Oct 2006 20:17:45 -0000	1.98
+++ symtab.h	18 Nov 2006 01:02:57 -0000
@@ -1147,6 +1147,12 @@ extern struct minimal_symbol *prim_recor
    enum minimal_symbol_type,
    char *info, int section, asection * bfd_section, struct objfile *);
 
+extern void prim_record_minimal_symbol_and_bfd_section
+  (const char *name, CORE_ADDR address,
+   enum minimal_symbol_type ms_type,
+   asection *bfd_section,
+   struct objfile *objfile);
+
 extern unsigned int msymbol_hash_iw (const char *);
 
 extern unsigned int msymbol_hash (const char *);
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	18 Nov 2006 01:02:58 -0000
@@ -605,8 +605,9 @@ init_minimal_symbol_collection (void)
 }
 
 void
-prim_record_minimal_symbol (const char *name, CORE_ADDR address,
+prim_record_minimal_symbol_and_bfd_section (const char *name, CORE_ADDR address,
 			    enum minimal_symbol_type ms_type,
+			    asection *bfd_section,
 			    struct objfile *objfile)
 {
   int section;
@@ -631,9 +632,18 @@ prim_record_minimal_symbol (const char *
     }
 
   prim_record_minimal_symbol_and_info (name, address, ms_type,
-				       NULL, section, NULL, objfile);
+				       NULL, section, bfd_section, objfile);
 }
 
+void
+prim_record_minimal_symbol (const char *name, CORE_ADDR address,
+                                        enum minimal_symbol_type ms_type,
+                                        struct objfile *objfile)
+{
+  prim_record_minimal_symbol_and_bfd_section (name, address, ms_type, NULL, objfile);
+}
+
+
 /* Record a minimal symbol in the msym bunches.  Returns the symbol
    newly created.  */
 
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	18 Nov 2006 01:03:01 -0000
@@ -259,17 +259,25 @@ find_targ_sec (bfd *abfd, asection *sect
     *args->resultp = sect;
 }
 
-/* Return the section number (SECT_OFF_*) that CS points to.  */
-static int
-cs_to_section (struct coff_symbol *cs, struct objfile *objfile)
+/* Return the bfd_section that CS points to.  */
+static struct bfd_section*
+cs_to_bfd_section (struct coff_symbol *cs, struct objfile *objfile)
 {
   asection *sect = NULL;
   struct find_targ_sec_arg args;
-  int off = SECT_OFF_TEXT (objfile);
 
   args.targ_index = cs->c_secnum;
   args.resultp = &sect;
   bfd_map_over_sections (objfile->obfd, find_targ_sec, &args);
+  return sect;
+}
+
+/* Return the section number (SECT_OFF_*) that CS points to.  */
+static int
+cs_to_section (struct coff_symbol *cs, struct objfile *objfile)
+{
+  int off = SECT_OFF_TEXT (objfile);
+  asection *sect = cs_to_bfd_section (cs, objfile);
   if (sect != NULL)
     {
       /* This is the section.  Figure out what SECT_OFF_* code it is.  */
@@ -410,13 +418,14 @@ coff_end_symtab (struct objfile *objfile
 
 static void
 record_minimal_symbol (char *name, CORE_ADDR address,
-		       enum minimal_symbol_type type, struct objfile *objfile)
+		       enum minimal_symbol_type type, asection *bfd_section, 
+		       struct objfile *objfile)
 {
   /* We don't want TDESC entry points in the minimal symbol table */
   if (name[0] == '@')
     return;
 
-  prim_record_minimal_symbol (name, address, type, objfile);
+  prim_record_minimal_symbol_and_bfd_section (name, address, type, bfd_section, objfile);
 }
 
 /* coff_symfile_init ()
@@ -761,9 +770,11 @@ coff_symtab_read (long symtab_offset, un
       /* Typedefs should not be treated as symbol definitions.  */
       if (ISFCN (cs->c_type) && cs->c_sclass != C_TPDEF)
 	{
+	  struct bfd_section *bfd_section = cs_to_bfd_section (cs, objfile);
+
 	  /* Record all functions -- external and static -- in minsyms. */
 	  tmpaddr = cs->c_value + ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile));
-	  record_minimal_symbol (cs->c_name, tmpaddr, mst_text, objfile);
+	  record_minimal_symbol (cs->c_name, tmpaddr, mst_text, bfd_section, objfile);
 
 	  fcn_line_ptr = main_aux.x_sym.x_fcnary.x_fcn.x_lnnoptr;
 	  fcn_start_addr = tmpaddr;
@@ -926,9 +937,10 @@ coff_symtab_read (long symtab_offset, un
 	    if (cs->c_name[0] != '@' /* Skip tdesc symbols */ )
 	      {
 		struct minimal_symbol *msym;
+		struct bfd_section *bfd_section = cs_to_bfd_section (cs, objfile);
 		msym = prim_record_minimal_symbol_and_info
 		  (cs->c_name, tmpaddr, ms_type, NULL,
-		   sec, NULL, objfile);
+		   sec, bfd_section, objfile);
 		if (msym)
 		  COFF_MAKE_MSYMBOL_SPECIAL (cs->c_sclass, msym);
 	      }