On 8/22/19 4:23 PM, Martin Sebor wrote: > On 8/22/19 3:27 PM, Jeff Law wrote: >> On 8/21/19 2:50 PM, Martin Sebor wrote: >>> This patch is a subset of the solution for PR 91457 whose main >>> goal is to eliminate inconsistencies in warnings issued for >>> out-of-bounds accesses to the various flavors of flexible array >>> members of constant objects. That patch was posted here: >>> https://gcc.gnu.org/ml/gcc-patches/2019-08/msg01202.html >>> >>> Like PR 91457, this patch also relies on improving optimization >>> to issue better quality warnings. (I.e., with the latter being >>> what motivated it.) >>> >>> The optimization enhances string_constant to recognize empty >>> CONSTRUCTORs returned by fold_ctor_reference for references >>> to members of constant objects with either "insufficient" >>> initializers (e.g., given struct S { char n, a[]; } s = { 0 };) >>> or with braced-list initializers (e.g., given >>> struct S s = { 3 { 1, 2, 3, 0 } }; The patch lets string_constant >>> convert the CONSTRUCTOR for s.a into a STRING_CST, which in turn >>> enables the folding of calls to built-ins like strlen, strchr, or >>> strcmp with such arguments. >>> >>> Exposing the strings to the folder then also lets it detect and >>> issue warnings for out-of-bounds offsets in more instances of >>> such references than before. >>> >>> The remaining changes in the patch mostly enhance the places >>> that use the no-warning bit to prevent redundant diagnostics. >>> >>> Tested on x86_64-linux. >>> >>> Martin >>> >>> gcc-91490.diff >>> >>> PR middle-end/91490 - bogus argument missing terminating nul warning >>> on strlen of a flexible array member >>> >>> gcc/c-family/ChangeLog: >>> >>> PR middle-end/91490 >>> * c-common.c (braced_list_to_string): Add argument and overload. >>> Handle flexible length arrays. >>> >>> gcc/testsuite/ChangeLog: >>> >>> PR middle-end/91490 >>> * c-c++-common/Warray-bounds-7.c: New test. >>> * gcc.dg/Warray-bounds-39.c: Expect either -Warray-bounds or >>> -Wstringop-overflow. >>> * gcc.dg/strlenopt-78.c: New test. >>> >>> gcc/ChangeLog: >>> >>> PR middle-end/91490 >>> * builtins.c (c_strlen): Rename argument and introduce new local. >>> Set no-warning bit on original argument. >>> * expr.c (string_constant): Pass argument type to >>> fold_ctor_reference. >>> * gimple-fold.c (fold_nonarray_ctor_reference): Return a STRING_CST >>> for missing initializers. >>> * tree.c (build_string_literal): Handle optional argument. >>> * tree.h (build_string_literal): Add defaulted argument. >>> * gimple-ssa-warn-restrict.c (maybe_diag_access_bounds): Check >>> no-warning bit on original expression. >>> >>> diff --git a/gcc/builtins.c b/gcc/builtins.c >>> --- a/gcc/expr.c >>> +++ b/gcc/expr.c >>> @@ -11402,6 +11402,16 @@ is_aligning_offset (const_tree offset, >>> const_tree exp) >>> tree >>> string_constant (tree arg, tree *ptr_offset, tree *mem_size, tree >>> *decl) >>> { >>> + tree dummy = NULL_TREE;; >>> + if (!mem_size) >>> + mem_size = &dummy; >>> + >>> + tree chartype = TREE_TYPE (arg); >>> + if (POINTER_TYPE_P (chartype)) >>> + chartype = TREE_TYPE (chartype); >>> + while (TREE_CODE (chartype) == ARRAY_TYPE) >>> + chartype = TREE_TYPE (chartype); >>> + >>> tree array; >>> STRIP_NOPS (arg); >> So per our conversation today, I took a closer look at this. As you >> noted CHARTYPE is only used for the empty constructor code you're adding >> as a part of this patch. >> >> Rather than stripping away types like this to compute chartype, couldn't >> we just use char_type_node instead of chartype in this code below? > > We can't. string_constant is also called for wide strings (e.g., > by the sprintf pass). Returning a narrow string when the caller > asks for a wide one breaks the sprintf stuff. Sigh. And presumably we can't just move the block down because other bits in string_constant modify ARG.
So I think a quick comment before that fragment about its purpose and we're good to go for the patch as a whole based on the one you posted an hour or so ago. Thanks, jeff
