On 10/01/2022 16:20, Jakub Jelinek wrote:
On Sat, Dec 18, 2021 at 06:05:09PM +0530, Siddhesh Poyarekar wrote:
@@ -1440,6 +1441,53 @@ cond_expr_object_size (struct object_size_info *osi,
tree var, gimple *stmt)
return reexamine;
}
+/* Find size of an object passed as a parameter to the function. */
+
+static void
+parm_object_size (struct object_size_info *osi, tree var)
+{
+ int object_size_type = osi->object_size_type;
+ tree parm = SSA_NAME_VAR (var);
+
+ if (!(object_size_type & OST_DYNAMIC) || !POINTER_TYPE_P (TREE_TYPE (parm)))
+ expr_object_size (osi, var, parm);
This looks very suspicious. Didn't you mean { expr_object_size (...); return;
} here?
Because the code below e.g. certainly assumes OST_DYNAMIC and that TREE_TYPE
(parm)
is a pointer type (otherwise TREE_TYPE (TREE_TYPE (...) wouldn't work.
Indeed, fixed.
+
+ /* Look for access attribute. */
+ rdwr_map rdwr_idx;
+
+ tree fndecl = cfun->decl;
+ const attr_access *access = get_parm_access (rdwr_idx, parm, fndecl);
+ tree typesize = TYPE_SIZE_UNIT (TREE_TYPE (TREE_TYPE (parm)));
+ tree sz = NULL_TREE;
+
+ if (access && access->sizarg != UINT_MAX)
Perhaps && typesize here? It makes no sense to e.g. create ssa default def
when you aren't going to use it in any way.
The typesize is only for scaling; the result of
get_or_create_ssa_default_def should get returned unscaled if it is
non-NULL and typesize is NULL; the latter happens when the type is void *:
sz = get_or_create_ssa_default_def (cfun, arg);
if (sz != NULL_TREE)
{
sz = fold_convert (sizetype, sz);
if (typesize)
sz = size_binop (MULT_EXPR, sz, typesize);
}
}
+ {
+ 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);
Instead of a loop with empty body wouldn't it be better to
do the work in that for loop?
I.e. take argpos != access->sizarg && from the condition,
replace arg != NULL_TREE with that argpos == access->sizarg
and add a break;?
Fixed.
+
+ if (arg != NULL_TREE && INTEGRAL_TYPE_P (TREE_TYPE (arg)))
+ {
+ sz = get_or_create_ssa_default_def (cfun, arg);
Also, I must say I'm little bit worried about this
get_or_create_ssa_default_def call. If the SSA_NAME doesn't exist,
so you create it and then attempt to use it but in the end don't
because e.g. some PHI's another argument was unknown etc., will
that SSA_NAME be released through release_ssa_name?
I think GIMPLE is fairly unhappy if there are SSA_NAMEs created and not
released that don't appear in the IL anywhere.
AFAICT, set_ss_default_def ends up creating a definition for the new
SSA_NAME it creates, so it does end up in the IR and in case of object
size computation failure, it just ends up being a dead store. I've
added a test to verify this:
size_t
__attribute__ ((access (__read_write__, 1, 3)))
__attribute__ ((noinline))
test_parmsz_unknown (void *obj, void *unknown, size_t sz, int cond)
{
return __builtin_dynamic_object_size (cond ? obj : unknown, 0);
}
which works as expected and returns -1.
Thanks,
Siddhesh
