ub fix in magenta

ub fix in magenta

[Szabolcs Nagy]

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


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

(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)

Re: ub fix in magenta

[Rich Felker]

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