This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] make gengtype more robust against user error


On 10/25/13 15:37, David Malcolm wrote:
This patch addresses various forms of failure described in
http://gcc.gnu.org/ml/gcc-patches/2013-10/msg01974.html

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
{
public:
   int kind;
   tree p;
};

class GTY((tag("1"))) bar : public foo
{
public:
   tree q;
};

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?

gcc/
	* 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
	linked list.
	* gengtype.c (get_ultimate_base_class): New.
	(add_subclass): 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,

Jeff


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]