This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFA-v2] Fix bug report 11479
> 2010-04-22 Pierre Muller <muller@ics.u-strasbg.fr>
>
> PR stabs/11479.
> * stabsread.c (set_length_in_type_chain): New function.
> (read_struct_type): Call set_length_in_type_chain function.
> (read_enum_type): Idem.
The code portion is pre-approved with the comments below. Just go ahead
and check it in.
> 2010-04-22 Pierre Muller <muller@ics.u-strasbg.fr>
>
> PR stabs/11479.
> * gdb.stabs/gdb11479.exp: New file.
> * gdb.stabs/gdb11479.c: New file.
This portion can be checked in separately after we answer the few questions
I have (see below).
> +/* Set the length for all variants of a same main_type, which are
> + connected in the closed chain. */
We're missing the "why" this is useful. How about the following?
/* Set the length for all variants of a same main_type, which are
connected in the closed chain.
This is something that needs to be done when a type is defined *after*
some cross references to this type have already been read. Consider
for instance the following scenario where we have the following two
stabs entries:
.stabs "t:p(0,21)=*(0,22)=k(0,23)=xsdummy:",160,0,28,-24
.stabs "dummy:T(0,23)=s16x:(0,1),0,3[...]"
A stubbed version of type dummy is created while processing the first
stabs entry. The length of that type is initially set to zero, since
it is unknown at this point. Also, a "constant" variation of type
"dummy" is created as well (this is the "(0,22)=k(0,23)" section of
the stabs line).
The second stabs entry allows us to replace the stubbed definition
with the real definition. However, we still need to adjust the length
of the "constant" variation of that type, as its length was left
untouched during the main type replacement... */
> +static void
> +set_length_in_type_chain (struct type * type)
^^ No space after the *
> + Please email any bugs, comments, and/or additions to this file to:
> + bug-gdb@gnu.org */
Can you remove this bit?
> +# Please email any bugs, comments, and/or additions to this file to:
> +# bug-gdb@gnu.org
Same for this one...
> +set testfile "gdb11479"
> +set srcfile ${testfile}.c
> +set binfile ${objdir}/${subdir}/${testfile}
> +if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable
> {debug additional_flags=-gstabs}] != "" } {
> + if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}"
> executable {debug}] != "" } {
> + untested "couldn't compile ${srcdir}/${subdir}/${srcfile}"
> + return -1
> + }
> +}
> +
> +# Start with a fresh gdb.
> +gdb_exit
> +gdb_start
> +gdb_reinitialize_dir $srcdir/$subdir
> +gdb_load ${binfile}
Can you actually use "prepare_for_testing" here? The only thing that
is a bit out of the ordinary is request the -gstabs debug flag. I am
actually wondering if we really want that, since this may not work on
certain platforms while we might still want to use the test with the
default debug format. On the other hand, you probably want to make sure
that your test gets run with -gstabs on Windows, so we have two
incompatible objectives...
How's about we do 2 builds? One with standard debug, and another with
-gstabs? We can repeat the testing by putting the gdb_test calls inside
a proc and then call it twice? I think that prepare_for_testing should
be able to handle the case where you need to add additional_flags as
well. In fact:
gdb.base/commands.exp:if { [prepare_for_testing commands.exp commands run.c {debug additional_flags=-DFAKEARGV}] } {
> +# Regression test for a cleanup bug in the charset code.
?
> +gdb_exit
Harmless, but unnecessary. Let's toss it.
--
Joel