failed assertion hit in check_typedef

failed assertion hit in check_typedef

[Joel Brobecker]

Hello,

I just hit that assertion when working on the mips-irix port of GDB,
in order to bring that port back in shape. Here is what happens:

> (gdb) catch exception Constraint_Error
> Breakpoint 2 at 0x9947200: file s-except.adb, line 44.
> (gdb) n
> gdbtypes.c:567: internal-error: make_cv_type: Assertion `TYPE_OBJFILE (*typeptr) == TYPE_OBJFILE (type)' failed.

Here, the "next" steps over a call that leads to an exception being
raised. And during the course of handling that exception, GDB hits
that assertion.

To refresh your memory on how Ada exception catchpoints are implemented,
it's a simple breakpoint on a specific function, and the exception name
is extracted from the addressed returned by the evaluation of the following
expression: "e.full_name". "e" here is the name of the parameter of our
function.

This function is part of the Ada runtime, and in the case of mips-irix,
the default runtime used is the shared GNAT runtime. Thus, our function
and its parameter are defined in a DSO, not in the main executable.
As as result, when we lookup "e", the symbol we get refers to the objfile
of that DSO, not the one of the main executable.

"E" is described as a ptr to a struct. However, the struct type is
a STUB (flags = 4):

$18 = {code = TYPE_CODE_STRUCT, upper_bound_type = BOUND_SIMPLE,
  lower_bound_type = BOUND_SIMPLE, name = 0x0,
  tag_name = 0x105dd636 "system__standard_library__exception_data",
  objfile = 0x1059b4d0, target_type = 0x0, flags = 4, nfields = 0,
  vptr_fieldno = -1, fields = 0x0, vptr_basetype = 0x0, type_specific = {
    cplus_stuff = 0x104c8480, floatformat = 0x104c8480,
    gnat_stuff = 0x104c8480}}

So at some point, we do a "check_typedef" on it to resolve it to
the complete type. This is where the fun begins: GCC, or maybe it
is just GNAT, has the little habit of duplicating the definition
of all the types it uses in all the compilation units that use them.

So one of the unfortunate effects is that we end up in a situation
when we have a copy of that type definition in our DSO, and also
another copy in the main executable (more than one actually, but
one is already too many).

As a result, we have a problem there (gdbtypes.c:check_typedef):

  else if (TYPE_STUB (type) && !currently_reading_symtab)
    {
      char *name = type_name_no_tag (type);
      [...]
      sym = lookup_symbol (name, 0, STRUCT_DOMAIN, 0, (struct symtab **) NULL);
      if (sym)
        make_cv_type (is_const, is_volatile, SYMBOL_TYPE (sym), &type);
    }

the lookup_symbol routine finds the first one which, bad luck,
just happens to be the definition from the main exectuable.
As a result, the following assertion in make_cv_type breaks:

      /* TYPE and *TYPEPTR must be in the same objfile.  We can't have
         a C-V variant chain that threads across objfiles: if one
         objfile gets freed, then the other has a broken C-V chain.

         This code used to try to copy over the main type from TYPE to
         *TYPEPTR if they were in different objfiles, but that's
         wrong, too: TYPE may have a field list or member function
         lists, which refer to types of their own, etc. etc.  The
         whole shebang would need to be copied over recursively; you
         can't have inter-objfile pointers.  The only thing to do is
         to leave stub types as stub types, and look them up afresh by
         name each time you encounter them.  */
      gdb_assert (TYPE_OBJFILE (*typeptr) == TYPE_OBJFILE (type));

I'm not exactly too sure at this point on how to fix this problem.
Maybe we should just change the call to make_cv_type to:

    type = make_cv_type (is_const, is_volatile, SYMBOL_TYPE (sym), type);

But I think this would be bad, because that would leave the incomplete
type as is, instead of replacing it by the complete definition. So
instead, I suggest the following: Remove the assertion, and turn it
into a check. Something like this:

      /* [repeat the comment already there (the one just above),
         and explain that we avoid the overwrite if the two types are
         not stored in the same obstack. Add explanation as to when
         this can happen].  */
      if (typeptr && *typeptr != NULL
        && TYPE_OBJFILE (*typeptr) != TYPE_OBJFILE (type))
      typeptr = NULL;

It takes quite a long while to run the testsuite on mips-irix,
so I was wondering if I could get some feedback before I go too
far into this route...

Thanks,
-- 
Joel

Re: failed assertion hit in check_typedef

[Daniel Jacobowitz]

On Mon, Feb 26, 2007 at 11:16:58PM -0800, Joel Brobecker wrote:
> So one of the unfortunate effects is that we end up in a situation
> when we have a copy of that type definition in our DSO, and also
> another copy in the main executable (more than one actually, but
> one is already too many).
> 
> As a result, we have a problem there (gdbtypes.c:check_typedef):
> 
>   else if (TYPE_STUB (type) && !currently_reading_symtab)
>     {
>       char *name = type_name_no_tag (type);
>       [...]
>       sym = lookup_symbol (name, 0, STRUCT_DOMAIN, 0, (struct symtab **) NULL);
>       if (sym)
>         make_cv_type (is_const, is_volatile, SYMBOL_TYPE (sym), &type);
>     }
> 
> the lookup_symbol routine finds the first one which, bad luck,
> just happens to be the definition from the main exectuable.

Maybe we really want a lookup_symbol call which looks just in the
correct objfile... well, I guess that wouldn't always help.

>       /* [repeat the comment already there (the one just above),
>          and explain that we avoid the overwrite if the two types are
>          not stored in the same obstack. Add explanation as to when
>          this can happen].  */
>       if (typeptr && *typeptr != NULL
>         && TYPE_OBJFILE (*typeptr) != TYPE_OBJFILE (type))
>       typeptr = NULL;
> 
> It takes quite a long while to run the testsuite on mips-irix,
> so I was wondering if I could get some feedback before I go too
> far into this route...

How about doing it in the caller?  If you go to check_typedef, you'll
see that there's already an example of the same thing.  If this is
DWARF2 and only started happening recently it may be the fault of your
recent fixes for what empty types constitute a declaration.

-- 
Daniel Jacobowitz
CodeSourcery