Hi, On 2018-08-24 11:47:43 -0400, Tom Lane wrote: > Um ... this would be enough to document that we don't think there's a > *read* hazard, but Andres was claiming that there's also a *write* hazard. > That is, if you store into (say) a bool in the last declared column, > the compiler will think it's free to trash what it believes are padding > bytes at the end of the struct. This is problematic: if there's a > short-header varlena there, the overwrite clobbers data directly; and even > if the varlena is aligned, our tuple traversal code requires the alignment > pad bytes to be zero.
Right. The relevant standardese, in C11 (C99 very similar), is: 6.2.6.1 General, 6): "When a value is stored in an object of structure or union type, including in a member object, the bytes of the object representation that correspond to any padding bytes take unspecified values." I don't have the references at hand, but I'm fairly sure that at least gcc and clang can be made to exploit that. > I think what we might need to do, in places where the end of struct is > not already aligned, is to do something like > > bool is_instead; > > #ifdef CATALOG_VARLEN /* variable-length fields start here */ > pg_node_tree ev_qual; > pg_node_tree ev_action; > #else > uint8 dummy; > #endif > > and then teach the Catalog.pm routines to ignore the "dummy" fields while > emitting the .bki data. However, I'd want some automatic verification > that we'd added dummy fields where and only where needed. > > BTW, I have no objection to documenting should-not-be-null varlena > fields as you suggest. But it's cosmetic and doesn't really fix > the problem at hand. Hm, this is fairly ugly. I can't quite come up with something better right now tho :(. I occasionally wonder when it's time to give up 1:1 mapping between the structs and the catalog and force explicit conversion inbetween. But obviously that's a large change.. Greetings, Andres Freund
