On Fri, Nov 19, 2021 at 2:16 AM Richard Biener via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> On Wed, Nov 17, 2021 at 7:25 AM apinski--- via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
> >
> > From: Andrew Pinski <apin...@marvell.com>
> >
> > The Linux kernel started to fail compile when the jump threader was improved
> > (r12-2591-g2e96b5f14e4025691). This failure was due to the IPA splitting
> > code
> > decided now to split off the basic block which contained two functions,
> > one of those functions included the error attribute on them. This patch
> > fixes
> > the problem by disallowing basic blocks from being split which contain
> > functions
> > that have either the error or warning attribute on them.
> >
> > The two new testcases are to make sure we still split the function for other
> > places if we reject the one case.
>
> Hmm, it's only problematic if the immediate(?) controlling condition of the
> error/warning annotated call is not in the split part, no?
No, if we have something like:
if (p_size < 32) {
if (ctx != 0) {
__write_overflow();
}
fortify_panic(__func__);
}
We would still run into the problem if we just disable the splitting
for the innermost bb and split off the 3 other bb's
I have a testcase where the above would fail if we decide only to make
sure the split point is not after ctx !=0 check.
> Interestingly we for example don't avoid splitting away __builtin_constant_p
> either.
__builtin_constant_p is handled a different way already; in
check_forbidden_calls we set forbidden_dominators to include the bb
where the builtin_constant_p would have been true.
And then when we consider the split, we reject if the entry point
would have been dominated by one of those bbs.
This was PR49642.
Thanks,
Andrew Pinski
>
> Richard.
>
> > OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions.
> >
> > PR tree-optimization/101941
> >
> > gcc/ChangeLog:
> >
> > * ipa-split.c (visit_bb): Disallow function calls where
> > the function has either error or warning attribute.
> >
> > gcc/testsuite/ChangeLog:
> >
> > * gcc.c-torture/compile/pr101941-1.c: New test.
> > * gcc.dg/tree-ssa/pr101941-1.c: New test.
> > ---
> > gcc/ipa-split.c | 12 ++++-
> > .../gcc.c-torture/compile/pr101941-1.c | 44 +++++++++++++++++
> > gcc/testsuite/gcc.dg/tree-ssa/pr101941-1.c | 48 +++++++++++++++++++
> > 3 files changed, 103 insertions(+), 1 deletion(-)
> > create mode 100644 gcc/testsuite/gcc.c-torture/compile/pr101941-1.c
> > create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr101941-1.c
> >
> > diff --git a/gcc/ipa-split.c b/gcc/ipa-split.c
> > index c68577d04a9..070e894ef31 100644
> > --- a/gcc/ipa-split.c
> > +++ b/gcc/ipa-split.c
> > @@ -873,7 +873,7 @@ visit_bb (basic_block bb, basic_block return_bb,
> > gimple *stmt = gsi_stmt (bsi);
> > tree op;
> > ssa_op_iter iter;
> > - tree decl;
> > + tree decl = NULL_TREE;
> >
> > if (is_gimple_debug (stmt))
> > continue;
> > @@ -927,6 +927,16 @@ visit_bb (basic_block bb, basic_block return_bb,
> > break;
> > }
> >
> > + /* If a function call and that function has either the
> > + warning or error attribute on it, don't split. */
> > + if (decl && (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)
> > bitmap_set_bit (set_ssa_names, SSA_NAME_VERSION (op));
> > FOR_EACH_SSA_TREE_OPERAND (op, stmt, iter, SSA_OP_USE)
> > diff --git a/gcc/testsuite/gcc.c-torture/compile/pr101941-1.c
> > b/gcc/testsuite/gcc.c-torture/compile/pr101941-1.c
> > new file mode 100644
> > index 00000000000..ab3bbea8ed7
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.c-torture/compile/pr101941-1.c
> > @@ -0,0 +1,44 @@
> > +/* { 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);
> > +}
> > +
> > diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr101941-1.c
> > b/gcc/testsuite/gcc.dg/tree-ssa/pr101941-1.c
> > new file mode 100644
> > index 00000000000..21c1d1ec466
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr101941-1.c
> > @@ -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" } } */
> > --
> > 2.17.1
> >
