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

[Max Romanov]

I'm sorry to be a pain, but the key thing is: ".. read from another object..". In the statement you are trying to "fix", the "base" does not actually _read_ from the object (structure, to be precise), instead the address of the structure itself used. In other words, "p->base" is a shortcut for "(uint8_t *) p".

--
Max

-----Original Message-----
From: Andrew Clayton <andrew at digital-domain.net>
To: Alejandro Colomar <alx.manpages at gmail.com>
Cc: unit at nginx.org, Max Romanov <max.romanov at gmail.com>
Sent: пт, 17 июн. 2022 2:05
Subject: Re: [PATCH 06/11] Sptr: avoided potentially undefined behaviour.

On Fri, 17 Jun 2022 00:17:20 +0200
Alejandro Colomar <alx.manpages at gmail.com> wrote:

> 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.

I knew it! ;)
 
> 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.

Thanks for looking into this.
 
> So, for the patch:
> 
> Reviewed-by: Alejandro Colomar <alx.manpages at gmail.com>

Well, I'm glad I wasn't completely wrong! I'll maybe tweak the commit
message and quote the spec...
 
> BTW, updated Stackoverflow: <https://stackoverflow.com/a/72652427/6872717>

And the above paragraph is in at least the C99 spec.

Cheers,
Andrew
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.nginx.org/pipermail/unit/attachments/20220618/13373fc7/attachment.htm>