[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20240318213441.GH4163@brightrain.aerifal.cx>
Date: Mon, 18 Mar 2024 17:34:42 -0400
From: Rich Felker <dalias@...c.org>
To: Mike Cui <cuicui@...il.com>
Cc: musl@...ts.openwall.com
Subject: Re: Potential bug in __res_msend_rc() wrt to union
initialization.
On Mon, Mar 18, 2024 at 12:56:41PM -0700, Mike Cui wrote:
> I recently upgraded to clang-18, and after compiling musl with it, I
> noticed that all my getaddrinfo() calls are failing. I tracked this to be
> an issue in __res_msend_rc(), where the 'sin6' member of union 'sa' is
> initialized to garbage, rather than 0. Then later bind() fails
> with EADDRNOTAVAIL.
>
> I reported this bug on clang discourse:
> https://discourse.llvm.org/t/union-initialization-and-aliasing-clang-18-seems-to-miscompile-musl/77724,
> and the discussion seems to suggest that there is potentially a bug in musl
> as well. TL;DR:
>
> - According to strict interpretation of the C standard, initializing a
> union with '{0}', only initializes the first member of the union to 0 (in
> this case, sin4), and "default" initializes the rest. This interpretation
> is still up for debate. The proper way to initialize the entire union is '{
> }' not '{ 0 }'.
No, { } is a constraint violation. It may be valid in C++ or C23, but
it's not in C99, which is the source language for musl. { 0 } has
always been the universal zero-initializer for C.
Moreover, the C language has no such thing as a "partially initialized
object". I guess it's possible for an implementor to interpret
zero-initialization of a union to produce something other than zero
bits in the storage that is not part of the first union member, but
it's rather hostile.
> - There is currently a bug in clang-18 that treats '{ }' to be the same as
> '{ 0 }'. The proposed fix is to just zero out the entire union for both "{
> 0 }" and "{ }". However we cannot rely on "{ 0 }" to always zero out the
> entire union in the future.
>
> musl should be fixed to use "{ }" for initialization. And to work around
> the current buggy release of clang-18, perhaps flip the order to make sin6
> the first member of the struct? I've attached a patch that works for me.
> There may be other instances of the same bug in the musl code base.
If the clang interpretation is going to be this, we can just reorder
the union members so that the largest one is first. This should avoid
dependency on how the compiler decided to interpret the universal zero
initializer for unions.
Rich
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
