On 10/4/24 1:02 PM, Jakub Jelinek wrote:
On Thu, Oct 03, 2024 at 12:14:35PM -0400, Jason Merrill wrote:
Agreed, the padding bits have indeterminate values (or erroneous in C++26),
so it's correct for infoleak-1.c to complain about 4b.
I've been afraid what the kernel people would say about this change (because
reading Linus' mails shows he doesn't care about what the standards say,
but what he expects to see, anything else is "broken").
Indeed.
Though, looking at godbolt, clang and icc 19 and older gcc all do zero
initialize the whole union before storing the single member in there (if
non-zero, otherwise just clear).
So whether we want to do this or do it by default is another question.
We will want to initialize the padding (for all types) to something for
C++26, but that's a separate issue...
But ideally in a way where uninit warnings know the bits aren't initialized
even if they are.
Yes.
Anyway, bootstrapped/regtested on x86_64-linux and i686-linux successfully.
2024-09-28 Jakub Jelinek <ja...@redhat.com>
PR c++/116416
* expr.cc (categorize_ctor_elements_1): Fix up union handling of
*p_complete. Clear it only if num_fields is 0 and the union has
at least one FIELD_DECL, set to -1 if either union has no fields
and non-zero size, or num_fields is 1 and complete_ctor_at_level_p
returned false.
Hmm, complete_ctor_at_level_p also seems to need a change for this
understanding of union semantics: "every meaningful byte" depends on the
active member, so it seems like it should return true for a union iff
num_elts == 1.
I thought complete_ctor_at_level_p has a single caller, but apparently
that isn't the case, cp/typeck2.cc uses it too.
Here is an updated version of the patch, which
a) moves some of the stuff into complete_ctor_at_level_p (but not
all the *p_complete = 0; case, for that it would need to change
so that it passes around the ctor rather than just its type)
b) introduces a new option, so that users can either get the new
behavior (only what is guaranteed by the standards, the default),
or previous behavior (union padding zero initialization, no such
guarantees in structures) or also a guarantee in structures
c) introduces a new CONSTRUCTOR flag which says that the padding bits
(if any) should be zero initialized (and sets it for now in the C++
FE for C23 {} initializers).
Am not sure the CONSTRUCTOR_ZERO_PADDING_BITS flag is really needed
for C23, if there is just empty initializer, I think we already mark
it as incomplete if there are any missing initializers. Maybe with
some designated initializer games, say
void foo () {
struct S { char a; long long b; };
struct T { struct S c; } t = { .c = {}, .c.a = 1, .c.b = 2 };
...
}
Is this supposed to initialize padding bits in C23 and then the .c.a = 1
and .c.b = 2 stores preserve those padding bits, so is that supposed
to be different from struct T t2 = { .c = { 1, 2 } };
? What about just struct T t3 = { .c.a = 1, .c.b = 2 }; ?
And I haven't touched the C++ FE for the flag, because I'm afraid I'm lost
on where exactly is zero-initialization done (vs. other types of
initialization) and where is e.g. zero-initialization of a temporary then
(member-wise) copied.
Say
struct S { char a; long long b; };
struct T { constexpr T (int a, int b) : c () { c.a = a; c.b = b; } S c; };
void bar (T *);
void
foo ()
{
T t (1, 2);
bar (&t);
}
Is the c () value-initialization of t.c followed by c.a and c.b updates
which preserve the zero initialized padding bits?
Yes.
Or is there some
copy construction involved which does member-wise copying and makes the
padding bits undefined?
No, c() directly value-initializes the c member.
Looking at (older) clang++ with -O2, it initializes also the padding bits
when c () is used and doesn't with c {}.
That seems correct; since S is an aggregate c{} is member-wise
initialization rather than value-initialization.
For GCC, note that there is that optimization from Alex to zero padding bits
for optimization purposes for small aggregates, so either one needs to look
at -O0 -fdump-tree-gimple dumps, or use larger structures which aren't
optimized that way.
Only lightly tested so far, this is mostly for further discussions.
And also a question what exactly does cp/typeck2.cc want from
complete_ctor_at_level_p, e.g. if it wants false for all the cases where
categorize_ctor_elements_1 does *p_complete = 0; (in that case it would need
to know whether CONSTRUCTOR_ZERO_PADDING_BITS flag was set).
Hmm, split_nonconstant_init uses that result to decide whether to
discard the CONSTRUCTOR; I guess if _ZERO_PADDING_BITS is set we would
want to keep the CONSTRUCTOR for clearing the padding.
Jason
