This is the mail archive of the
mailing list for the GCC project.
Re: [PATCH] make gengtype more robust against user error
- From: Jeff Law <law at redhat dot com>
- To: David Malcolm <dmalcolm at redhat dot com>
- Cc: gcc-patches at gcc dot gnu dot org
- Date: Mon, 28 Oct 2013 15:19:22 -0600
- Subject: Re: [PATCH] make gengtype more robust against user error
- Authentication-results: sourceware.org; auth=none
- References: <526A0427 dot 6040705 at redhat dot com> <1382737048-28620-1-git-send-email-dmalcolm at redhat dot com>
On 10/25/13 15:37, David Malcolm wrote:
This patch addresses various forms of failure described in
It adds a "default: gcc_unreachable();" to the autogenerated switch()
statement in the routines for a base class, converting various kinds of
tag errors from leading to silent lack-of-field traversal into giving
run-time assertion failures (addressing (F) and (G))
It also issues an error within gengtype itself for a missing "desc"
(failure "B"), turning this into an error message from gengtype.
I found another potential failure mode:
(H) If you had:
class GTY((desc("%0.kind"), tag("0"))) foo
class GTY((tag("1"))) bar : public foo
static GTY(()) foo *dummy_foo;
and there are no explicit pointers to "bar" in the code, gengtype
treated it as unused, and silently omitted the case for it from
foo's marking routine.
I've updated set_gc_type_used so that it propagates usage down into
subclasses (which recurses), fixing this issue.
To do this efficiently we need to track subclasses in within gengtype,
so the patch also adds that, and uses it to eliminate an O(N^2).
Note that for error (G), if a class within the hierarchy omits a GTY
marker, gengtype doesn't parse it at all, and so doesn't parse the
inheritance information - so we can't issue a warning about this.
However, the lack of a tag will trigger a run-time assertion failure
due to hitting the "default: gcc_unreachable();" in the switch.
The patch also adds a paragraph to the docs, spelling out the need
for evary class in such a hierarchy to have a GTY marker.
I believe this addresses all of the silent-lack-of-field-traversal
issues, converting them to gengtype errors or runtime assertions.
It also adds a handler for (E), turning this from a failure to
compile bogus C to a specific error in gengtype.
I'm bootstrapping/regtesting now.
OK for trunk if that passes?
* doc/gty.texi ("Inheritance and GTY"): Make it clear that
to use autogenerated markers for a class-hierarchy, every class
must have a GTY marker.
* gengtype.h (struct type): Add linked list of subclasses to
the "s" member of the union.
(add_subclass): New decl.
* gengtype-state.c (read_state_struct_type): Set up subclass
* gengtype.c (get_ultimate_base_class): New.
(new_structure): Set up subclass linked list.
(set_gc_used_type): Propagate usage information to subclasses.
(output_mangled_typename): Use get_ultimate_base_class.
(walk_subclasses): Use the subclass linked list, avoiding an
O(N^2) when writing out all types.
(walk_type): Issue an error if the base class is missing a tag,
rather than generating bogus C code. Add a gcc_unreachable
default case, in case people omit tags from concrete subclasses,
or get the values wrong.
(write_func_for_structure): Issue an error for subclasses for
which the base doesn't have a "desc", since otherwise the
autogenerated routines for the base would silently fail to visit
any subclass fields.
(write_root): Use get_ultimate_base_class, tweaking constness of
tp to match that function's signature.
Thanks for diving into this stuff and getting it fixed.
OK for the trunk,