This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] Fix check for ICC incomplete struct types
- From: Michael Eager <eager at eagerm dot com>
- To: Tom Tromey <tromey at redhat dot com>
- Cc: "gdb-patches at sourceware dot org" <gdb-patches at sourceware dot org>
- Date: Tue, 07 Jan 2014 12:16:55 -0800
- Subject: Re: [PATCH] Fix check for ICC incomplete struct types
- Authentication-results: sourceware.org; auth=none
- References: <52CC3790 dot 4030702 at eagerm dot com> <87bnznhbtn dot fsf at fleche dot redhat dot com>
On 01/07/14 10:43, Tom Tromey wrote:
"Michael" == Michael Eager <eager@eagerm.com> writes:
Michael> GDB contains code in read_structure_type() which is supposed
Michael> to check for incorrect DWARF generated by ICC for an incomplete
Michael> structure type. The code is incomplete, in that it doesn't
Michael> check for length == 0, and it doesn't set the STUB flag.
Thanks.
Michael> - if (producer_is_icc (cu))
Michael> + if (producer_is_icc (cu) && (TYPE_LENGTH (type) == 0))
Michael> {
Michael> /* ICC does not output the required DW_AT_declaration
Michael> on incomplete types, but gives them a size of zero. */
Michael> + TYPE_STUB (type) = 1;
Michael> }
Michael> else
Michael> TYPE_STUB_SUPPORTED (type) = 1;
It seems to me that TYPE_STUB_SUPPORTED should be set unconditionally
(and, btw, eww, what a big hack TYPE_STUB_SUPPORTED is). Without the
TYPE_STUB_SUPPORTED setting, TYPE_IS_OPAQUE does not honor TYPE_STUB.
Then the icc check moved to the next stanza, say like:
TYPE_STUB_SUPPORTED (type) = 1;
if (die_is_declaration (die, cu))
TYPE_STUB (type) = 1;
else if (producer_is_icc (cu) && TYPE_LENGTH (type) == 0)
{
/* ICC does not output the required DW_AT_declaration
on incomplete types, but gives them a size of zero. */
TYPE_STUB (type) = 1;
}
else if (attr == NULL && die->child == NULL
&& producer_is_realview (cu->producer))
/* RealView does not output the required DW_AT_declaration
on incomplete types. */
TYPE_STUB (type) = 1;
What do you think of this?
TYPE_IS_OPAQUE does honor TYPE_STUB and checks it before TYPE_STUB_SUPPORTED.
Which is absolutely screwy, since TYPE_STUB_SUPPORTED is set for all non-ICC
producers.
#define TYPE_IS_OPAQUE(thistype) \
(((TYPE_CODE (thistype) == TYPE_CODE_STRUCT) \
|| (TYPE_CODE (thistype) == TYPE_CODE_UNION)) \
&& (TYPE_NFIELDS (thistype) == 0) \
&& (!HAVE_CPLUS_STRUCT (thistype) \
|| TYPE_NFN_FIELDS (thistype) == 0) \
&& (TYPE_STUB (thistype) || !TYPE_STUB_SUPPORTED (thistype)))
So the last expression for non-ICC is equivalent to
(TYPE_STUB (thistype) || 0))
The only other place that TYPE_STUB is set in dwarf2read.c:
read_structure_type() is for TYPE_CODE_ENUM, which is excluded from
TYPE_IS_OPAQUE. (Not that I understand why.) It is also set in gdbtypes.c:
allocate_stub_method() with TYPE_CODE_METHOD, which is excluded from the test.
I don't understand why TYPE_CODE_CLASS is excluded from TYPE_IS_OPAQUE
either, since everything else is the same for structs and classes.
A test with ICC (after the patch) doesn't show any difference
whether TYPE_STUB_SUPPORTED is set or not, as expected.
As far as I can see, TYPE_STUB_SUPPORTED does nothing.
I'm happy to remove it and all the associated dreck and
reorder the test as you suggest.
--
Michael Eager eager@eagercon.com
1960 Park Blvd., Palo Alto, CA 94306 650-325-8077