From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr1-x42f.google.com (mail-wr1-x42f.google.com [IPv6:2a00:1450:4864:20::42f]) by sourceware.org (Postfix) with ESMTPS id 834393858D28 for ; Mon, 31 Jul 2023 20:42:36 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 834393858D28 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=linaro.org Received: by mail-wr1-x42f.google.com with SMTP id ffacd0b85a97d-31792ac0fefso2118108f8f.2 for ; Mon, 31 Jul 2023 13:42:36 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1690836155; x=1691440955; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=a/FJ6x1Xppbl3C/FfPsjbWe0Tu4deStNaEB7Nmi5ujI=; b=tNCcyC14WWjUS93W2bXrrn8qLxuQG8u8hvnJjRldwwQcLDteaEIDBzH6aKaNT5hRET m/7wh+A2QRn9/cb8kRKDYSWsz39t90GNDj3PE3x64sSgkTFOFQtQ9mqLhw7Dl7OCh+S3 2xiwkpBMx+cUxwvJgsLFCptqg8AImfMwFM3dxVOys9MzUCfCSVfolEfoPg0x6JqXBbuB zkTcWKfIzosCOavBNFmpt3qwJCNXTyZ9PahspMwiu9q8rZSvy4wEhIpK8Jji3ahnMIKz cj1GpDXEnQ1Cjr535J/dg6mjNRnu00UAaO6+N2OUtAGz1BX8QwND0GrfLzkmwvQsNv2r uXWA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1690836155; x=1691440955; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=a/FJ6x1Xppbl3C/FfPsjbWe0Tu4deStNaEB7Nmi5ujI=; b=XMWqIKTv2Kc/6djfLR9gQ8emp/9s9pYCreSpnurTkqeo+UGX3dben7hJ7ZMv/nfglA kFt/XJLpnGkwOqikHaZYen1iSUUEcXZQjx41jvgI311W+1ZYcDfISGIkNlZ9wwtDPaj3 V1HUK/gMG3pjW6RZ2cIkMDL3AUpaZjZ8qDZj1j1BzzqQOPDC7X1xZsWJg+48iVZtVUrC pmjekAxTCCTp+ux2ZthOWY6S3zPBTTAp8KiTITpOVybbKnOMRqS8Wn8cNbGQTYTEhdSi k0Owi9IvwU8oqniyNxSJpc9fMWNtjdoNKNRUB5tLbyhP42S4v9YDgRTRy0ZFxpLl/3nI zxFA== X-Gm-Message-State: ABy/qLYq1jlJAj/Iy1ldmZvbWa36f1WbAA0ZFrtTJPopA6vJDFM/3aJP 9kVvcC+M+druLhiqkgQhHmIfDuQO82ovewOIBOObmiMIrGl5YpbXmps= X-Google-Smtp-Source: APBJJlG8wrwtkHMvJtUkIGDif4IY1spd+oc836FHe5g4TAGwfuDXY7OFAsTA61Fz1sY8ElwTD4Soj5y0SRLyrjoxMlQ= X-Received: by 2002:adf:f248:0:b0:315:ade6:a52d with SMTP id b8-20020adff248000000b00315ade6a52dmr669382wrp.19.1690836154459; Mon, 31 Jul 2023 13:42:34 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Prathamesh Kulkarni Date: Tue, 1 Aug 2023 02:11:58 +0530 Message-ID: Subject: Re: [C PATCH]: Add Walloc-type to warn about insufficient size in allocations To: Martin Uecker Cc: gcc-patches@gcc.gnu.org, Joseph Myers Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-9.5 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On Fri, 21 Jul 2023 at 16:52, Martin Uecker via Gcc-patches wrote: > > > > This patch adds a warning for allocations with insufficient size > based on the "alloc_size" attribute and the type of the pointer > the result is assigned to. While it is theoretically legal to > assign to the wrong pointer type and cast it to the right type > later, this almost always indicates an error. Since this catches > common mistakes and is simple to diagnose, it is suggested to > add this warning. > > > Bootstrapped and regression tested on x86. > > > Martin > > > > Add option Walloc-type that warns about allocations that have > insufficient storage for the target type of the pointer the > storage is assigned to. > > gcc: > * doc/invoke.texi: Document -Wstrict-flex-arrays option. > > gcc/c-family: > > * c.opt (Walloc-type): New option. > > gcc/c: > * c-typeck.cc (convert_for_assignment): Add Walloc-type warning. > > gcc/testsuite: > > * gcc.dg/Walloc-type-1.c: New test. > > > diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt > index 4abdc8d0e77..8b9d148582b 100644 > --- a/gcc/c-family/c.opt > +++ b/gcc/c-family/c.opt > @@ -319,6 +319,10 @@ Walloca > C ObjC C++ ObjC++ Var(warn_alloca) Warning > Warn on any use of alloca. > > +Walloc-type > +C ObjC Var(warn_alloc_type) Warning > +Warn when allocating insufficient storage for the target type of the > assigned pointer. > + > Walloc-size-larger-than= > C ObjC C++ LTO ObjC++ Var(warn_alloc_size_limit) Joined Host_Wide_Int > ByteSize Warning Init(HOST_WIDE_INT_MAX) > -Walloc-size-larger-than= Warn for calls to allocation > functions that > diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc > index 7cf411155c6..2e392f9c952 100644 > --- a/gcc/c/c-typeck.cc > +++ b/gcc/c/c-typeck.cc > @@ -7343,6 +7343,32 @@ convert_for_assignment (location_t location, > location_t expr_loc, tree type, > "request for implicit conversion " > "from %qT to %qT not permitted in C++", rhstype, > type); > > + /* Warn of new allocations are not big enough for the target > type. */ > + tree fndecl; > + if (warn_alloc_type > + && TREE_CODE (rhs) == CALL_EXPR > + && (fndecl = get_callee_fndecl (rhs)) != NULL_TREE > + && DECL_IS_MALLOC (fndecl)) > + { > + tree fntype = TREE_TYPE (fndecl); > + tree fntypeattrs = TYPE_ATTRIBUTES (fntype); > + tree alloc_size = lookup_attribute ("alloc_size", > fntypeattrs); > + if (alloc_size) > + { > + tree args = TREE_VALUE (alloc_size); > + int idx = TREE_INT_CST_LOW (TREE_VALUE (args)) - 1; > + /* For calloc only use the second argument. */ > + if (TREE_CHAIN (args)) > + idx = TREE_INT_CST_LOW (TREE_VALUE (TREE_CHAIN > (args))) - 1; > + tree arg = CALL_EXPR_ARG (rhs, idx); > + if (TREE_CODE (arg) == INTEGER_CST > + && tree_int_cst_lt (arg, TYPE_SIZE_UNIT (ttl))) Hi Martin, Just wondering if it'd be a good idea perhaps to warn if alloc size is not a multiple of TYPE_SIZE_UNIT instead of just less-than ? So it can catch cases like: int *p = malloc (sizeof (int) + 2); // probably intended malloc (sizeof (int) * 2) FWIW, this is caught using -fanalyzer: f.c: In function 'f': f.c:3:12: warning: allocated buffer size is not a multiple of the pointee's size [CWE-131] [-Wanalyzer-allocation-size] 3 | int *p = __builtin_malloc (sizeof(int) + 2); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Thanks, Prathamesh > + warning_at (location, OPT_Walloc_type, "allocation of > " > + "insufficient size %qE for type %qT with > " > + "size %qE", arg, ttl, TYPE_SIZE_UNIT > (ttl)); > + } > + } > + > /* See if the pointers point to incompatible address spaces. */ > asl = TYPE_ADDR_SPACE (ttl); > asr = TYPE_ADDR_SPACE (ttr); > diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi > index 88e3c625030..6869bed64c3 100644 > --- a/gcc/doc/invoke.texi > +++ b/gcc/doc/invoke.texi > @@ -8076,6 +8076,15 @@ always leads to a call to another @code{cold} > function such as wrappers of > C++ @code{throw} or fatal error reporting functions leading to > @code{abort}. > @end table > > +@opindex Wno-alloc-type > +@opindex Walloc-type > +@item -Walloc-type > +Warn about calls to allocation functions decorated with attribute > +@code{alloc_size} that specify insufficient size for the target type > of > +the pointer the result is assigned to, including those to the built-in > +forms of the functions @code{aligned_alloc}, @code{alloca}, > @code{calloc}, > +@code{malloc}, and @code{realloc}. > + > @opindex Wno-alloc-zero > @opindex Walloc-zero > @item -Walloc-zero > diff --git a/gcc/testsuite/gcc.dg/Walloc-type-1.c > b/gcc/testsuite/gcc.dg/Walloc-type-1.c > new file mode 100644 > index 00000000000..bc62e5e9aa3 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/Walloc-type-1.c > @@ -0,0 +1,37 @@ > +/* Tests the warnings for insufficient allocation size. > + { dg-do compile } > + * { dg-options "-Walloc-type" } > + * */ > +#include > +#include > + > +struct b { int x[10]; }; > + > +void fo0(void) > +{ > + struct b *p = malloc(sizeof *p); > +} > + > +void fo1(void) > +{ > + struct b *p = malloc(sizeof p); /* { dg- > warning "allocation of insufficient size" } */ > +} > + > +void fo2(void) > +{ > + struct b *p = alloca(sizeof p); /* { dg- > warning "allocation of insufficient size" } */ > +} > + > +void fo3(void) > +{ > + struct b *p = calloc(1, sizeof p); /* { dg-warning > "allocation of insufficient size" } */ > +} > + > +void g(struct b* p); > + > +void fo4(void) > +{ > + g(malloc(4)); /* { dg-warning "allocation of > insufficient size" } */ > +} > + > + > > >