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 1E61E385801E for ; Tue, 18 Jan 2022 08:19:00 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 1E61E385801E Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-192-OnplWC0dNem3AlN4kT_SIw-1; Tue, 18 Jan 2022 03:18:54 -0500 X-MC-Unique: OnplWC0dNem3AlN4kT_SIw-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 52D9C190B2A0; Tue, 18 Jan 2022 08:18:53 +0000 (UTC) Received: from tucnak.zalov.cz (unknown [10.40.192.74]) by smtp.corp.redhat.com (Postfix) with ESMTPS id B434860C5A; Tue, 18 Jan 2022 08:18:52 +0000 (UTC) Received: from tucnak.zalov.cz (localhost [127.0.0.1]) by tucnak.zalov.cz (8.16.1/8.16.1) with ESMTPS id 20I8IouD3059399 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NOT); Tue, 18 Jan 2022 09:18:50 +0100 Received: (from jakub@localhost) by tucnak.zalov.cz (8.16.1/8.16.1/Submit) id 20I8Io1K3059398; Tue, 18 Jan 2022 09:18:50 +0100 Date: Tue, 18 Jan 2022 09:18:50 +0100 From: Jakub Jelinek To: Andrew Pinski Cc: Jan Hubicka , Andrew Pinski , GCC Patches Subject: Re: [PATCH] Fix tree-optimization/101941: IPA splitting out function with error attribute Message-ID: <20220118081850.GT2646553@tucnak> Reply-To: Jakub Jelinek References: <1637130270-3920-1-git-send-email-apinski@marvell.com> <20220117183512.GO2646553@tucnak> MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 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=-5.5 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_NONE, 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: Tue, 18 Jan 2022 08:19:01 -0000 On Mon, Jan 17, 2022 at 09:08:22PM -0800, Andrew Pinski wrote: > > Perhaps even > > /* Check builtins that prevent splitting. */ > > if (gimple_code (stmt) == GIMPLE_CALL) > > if (tree decl = gimple_call_fndecl (stmt)) > > { > > if (fndecl_built_in_p (decl, BUILT_IN_NORMAL)) > > ... > > if (lookup_attribute || lookup_attribute) > > ... > > } > > ? > > Yes that looks good, I am just finished up the patch and going to run > a full bootstrap/test to make sure I didn't mess up. > I had originally trying not to reindent the code to make it easier to > see what was being added but I think re-indentation is correct after > all. Here is what I've bootstrapped/regtested successfully on x86_64-linux and i686-linux. Note, besides the reformatting I've changed the wording of the added comment. 2022-01-18 Andrew Pinski PR tree-optimization/101941 * ipa-split.c (visit_bb): Disallow function calls where the function has either error or warning attribute. * gcc.c-torture/compile/pr101941.c: New test. * gcc.dg/tree-ssa/pr101941.c: New test. --- gcc/ipa-split.c.jj 2022-01-11 23:11:22.664286304 +0100 +++ gcc/ipa-split.c 2022-01-17 19:40:23.837234404 +0100 @@ -873,7 +873,6 @@ visit_bb (basic_block bb, basic_block re gimple *stmt = gsi_stmt (bsi); tree op; ssa_op_iter iter; - tree decl; if (is_gimple_debug (stmt)) continue; @@ -900,31 +899,45 @@ visit_bb (basic_block bb, basic_block re } /* Check builtins that prevent splitting. */ - if (gimple_code (stmt) == GIMPLE_CALL - && (decl = gimple_call_fndecl (stmt)) != NULL_TREE - && fndecl_built_in_p (decl, BUILT_IN_NORMAL)) - switch (DECL_FUNCTION_CODE (decl)) + if (gimple_code (stmt) == GIMPLE_CALL) + if (tree decl = gimple_call_fndecl (stmt)) { - /* FIXME: once we will allow passing non-parm values to split part, - we need to be sure to handle correct builtin_stack_save and - builtin_stack_restore. At the moment we are safe; there is no - way to store builtin_stack_save result in non-SSA variable - since all calls to those are compiler generated. */ - case BUILT_IN_APPLY: - case BUILT_IN_APPLY_ARGS: - case BUILT_IN_VA_START: - if (dump_file && (dump_flags & TDF_DETAILS)) - fprintf (dump_file, - "Cannot split: builtin_apply and va_start.\n"); - can_split = false; - break; - case BUILT_IN_EH_POINTER: - if (dump_file && (dump_flags & TDF_DETAILS)) - fprintf (dump_file, "Cannot split: builtin_eh_pointer.\n"); - can_split = false; - break; - default: - break; + if (fndecl_built_in_p (decl, BUILT_IN_NORMAL)) + switch (DECL_FUNCTION_CODE (decl)) + { + /* FIXME: once we will allow passing non-parm values to + split part, we need to be sure to handle correct + builtin_stack_save and builtin_stack_restore. At the + moment we are safe; there is no way to store + builtin_stack_save result in non-SSA variable + since all calls to those are compiler generated. */ + case BUILT_IN_APPLY: + case BUILT_IN_APPLY_ARGS: + case BUILT_IN_VA_START: + if (dump_file && (dump_flags & TDF_DETAILS)) + fprintf (dump_file, + "Cannot split: builtin_apply and va_start.\n"); + can_split = false; + break; + case BUILT_IN_EH_POINTER: + if (dump_file && (dump_flags & TDF_DETAILS)) + fprintf (dump_file, "Cannot split: builtin_eh_pointer.\n"); + can_split = false; + break; + default: + break; + } + + /* If there is a function call and that function has either the + warning or error attribute on it, don't split. */ + if (lookup_attribute ("warning", DECL_ATTRIBUTES (decl)) + || lookup_attribute ("error", DECL_ATTRIBUTES (decl))) + { + if (dump_file && (dump_flags & TDF_DETAILS)) + fprintf (dump_file, "Cannot split: warning or error " + "attribute.\n"); + can_split = false; + } } FOR_EACH_SSA_TREE_OPERAND (op, stmt, iter, SSA_OP_DEF) --- gcc/testsuite/gcc.dg/tree-ssa/pr101941.c.jj 2022-01-17 19:37:01.609066831 +0100 +++ gcc/testsuite/gcc.dg/tree-ssa/pr101941.c 2022-01-17 19:37:01.609066831 +0100 @@ -0,0 +1,48 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fconserve-stack -fdump-tree-optimized" } */ +struct crypto_aes_ctx { + char key_dec[128]; +}; + +int rfc4106_set_hash_subkey_hash_subkey; + +void __write_overflow(void)__attribute__((__error__(""))); +void __write_overflow1(void); +void aes_encrypt(void*); + +void fortify_panic(const char*) __attribute__((__noreturn__)) ; + +char *rfc4106_set_hash_subkey(struct crypto_aes_ctx *ctx) { + void *a = &ctx->key_dec[0]; + unsigned p_size = __builtin_object_size(a, 0); +#ifdef __OPTIMIZE__ + if (p_size < 16) { + __write_overflow1(); + fortify_panic(__func__); + } + if (p_size < 32) { + __write_overflow(); + fortify_panic(__func__); + } +#endif + aes_encrypt(ctx); + return ctx->key_dec; +} + +char *(*gg)(struct crypto_aes_ctx *) = rfc4106_set_hash_subkey; + +void a(void) +{ + struct crypto_aes_ctx ctx; + rfc4106_set_hash_subkey(&ctx); +} +void b(void) +{ + struct crypto_aes_ctx ctx; + ctx.key_dec[0] = 0; + rfc4106_set_hash_subkey(&ctx); +} + +/* This testcase should still split out one of the above basic blocks dealing + with __write_overflow. */ +/* { dg-final { scan-tree-dump-times "Function rfc4106_set_hash_subkey.part" 1 "optimized" } } */ --- gcc/testsuite/gcc.c-torture/compile/pr101941.c.jj 2022-01-17 19:37:01.609066831 +0100 +++ gcc/testsuite/gcc.c-torture/compile/pr101941.c 2022-01-17 19:37:01.609066831 +0100 @@ -0,0 +1,43 @@ +/* { dg-additional-options "-fconserve-stack" } */ +struct crypto_aes_ctx { + char key_dec[128]; +}; + +int rfc4106_set_hash_subkey_hash_subkey; + +void __write_overflow(void)__attribute__((__error__(""))); +void __write_overflow1(void); +void aes_encrypt(void*); + +void fortify_panic(const char*) __attribute__((__noreturn__)) ; + +char *rfc4106_set_hash_subkey(struct crypto_aes_ctx *ctx) { + void *a = &ctx->key_dec[0]; + unsigned p_size = __builtin_object_size(a, 0); +#ifdef __OPTIMIZE__ + if (p_size < 16) { + __write_overflow1(); + fortify_panic(__func__); + } + if (p_size < 32) { + __write_overflow(); + fortify_panic(__func__); + } +#endif + aes_encrypt(ctx); + return ctx->key_dec; +} + +char *(*gg)(struct crypto_aes_ctx *) = rfc4106_set_hash_subkey; + +void a(void) +{ + struct crypto_aes_ctx ctx; + rfc4106_set_hash_subkey(&ctx); +} +void b(void) +{ + struct crypto_aes_ctx ctx; + ctx.key_dec[0] = 0; + rfc4106_set_hash_subkey(&ctx); +} Jakub