Fix PCH crash on GTYed pointer-to-scalar field of a struct

When -ftrack-macro-expansion is activated, the PCH generation
machinery can crash in gt_pch_save when it's about to relocate the
pointer for the
line_maps::info_macro::maps[i]::d.macro.macro_locations member.

The call that crashes (in ggc-common.c) is:

    state.ptrs[i]->note_ptr_fn (state.ptrs[i]->obj,
				state.ptrs[i]->note_ptr_cookie,
				relocate_ptrs, &state);

The ->note_ptr_fn called in this case is the gengtype-generated
gt_pch_p_9line_maps function.  It crashes because the second argument
passed to it is a pointer to struct line_map, instead of being a
pointer to struct line_maps (extra 's') like what the function
expects.

You can see the crash for the test case:

    runtest --tool g++ --tool_opts="-ftrack-macro-expansion" pch.exp=system-1.C

I believe it's because a part of the code of gt_pch_nx_line_maps
(generated as part of gtype-desc.c by gengtype) is not correct.  Note
that this gt_pch_nx_line_maps function is called from gt_pch_save in
the snippet:

  for (rt = gt_ggc_rtab; *rt; rt++)
    for (rti = *rt; rti->base != NULL; rti++)
      for (i = 0; i < rti->nelt; i++)
	(*rti->pchw)(*(void **)((char *)rti->base + rti->stride * i));

So, in that gt_pch_nx_line_maps, in the branch that starts with the
code:

      if ((*x).info_macro.maps != NULL) {
        size_t i3;
        for (i3 = 0; i3 != (size_t)(((*x).info_macro).used); i3++) {
          switch (((*x).info_macro.maps[i3]).reason == LC_ENTER_MACRO)

we have the code:

    gt_pch_note_object ((*x).info_macro.maps[i3].d.macro.macro_locations,
			(*x).info_macro.maps,
			 gt_pch_p_9line_maps,
			  gt_types_enum_last);

This last snippet registers gt_pch_p_9line_maps to be called on the
object pointed by (*x).info_macro.maps[i3].d.macro.macro_locations (as
a first argument), with (*x).info_macro.maps as its second argument.

Note that (*x).info_macro.maps is of type struct line_map*, while 'x'
is of type struct line_maps* - beware, there is an 's' at the end of
the latter.

The problem is that gt_pch_p_9line_maps requires that its second
argument be an instance of _struct line_maps_, not struct line_map.
So later when gt_pch_p_9line_maps is called, it just crashes.

More generally, these gt_pch_p_xxx functions seem to require that
their second argument be an instance of the xxx in question.  And that
invariant is violated by the snippet of code above.

The invariant seems to be violated only for the case where a GTYed
structure (possibly embedded in another GTYed structure) contains a
pointer to a scalar (that is not a string) which memory is ggc/GTY
managed, like the line_map_macro::macro_locations field.  And this
only happens for PCH generation.

Looking at gengtype.c, it seems like write_types_process_field can be
fooled in that case.  It expects that the expression d->prev_val[3]
contains the name of the second argument of the gt_pch_p_xxx (which is
generically referenced by wtd->subfield_marker_routine there).  That
expression can resolve to either "x", as we would like it to be, but
can also resolve to another arbitrary name for e.g, the case of a
pointer-to-struct used as a root).

This patch simply forces the second argument of gt_pch_p_xxx to be 'x'
even in the case of a member that is a pointer to a scalar.

As a result, here is the the diff the new generated gtype-desc.c file:

@@ -5234,7 +5234,7 @@ gt_pch_nx_line_maps (void *x_p)
                 size_t i2;
                 for (i2 = 0; i2 != (size_t)(2 * ((*x).info_ordinary.maps[i0].d.macro).n_tokens); i2++) {
                 }
-                gt_pch_note_object ((*x).info_ordinary.maps[i0].d.macro.macro_locations, (*x).info_ordinary.maps, gt_pch_p_9line_maps, gt_types_enum_last);
+                gt_pch_note_object ((*x).info_ordinary.maps[i0].d.macro.macro_locations, x, gt_pch_p_9line_maps, gt_types_enum_last);
               }
               break;
             default:
@@ -5261,7 +5261,7 @@ gt_pch_nx_line_maps (void *x_p)
                 size_t i5;
                 for (i5 = 0; i5 != (size_t)(2 * ((*x).info_macro.maps[i3].d.macro).n_tokens); i5++) {
                 }
-                gt_pch_note_object ((*x).info_macro.maps[i3].d.macro.macro_locations, (*x).info_macro.maps, gt_pch_p_9line_maps, gt_types_enum_last);
+                gt_pch_note_object ((*x).info_macro.maps[i3].d.macro.macro_locations, x, gt_pch_p_9line_maps, gt_types_enum_last);
               }
               break;
             default:
@@ -9366,7 +9366,7 @@ gt_pch_na_regno_reg_rtx (ATTRIBUTE_UNUSED void *x_p)
     for (i1 = 0; i1 != (size_t)(crtl->emit.x_reg_rtx_no); i1++) {
       gt_pch_n_7rtx_def (regno_reg_rtx[i1]);
     }
-    gt_pch_note_object (regno_reg_rtx, &regno_reg_rtx, gt_pch_pa_regno_reg_rtx, gt_types_enum_last);
+    gt_pch_note_object (regno_reg_rtx, x, gt_pch_pa_regno_reg_rtx, gt_types_enum_last);
   }
 }

I think it's pretty much what I was willing to have.

Bootstrapped and tested on x86_64-unknown-linux-gnu against trunk.

Note that the bootstrap with -ftrack-macro-expansion exhibits
other separate issues that are addressed in subsequent patches.
This patch just fixes one class of problems.

The patch does pass bootstrap with -ftrack-macro-expansion turned
off, though.

gcc/

	* gengtype.c (write_types_process_field):  Force second argument
	of the call to the PCH object hierarchy walker to be 'x'.

From-SVN: r186967

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 86a2f2d..a1cdc88 100644 (file)
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,9 @@
+2012-04-30  Dodji Seketeli  <dodji@redhat.com>
+
+       Fix PCH crash on GTYed pointer-to-scalar field of a struct
+       * gengtype.c (write_types_process_field): Force second argument of
+       the call to the PCH object hierarchy walker to be 'x'.
+
 2012-04-30  Eric Botcazou  <ebotcazou@adacore.com>
 
        * config/i386/i386.c (ix86_handle_struct_attribute): Use the proper

diff --git a/gcc/gengtype.c b/gcc/gengtype.c
index abafaa9..02d4c8c 100644 (file)
--- a/gcc/gengtype.c
+++ b/gcc/gengtype.c
@@ -2930,7 +2930,26 @@ write_types_process_field (type_p f, const struct walk_type_data *d)
               wtd->subfield_marker_routine, cast, d->val);
       if (wtd->param_prefix)
        {
-         oprintf (d->of, ", %s", d->prev_val[3]);
+         if (f->u.p->kind == TYPE_SCALAR)
+           /* The current type is a pointer to a scalar (so not
+              considered like a pointer to instances of user defined
+              types) and we are seeing it; it means we must be even
+              more careful about the second argument of the
+              SUBFIELD_MARKER_ROUTINE call.  That argument must
+              always be the instance of the type for which
+              write_func_for_structure was called - this really is
+              what the function SUBFIELD_MARKER_ROUTINE expects.
+              That is, it must be an instance of the ORIG_S type
+              parameter of write_func_for_structure.  The convention
+              is that that argument must be "x" in that case (as set
+              by write_func_for_structure).  The problem is, we can't
+              count on d->prev_val[3] to be always set to "x" in that
+              case.  Sometimes walk_type can set it to something else
+              (to e.g cooperate with write_array when called from
+              write_roots).  So let's set it to "x" here then.  */
+           oprintf (d->of, ", x");
+         else
+           oprintf (d->of, ", %s", d->prev_val[3]);
          if (d->orig_s)
            {
              oprintf (d->of, ", gt_%s_", wtd->param_prefix);