Bug 94417 - -fcf-protection -mcmodel=large/-mforce-indirect-call is broken
Summary: -fcf-protection -mcmodel=large/-mforce-indirect-call is broken
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: target (show other bugs)
Version: 10.0
: P3 normal
Target Milestone: 10.0
Assignee: Not yet assigned to anyone
URL: https://gcc.gnu.org/pipermail/gcc-pat...
Keywords: patch
Depends on:
Blocks:
 
Reported: 2020-03-30 23:37 UTC by H.J. Lu
Modified: 2020-04-17 22:34 UTC (History)
1 user (show)

See Also:
Host:
Target: x86-64
Build:
Known to work:
Known to fail:
Last reconfirmed: 2020-03-31 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description H.J. Lu 2020-03-30 23:37:33 UTC
From:

https://bugs.llvm.org/show_bug.cgi?id=45364

Testcase:

void ext();
__attribute((noinline)) static void a() { ext(); }
void b() { a(); }

Compile with "gcc -O2 -mcmodel=large -fcf-protection".  Note the indirect
call to "a", which does not have an endbr64.
Comment 1 H.J. Lu 2020-03-31 05:30:16 UTC
-mforce-indirect-call has the same issue.
Comment 2 H.J. Lu 2020-03-31 05:36:33 UTC
This seems to work:

diff --git a/gcc/config/i386/i386-features.c b/gcc/config/i386/i386-features.c
index 66b120d21a7..78fb373db6e 100644
--- a/gcc/config/i386/i386-features.c
+++ b/gcc/config/i386/i386-features.c
@@ -1963,7 +1963,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)
Comment 3 H.J. Lu 2020-03-31 15:12:34 UTC
A patch is posted at

https://gcc.gnu.org/pipermail/gcc-patches/2020-March/542984.html
Comment 4 GCC Commits 2020-04-08 16:48:36 UTC
The master branch has been updated by H.J. Lu <hjl@gcc.gnu.org>:

https://gcc.gnu.org/g:c5f379653964a1d2c7037b2de3e947a48370a198

commit r10-7633-gc5f379653964a1d2c7037b2de3e947a48370a198
Author: H.J. Lu <hjl.tools@gmail.com>
Date:   Wed Apr 8 09:47:35 2020 -0700

    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.
    
    gcc/
    
            PR target/94417
            * config/i386/i386-features.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.
Comment 5 H.J. Lu 2020-04-08 16:52:17 UTC
Fixed for GCC 10.
Comment 6 GCC Commits 2020-04-17 22:24:41 UTC
The releases/gcc-9 branch has been updated by H.J. Lu <hjl@gcc.gnu.org>:

https://gcc.gnu.org/g:4a745938b56da04ed01055d5bcb520dc1c760414

commit r9-8508-g4a745938b56da04ed01055d5bcb520dc1c760414
Author: H.J. Lu <hjl.tools@gmail.com>
Date:   Fri Apr 17 15:23:27 2020 -0700

    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/
    
            Backport from master
            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)
Comment 7 GCC Commits 2020-04-17 22:33:25 UTC
The releases/gcc-8 branch has been updated by H.J. Lu <hjl@gcc.gnu.org>:

https://gcc.gnu.org/g:99ddb11c0840f68466a14fd583dd4d3a558d4961

commit r8-10186-g99ddb11c0840f68466a14fd583dd4d3a558d4961
Author: H.J. Lu <hjl.tools@gmail.com>
Date:   Fri Apr 17 15:23:27 2020 -0700

    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/
    
            Backport from master
            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)
Comment 8 H.J. Lu 2020-04-17 22:34:50 UTC
Fixed for GCC 10, GCC 9.4 and GCC 8.5.