Re: remove ATTRIBUTE_FIXED_PART_SIZE

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