[PATCH 06/11] Sptr: avoided potentially undefined behaviour.

[Alejandro Colomar]

Hi, Andrew, Max!

On 6/16/22 19:43, Andrew Clayton wrote:
> On Thu, 16 Jun 2022 17:32:22 +0200
> Alejandro Colomar <alx.manpages at gmail.com> wrote:
> 
>> Hmmm, AFAIR, `u.a = u.b + 1;` is fine, since the = creates a sequence
>> point, isn't it?
>>
>> <https://stackoverflow.com/a/33291213/6872717>
> 
> So we're saying that
> 
>      6.5.16 (p3):
> 
>      [...] The side effect of updating the stored value of the left operand
>      is sequenced after the value computations of the left and right operands.
> 
> makes it OK?

Woah, wait a bit.  No it doesn't.  Today I learnt:

6.5.16.1/3 (Assignment operators::Simple Assignment::Semantics):

If the value being stored in an object is read from another object that 
overlaps in any way the storage of the first object, then the overlap 
shall be exact and the two objects shall have qualified or unqualified 
versions of a compatible type; otherwise, the behavior is undefined.

<https://www.open-std.org/JTC1/SC22/WG14/www/docs/n2731.pdf>

I thought union would have this issue in C++, but not in C; but it does. 
  So this patch is actually correct.

We have:

$ grepc nxt_unit_sptr_.
./src/nxt_unit_sptr.h:18:
union nxt_unit_sptr_u {
     uint8_t   base[1];
     uint32_t  offset;
};


./src/nxt_unit_typedefs.h:18:
typedef union nxt_unit_sptr_u              nxt_unit_sptr_t;


Since `uint8_t [1]` (or `uint8_t *`, to which it decays) is incompatible 
with `uint32_t`, it is UB, and storing it in a temporary fixes that.

So, for the patch:

Reviewed-by: Alejandro Colomar <alx.manpages at gmail.com>

BTW, updated Stackoverflow: <https://stackoverflow.com/a/72652427/6872717>


Cheers,

Alex

-- 
Alejandro Colomar
<http://www.alejandro-colomar.es/>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <http://mailman.nginx.org/pipermail/unit/attachments/20220617/73ef6cd0/attachment.bin>