On Sun, Sep 30, 2018 at 10:44 AM Amit Kapila <amit.kapil...@gmail.com> wrote: > > On Mon, Sep 10, 2018 at 7:05 PM Tom Lane <t...@sss.pgh.pa.us> wrote: > > > > Amit Kapila <amit.kapil...@gmail.com> writes: > > > On Mon, Sep 10, 2018 at 8:58 AM Tom Lane <t...@sss.pgh.pa.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. >
Attached patch contains a test case as well to cover this code. It will help us in identifying if there is any failure on alignment-sensitive hardware in this code path. Kindly suggest what is the best path forward, is it a good idea to commit this on HEAD first and see if the test passes on all machines in buildfarm and then back-patch it? -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
fix_datum_serialization_v2.patch
Description: Binary data