From patchwork Thu Oct 7 22:14:24 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Siddhesh Poyarekar X-Patchwork-Id: 45971 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 3A93C3857C70 for ; Thu, 7 Oct 2021 22:15:04 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from aye.elm.relay.mailchannels.net (aye.elm.relay.mailchannels.net [23.83.212.6]) by sourceware.org (Postfix) with ESMTPS id 15F263858428 for ; Thu, 7 Oct 2021 22:14:45 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 15F263858428 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=gotplt.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gotplt.org X-Sender-Id: dreamhost|x-authsender|siddhesh@gotplt.org Received: from relay.mailchannels.net (localhost [127.0.0.1]) by relay.mailchannels.net (Postfix) with ESMTP id 68E1D6822F2; Thu, 7 Oct 2021 22:14:44 +0000 (UTC) Received: from pdx1-sub0-mail-a17.g.dreamhost.com (100-96-133-192.trex.outbound.svc.cluster.local [100.96.133.192]) (Authenticated sender: dreamhost) by relay.mailchannels.net (Postfix) with ESMTPA id EC4CB68226A; Thu, 7 Oct 2021 22:14:43 +0000 (UTC) X-Sender-Id: dreamhost|x-authsender|siddhesh@gotplt.org Received: from pdx1-sub0-mail-a17.g.dreamhost.com (pop.dreamhost.com [64.90.62.162]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384) by 100.96.133.192 (trex/6.4.3); Thu, 07 Oct 2021 22:14:44 +0000 X-MC-Relay: Neutral X-MailChannels-SenderId: dreamhost|x-authsender|siddhesh@gotplt.org X-MailChannels-Auth-Id: dreamhost X-Sponge-Robust: 54789f5e249185a0_1633644884237_4205420698 X-MC-Loop-Signature: 1633644884237:2826253584 X-MC-Ingress-Time: 1633644884237 Received: from pdx1-sub0-mail-a17.g.dreamhost.com (localhost [127.0.0.1]) by pdx1-sub0-mail-a17.g.dreamhost.com (Postfix) with ESMTP id 9839083495; Thu, 7 Oct 2021 15:14:43 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gotplt.org; h=from:to:cc :subject:date:message-id:mime-version:content-transfer-encoding; s=gotplt.org; bh=jbbokFL3DE0CcFLYK4jvVoBjQlc=; b=of2rHBNpOLDgca Thcfmpx4ML76Xol+cAv3WNiwtWcJ9lsOdn383NfoMF7xJjjcIndl2fHOc8lnDhL7 d7PIGtY+XGFJyQD6dnLREVXSgOGx2FvtlFmmap+bELSM/FuL8J2Q5kN+I+8G2auX kK1ukTKPRL+AkMVzSlk/DNruUVtKA= Received: from rhbox.redhat.com (unknown [1.186.223.58]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) (Authenticated sender: siddhesh@gotplt.org) by pdx1-sub0-mail-a17.g.dreamhost.com (Postfix) with ESMTPSA id 85E468348D; Thu, 7 Oct 2021 15:14:40 -0700 (PDT) X-DH-BACKEND: pdx1-sub0-mail-a17 From: Siddhesh Poyarekar To: gcc-patches@gcc.gnu.org Subject: [PATCH 0/8] __builtin_dynamic_object_size and more Date: Fri, 8 Oct 2021 03:44:24 +0530 Message-Id: <20211007221432.1029249-1-siddhesh@gotplt.org> X-Mailer: git-send-email 2.31.1 MIME-Version: 1.0 X-Spam-Status: No, score=-3032.8 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, 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: , Cc: jakub@redhat.com Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" This patchset implements the __builtin_dynamic_object_size builtin for gcc. The primary motivation to have this builtin in gcc is to enable _FORTIFY_SOURCE=3 support with gcc, thus allowing greater fortification in use cases where the potential performance tradeoff is acceptable. Semantics: ---------- __builtin_dynamic_object_size has the same signature as __builtin_object_size; it accepts a pointer and type ranging from 0 to 3 and it returns an object size estimate for the pointer based on an analysis of which objects the pointer could point to. The actual properties of the object size estimate are different: - In the best case __builtin_dynamic_object_size evaluates to an expression that represents a precise size of the object being pointed to. - In case a precise object size expression cannot be evaluated, __builtin_dynamic_object_size attempts to evaluate an estimate size expression based on the object size type. - In what situations the builtin returns an estimate vs a precise expression is an implementation detail and may change in future. Users must always assume, as in the case of __builtin_object_size, that the returned value is the maximum or minimum based on the object size type they have provided. - In the worst case of failure, __builtin_dynamic_object_size returns a constant (size_t)-1 or (size_t)0. - If __builtin_dynamic_object_size returns a non-constant expression, the value of that expression is never (size_t)-1. Implementation: --------------- - The __builtin_dynamic_object_size support is implemented in tree-dynamic-object-size.c as two passes, just like tree-object-size. In most cases the first pass (early_dynobjsz) is able to evaluate object size expressions. The second phase mainly ends up simplifying the __builtin_dynamic_object_size to __builtin_object_size. Speaking of which... - Currently if __builtin_dynamic_object_size fails to evaluate a precise size expression, it replaces the call with __builtin_object_size and punts to the last tree-object-size pass. In future, this could become more sophisticated. - Size expressions returned directly by __builtin_dynamic_object_size are bounded in the range of [0, SIZE_MAX - 1] so that equality tests with (size_t)-1 fail. This is necessary to eliminate unnecessary branches in _FORTIFY_SOURCE=3. This could be tightened further in future to SIZE_MAX / 2 since there are already assumptions built in that prevent object sizes from crossing that limit. - I have split the implementation into one feature per patchset (calls, function parameters, PHI, etc.) to hopefully ease review. - Some code duplication from tree-object-size is intentional. The immediate need for that is to reduce impact on existing passes. Over the medium/long term though, the intention is to fully replace tree-object-size. See Future work for more information. - I have marked the new files as Copyright (C) me (since I'm contributing under DCO), but I've based some of the code on the existing tree-object-size.c, so I need advice on the correct copyright notice. Performance: ------------ Expressions generated by this pass in theory could be arbitrarily complex. I have not made an attempt to limit nesting of objects since it seemed too early to do that. In practice based on the few applications I built, most of the complexity of the expressions got folded away. Even so, the performance overhead is likely to be non-zero. If we find performance degradation to be significant, we could later add nesting limits to bail out if a size expression gets too complex. I have also not implemented simplification of __*_chk to their normal variants if we can determine at compile time that it is safe. I am working on that right now and hope to have it ready before stage 1 closes. Testing: -------- I have added tests for dynamic object sizes as well as wrappers for all __builtin_object_size tests to provide wide coverage. With that in place I have run full bootstrap builds on Fedora rawhide by backporting the patches to the gcc11 branch and x86_64 and i686 have no failures in any of the builtin-*object-size* tests and no new failures. I have also built bash, cmake, zfs-fuse and systemtap with _FORTIFY_SOURCE=3 (with a hacked up glibc to make sure it works) and saw no issues in any of those builds. I did some rudimentary analysis of the generated binaries to see if there was any difference in coverage and found that there was. In terms of pure numbers, there were far more _chk variants of calls than the regular ones due to _FORTIFY_SOURCE (from about 4% to 70% in bash), but that could well be due to the _chk variants not being folded into regular variants when safe. However, on manual inspection of some of these sites, it was clear that coverage was increasing significantly where __builtin_object_size was previously bailing out. I'm running more tests with _FORTIFY_SOURCE=3 and will do more analysis to quantify the coverage improvement more clearly. Limitations/Future work: - The most immediate work is to fold _chk variants of builtins into regular ones when it can be proven at compile time that the object size will alwasy be less than the length of the write. I am working on it right now. - I need to enable _FORTIFY_SOURCE=3 for gcc in glibc; currently it is llvm-only. I've started working on these patches too on the side. - AFAICT, there are no cases where __builtin_dynamic_object_size misses and __builtin_object_size gets a non-fail estimate. I have still kept the backstop in place since this is a new pass and it's very likely that I've missed something. It should be possible to use ranger to get an upper and lower bound on the size expression computed by __builtin_dynamic_object_size and use that to implement __builtin_object_size. In practice though, ranger currently seems to have some difficulty evaluating ranges for some PHI objects. I intend to look into that a bit more once this pass has settled in. - More work could to be done to reduce the performance impact of the computation. One way could be to add a heuristic where the pass keeps track of nesting in the expression and either bail out or compute an estimate if nesting crosses a threshold. I'll take this up once we have more data on the nature of the bottlenecks. Siddhesh Poyarekar (8): __builtin_dynamic_object_size: Recognize builtin name tree-dynamic-object-size: New pass tree-dynamic-object-size: Handle GIMPLE_PHI tree-dynamic-object-size: Support ADDR_EXPR tree-dynamic-object-size: Handle GIMPLE_ASSIGN tree-dynamic-object-size: Handle function parameters tree-dynamic-object-size: Get subobject sizes tree-dynamic-object-size: Add test wrappers to extend testing gcc/Makefile.in | 19 +- gcc/builtins.c | 71 +- gcc/builtins.def | 1 + gcc/doc/extend.texi | 11 + gcc/passes.def | 3 + .../g++.dg/ext/builtin-dynamic-object-size1.C | 5 + .../g++.dg/ext/builtin-dynamic-object-size2.C | 5 + .../gcc.dg/builtin-dynamic-alloc-size.c | 7 + .../gcc.dg/builtin-dynamic-object-size-0.c | 401 +++++ .../gcc.dg/builtin-dynamic-object-size-1.c | 7 + .../gcc.dg/builtin-dynamic-object-size-10.c | 9 + .../gcc.dg/builtin-dynamic-object-size-11.c | 7 + .../gcc.dg/builtin-dynamic-object-size-12.c | 5 + .../gcc.dg/builtin-dynamic-object-size-13.c | 5 + .../gcc.dg/builtin-dynamic-object-size-14.c | 5 + .../gcc.dg/builtin-dynamic-object-size-15.c | 6 + .../gcc.dg/builtin-dynamic-object-size-16.c | 7 + .../gcc.dg/builtin-dynamic-object-size-17.c | 8 + .../gcc.dg/builtin-dynamic-object-size-18.c | 8 + .../gcc.dg/builtin-dynamic-object-size-19.c | 104 ++ .../gcc.dg/builtin-dynamic-object-size-2.c | 7 + .../gcc.dg/builtin-dynamic-object-size-3.c | 7 + .../gcc.dg/builtin-dynamic-object-size-4.c | 7 + .../gcc.dg/builtin-dynamic-object-size-6.c | 5 + .../gcc.dg/builtin-dynamic-object-size-7.c | 5 + .../gcc.dg/builtin-dynamic-object-size-8.c | 5 + .../gcc.dg/builtin-dynamic-object-size-9.c | 5 + gcc/testsuite/gcc.dg/builtin-object-size-1.c | 110 ++ gcc/testsuite/gcc.dg/builtin-object-size-16.c | 10 + gcc/testsuite/gcc.dg/builtin-object-size-17.c | 2 + gcc/testsuite/gcc.dg/builtin-object-size-2.c | 126 ++ gcc/testsuite/gcc.dg/builtin-object-size-3.c | 148 ++ gcc/testsuite/gcc.dg/builtin-object-size-4.c | 117 ++ gcc/tree-dynamic-object-size.c | 1513 +++++++++++++++++ gcc/tree-dynamic-object-size.h | 26 + gcc/tree-pass.h | 2 + 36 files changed, 2769 insertions(+), 20 deletions(-) create mode 100644 gcc/testsuite/g++.dg/ext/builtin-dynamic-object-size1.C create mode 100644 gcc/testsuite/g++.dg/ext/builtin-dynamic-object-size2.C create mode 100644 gcc/testsuite/gcc.dg/builtin-dynamic-alloc-size.c create mode 100644 gcc/testsuite/gcc.dg/builtin-dynamic-object-size-0.c create mode 100644 gcc/testsuite/gcc.dg/builtin-dynamic-object-size-1.c create mode 100644 gcc/testsuite/gcc.dg/builtin-dynamic-object-size-10.c create mode 100644 gcc/testsuite/gcc.dg/builtin-dynamic-object-size-11.c create mode 100644 gcc/testsuite/gcc.dg/builtin-dynamic-object-size-12.c create mode 100644 gcc/testsuite/gcc.dg/builtin-dynamic-object-size-13.c create mode 100644 gcc/testsuite/gcc.dg/builtin-dynamic-object-size-14.c create mode 100644 gcc/testsuite/gcc.dg/builtin-dynamic-object-size-15.c create mode 100644 gcc/testsuite/gcc.dg/builtin-dynamic-object-size-16.c create mode 100644 gcc/testsuite/gcc.dg/builtin-dynamic-object-size-17.c create mode 100644 gcc/testsuite/gcc.dg/builtin-dynamic-object-size-18.c create mode 100644 gcc/testsuite/gcc.dg/builtin-dynamic-object-size-19.c create mode 100644 gcc/testsuite/gcc.dg/builtin-dynamic-object-size-2.c create mode 100644 gcc/testsuite/gcc.dg/builtin-dynamic-object-size-3.c create mode 100644 gcc/testsuite/gcc.dg/builtin-dynamic-object-size-4.c create mode 100644 gcc/testsuite/gcc.dg/builtin-dynamic-object-size-6.c create mode 100644 gcc/testsuite/gcc.dg/builtin-dynamic-object-size-7.c create mode 100644 gcc/testsuite/gcc.dg/builtin-dynamic-object-size-8.c create mode 100644 gcc/testsuite/gcc.dg/builtin-dynamic-object-size-9.c create mode 100644 gcc/tree-dynamic-object-size.c create mode 100644 gcc/tree-dynamic-object-size.h