Re: [HACKERS] Constifying numeric.c's local vars

> This patch got committed as c1898c3e1e235ae35b4759d233253eff221b976a
> on Sun Sep 10 16:20:41 2017 -0700, but I've only just gotten around to
> reviewing it.
>
> I believe this is wrong and should be reverted, at least in part.
>
> The NumericVar struct has the field 'digits' as non-const:
>
> typedef struct NumericVar
> {
> int ndigits; /* # of digits in digits[] - can be 0! */
> int weight; /* weight of first digit */
> int sign; /* NUMERIC_POS, NUMERIC_NEG, or NUMERIC_NAN */
> int dscale; /* display scale */
> NumericDigit *buf; /* start of palloc'd space for digits[] */
> NumericDigit *digits; /* base-NBASE digits */
> } NumericVar;
>
> The static const data which is getting put in read only memory sets that data
> by casting away const as follows:
>
> static const NumericDigit const_zero_data[1] = {0};
> static const NumericVar const_zero =
> {0, 0, NUMERIC_POS, 0, NULL, (NumericDigit *) const_zero_data};
>
> This means that the const variable 'const_zero' contains a pointer that is
> non-const, pointing at something that is static const, stored in read only
> memory. Yikes.

I still believe this is unsafe.

> The function set_var_from_var(const NumericVar *value, NumericVar *dest)
> uses memcpy to copy the contents of value into dest. In cases where the value
> is a static const variable (eg, const_zero), the memcpy is copying a pointer to
> static const read only data into the dest and implicitly casting away const.
> Since that static const data is stored in read only memory, this has undefined
> semantics, and I believe could lead to a server crash, at least on some
> architectures with some compilers.

This is probably safe, though, since NumericDigit seems to just be a typedef
to int16. I should have checked that definition before complaining about that
part.