On Wed, Feb 05, 2025 at 10:53:24AM -0800, Kees Cook wrote:
> On Wed, Feb 05, 2025 at 12:59:58PM +0100, Jakub Jelinek wrote:
> > Kees, any progress on this?
>
> I need to take another run at it. I got stalled out when I discovered
> that I array-of-char-arrays attributes got applied at the "wrong" depth,
> and stuff wasn't working.
>
> e.g.:
>
> char acpi_table[TABLE_SIZE][4] __attribute((nonstring)) = {
> { "ohai" },
> { "1234" },
> };
>
> when nonstring was checked for on something like "acpi_table[2]" it
> wouldn't be found, since it was applied at the top level.
While I think we should address that, I think it should be handled
incrementally, it is basically a change in the nonstring attribute and
needs to be dealt wherever nonstring is handled.
In order to speed things up, I took your patch and applied Marek's and my
review comments to it, furthermore removed unreachable code -
if (warn_cxx_compat || len - unit > avail)
...
else if (warn_unterminated_string_initialization)
{
if (len - unit > avail)
...
else
...
}
makes no sense, as the second len - unit > avail will be always false.
And tweaked the test coverage a little bit as well.
Kees, are you submitting this under assignment to FSF (maybe the Google one
if it has one) or DCO? See https://gcc.gnu.org/contribute.html#legal
for details. If DCO, can you add your Signed-off-by: tag for it?
So far lightly tested, ok for trunk if it passes bootstrap/regtest?
2025-02-13 Kees Cook <k...@kernel.org>
Jakub Jelinek <ja...@redhat.com>
PR c/117178
gcc/
* doc/invoke.texi (Wunterminated-string-initialization): Document
the new interaction between this warning and -Wc++-compat and that
initialization of decls with nonstring attribute aren't warned about.
gcc/c-family/
* c.opt (Wunterminated-string-initialization): Don't depend on
-Wc++-compat.
gcc/c/
* c-typeck.cc (digest_init): Add DECL argument. Adjust wording of
pedwarn_init for too long strings and provide details on the lengths,
for string literals where just the trailing NULL doesn't fit warn for
warn_cxx_compat with OPT_Wc___compat, wording which mentions "for C++"
and provides details on lengths, otherwise for
warn_unterminated_string_initialization adjust the warning, provide
details on lengths and don't warn if get_attr_nonstring_decl (decl).
(build_c_cast, store_init_value, output_init_element): Adjust
digest_init callers.
gcc/testsuite/
* gcc.dg/Wunterminated-string-initialization.c: Add additional test
coverage.
* gcc.dg/Wcxx-compat-14.c: Check in dg-warning for "for C++" part of
the diagnostics.
* gcc.dg/Wcxx-compat-23.c: New test.
* gcc.dg/Wcxx-compat-24.c: New test.
--- gcc/doc/invoke.texi.jj 2025-02-13 10:17:17.320789358 +0100
+++ gcc/doc/invoke.texi 2025-02-13 13:11:42.089042791 +0100
@@ -8661,17 +8661,20 @@ give a larger number of false positives
@opindex Wunterminated-string-initialization
@opindex Wno-unterminated-string-initialization
@item -Wunterminated-string-initialization @r{(C and Objective-C only)}
-Warn about character arrays
-initialized as unterminated character sequences
-with a string literal.
+Warn about character arrays initialized as unterminated character sequences
+with a string literal, unless the declaration being initialized has
+the @code{nonstring} attribute.
For example:
@smallexample
-char arr[3] = "foo";
+char arr[3] = "foo"; /* Warning. */
+char arr2[3] __attribute__((nonstring)) = "bar"; /* No warning. */
@end smallexample
-This warning is enabled by @option{-Wextra} and @option{-Wc++-compat}.
-In C++, such initializations are an error.
+This warning is enabled by @option{-Wextra}. If @option{-Wc++-compat}
+is enabled, the warning has slightly different wording and warns even
+if the declaration being initialized has the @code{nonstring} warning,
+as in C++ such initializations are an error.
@opindex Warray-compare
@opindex Wno-array-compare
--- gcc/c-family/c.opt.jj 2025-01-02 11:47:29.681229781 +0100
+++ gcc/c-family/c.opt 2025-02-13 12:49:47.187320829 +0100
@@ -1550,7 +1550,7 @@ C ObjC Var(warn_unsuffixed_float_constan
Warn about unsuffixed float constants.
Wunterminated-string-initialization
-C ObjC Var(warn_unterminated_string_initialization) Warning LangEnabledBy(C
ObjC,Wextra || Wc++-compat)
+C ObjC Var(warn_unterminated_string_initialization) Warning LangEnabledBy(C
ObjC,Wextra)
Warn about character arrays initialized as unterminated character sequences
with a string literal.
Wunused
--- gcc/c/c-typeck.cc.jj 2025-01-14 09:36:43.751522483 +0100
+++ gcc/c/c-typeck.cc 2025-02-13 12:52:14.366275230 +0100
@@ -116,8 +116,8 @@ static void push_member_name (tree);
static int spelling_length (void);
static char *print_spelling (char *);
static void warning_init (location_t, int, const char *);
-static tree digest_init (location_t, tree, tree, tree, bool, bool, bool, bool,
- bool, bool);
+static tree digest_init (location_t, tree, tree, tree, tree, bool, bool, bool,
+ bool, bool, bool);
static void output_init_element (location_t, tree, tree, bool, tree, tree,
bool,
bool, struct obstack *);
static void output_pending_init_elements (int, struct obstack *);
@@ -6844,7 +6844,7 @@ build_c_cast (location_t loc, tree type,
t = build_constructor_single (type, field, t);
if (!maybe_const)
t = c_wrap_maybe_const (t, true);
- t = digest_init (loc, type, t,
+ t = digest_init (loc, field, type, t,
NULL_TREE, false, false, false, true, false, false);
TREE_CONSTANT (t) = TREE_CONSTANT (value);
return t;
@@ -8874,8 +8874,8 @@ store_init_value (location_t init_loc, t
}
bool constexpr_p = (VAR_P (decl)
&& C_DECL_DECLARED_CONSTEXPR (decl));
- value = digest_init (init_loc, type, init, origtype, npc, int_const_expr,
- arith_const_expr, true,
+ value = digest_init (init_loc, decl, type, init, origtype, npc,
+ int_const_expr, arith_const_expr, true,
TREE_STATIC (decl) || constexpr_p, constexpr_p);
/* Store the expression if valid; else report error. */
@@ -9201,7 +9201,8 @@ check_constexpr_init (location_t loc, tr
"type of object");
}
-/* Digest the parser output INIT as an initializer for type TYPE.
+/* Digest the parser output INIT as an initializer for type TYPE
+ initializing DECL.
Return a C expression of type TYPE to represent the initial value.
If ORIGTYPE is not NULL_TREE, it is the original type of INIT.
@@ -9224,8 +9225,8 @@ check_constexpr_init (location_t loc, tr
on initializers for 'constexpr' objects apply. */
static tree
-digest_init (location_t init_loc, tree type, tree init, tree origtype,
- bool null_pointer_constant, bool int_const_expr,
+digest_init (location_t init_loc, tree decl, tree type, tree init,
+ tree origtype, bool null_pointer_constant, bool int_const_expr,
bool arith_const_expr, bool strict_string,
bool require_constant, bool require_constexpr)
{
@@ -9360,27 +9361,38 @@ digest_init (location_t init_loc, tree t
&& TREE_CODE (TYPE_SIZE (type)) == INTEGER_CST)
{
unsigned HOST_WIDE_INT len = TREE_STRING_LENGTH (inside_init);
- unsigned unit = TYPE_PRECISION (typ1) / BITS_PER_UNIT;
- /* Subtract the size of a single (possibly wide) character
- because it's ok to ignore the terminating null char
- that is counted in the length of the constant. */
- if (compare_tree_int (TYPE_SIZE_UNIT (type), len - unit) < 0)
- pedwarn_init (init_loc, 0,
- ("initializer-string for array of %qT "
- "is too long"), typ1);
- else if (warn_unterminated_string_initialization
- && compare_tree_int (TYPE_SIZE_UNIT (type), len) < 0)
- warning_at (init_loc, OPT_Wunterminated_string_initialization,
- ("initializer-string for array of %qT "
- "is too long"), typ1);
if (compare_tree_int (TYPE_SIZE_UNIT (type), len) < 0)
{
- unsigned HOST_WIDE_INT size
+ unsigned HOST_WIDE_INT avail
= tree_to_uhwi (TYPE_SIZE_UNIT (type));
+ unsigned unit = TYPE_PRECISION (typ1) / BITS_PER_UNIT;
const char *p = TREE_STRING_POINTER (inside_init);
- inside_init = build_string (size, p);
+ /* Construct truncated string. */
+ inside_init = build_string (avail, p);
+
+ /* Subtract the size of a single (possibly wide) character
+ because it may be ok to ignore the terminating NUL char
+ that is counted in the length of the constant. */
+ if (len - unit > avail)
+ pedwarn_init (init_loc, 0,
+ "initializer-string for array of %qT "
+ "is too long (%wu chars into %wu "
+ "available)", typ1, len, avail);
+ else if (warn_cxx_compat)
+ warning_at (init_loc, OPT_Wc___compat,
+ "initializer-string for array of %qT "
+ "is too long for C++ (%wu chars into %wu "
+ "available)", typ1, len, avail);
+ else if (warn_unterminated_string_initialization
+ && get_attr_nonstring_decl (decl) == NULL_TREE)
+ warning_at (init_loc,
+ OPT_Wunterminated_string_initialization,
+ "initializer-string for array of %qT "
+ "truncates NUL terminator but destination "
+ "lacks %qs attribute (%wu chars into %wu "
+ "available)", typ1, "nonstring", len, avail);
}
}
@@ -11405,7 +11417,7 @@ output_init_element (location_t loc, tre
if (!require_constexpr_value
|| !npc
|| TREE_CODE (constructor_type) != POINTER_TYPE)
- new_value = digest_init (loc, type, new_value, origtype, npc,
+ new_value = digest_init (loc, field, type, new_value, origtype, npc,
int_const_expr, arith_const_expr, strict_string,
require_constant_value, require_constexpr_value);
if (new_value == error_mark_node)
--- gcc/testsuite/gcc.dg/Wunterminated-string-initialization.c.jj
2024-07-16 13:36:54.199749481 +0200
+++ gcc/testsuite/gcc.dg/Wunterminated-string-initialization.c 2025-02-13
12:43:26.928616073 +0100
@@ -1,6 +1,33 @@
+/* PR c/117178 */
/* { dg-do compile } */
/* { dg-options "-Wunterminated-string-initialization" } */
char a1[] = "a";
-char a2[1] = "a"; /* { dg-warning "initializer-string for array of 'char'
is too long" } */
-char a3[2] = "a";
+char a2[1] = "a"; /* { dg-warning "initializer-string for array of 'char'
truncates" } */
+char a2nonstring[1] __attribute__((nonstring)) = "a";
+char a3[1] = "aa"; /* { dg-warning "initializer-string for array of 'char'
is too long" } */
+char a4[2] = "a";
+
+struct has_str {
+ int a;
+ char str1[4];
+ char str2[4];
+ char str3[4];
+ char str4[4];
+ char tag1[4] __attribute__((nonstring));
+ char tag2[4] __attribute__((nonstring));
+ char tag3[4] __attribute__((nonstring));
+ char tag4[4] __attribute__((nonstring));
+ int b;
+};
+
+struct has_str foo = {
+ .str1 = "111",
+ .str2 = "2222", /* { dg-warning "initializer-string for array of 'char'
truncates" } */
+ .str3 = "33333", /* { dg-warning "initializer-string for array of 'char'
is too long" } */
+ .str4 = { '4', '4', '4', '4' },
+ .tag1 = "AAA",
+ .tag2 = "BBBB",
+ .tag3 = "CCCCC", /* { dg-warning "initializer-string for array of 'char'
is too long" } */
+ .tag4 = { 'D', 'D', 'D', 'D' }
+};
--- gcc/testsuite/gcc.dg/Wcxx-compat-14.c.jj 2024-07-16 13:36:54.199749481
+0200
+++ gcc/testsuite/gcc.dg/Wcxx-compat-14.c 2025-02-13 13:20:49.631427826
+0100
@@ -2,5 +2,5 @@
/* { dg-options "-Wc++-compat" } */
char a1[] = "a";
-char a2[1] = "a"; /* { dg-warning "initializer-string for array of 'char'
is too long" } */
+char a2[1] = "a"; /* { dg-warning "initializer-string for array of 'char'
is too long for C\\\+\\\+" } */
char a3[2] = "a";
--- gcc/testsuite/gcc.dg/Wcxx-compat-23.c.jj 2025-02-13 13:21:12.635107900
+0100
+++ gcc/testsuite/gcc.dg/Wcxx-compat-23.c 2025-02-13 13:27:43.992665072
+0100
@@ -0,0 +1,33 @@
+/* PR c/117178 */
+/* { dg-do compile } */
+/* { dg-options "-Wc++-compat -Wunterminated-string-initialization" } */
+
+char a1[] = "a";
+char a2[1] = "a"; /* { dg-warning "initializer-string for array of 'char'
is too long for C\\\+\\\+" } */
+char a2nonstring[1] __attribute__((nonstring)) = "a"; /* { dg-warning
"initializer-string for array of 'char' is too long for C\\\+\\\+" } */
+char a3[1] = "aa"; /* { dg-warning "initializer-string for array of 'char'
is too long" } */
+char a4[2] = "a";
+
+struct has_str {
+ int a;
+ char str1[4];
+ char str2[4];
+ char str3[4];
+ char str4[4];
+ char tag1[4] __attribute__((nonstring));
+ char tag2[4] __attribute__((nonstring));
+ char tag3[4] __attribute__((nonstring));
+ char tag4[4] __attribute__((nonstring));
+ int b;
+};
+
+struct has_str foo = {
+ .str1 = "111",
+ .str2 = "2222", /* { dg-warning "initializer-string for array of 'char'
is too long for C\\\+\\\+" } */
+ .str3 = "33333", /* { dg-warning "initializer-string for array of 'char'
is too long" } */
+ .str4 = { '4', '4', '4', '4' },
+ .tag1 = "AAA",
+ .tag2 = "BBBB", /* { dg-warning "initializer-string for array of 'char'
is too long for C\\\+\\\+" } */
+ .tag3 = "CCCCC", /* { dg-warning "initializer-string for array of 'char'
is too long" } */
+ .tag4 = { 'D', 'D', 'D', 'D' }
+};
--- gcc/testsuite/gcc.dg/Wcxx-compat-24.c.jj 2025-02-13 13:27:57.727474060
+0100
+++ gcc/testsuite/gcc.dg/Wcxx-compat-24.c 2025-02-13 13:28:04.309382516
+0100
@@ -0,0 +1,33 @@
+/* PR c/117178 */
+/* { dg-do compile } */
+/* { dg-options "-Wc++-compat -Wno-unterminated-string-initialization" } */
+
+char a1[] = "a";
+char a2[1] = "a"; /* { dg-warning "initializer-string for array of 'char'
is too long for C\\\+\\\+" } */
+char a2nonstring[1] __attribute__((nonstring)) = "a"; /* { dg-warning
"initializer-string for array of 'char' is too long for C\\\+\\\+" } */
+char a3[1] = "aa"; /* { dg-warning "initializer-string for array of 'char'
is too long" } */
+char a4[2] = "a";
+
+struct has_str {
+ int a;
+ char str1[4];
+ char str2[4];
+ char str3[4];
+ char str4[4];
+ char tag1[4] __attribute__((nonstring));
+ char tag2[4] __attribute__((nonstring));
+ char tag3[4] __attribute__((nonstring));
+ char tag4[4] __attribute__((nonstring));
+ int b;
+};
+
+struct has_str foo = {
+ .str1 = "111",
+ .str2 = "2222", /* { dg-warning "initializer-string for array of 'char'
is too long for C\\\+\\\+" } */
+ .str3 = "33333", /* { dg-warning "initializer-string for array of 'char'
is too long" } */
+ .str4 = { '4', '4', '4', '4' },
+ .tag1 = "AAA",
+ .tag2 = "BBBB", /* { dg-warning "initializer-string for array of 'char'
is too long for C\\\+\\\+" } */
+ .tag3 = "CCCCC", /* { dg-warning "initializer-string for array of 'char'
is too long" } */
+ .tag4 = { 'D', 'D', 'D', 'D' }
+};
Jakub
