On Sun, Dec 15, 2024 at 08:02:57PM -0800, Kees Cook wrote:
> When initializing a nonstring char array when compiled with
> -Wunterminated-string-initialization the warning trips even when
> truncating the trailing NUL character from the string constant. Only
> warn about this when running under -Wc++-compat since under C++ we should
> not initialize nonstrings from C strings.
Thanks for the patch.
>
> PR c/117178
>
> gcc/c/ChangeLog:
>
> * c-typeck.cc (digest_init): Check for nonstring attribute and
> avoid warning when only the trailing NUL is truncated.
> (build_c_cast): Update function call prototype.
> (store_init_value): Ditto.
> (output_init_element): Ditto.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.dg/Wunterminated-string-initialization.c: Update for
> new warning text and check for nonstring cases.
> * gcc.dg/Wunterminated-string-initialization-c++.c: Duplicate
> C test for -Wc++-compat.
> ---
> gcc/c/c-typeck.cc | 73 ++++++++++++-------
> .../Wunterminated-string-initialization-c++.c | 28 +++++++
> .../Wunterminated-string-initialization.c | 26 ++++++-
> 3 files changed, 98 insertions(+), 29 deletions(-)
> create mode 100644
> gcc/testsuite/gcc.dg/Wunterminated-string-initialization-c++.c
>
> diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc
> index 10b02da8752d..5cd0c07b87b1 100644
> --- a/gcc/c/c-typeck.cc
> +++ b/gcc/c/c-typeck.cc
> @@ -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 *);
> @@ -6731,7 +6731,7 @@ build_c_cast (location_t loc, tree type, tree expr)
> 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;
> @@ -8646,8 +8646,8 @@ store_init_value (location_t init_loc, tree decl, tree
> init, tree origtype)
> }
> 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. */
> @@ -8996,8 +8996,8 @@ check_constexpr_init (location_t loc, tree type, tree
> init,
> 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 field, 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)
Please document the new FIELD parameter.
> {
> @@ -9132,27 +9132,46 @@ digest_init (location_t init_loc, tree type, tree
> init, tree origtype,
> && 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
> - = tree_to_uhwi (TYPE_SIZE_UNIT (type));
> - const char *p = TREE_STRING_POINTER (inside_init);
> -
> - inside_init = build_string (size, p);
> + 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);
> +
> + /* 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 (warn_cxx_compat || len - unit > avail)
> + {
> + pedwarn_init (init_loc, 0,
> + ("initializer-string for array of %qT "
> + "is too long (%lu chars into %lu "
> + "available)"), typ1, len, avail);
> + }
No need for the { } here.
> + else if (warn_unterminated_string_initialization)
> + {
> + if (len - unit > avail)
Might make sense to factor out the check into
const bool too_long_p = len - unit > avail;
since it's used twice. Up to you.
> + warning_at (init_loc,
> + OPT_Wunterminated_string_initialization,
> + ("initializer-string for array of %qT "
> + "is too long (%lu chars into %lu "
> + "available)"), typ1, len, avail);
> + else if (get_attr_nonstring_decl (field) == NULL_TREE)
> + {
> + warning_at (init_loc,
> + OPT_Wunterminated_string_initialization,
> + ("initializer-string for array of %qT "
> + "truncates NUL terminator but "
> + "destination lacks 'nonstring' attribute "
Better to say "destination lacks %qs attribute" and add a "nonstring"
argument. And I don't think we add () around strings like that (I know
it we there before).
> + "(%lu chars into %lu available)"),
> + typ1, len, avail);
> + }
You can drop the {} here too.
> + }
> }
> }
>
> @@ -11173,7 +11192,7 @@ output_init_element (location_t loc, tree value, tree
> origtype,
> 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)
> diff --git a/gcc/testsuite/gcc.dg/Wunterminated-string-initialization-c++.c
> b/gcc/testsuite/gcc.dg/Wunterminated-string-initialization-c++.c
> new file mode 100644
> index 000000000000..18e8054eb5d3
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/Wunterminated-string-initialization-c++.c
> @@ -0,0 +1,28 @@
Please add
/* PR c/117178 */
here and in the other new test.
> +/* { 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" } */
> +char a2nonstring[1] __attribute__((nonstring)) = "a"; /* { dg-warning
> "initializer-string for array of 'char' is too long" } */
> +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 tag1[4] __attribute__((nonstring));
> + char tag2[4] __attribute__((nonstring));
> + char tag3[4] __attribute__((nonstring));
> + int b;
> +};
> +
> +struct has_str foo = {
> + .str1 = "111",
> + .str2 = "2222", /* { dg-warning "initializer-string for array of 'char'
> is too long" } */
> + .str3 = "33333", /* { dg-warning "initializer-string for array of 'char'
> is too long" } */
> + .tag1 = "AAA",
> + .tag2 = "BBBB", /* { dg-warning "initializer-string for array of 'char'
> is too long" } */
> + .tag3 = "CCCCC", /* { dg-warning "initializer-string for array of 'char'
> is too long" } */
> +};
> diff --git a/gcc/testsuite/gcc.dg/Wunterminated-string-initialization.c
> b/gcc/testsuite/gcc.dg/Wunterminated-string-initialization.c
> index 13d5dbc66400..315e7d53e253 100644
> --- a/gcc/testsuite/gcc.dg/Wunterminated-string-initialization.c
> +++ b/gcc/testsuite/gcc.dg/Wunterminated-string-initialization.c
> @@ -2,5 +2,27 @@
> /* { 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 tag1[4] __attribute__((nonstring));
> + char tag2[4] __attribute__((nonstring));
> + char tag3[4] __attribute__((nonstring));
> + int b;
> +};
> +
> +struct has_str foo = {
> + .str1 = "111",
> + .str2 = "2222", /* { dg-warning "initializer-string for array of 'char'
> truncates" } */
Would it be worth it checking { '2', '2', '2', '2' } here too?
> + .str3 = "33333", /* { dg-warning "initializer-string for array of 'char'
> is too long" } */
> + .tag1 = "AAA",
> + .tag2 = "BBBB",
> + .tag3 = "CCCCC", /* { dg-warning "initializer-string for array of 'char'
> is too long" } */
> +};
> --
> 2.34.1
>
Marek
