Re: LLVM miscompiles numeric.c access to short numeric var headers - Mailing list pgsql-hackers

From Tom Lane
Subject Re: LLVM miscompiles numeric.c access to short numeric var headers
Date
Msg-id 18280.1447343484@sss.pgh.pa.us
Whole thread Raw
In response to Re: LLVM miscompiles numeric.c access to short numeric var headers  (Greg Stark <stark@mit.edu>)
Responses Re: LLVM miscompiles numeric.c access to short numeric var headers  (Greg Stark <stark@mit.edu>)
List pgsql-hackers
Greg Stark <stark@mit.edu> writes:
> On Thu, Nov 12, 2015 at 2:39 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Either that's a reportable compiler bug, or someplace nearby we've
>> casted the pointer to something that would require a 4-byte struct.
>> I'm not sure which code you're looking at exactly, but maybe we're
>> using "union NumericChoice" prematurely?

> It triggers on the line with the NUMERIC_WEIGHT() macro call in
> init_var_from_num():

Hmm.  I'd argue that that is a compiler bug, but I dunno if the LLVM
guys would see it that way.  The existence of other union members in
the declaration shouldn't license them to assume that the particular
instance we're accessing is large enough for any possible member.

> I think it wouldn't be too hard to just give up on the structs
> and unions and use a char * as the underlying type. We could access
> the meta information directly using byte accesses and memcpy the
> digits to an aligned array of digits when setting up the var.

Meh.  The palloc to create an aligned array of digits would eat up
any possible performance win --- it'd be just about as expensive
as the existing unpack operation.

I think we could fix the immediate issue by redeclaring numeric
headers as arrays of (u)int16 rather than structs.  I'm not
very excited about the packed-header case.
        regards, tom lane



pgsql-hackers by date:

Previous
From: Andres Freund
Date:
Subject: Re: checkpointer continuous flushing
Next
From: Greg Stark
Date:
Subject: Re: LLVM miscompiles numeric.c access to short numeric var headers