Hi Jakub,
Thanks for your review.
On 21/05/2025 13:51, Jakub Jelinek wrote:
On Wed, May 21, 2025 at 01:42:01PM +0100, Christopher Bazley wrote:
On 21/05/2025 12:26, Christopher Bazley wrote:
Hi Joseph,
Thanks for reviewing my patch.
On 20/05/2025 18:02, Joseph Myers wrote:
On Tue, 20 May 2025, Christopher Bazley wrote:
+ if (!cleared)
+ {
+ if (complete_p.padded_non_union
+ && warn_zero_init_padding_bits >= ZERO_INIT_PADDING_BITS_ALL)
+ {
+ warning (OPT_Wzero_init_padding_bits_,
+ "Padding bits might not be initialized to zero; "
+ "consider using %<-fzero-init-padding-bits=all%>");
+ }
+ else if (complete_p.padded_union
+ && warn_zero_init_padding_bits
+ >= ZERO_INIT_PADDING_BITS_UNIONS)
+ {
+ warning (OPT_Wzero_init_padding_bits_,
+ "Padding bits might not be initialized to zero; "
+ "consider using %<-fzero-init-padding-bits=unions%> "
+ "or %<-fzero-init-padding-bits=all%>");
Diagnostics should start with a lowercase letter.
I'll change it to "padding might not be initialized to zero..." OK?
("padding" seems to be more common than "padding bits" in existing
messages.)
Is there anything I need to do for initialization?
Sorry, I meant "internationalisation" (i.e. to support translation), not
"initialization"!
For translations I think it is fine as is.
Please avoid {}s around single statements (e.g. the warning call above).
Originally, it was more than one statement to produce information on how
to initialise padding bits. That information was language (and dialect)
specific, which wasn't acceptable in the Gimplifier. I'll remove the
redundant braces.
And, I think the warnings should include text like
"if code relies on it being zero, consider"
because telling people to use those options mindlessly is undesirable.
Nobody is going to see these warnings unless they explicitly enable
them, in which case I hope they would have read the documentation first.
The current text is:
+ warning (OPT_Wzero_init_padding_bits_,
+ "padding might not be initialized to zero; "
+ "consider using
%<-fzero-init-padding-bits=unions%> "
+ "or %<-fzero-init-padding-bits=all%>");
I can add "if code relies on it being zero," although I don't think
"consider" is a very strong prescription and I highly doubt whether the
people reading these warnings will know whether the code they are
compiling relies on padding being zeroed or not. My presumption was that
programmers who know their own code intimately won't enable the warnings
in the first place.
Having uninitialized padding bits is most of the time just fine, only if
code accesses it (whether writing it to disk, or say computing checksum
of all the bytes in it etc.), it is problematic.
I'm well aware of that. You can see what I wrote in the documentation:
"It does not follow that all source code which causes warnings about
uninitialized padding bits has undefined behaviour: usually, the values
of padding bits have no effect on execution of a program."
--
Christopher Bazley
Staff Software Engineer, GNU Tools Team.
Arm Ltd, 110 Fulbourn Road, Cambridge, CB1 9NJ, UK.
http://www.arm.com/
