Re: ub fix in magenta

[Rich Felker <dalias@libc.org>]

On Sat, Nov 05, 2016 at 09:25:59PM +0100, Szabolcs Nagy wrote:
> why do you think union based type punning is ub?
> are you compiling musl as c++ code?
> 
> commit 224516687417d5e9dcbb0ba300c3e34bb47bb12b
> Author: George Kulakowski <kulakowski@google.com>
> Date:   2016-10-19 17:11:59 -0700
> 
>     [musl][malloc] Remove undefined behavior in malloc
>     
>     This bit of code computes an approximation to log2(x) by extracting the
>     exponent from a float. Doing it via a union this way is bad, so memcpy
>     instead.
> 
> https://fuchsia.googlesource.com/magenta/+/224516687417d5e9dcbb0ba300c3e34bb47bb12b

It's definitely not UB (this usage is explicitly permitted by C), and
the memcpy approach is much slower (requires store/call/load) because
-ffreestanding implies -fno-builtin. I'd like to try overriding that
with -fbuiltin-memcpy or a musl-internal header that defines memcpy to
__builtin_memcpy, etc., for all files but src/string/*, but there are
various subtle issues to be concerned about.

> this makes implementation internals publicly visible, introduce
> paddings and whenever you need to add new fields you will have
> to break the abi again.
> 
> note that the initializers are not valid c and thus non-conforming.
> 
> commit c751172f029e96a3208b37da91fd9e020a792834
> Author: George Kulakowski <kulakowski@google.com>
> Date:   2016-10-13 21:31:24 -0700
> 
>     [musl] Use a single proper struct definition for pthread types
>     
>     There is one slight change in layout here made for simplicity's
>     sake. Upstream's pthread_barrier_t overlays the _b_count and _b_inst
>     fields in the 32 bit case. Since this is so rarely used (in Fuchsia
>     outside of libc I pretty much only see tsan, gdb etc. test cases),
>     just do the simple thing.
> 
> https://fuchsia.googlesource.com/magenta/+/c751172f029e96a3208b37da91fd9e020a792834

Yes, this change looks highly problematic to ABI stability.

> (i don't plan to review all changes i just wanted to see if there
> was anything useful in the magenta repo for musl, havent found much
> yet, but some of the changes could have been discussed upstream)

Thanks.

Rich