On Wed, Apr 8, 2020 at 9:41 AM Jeff Law <l...@redhat.com> wrote: > > On Wed, 2020-04-08 at 09:23 -0700, H.J. Lu wrote: > > On Wed, Apr 8, 2020 at 9:16 AM Jeff Law <l...@redhat.com> wrote: > > > On Tue, 2020-03-31 at 08:11 -0700, H.J. Lu via Gcc-patches wrote: > > > > Since constant_call_address_operand has > > > > > > > > ;; Test for a pc-relative call operand > > > > (define_predicate "constant_call_address_operand" > > > > (match_code "symbol_ref") > > > > { > > > > if (ix86_cmodel == CM_LARGE || ix86_cmodel == CM_LARGE_PIC > > > > || flag_force_indirect_call) > > > > return false; > > > > if (TARGET_DLLIMPORT_DECL_ATTRIBUTES && SYMBOL_REF_DLLIMPORT_P (op)) > > > > return false; > > > > return true; > > > > }) > > > > > > > > even if cgraph_node::get (cfun->decl)->only_called_directly_p () returns > > > > false, the fuction may still be called indirectly. Copy the logic from > > > > constant_call_address_operand to rest_of_insert_endbranch to insert > > > > ENDBR > > > > at function entry if function will be called indirectly. > > > > > > > > gcc/ > > > > > > > > PR target/94417 > > > > * config/i386/i386-features.c (rest_of_insert_endbranch): Insert > > > > ENDBR at function entry if function will be called indirectly. > > > Can you just call constant_call_address_operand rather than copying its > > > contents? > > > > I wish I could. constant_call_address_operand uses SYMBOL_REF_DLLIMPORT_P > > (op) > > But I need to use DECL_DLLIMPORT_P (cfun->decl)). > Sigh. In that case I guess the patch is OK as-is. >
I'd like to backport this wrong code fix to GCC 9/8 branches. Is it OK for GCC 9/8 branches? Thanks. -- H.J.
From f86e27e71fb963aaa95b1da31515f888f6e146f8 Mon Sep 17 00:00:00 2001 From: "H.J. Lu" <hjl.to...@gmail.com> Date: Wed, 8 Apr 2020 09:47:35 -0700 Subject: [PATCH] x86: Insert ENDBR if function will be called indirectly Since constant_call_address_operand has ;; Test for a pc-relative call operand (define_predicate "constant_call_address_operand" (match_code "symbol_ref") { if (ix86_cmodel == CM_LARGE || ix86_cmodel == CM_LARGE_PIC || flag_force_indirect_call) return false; if (TARGET_DLLIMPORT_DECL_ATTRIBUTES && SYMBOL_REF_DLLIMPORT_P (op)) return false; return true; }) even if cgraph_node::get (cfun->decl)->only_called_directly_p () returns false, the fuction may still be called indirectly. Copy the logic from constant_call_address_operand to rest_of_insert_endbranch to insert ENDBR at function entry if function will be called indirectly. NB: gcc.target/i386/pr94417-2.c is updated to expect 4 ENDBRs, instead of 2, since only GCC 10 has the fix for PR target/89355 not to insert ENDBR after NOTE_INSN_DELETED_LABEL. gcc/ Backport from master PR target/94417 * config/i386/i386.c (rest_of_insert_endbranch): Insert ENDBR at function entry if function will be called indirectly. gcc/testsuite/ PR target/94417 * gcc.target/i386/pr94417-1.c: New test. * gcc.target/i386/pr94417-2.c: Likewise. * gcc.target/i386/pr94417-3.c: Likewise. (cherry picked from commit c5f379653964a1d2c7037b2de3e947a48370a198) --- gcc/config/i386/i386.c | 7 ++++++- gcc/testsuite/gcc.target/i386/pr94417-1.c | 20 ++++++++++++++++++++ gcc/testsuite/gcc.target/i386/pr94417-2.c | 21 +++++++++++++++++++++ gcc/testsuite/gcc.target/i386/pr94417-3.c | 19 +++++++++++++++++++ 4 files changed, 66 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/gcc.target/i386/pr94417-1.c create mode 100644 gcc/testsuite/gcc.target/i386/pr94417-2.c create mode 100644 gcc/testsuite/gcc.target/i386/pr94417-3.c diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c index 5d12d82db5b..3891d44ce85 100644 --- a/gcc/config/i386/i386.c +++ b/gcc/config/i386/i386.c @@ -2535,7 +2535,12 @@ rest_of_insert_endbranch (void) && (!flag_manual_endbr || lookup_attribute ("cf_check", DECL_ATTRIBUTES (cfun->decl))) - && !cgraph_node::get (cfun->decl)->only_called_directly_p ()) + && (!cgraph_node::get (cfun->decl)->only_called_directly_p () + || ix86_cmodel == CM_LARGE + || ix86_cmodel == CM_LARGE_PIC + || flag_force_indirect_call + || (TARGET_DLLIMPORT_DECL_ATTRIBUTES + && DECL_DLLIMPORT_P (cfun->decl)))) { /* Queue ENDBR insertion to x86_function_profiler. */ if (crtl->profile && flag_fentry) diff --git a/gcc/testsuite/gcc.target/i386/pr94417-1.c b/gcc/testsuite/gcc.target/i386/pr94417-1.c new file mode 100644 index 00000000000..5bbe057fa8f --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/pr94417-1.c @@ -0,0 +1,20 @@ +/* { dg-do compile } */ +/* { dg-require-effective-target lp64 } */ +/* { dg-options "-O2 -fcf-protection -mcmodel=large" } */ +/* { dg-final { scan-assembler-times {\mendbr} 2 } } */ + +extern void ext (void); + +__attribute((noclone, noinline)) +static +void +foo (void) +{ + ext (); +} + +void +bar (void) +{ + foo (); +} diff --git a/gcc/testsuite/gcc.target/i386/pr94417-2.c b/gcc/testsuite/gcc.target/i386/pr94417-2.c new file mode 100644 index 00000000000..9e9c277e07f --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/pr94417-2.c @@ -0,0 +1,21 @@ +/* { dg-do compile } */ +/* { dg-require-effective-target lp64 } */ +/* { dg-require-effective-target fpic } */ +/* { dg-options "-O2 -fpic -mcmodel=large -fcf-protection" } */ +/* { dg-final { scan-assembler-times {\mendbr} 4 } } */ + +extern void ext (void); + +__attribute((noclone, noinline)) +static +void +foo (void) +{ + ext (); +} + +void +bar (void) +{ + foo (); +} diff --git a/gcc/testsuite/gcc.target/i386/pr94417-3.c b/gcc/testsuite/gcc.target/i386/pr94417-3.c new file mode 100644 index 00000000000..07c451796c2 --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/pr94417-3.c @@ -0,0 +1,19 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fcf-protection -mforce-indirect-call" } */ +/* { dg-final { scan-assembler-times {\mendbr} 2 } } */ + +extern void ext (void); + +__attribute((noclone, noinline)) +static +void +foo (void) +{ + ext (); +} + +void +bar (void) +{ + foo (); +} -- 2.25.2