From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id 3B63E3858299 for ; Thu, 22 Sep 2022 13:02:29 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 3B63E3858299 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1663851748; h=from:from:reply-to:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:in-reply-to:in-reply-to: references:references; bh=hlsx8KNl4J/mW3WhU59ODuWVz1+qOAACprAvw6WEaEA=; b=NSDuh7MGma69MKbt73y4LcgmbOd08LCF6OdY9/COR7NjIPYbvYQtQJen/GCt93fFJfTd6e pQbY/iigu7zojLnGrg0h9Yr3+uiyPv3/GlRI+B2PNnDrVEZDhmBAICxNZO/vGzM2ALi366 ppbm6x6khXZOFkWY6BOLuHkfqUE/X5Y= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-624-UuePvtKaMDCHQvVGEyqKZQ-1; Thu, 22 Sep 2022 09:02:26 -0400 X-MC-Unique: UuePvtKaMDCHQvVGEyqKZQ-1 Received: from smtp.corp.redhat.com (int-mx10.intmail.prod.int.rdu2.redhat.com [10.11.54.10]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 68EF4101A52A; Thu, 22 Sep 2022 13:02:25 +0000 (UTC) Received: from tucnak.zalov.cz (unknown [10.39.192.194]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 4D5F8492B1B; Thu, 22 Sep 2022 13:02:24 +0000 (UTC) Received: from tucnak.zalov.cz (localhost [127.0.0.1]) by tucnak.zalov.cz (8.17.1/8.17.1) with ESMTPS id 28MD2Lp22547352 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NOT); Thu, 22 Sep 2022 15:02:21 +0200 Received: (from jakub@localhost) by tucnak.zalov.cz (8.17.1/8.17.1/Submit) id 28MD2Kf42547350; Thu, 22 Sep 2022 15:02:20 +0200 Date: Thu, 22 Sep 2022 15:02:20 +0200 From: Jakub Jelinek To: Siddhesh Poyarekar Cc: gcc-patches@gcc.gnu.org Subject: Re: [PATCH] tree-object-size: Support strndup and strdup Message-ID: Reply-To: Jakub Jelinek References: <20220815192311.763473-1-siddhesh@gotplt.org> MIME-Version: 1.0 In-Reply-To: <20220815192311.763473-1-siddhesh@gotplt.org> X-Scanned-By: MIMEDefang 3.1 on 10.11.54.10 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=us-ascii Content-Disposition: inline X-Spam-Status: No, score=-4.0 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_LOW,SPF_HELO_NONE,SPF_NONE,TXREP 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 Mon, Aug 15, 2022 at 03:23:11PM -0400, Siddhesh Poyarekar wrote: > --- a/gcc/tree-object-size.cc > +++ b/gcc/tree-object-size.cc > @@ -495,6 +495,18 @@ decl_init_size (tree decl, bool min) > return size; > } > > +/* Get the outermost object that PTR may point into. */ > + > +static tree > +get_whole_object (const_tree ptr) > +{ > + tree pt_var = TREE_OPERAND (ptr, 0); > + while (handled_component_p (pt_var)) > + pt_var = TREE_OPERAND (pt_var, 0); > + > + return pt_var; > +} Not sure why you want a new function for this. This is essentially get_base_address (TREE_OPERAND (ptr, 0)). > /* Compute __builtin_object_size for PTR, which is a ADDR_EXPR. > OBJECT_SIZE_TYPE is the second argument from __builtin_object_size. > If unknown, return size_unknown (object_size_type). */ > + if (!size_valid_p (sz, object_size_type) > + || size_unknown_p (sz, object_size_type)) > + { > + tree wholesrc = NULL_TREE; > + if (TREE_CODE (src) == ADDR_EXPR) > + wholesrc = get_whole_object (src); > + > + if (!(object_size_type & OST_MINIMUM) > + || (wholesrc && TREE_CODE (wholesrc) == STRING_CST)) Is this safe? I mean get_whole_object will also skip ARRAY_REFs with variable indexes etc. and the STRING_CST could have embedded '\0's in it. Even if c_strlen (src, 1) is constant, I don't see what you can assume for object size of strndup ("abcd\0efgh", n); for minimum, except 1. But on the other side, 1 is a safe minimum for OST_MINIMUM of both strdup and strndup if you don't find anything more specific (exact strlen for strndup) because the terminating '\0' will be always there. Other than that you'd need to consider INTEGER_CST second strndup argument or ranges of the second argument etc. E.g. maximum for OST_DYNAMIC could be for strndup (src, n) MIN (__bdos (src, ?), n + 1). > @@ -2113,7 +2177,7 @@ const pass_data pass_data_object_sizes = > PROP_objsz, /* properties_provided */ > 0, /* properties_destroyed */ > 0, /* todo_flags_start */ > - 0, /* todo_flags_finish */ > + TODO_update_ssa_no_phi, /* todo_flags_finish */ > }; > > class pass_object_sizes : public gimple_opt_pass > @@ -2153,7 +2217,7 @@ const pass_data pass_data_early_object_sizes = > 0, /* properties_provided */ > 0, /* properties_destroyed */ > 0, /* todo_flags_start */ > - 0, /* todo_flags_finish */ > + TODO_update_ssa_no_phi, /* todo_flags_finish */ > }; This is quite expensive. Do you really need full ssa update, or just TODO_update_ssa_only_virtuals would be enough (is it for the missing vuse on the strlen call if you emit it)? In any case, would be better not to do that always, but only if you really need it (emitted the strlen call somewhere; e.g. if __bdos is never used, only __bos, it is certainly not needed), todo flags can be both in todo_flags_finish and in return value from execute method. Jakub