Re: Cleanup gcc trick with varattrib_1b_e in VARATT_EXTERNAL_GET_POINTER()

On 09.06.25 10:21, Michael Paquier wrote:
> Back in b89e151054a0, the following macro has been introduced to
> retrieve the varatt_external of an on-disk external TOAST Datum, stuff
> now in detoast.h:
> /*
> * Macro to fetch the possibly-unaligned contents of an EXTERNAL datum
> * into a local "struct varatt_external" toast pointer. This should be
> * just a memcpy, but some versions of gcc seem to produce broken code
> * that assumes the datum contents are aligned. Introducing an explicit
> * intermediate "varattrib_1b_e *" variable seems to fix it.
> */
> #define VARATT_EXTERNAL_GET_POINTER(toast_pointer, attr) \
> do { \
> varattrib_1b_e *attre = (varattrib_1b_e *) (attr); \
> Assert(VARATT_IS_EXTERNAL(attre)); \
> Assert(VARSIZE_EXTERNAL(attre) == sizeof(toast_pointer) + VARHDRSZ_EXTERNAL); \
> memcpy(&(toast_pointer), VARDATA_EXTERNAL(attre), sizeof(toast_pointer)); \
> } while (0)
>
> I vaguely recall that this has been mentioned during the unconference
> session dedicated to TOAST, or perhaps not. Anyway, I've just bumped
> into that again while working on this area, and I am wondering if this
> is relevant these days.

I'm not sure that the original reason applies anymore.

If attr in the above code is of type Datum, then I think the original
problem still exists. The compiler can assume that values of type Datum
have alignment fitting for Datum. But all the callers in the current
code have type struct varlena *, and the cast target behind
VARDATA_EXTERNAL() is varattrib_1b_e, both of which AFAICT have no
higher alignment requirement.

I can see how this might have been different historically. I have
noticed that there are some areas of code where Datum and struct varlena
* or similar are used interchangeably. Macros tend to hide that kind of
confusion. But some of this has been cleaned up with changing some
macros to inline functions. Maybe doing the same would help here, too.