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.