Re: SerializeParamList vs machines with strict alignment

On Mon, Sep 10, 2018 at 7:05 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> writes:
> > On Mon, Sep 10, 2018 at 8:58 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> >> In particular, SerializeParamList does this:
> >>
> >> /* Write flags. */
> >> memcpy(*start_address, &prm->pflags, sizeof(uint16));
> >> *start_address += sizeof(uint16);
> >>
> >> immediately followed by this:
> >>
> >> datumSerialize(prm->value, prm->isnull, typByVal, typLen,
> >> start_address);
>
> > datumSerialize does this:
>
> > memcpy(*start_address, &header, sizeof(int));
> > *start_address += sizeof(int);
>
> > before calling EOH_flatten_into, so it seems to me it should be 4-byte aligned.
>
> But that doesn't undo the fact that you're now on an odd halfword
> boundary. In the case I observed, EA_flatten_into was trying to
> store an int32 through a pointer whose hex value ended in E, which
> is explained by the "+= sizeof(uint16)".
>
> > Yeah, I think as suggested by you, start_address should be maxaligned.
>
> A localized fix would be for datumSerialize to temporarily palloc the
> space for EOH_flatten_into to write into, and then it could memcpy
> that to whatever random address it'd been passed.

Attached is a patch along these lines, let me know if you have
something else in mind? I have manually verified this with
force_parallel_mode=regress configuration in my development
environment. I don't have access to alignment-sensitive hardware, so
can't test in such an environment. I will see if I can write a test
as well.

> Seems kind of
> inefficient though, especially since you'd likely have to do the same
> thing on the receiving side.
>

I am not sure what exactly you have in mind for receiving side.
datumRestore does below for pass-by-reference values:

/* Pass-by-reference case; copy indicated number of bytes. */
Assert(header > 0);
d = palloc(header);
memcpy(d, *start_address, header);
*start_address += header;
return PointerGetDatum(d);

Do we need any other special handling here?

--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com