[patch/mdebug] symbol symtab not set on Tru64

[patch/mdebug] symbol symtab not set on Tru64

[Joel Brobecker]

Hello,

Something I just noticed while working on alpha-tru64.  When reading
symbols from ECOFF, the produced symbols do not have their symtab
field set.  I did a little of archeology, and basically the reason
for it is because the symbol reader does its own symbol and symtab
building, rather than using buildsym.  So when the symtab field
was added to struct symbol, we forgot to update part of mdebugread.c.

It's kind of confusing, because mdebugread also handles stabs-in-coff,
in which case buildsym.c is being used.  This may be why we missed
the parts in mdebugread.c that needed an update.

As a result, we end up with SEGVs or failed assertions when trying
to access this field because it was NULL. Fixed thusly.

2008-05-26  Joel Brobecker  <brobecker@adacore.com>

        Set the symtab field of symbols read from ECOFF debugging entries.
        * mdebugread.c (add_symbol): Add new parameter symtab.
        (parse_symbol): Update calls to add_symbol throughout.

Tested on alpha-tru64 with the AdaCore testsuite (I can't seem to get
expect to work on Tru64 anymore - it looks like I have an echo issue).
I will commit in a week unless there are comments about the patch.

-- 
Joel

Re: [patch/mdebug] symbol symtab not set on Tru64

[Joel Brobecker]

[-- Attachment #1: Type: text/plain, Size: 1235 bytes --]

[with the patch, this time. Sigh...]

Hello,

Something I just noticed while working on alpha-tru64.  When reading
symbols from ECOFF, the produced symbols do not have their symtab
field set.  I did a little of archeology, and basically the reason
for it is because the symbol reader does its own symbol and symtab
building, rather than using buildsym.  So when the symtab field
was added to struct symbol, we forgot to update part of mdebugread.c.

It's kind of confusing, because mdebugread also handles stabs-in-coff,
in which case buildsym.c is being used.  This may be why we missed
the parts in mdebugread.c that needed an update.

As a result, we end up with SEGVs or failed assertions when trying
to access this field because it was NULL. Fixed thusly.

2008-05-26  Joel Brobecker  <brobecker@adacore.com>

        Set the symtab field of symbols read from ECOFF debugging entries.
        * mdebugread.c (add_symbol): Add new parameter symtab.
        (parse_symbol): Update calls to add_symbol throughout.

Tested on alpha-tru64 with the AdaCore testsuite (I can't seem to get
expect to work on Tru64 anymore - it looks like I have an echo issue).
I will commit in a week unless there are comments about the patch.

-- 
Joel

[-- Attachment #2: mdebugread.c.diff --]
[-- Type: text/plain, Size: 3709 bytes --]

Modified: mdebugread.c
===================================================================
--- mdebugread.c	2008-05-23 22:51:11
+++ mdebugread.c	2008-05-23 23:37:18
@@ -279,7 +279,7 @@
 
 static void add_block (struct block *, struct symtab *);
 
-static void add_symbol (struct symbol *, struct block *);
+static void add_symbol (struct symbol *, struct symtab *, struct block *);
 
 static int add_line (struct linetable *, int, CORE_ADDR, int);
 
@@ -649,7 +649,7 @@
     data:			/* Common code for symbols describing data */
       SYMBOL_DOMAIN (s) = VAR_DOMAIN;
       SYMBOL_CLASS (s) = class;
-      add_symbol (s, b);
+      add_symbol (s, top_stack->cur_st, b);
 
       /* Type could be missing if file is compiled without debugging info.  */
       if (SC_IS_UNDEF (sh->sc)
@@ -694,7 +694,7 @@
 	}
       SYMBOL_VALUE (s) = svalue;
       SYMBOL_TYPE (s) = parse_type (cur_fd, ax, sh->index, 0, bigend, name);
-      add_symbol (s, top_stack->cur_block);
+      add_symbol (s, top_stack->cur_st, top_stack->cur_block);
       break;
 
     case stLabel:		/* label, goes into current block */
@@ -703,7 +703,7 @@
       SYMBOL_CLASS (s) = LOC_LABEL;	/* but not misused */
       SYMBOL_VALUE_ADDRESS (s) = (CORE_ADDR) sh->value;
       SYMBOL_TYPE (s) = mdebug_type_int;
-      add_symbol (s, top_stack->cur_block);
+      add_symbol (s, top_stack->cur_st, top_stack->cur_block);
       break;
 
     case stProc:		/* Procedure, usually goes into global block */
@@ -780,7 +780,7 @@
 	  else if (sh->value == top_stack->procadr)
 	    b = BLOCKVECTOR_BLOCK (bv, GLOBAL_BLOCK);
 	}
-      add_symbol (s, b);
+      add_symbol (s, top_stack->cur_st, b);
 
       /* Make a type for the procedure itself */
       SYMBOL_TYPE (s) = lookup_function_type (t);
@@ -1066,7 +1066,7 @@
 		SYMBOL_VALUE (enum_sym) = tsym.value;
 		if (SYMBOL_VALUE (enum_sym) < 0)
 		  unsigned_enum = 0;
-		add_symbol (enum_sym, top_stack->cur_block);
+		add_symbol (enum_sym, top_stack->cur_st, top_stack->cur_block);
 
 		/* Skip the stMembers that we've handled. */
 		count++;
@@ -1096,7 +1096,7 @@
 	SYMBOL_CLASS (s) = LOC_TYPEDEF;
 	SYMBOL_VALUE (s) = 0;
 	SYMBOL_TYPE (s) = t;
-	add_symbol (s, top_stack->cur_block);
+	add_symbol (s, top_stack->cur_st, top_stack->cur_block);
 	break;
 
 	/* End of local variables shared by struct, union, enum, and
@@ -1158,7 +1158,7 @@
 	  SYMBOL_VALUE (s) = (long) e;
 	  e->numargs = top_stack->numargs;
 	  e->pdr.framereg = -1;
-	  add_symbol (s, top_stack->cur_block);
+	  add_symbol (s, top_stack->cur_st, top_stack->cur_block);
 
 	  /* f77 emits proc-level with address bounds==[0,0],
 	     So look for such child blocks, and patch them.  */
@@ -1297,7 +1297,7 @@
       SYMBOL_CLASS (s) = LOC_TYPEDEF;
       SYMBOL_BLOCK_VALUE (s) = top_stack->cur_block;
       SYMBOL_TYPE (s) = t;
-      add_symbol (s, top_stack->cur_block);
+      add_symbol (s, top_stack->cur_st, top_stack->cur_block);
 
       /* Incomplete definitions of structs should not get a name.  */
       if (TYPE_NAME (SYMBOL_TYPE (s)) == NULL
@@ -1904,7 +1904,7 @@
       SYMBOL_CLASS (s) = LOC_BLOCK;
       /* Donno its type, hope int is ok */
       SYMBOL_TYPE (s) = lookup_function_type (mdebug_type_int);
-      add_symbol (s, top_stack->cur_block);
+      add_symbol (s, top_stack->cur_st, top_stack->cur_block);
       /* Wont have symbols for this one */
       b = new_block (2);
       SYMBOL_BLOCK_VALUE (s) = b;
@@ -4434,8 +4434,9 @@
 /* Add a new symbol S to a block B.  */
 
 static void
-add_symbol (struct symbol *s, struct block *b)
+add_symbol (struct symbol *s, struct symtab *symtab, struct block *b)
 {
+  SYMBOL_SYMTAB (s) = symtab;
   dict_add_symbol (BLOCK_DICT (b), s);
 }
 

Re: [patch/mdebug] symbol symtab not set on Tru64

[Ulrich Weigand]

Joel Brobecker wrote:

> As a result, we end up with SEGVs or failed assertions when trying
> to access this field because it was NULL. Fixed thusly.

Is this a regression due to my recent SYMBOL_SYMTAB cleanup patch?

> 2008-05-26  Joel Brobecker  <brobecker@adacore.com>
> 
>         Set the symtab field of symbols read from ECOFF debugging entries.
>         * mdebugread.c (add_symbol): Add new parameter symtab.
>         (parse_symbol): Update calls to add_symbol throughout.

In any case, this looks good to me ...

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: [patch/mdebug] symbol symtab not set on Tru64

[Joel Brobecker]

> > As a result, we end up with SEGVs or failed assertions when trying
> > to access this field because it was NULL. Fixed thusly.
> 
> Is this a regression due to my recent SYMBOL_SYMTAB cleanup patch?

Your patch might have made the problem more visible, not sure, but
the issue was introduced much earlier than this (I think early 2007),
when this new field was added. I noticed it a couple of weeks ago,
before you checked your cleanup patch in.

> > 2008-05-26  Joel Brobecker  <brobecker@adacore.com>
> > 
> >         Set the symtab field of symbols read from ECOFF debugging entries.
> >         * mdebugread.c (add_symbol): Add new parameter symtab.
> >         (parse_symbol): Update calls to add_symbol throughout.
> 
> In any case, this looks good to me ...

Thanks for double-checking it! I will check it in shortly.

-- 
Joel

Re: [patch/mdebug] symbol symtab not set on Tru64

[Ulrich Weigand]

Joel Brobecker wrote:

> > > As a result, we end up with SEGVs or failed assertions when trying
> > > to access this field because it was NULL. Fixed thusly.
> > 
> > Is this a regression due to my recent SYMBOL_SYMTAB cleanup patch?
> 
> Your patch might have made the problem more visible, not sure, but
> the issue was introduced much earlier than this (I think early 2007),
> when this new field was added. I noticed it a couple of weeks ago,
> before you checked your cleanup patch in.

Ah, OK.  Just wondering whether I missed something ...

Thanks,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com