From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pj1-x1032.google.com (mail-pj1-x1032.google.com [IPv6:2607:f8b0:4864:20::1032]) by sourceware.org (Postfix) with ESMTPS id 086033858409 for ; Wed, 20 Oct 2021 16:56:58 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 086033858409 Received: by mail-pj1-x1032.google.com with SMTP id a15-20020a17090a688f00b001a132a1679bso983713pjd.0 for ; Wed, 20 Oct 2021 09:56:57 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=WkIwC7NBLU6AW0TcG2O0+/5+teEr1oEVyGzSTPLUxdA=; b=0jsTKprtmqz/L0n8u3PmcKjyZPa/5vmsrBdbRkNXk6uA7Xnab4jcXoZAkccx0A/HIL PeGkEpGpjzpuCnuRLgEGLZc8rVSj7Qpukzt4RvPNBh/5lMCu5VZ1K0dJMg6mEwjSfqOc GaBl/9NXLrQINRD3T3cxlUIsqFLvZ4DXyQnB1oD6E9iVXOpZQa2OyuNIv/bTT2HrJgKV aS+ebFjh3FPlRwgnozpPk908IKGDkXQXVCaJzsABjzbN9S+ADxdOu9HSdrD3QRUuRwN0 dHVqBtU7nrHh4pN7+NQWy7qvOSCI1UTQQfLF1WyCwQbrlkEEbaaJdwBXbXn45agsyrAe qb0g== X-Gm-Message-State: AOAM5319oBNAWNpQvvvkgztNxXuWn+w23eStYBKSG/tFT4uPUcJHKrfD e86TMhyDfnhvAq1v5PQTaS0= X-Google-Smtp-Source: ABdhPJwCRoLE9iitWhGUbuc26e1it1UXYaIavlzrXPEPffyEcDvKHm5Du1ULeoxKZhiFL38I6u8Ymw== X-Received: by 2002:a17:90a:fa2:: with SMTP id 31mr114628pjz.175.1634749016990; Wed, 20 Oct 2021 09:56:56 -0700 (PDT) Received: from [192.168.0.41] (184-96-250-116.hlrn.qwest.net. [184.96.250.116]) by smtp.gmail.com with ESMTPSA id a17sm3217453pfd.54.2021.10.20.09.56.54 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 20 Oct 2021 09:56:56 -0700 (PDT) Subject: Re: [PATCH 6/8] tree-dynamic-object-size: Handle function parameters To: Siddhesh Poyarekar , gcc-patches@gcc.gnu.org Cc: jakub@redhat.com References: <20211007221432.1029249-1-siddhesh@gotplt.org> <20211007221432.1029249-7-siddhesh@gotplt.org> From: Martin Sebor Message-ID: <44de9395-0a81-9680-a977-231ba60ae4c7@gmail.com> Date: Wed, 20 Oct 2021 10:56:51 -0600 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.2.2 MIME-Version: 1.0 In-Reply-To: <20211007221432.1029249-7-siddhesh@gotplt.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-11.2 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 20 Oct 2021 16:57:01 -0000 On 10/7/21 4:14 PM, Siddhesh Poyarekar wrote: > Handle either static sizes in function parameters or hints provided by > __attribute__ ((access (...))) to compute sizes for objects. It's been my hope to eventually teach __builtin_object_size about attribute access but implementing it in the new built-in might be preferable. Glad to see you noticed it and took advantage of it! Does this include handling "VLA function parameters" as in void f (int n, char d[n]); // (or char d[static n]) I don't see tests for it in this patch but since internally, GCC describes VLA (and array) function arguments using attribute access hanndling it should automatically give us VLA (and array) support as well unless we disable it, either intentionally or by accident. Either way, I would recommend adding tests for VLA parameters. Martin > > gcc/ChangeLog: > > * tree-dynamic-object-size.c: Include tree-dfa.h. > (emit_size_stmts): New argument osi. Handle GIMPLE_NOP. > (eval_size_expr, gimplify_size_exprs): Adjust. > (parm_object_size): New function. > (collect_object_sizes_for): Handle GIMPLE_NOP. > > gcc/testsuite/ChangeLog: > > * gcc.dg/builtin-dynamic-object-size-0.c (test_parmsz): New > test. > (main): Call it. > > Signed-off-by: Siddhesh Poyarekar > --- > .../gcc.dg/builtin-dynamic-object-size-0.c | 22 +++++ > gcc/tree-dynamic-object-size.c | 98 +++++++++++++++++-- > 2 files changed, 112 insertions(+), 8 deletions(-) > > diff --git a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-0.c b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-0.c > index 3c2c4c84264..c72fa0508db 100644 > --- a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-0.c > +++ b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-0.c > @@ -255,6 +255,15 @@ test_substring_ptrplus (size_t sz, size_t off) > return __builtin_dynamic_object_size (str + off, 0); > } > > +size_t > +__attribute__ ((noinline)) I think attribute noipa might be a better choice if the goal is to keep GCC from sneaking in data from the caller and so defeating the purpose of the test. Martin > +__attribute__ ((access (__read_write__, 1, 2))) > +test_parmsz (void *obj, size_t sz, size_t off) > +{ > + return __builtin_dynamic_object_size (obj + off, 0); > +} > + > + > int > main (int argc, char **argv) > { > @@ -338,6 +347,19 @@ main (int argc, char **argv) > if (test_deploop (128, 129) != 32) > FAIL (); > > + if (test_parmsz (argv[0], __builtin_strlen (argv[0]) + 1, -1)!= 0) > + FAIL (); > + > + if (test_parmsz (argv[0], __builtin_strlen (argv[0]) + 1, 0) > + != __builtin_strlen (argv[0]) + 1) > + FAIL (); > + if (test_parmsz (argv[0], __builtin_strlen (argv[0]) + 1, > + __builtin_strlen (argv[0]))!= 1) > + FAIL (); > + if (test_parmsz (argv[0], __builtin_strlen (argv[0]) + 1, > + __builtin_strlen (argv[0]) + 2)!= 0) > + FAIL (); > + > if (nfails > 0) > __builtin_abort (); > > diff --git a/gcc/tree-dynamic-object-size.c b/gcc/tree-dynamic-object-size.c > index f143a64777c..8d7283623dc 100644 > --- a/gcc/tree-dynamic-object-size.c > +++ b/gcc/tree-dynamic-object-size.c > @@ -58,6 +58,7 @@ along with GCC; see the file COPYING3. If not see > #include "gimple-fold.h" > #include "gimple-iterator.h" > #include "tree-cfg.h" > +#include "tree-dfa.h" > #include "stringpool.h" > #include "attribs.h" > #include "builtins.h" > @@ -456,7 +457,7 @@ pass_through_call (const gcall *call) > > > static void > -emit_size_stmts (gimple *stmt, tree wholesize_ssa, > +emit_size_stmts (object_size_info *osi, gimple *stmt, tree wholesize_ssa, > tree wholesize_expr, tree size_ssa, tree size_expr) > { > gimple_seq seq = NULL; > @@ -481,7 +482,14 @@ emit_size_stmts (gimple *stmt, tree wholesize_ssa, > statements involved in evaluation of the object size expression precede > the definition statement. For parameters, we don't have a definition > statement, so insert into the first code basic block. */ > - gimple_stmt_iterator i = gsi_for_stmt (stmt); > + gimple_stmt_iterator i; > + if (gimple_code (stmt) == GIMPLE_NOP) > + { > + basic_block first_bb = single_succ (ENTRY_BLOCK_PTR_FOR_FN (osi->fun)); > + i = gsi_start_bb (first_bb); > + } > + else > + i = gsi_for_stmt (stmt); > gsi_insert_seq_before (&i, seq, GSI_CONTINUE_LINKING); > } > > @@ -542,8 +550,8 @@ size_bound_expr (tree sz) > } > > static void > -eval_size_expr (tree var, tree wholesize, tree *wholesize_expr, > - tree size, tree *size_expr) > +eval_size_expr (struct object_size_info *osi, tree var, tree wholesize, > + tree *wholesize_expr, tree size, tree *size_expr) > { > if (size_expr != NULL) > { > @@ -560,7 +568,7 @@ eval_size_expr (tree var, tree wholesize, tree *wholesize_expr, > } > else > { > - emit_size_stmts (stmt, wholesize, *wholesize_expr, size, > + emit_size_stmts (osi, stmt, wholesize, *wholesize_expr, size, > size_bound_expr (*size_expr)); > delete wholesize_expr; > delete size_expr; > @@ -611,7 +619,7 @@ gimplify_size_exprs (object_size_info *osi, tree ptr) > for (int object_size_type = 0; object_size_type <= 3; object_size_type++) > EXECUTE_IF_SET_IN_BITMAP (computed[object_size_type], 0, i, bi) > { > - eval_size_expr (ssa_name (i), > + eval_size_expr (osi, ssa_name (i), > object_whole_sizes[object_size_type][i], > object_whole_size_exprs[object_size_type][i], > object_sizes[object_size_type][i], > @@ -939,6 +947,71 @@ call_object_size (struct object_size_info *osi, tree ptr, gcall *call) > cache_object_size_copy (osi, SSA_NAME_VERSION (ptr), sz, sz); > } > > +/* Find size of an object passed as a parameter to the function. */ > + > +static void > +parm_object_size (struct object_size_info *osi, tree var) > +{ > + tree parm = SSA_NAME_VAR (var); > + unsigned varno = SSA_NAME_VERSION (var); > + tree sz = NULL_TREE, wholesz = NULL_TREE; > + > + if (dump_file) > + { > + fprintf (dump_file, "Object is a parameter: "); > + print_generic_expr (dump_file, parm, dump_flags); > + fprintf (dump_file, " which is %s a pointer type\n", > + POINTER_TYPE_P (TREE_TYPE (parm)) ? "" : "not"); > + } > + > + if (POINTER_TYPE_P (TREE_TYPE (parm))) > + { > + /* Look for access attribute. */ > + rdwr_map rdwr_idx; > + > + tree fndecl = osi->fun->decl; > + const attr_access *access = get_parm_access (rdwr_idx, parm, fndecl); > + tree typesize = TYPE_SIZE_UNIT (TREE_TYPE (TREE_TYPE (parm))); > + > + if (access && access->sizarg != UINT_MAX) > + { > + tree fnargs = DECL_ARGUMENTS (fndecl); > + tree arg = NULL_TREE; > + unsigned argpos = 0; > + > + /* Walk through the parameters to pick the size parameter and safely > + scale it by the type size. */ > + for (arg = fnargs; argpos != access->sizarg && arg; > + arg = TREE_CHAIN (arg), ++argpos); > + > + if (arg != NULL_TREE && INTEGRAL_TYPE_P (TREE_TYPE (arg))) > + { > + tree arrsz = get_or_create_ssa_default_def (osi->fun, arg); > + if (arrsz != NULL_TREE) > + { > + arrsz = fold_convert (sizetype, arrsz); > + if (typesize) > + { > + tree res = fold_build2 (MULT_EXPR, sizetype, arrsz, > + typesize); > + tree check = fold_build2 (EXACT_DIV_EXPR, sizetype, > + TYPE_MAX_VALUE (sizetype), > + typesize); > + check = fold_build2 (LT_EXPR, sizetype, arrsz, check); > + arrsz = fold_build3 (COND_EXPR, sizetype, check, res, > + size_zero_node); > + } > + } > + sz = wholesz = arrsz; > + } > + } > + } > + else > + expr_object_size (osi, parm, &sz, &wholesz); > + > + cache_object_size_copy (osi, varno, sz, wholesz); > +} > + > /* Compute object sizes for VAR. > > - For allocation GIMPLE_CALL like malloc or calloc object size is the size > @@ -948,7 +1021,9 @@ call_object_size (struct object_size_info *osi, tree ptr, gcall *call) > - For GIMPLE_PHI, compute a PHI node with sizes of all branches in the PHI > node of the object. > - For GIMPLE_ASSIGN, return the size of the object the SSA name points > - to. */ > + to. > + - For GIMPLE_NOP, if the variable is a function parameter, compute the size > + of the parameter. */ > > static void > collect_object_sizes_for (struct object_size_info *osi, tree var) > @@ -1076,8 +1151,15 @@ collect_object_sizes_for (struct object_size_info *osi, tree var) > break; > } > > - /* Bail out for all other cases. */ > case GIMPLE_NOP: > + if (SSA_NAME_VAR (var) && TREE_CODE (SSA_NAME_VAR (var)) == PARM_DECL) > + parm_object_size (osi, var); > + else > + /* Uninitialized SSA names point nowhere. */ > + cache_object_size_copy (osi, varno, NULL_TREE, NULL_TREE); > + break; > + > + /* Bail out for all other cases. */ > case GIMPLE_ASM: > cache_object_size_copy (osi, varno, NULL_TREE, NULL_TREE); > break; >