Pointers in union go AWOL (at least in gui/gm-editor)

[PatrikLundell]

The union in question is df.world.xml:incident.unk_v42_1. When viewed with gui/gm-editor the view is empty, i.e. no trace of t6-t10. I've then tried to find out what's going on:

• When one of the pointers (t6) is present together with two int16_t fields the integer fields are there, but the pointer does not appear. (The integer fields are just test subjects, not intended to mean anything, and I used only one pointer during the investigation).
• When I buried the pointer in a "compound" nothing changed, i.e. it still didn't appear.
• When I made an intermediate struct type containing only the pointer and then referenced this type as a compound field of the union it finally did appear...

As indicated, I don't know if this is some kind of issue with the Lua interface or if it's a deeper issue with the generated code. The issue can be worked around by hiding the pointers inside structs, as mentioned, but that's really just a work around, not a solution.

(What I was trying to do when running into the issue was to find out how thoughts refer to performances, which I believe are the subtype 6 version of the incident type. And it seems anon_4 of t6 is the id of the written_content object the performance relates to).

[lethosor]

From static.fields-i.inc:

  static const struct_field_info incident_doT_Dot_T_unk_v42_1_fields[] = {
    { FLD(POINTER, t6), identity_traits<df::incident_sub6 >::get(), 2, NULL },
    { FLD(POINTER, t7), identity_traits<df::incident_sub7 >::get(), 2, NULL },
    { FLD(POINTER, t8), identity_traits<df::incident_sub8 >::get(), 2, NULL },
    { FLD(POINTER, t9), identity_traits<df::incident_sub9 >::get(), 2, NULL },
    { FLD(POINTER, t10), identity_traits<df::incident_sub10 >::get(), 2, NULL },
    { FLD_END }
  };

The "2" is a flag which I believe indicates that these fields should get skipped in pairs(). This is intended to avoid crashes in things like the union in ui_look_list, which has a union including a few pointers to classes with vtables, and even iterating over those will crash. I'll try to figure out why that flag is getting set here, though, since it should be safe enough.

[lethosor]

Found in StructFields.pm:

        $count |= 2 if $in_union || is_attr_true($field, 'has-bad-pointers');

This was originally just

     $count |= 2 if $in_union

but was expanded to allow the has-bad-pointers attribute to skip uninitialized pointers to virtual class instances outside of unions too. Also, the check for this flag in LuaTypes.cpp was changed as follows:

-            // Skip class-typed pointers within unions
-            if ((fields[i].count & 2) != 0 && fields[i].type &&
-                fields[i].type->type() == IDTYPE_CLASS)
+            // Skip class-typed pointers within unions and other bad pointers
+            if ((fields[i].count & 2) != 0 && fields[i].type)

to support 4c224dd20567a9eda652ca9522bd0547be4bf358. Maybe we need to use two flags here - one for "virtual class in union" and one for "bad pointer".

[PatrikLundell]

Well, casting a pointer to the wrong type is dangerous, and that's essentially what a union of pointers is. The types pointed to contain vectors, and I'd assume interpreting binary patterns which aren't vectors as if they were to be potentially harmful. However, hiding them from view isn't the right thing either. Toady surely is using some kind of logic other than "just make sure to always select the correct one" to organize these unions in a somewhat safe manner, but I guess we just don't know what.

If I understand the current case correctly, it should be possible to change the "in union" automatic flag assignment to explicit "has-bad-pointers" assignments on the unions that do have elements with "bad" pointers in the form of vtables? It probably won't hurt to have two separate flags that for the time being have the same effect, though, as it would allow for future separation of the handling of the two cases.

[lethosor]

In the case of classes with vtable pointers, we have two choices: hide them or crash. In this case, they're structs, so I think we can make them visible. Printing an invalid vector (e.g. in a union) is fine; printing its contents is not.

The point I was making is that ALL classes with vtable pointers in unions need to be hidden in Lua. Formerly, this was done by setting the "in union" flag (2) on all fields in unions in perl, then checking whether a field was a virtual class (that's IDTYPE_CLASS above) in LuaTypes.cpp before actually skipping it. Requiring every class field in a union to have an explicit "in-union" or "has-bad-pointers" attribute in XML isn't good because it's something we can already determine automatically, it needs to be done for every class field in a union, and if someone forgets to add it (to an existing or new field), it will lead to crashes.