Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RISCV] Support .note.gnu.property for enable Zicfiss and Zicfilp extension #77414

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

SuHo-llrr
Copy link

@SuHo-llrr SuHo-llrr commented Jan 9, 2024

Emit Zicfiss/Zicfilp to .note.gnu.property sections

  1. for spec v0.4.0 Zicifss/Zicfilp is AND feature means that all objects need to have this feature.
  2. Emit note section when Zicifss/Zicfilp extension is enabled.
  3. Checking all objects enable Zicifss/Zicfilp when linking.
  4. Add -zforce-zicfilp/-zforce-zicfiss options to force emit Zicifss/Zicfilp flags on in .note.gnu.property

Ref: riscv-non-isa/riscv-elf-psabi-doc#417

Copy link

github-actions bot commented Jan 9, 2024

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be
notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write
permissions for the repository. In which case you can instead tag reviewers by
name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review
by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate
is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

Copy link

github-actions bot commented Jan 9, 2024

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:
git-clang-format --diff db78ee0cb82669302a5e0f18a15fd53346a73823 e01a93cb192e71947f693bdd07d534912e54fbef -- clang/test/Driver/riscv-cfi-property.c lld/ELF/Config.h lld/ELF/Driver.cpp lld/ELF/InputFiles.cpp lld/ELF/SyntheticSections.cpp llvm/include/llvm/BinaryFormat/ELF.h llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.cpp llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.h llvm/lib/Target/RISCV/MCTargetDesc/RISCVTargetStreamer.cpp llvm/lib/Target/RISCV/MCTargetDesc/RISCVTargetStreamer.h llvm/tools/llvm-readobj/ELFDumper.cpp
View the diff from clang-format here.
diff --git a/lld/ELF/Driver.cpp b/lld/ELF/Driver.cpp
index 182ca6d4df..8b1845fba7 100644
--- a/lld/ELF/Driver.cpp
+++ b/lld/ELF/Driver.cpp
@@ -2697,7 +2697,8 @@ static void readSecurityNotes() {
                       "GNU_PROPERTY_X86_FEATURE_1_SHSTK property");
 
     checkAndReportMissingFeature(
-        config->zZicfilpReport, features, GNU_PROPERTY_RISCV_FEATURE_1_CFI_LP_SIMPLE,
+        config->zZicfilpReport, features,
+        GNU_PROPERTY_RISCV_FEATURE_1_CFI_LP_SIMPLE,
         toString(f) + ": -z zicfilp-report: file does not have "
                       "GNU_PROPERTY_RISCV_FEATURE_1_CFI_LP_SIMPLE property");
 
@@ -2723,8 +2724,9 @@ static void readSecurityNotes() {
         !(features & GNU_PROPERTY_RISCV_FEATURE_1_CFI_LP_SIMPLE)) {
       features |= GNU_PROPERTY_RISCV_FEATURE_1_CFI_LP_SIMPLE;
       if (config->zZicfilpReport == "none")
-        warn(toString(f) + ": -z force-zicfilp: file does not have "
-                           "GNU_PROPERTY_RISCV_FEATURE_1_CFI_LP_SIMPLE property");
+        warn(toString(f) +
+             ": -z force-zicfilp: file does not have "
+             "GNU_PROPERTY_RISCV_FEATURE_1_CFI_LP_SIMPLE property");
     }
 
     if (config->zForceZicfiss &&

@SuHo-llrr SuHo-llrr force-pushed the upstream-main-cfi branch 3 times, most recently from 6c7684e to 319c547 Compare February 15, 2024 08:34
@topperc topperc requested a review from yetingk February 15, 2024 08:35
@kito-cheng kito-cheng changed the title Support .note.gnu.property for enable Zicfiss and Zicfilp extension [RISCV] Support .note.gnu.property for enable Zicfiss and Zicfilp extension Mar 1, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Mar 1, 2024

@llvm/pr-subscribers-lld-elf
@llvm/pr-subscribers-clang-driver
@llvm/pr-subscribers-clang
@llvm/pr-subscribers-lld

@llvm/pr-subscribers-backend-risc-v

Author: None (SuHo-llrr)

Changes

Emit Zicfiss/Zicfilp to .note.gnu.property sections

  1. for spec v0.4.0 Zicifss/Zicfilp is AND feature means that all objects need to have this feature.
  2. Emit note section when Zicifss/Zicfilp extension is enabled.
  3. Checking all objects enable Zicifss/Zicfilp when linking.
  4. Add -zforce-zicfilp/-zforce-zicfiss options to force emit Zicifss/Zicfilp flags on in .note.gnu.property

Ref: riscv-non-isa/riscv-elf-psabi-doc#417


Patch is 20.21 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/77414.diff

10 Files Affected:

  • (added) clang/test/Driver/riscv-cfi-property.c (+29)
  • (modified) lld/ELF/Config.h (+4)
  • (modified) lld/ELF/Driver.cpp (+40-2)
  • (modified) lld/ELF/InputFiles.cpp (+12-3)
  • (modified) lld/ELF/SyntheticSections.cpp (+12-3)
  • (added) lld/test/ELF/riscv-force-cfi-property.s (+35)
  • (modified) llvm/include/llvm/BinaryFormat/ELF.h (+7)
  • (modified) llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.cpp (+43)
  • (modified) llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.h (+1)
  • (modified) llvm/tools/llvm-readobj/ELFDumper.cpp (+34-15)
diff --git a/clang/test/Driver/riscv-cfi-property.c b/clang/test/Driver/riscv-cfi-property.c
new file mode 100644
index 00000000000000..7a4044c9328dc4
--- /dev/null
+++ b/clang/test/Driver/riscv-cfi-property.c
@@ -0,0 +1,29 @@
+// When -march with zicfiss0p4 or zicfilp0p4 add GNU property to file object
+
+// RUN: %clang --target=riscv32-linux-gnu -menable-experimental-extensions -march=rv32gc_zicfiss0p4 -c -o - %s | llvm-readobj -n - | FileCheck -check-prefix=CHECK -check-prefix=CHECK_ZICFISS %s
+// RUN: %clang --target=riscv64-linux-gnu -menable-experimental-extensions -march=rv64gc_zicfiss0p4 -c -o - %s | llvm-readobj -n - | FileCheck -check-prefix=CHECK -check-prefix=CHECK_ZICFISS %s
+// RUN: %clang --target=riscv32-linux-gnu -menable-experimental-extensions -march=rv32gc_zicfilp0p4 -c -o - %s | llvm-readobj -n - | FileCheck -check-prefix=CHECK -check-prefix=CHECK_ZICFILP %s
+// RUN: %clang --target=riscv64-linux-gnu -menable-experimental-extensions -march=rv64gc_zicfilp0p4 -c -o - %s | llvm-readobj -n - | FileCheck -check-prefix=CHECK -check-prefix=CHECK_ZICFILP %s
+// RUN: %clang --target=riscv32-linux-gnu -menable-experimental-extensions -march=rv32gc_zicfilp0p4_zicfiss0p4 -c -o - %s | llvm-readobj -n - | FileCheck -check-prefix=CHECK -check-prefix=CHECK_ZICFILP_ZICFISS %s
+// RUN: %clang --target=riscv64-linux-gnu -menable-experimental-extensions -march=rv64gc_zicfilp0p4_zicfiss0p4 -c -o - %s | llvm-readobj -n - | FileCheck -check-prefix=CHECK -check-prefix=CHECK_ZICFILP_ZICFISS %s
+
+
+// CHECK: Name: .note.gnu.property
+// CHECK: Type: NT_GNU_PROPERTY_TYPE_0
+// CHECK: Property [
+// CHECK_ZICFISS: riscv feature: ZICFISS
+// CHECK_ZICFILP: riscv feature: ZICFILP
+// CHECK_ZICFILP_ZICFISS: riscv feature: ZICFILP, ZICFISS
+// CHECK: ]
+
+// GNU Note Section Example
+/*.section .note.gnu.property, "a";
+.balign 8;
+.long 0x4;
+.long 0x10;
+.long 0x5
+.asciz "GNU"
+.long 0xc0000000
+.long 4
+.long 3
+.long 0*/
diff --git a/lld/ELF/Config.h b/lld/ELF/Config.h
index fcca8c42b29b71..552a5060452729 100644
--- a/lld/ELF/Config.h
+++ b/lld/ELF/Config.h
@@ -187,6 +187,8 @@ struct Config {
   llvm::StringRef cmseOutputLib;
   StringRef zBtiReport = "none";
   StringRef zCetReport = "none";
+  llvm::StringRef zZicfilpReport = "none";
+  llvm::StringRef zZicfissReport = "none";
   bool ltoBBAddrMap;
   llvm::StringRef ltoBasicBlockSections;
   std::pair<llvm::StringRef, llvm::StringRef> thinLTOObjectSuffixReplace;
@@ -324,6 +326,8 @@ struct Config {
   bool zText;
   bool zRetpolineplt;
   bool zWxneeded;
+  bool zForceZicfilp;
+  bool zForceZicfiss;
   DiscardPolicy discard;
   GnuStackKind zGnustack;
   ICFLevel icf;
diff --git a/lld/ELF/Driver.cpp b/lld/ELF/Driver.cpp
index 4bb9b7a0b2a983..1791d275a4d00c 100644
--- a/lld/ELF/Driver.cpp
+++ b/lld/ELF/Driver.cpp
@@ -463,6 +463,13 @@ static void checkOptions() {
       error("-z bti-report only supported on AArch64");
   }
 
+  if (config->emachine != EM_RISCV) {
+    if (config->zZicfilpReport != "none")
+      error("-z zicfilip-report only support on RISCV32/RISCV64");
+    if (config->zZicfissReport != "none")
+      error("-z zicfiss-report only support on RISCV32/RISCV64");
+  }
+
   if (config->emachine != EM_386 && config->emachine != EM_X86_64 &&
       config->zCetReport != "none")
     error("-z cet-report only supported on X86 and X86_64");
@@ -1455,6 +1462,8 @@ static void readConfigs(opt::InputArgList &args) {
   config->zWxneeded = hasZOption(args, "wxneeded");
   setUnresolvedSymbolPolicy(args);
   config->power10Stubs = args.getLastArgValue(OPT_power10_stubs_eq) != "no";
+  config->zForceZicfilp = hasZOption(args, "force-zicfilp");
+  config->zForceZicfiss = hasZOption(args, "force-zicfiss");
 
   if (opt::Arg *arg = args.getLastArg(OPT_eb, OPT_el)) {
     if (arg->getOption().matches(OPT_eb))
@@ -1497,7 +1506,9 @@ static void readConfigs(opt::InputArgList &args) {
   }
 
   auto reports = {std::make_pair("bti-report", &config->zBtiReport),
-                  std::make_pair("cet-report", &config->zCetReport)};
+                  std::make_pair("cet-report", &config->zCetReport),
+                  std::make_pair("zicfilp-report", &config->zZicfilpReport),
+                  std::make_pair("zicfiss-report", &config->zZicfissReport)};
   for (opt::Arg *arg : args.filtered(OPT_z)) {
     std::pair<StringRef, StringRef> option =
         StringRef(arg->getValue()).split('=');
@@ -2570,7 +2581,7 @@ static void checkAndReportMissingFeature(StringRef config, uint32_t features,
 // GNU_PROPERTY_AARCH64_FEATURE_1_AND mechanism.
 static uint32_t getAndFeatures() {
   if (config->emachine != EM_386 && config->emachine != EM_X86_64 &&
-      config->emachine != EM_AARCH64)
+      config->emachine != EM_AARCH64 && config->emachine != EM_RISCV)
     return 0;
 
   uint32_t ret = -1;
@@ -2592,6 +2603,16 @@ static uint32_t getAndFeatures() {
         toString(f) + ": -z cet-report: file does not have "
                       "GNU_PROPERTY_X86_FEATURE_1_SHSTK property");
 
+    checkAndReportMissingFeature(
+        config->zZicfilpReport, features, GNU_PROPERTY_RISCV_FEATURE_1_ZICFILP,
+        toString(f) + ": -z zicfilp-report: file does not have "
+                      "GNU_PROPERTY_RISCV_FEATURE_1_ZICFILP property");
+
+    checkAndReportMissingFeature(
+        config->zZicfissReport, features, GNU_PROPERTY_RISCV_FEATURE_1_ZICFISS,
+        toString(f) + ": -z zicfiss-report: file does not have "
+                      "GNU_PROPERTY_RISCV_FEATURE_1_ZICFISS property");
+
     if (config->zForceBti && !(features & GNU_PROPERTY_AARCH64_FEATURE_1_BTI)) {
       features |= GNU_PROPERTY_AARCH64_FEATURE_1_BTI;
       if (config->zBtiReport == "none")
@@ -2604,6 +2625,23 @@ static uint32_t getAndFeatures() {
                            "GNU_PROPERTY_X86_FEATURE_1_IBT property");
       features |= GNU_PROPERTY_X86_FEATURE_1_IBT;
     }
+
+    if (config->zForceZicfilp &&
+        !(features & GNU_PROPERTY_RISCV_FEATURE_1_ZICFILP)) {
+      features |= GNU_PROPERTY_RISCV_FEATURE_1_ZICFILP;
+      if (config->zZicfilpReport == "none")
+        warn(toString(f) + ": -z force-zicfilp: file does not have "
+                           "GNU_PROPERTY_RISCV_FEATURE_1_ZICFILP property");
+    }
+
+    if (config->zForceZicfiss &&
+        !(features & GNU_PROPERTY_RISCV_FEATURE_1_ZICFISS)) {
+      features |= GNU_PROPERTY_RISCV_FEATURE_1_ZICFISS;
+      if (config->zZicfissReport == "none")
+        warn(toString(f) + ": -z force-zicfiss: file does not have "
+                           "GNU_PROPERTY_RISCV_FEATURE_1_ZICFISS property");
+    }
+
     if (config->zPacPlt && !(features & GNU_PROPERTY_AARCH64_FEATURE_1_PAC)) {
       warn(toString(f) + ": -z pac-plt: file does not have "
                          "GNU_PROPERTY_AARCH64_FEATURE_1_PAC property");
diff --git a/lld/ELF/InputFiles.cpp b/lld/ELF/InputFiles.cpp
index 00aebb47640e84..5324c7b95935a8 100644
--- a/lld/ELF/InputFiles.cpp
+++ b/lld/ELF/InputFiles.cpp
@@ -926,9 +926,18 @@ template <class ELFT> static uint32_t readAndFeatures(const InputSection &sec) {
       continue;
     }
 
-    uint32_t featureAndType = config->emachine == EM_AARCH64
-                                  ? GNU_PROPERTY_AARCH64_FEATURE_1_AND
-                                  : GNU_PROPERTY_X86_FEATURE_1_AND;
+    uint32_t featureAndType = 0;
+    switch (config->emachine) {
+    default:
+      featureAndType = GNU_PROPERTY_X86_FEATURE_1_AND;
+      break;
+    case EM_AARCH64:
+      featureAndType = GNU_PROPERTY_AARCH64_FEATURE_1_AND;
+      break;
+    case EM_RISCV:
+      featureAndType = GNU_PROPERTY_RISCV_FEATURE_1_AND;
+      break;
+    }
 
     // Read a body of a NOTE record, which consists of type-length-value fields.
     ArrayRef<uint8_t> desc = note.getDesc(sec.addralign);
diff --git a/lld/ELF/SyntheticSections.cpp b/lld/ELF/SyntheticSections.cpp
index bada394aa30d7d..1065e5c5be6e14 100644
--- a/lld/ELF/SyntheticSections.cpp
+++ b/lld/ELF/SyntheticSections.cpp
@@ -314,9 +314,18 @@ GnuPropertySection::GnuPropertySection()
                        config->wordsize, ".note.gnu.property") {}
 
 void GnuPropertySection::writeTo(uint8_t *buf) {
-  uint32_t featureAndType = config->emachine == EM_AARCH64
-                                ? GNU_PROPERTY_AARCH64_FEATURE_1_AND
-                                : GNU_PROPERTY_X86_FEATURE_1_AND;
+  uint32_t featureAndType = 0;
+  switch (config->emachine) {
+  default:
+    featureAndType = GNU_PROPERTY_X86_FEATURE_1_AND;
+    break;
+  case EM_AARCH64:
+    featureAndType = GNU_PROPERTY_AARCH64_FEATURE_1_AND;
+    break;
+  case EM_RISCV:
+    featureAndType = GNU_PROPERTY_RISCV_FEATURE_1_AND;
+    break;
+  }
 
   write32(buf, 4);                                   // Name size
   write32(buf + 4, config->is64 ? 16 : 12);          // Content size
diff --git a/lld/test/ELF/riscv-force-cfi-property.s b/lld/test/ELF/riscv-force-cfi-property.s
new file mode 100644
index 00000000000000..65043d1e71261e
--- /dev/null
+++ b/lld/test/ELF/riscv-force-cfi-property.s
@@ -0,0 +1,35 @@
+# REQUIRES: riscv
+
+# RUN: llvm-mc -filetype=obj -triple=riscv32-unknown-elf %s -o %t.rv32_lp.o
+# RUN: ld.lld %t.rv32_lp.o -zforce-zicfilp -o %t.rv32_lp | count 0
+# RUN: llvm-readobj -n %t.rv32_lp | FileCheck -check-prefix=CHECK -check-prefix=CHECK_ZICFILP %s
+
+# RUN: llvm-mc -filetype=obj -triple=riscv64-unknown-elf %s -o %t.rv64_lp.o
+# RUN: ld.lld %t.rv64_lp.o -zforce-zicfilp -o %t.rv64_lp | count 0
+# RUN: llvm-readobj -n %t.rv64_lp | FileCheck -check-prefix=CHECK -check-prefix=CHECK_ZICFILP %s
+
+# RUN: llvm-mc -filetype=obj -triple=riscv32-unknown-elf %s -o %t.rv32_ss.o
+# RUN: ld.lld %t.rv32_ss.o -zforce-zicfiss -o %t.rv32_ss | count 0
+# RUN: llvm-readobj -n %t.rv32_ss | FileCheck -check-prefix=CHECK -check-prefix=CHECK_ZICFISS %s
+
+# RUN: llvm-mc -filetype=obj -triple=riscv64-unknown-elf %s -o %t.rv64_ss.o
+# RUN: ld.lld %t.rv64_ss.o -zforce-zicfiss -o %t.rv64_ss | count 0
+# RUN: llvm-readobj -n %t.rv64_ss | FileCheck -check-prefix=CHECK -check-prefix=CHECK_ZICFISS %s
+
+# RUN: llvm-mc -filetype=obj -triple=riscv32-unknown-elf %s -o %t.rv32_lp_ss.o
+# RUN: ld.lld %t.rv32_lp_ss.o -zforce-zicfilp -zforce-zicfiss -o %t.rv32_lp_ss | count 0
+# RUN: llvm-readobj -n %t.rv32_lp_ss | FileCheck -check-prefix=CHECK -check-prefix=CHECK_ZICFILP_ZICFISS %s
+
+# RUN: llvm-mc -filetype=obj -triple=riscv64-unknown-elf %s -o %t.rv64_lp_ss.o
+# RUN: ld.lld %t.rv64_lp_ss.o -zforce-zicfilp -zforce-zicfiss -o %t.rv64_lp_ss | count 0
+# RUN: llvm-readobj -n %t.rv64_lp_ss | FileCheck -check-prefix=CHECK -check-prefix=CHECK_ZICFILP_ZICFISS %s
+
+
+
+// CHECK: Name: .note.gnu.property
+// CHECK: Type: NT_GNU_PROPERTY_TYPE_0
+// CHECK: Property [
+// CHECK_ZICFISS: riscv feature: ZICFISS
+// CHECK_ZICFILP: riscv feature: ZICFILP
+// CHECK_ZICFILP_ZICFISS: riscv feature: ZICFILP, ZICFISS
+// CHECK: ]
diff --git a/llvm/include/llvm/BinaryFormat/ELF.h b/llvm/include/llvm/BinaryFormat/ELF.h
index 8e0356a8efeb61..34c6a2690cbf3f 100644
--- a/llvm/include/llvm/BinaryFormat/ELF.h
+++ b/llvm/include/llvm/BinaryFormat/ELF.h
@@ -1737,6 +1737,7 @@ enum : unsigned {
   GNU_PROPERTY_NO_COPY_ON_PROTECTED = 2,
   GNU_PROPERTY_AARCH64_FEATURE_1_AND = 0xc0000000,
   GNU_PROPERTY_X86_FEATURE_1_AND = 0xc0000002,
+  GNU_PROPERTY_RISCV_FEATURE_1_AND = 0xc0000000,
 
   GNU_PROPERTY_X86_UINT32_OR_LO = 0xc0008000,
   GNU_PROPERTY_X86_FEATURE_2_NEEDED = GNU_PROPERTY_X86_UINT32_OR_LO + 1,
@@ -1776,6 +1777,12 @@ enum : unsigned {
   GNU_PROPERTY_X86_ISA_1_V4 = 1 << 3,
 };
 
+// riscv processor feature bits.
+enum : unsigned {
+  GNU_PROPERTY_RISCV_FEATURE_1_ZICFILP = 1 << 0,
+  GNU_PROPERTY_RISCV_FEATURE_1_ZICFISS = 1 << 1,
+};
+
 // FreeBSD note types.
 enum {
   NT_FREEBSD_ABI_TAG = 1,
diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.cpp b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.cpp
index b375e8bb4b8fac..ef1d86624b771d 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.cpp
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.cpp
@@ -84,6 +84,35 @@ void RISCVTargetELFStreamer::finishAttributeSection() {
                           ELF::SHT_RISCV_ATTRIBUTES, AttributeSection);
 }
 
+void RISCVTargetELFStreamer::emitNoteSection(unsigned Flags) {
+  if (Flags == 0)
+    return;
+
+  MCStreamer &OutStreamer = getStreamer();
+  MCContext &Context = OutStreamer.getContext();
+  MCSectionELF *Nt = Context.getELFSection(".note.gnu.property", ELF::SHT_NOTE,
+                                           ELF::SHF_ALLOC);
+  MCSection *Cur = OutStreamer.getCurrentSectionOnly();
+  OutStreamer.switchSection(Nt);
+
+  // Emit the note header.
+  OutStreamer.emitValueToAlignment(Align(8));
+  OutStreamer.emitIntValue(4, 4);     // data size for note name
+  OutStreamer.emitIntValue(4 * 4, 4); // data size
+  OutStreamer.emitIntValue(ELF::NT_GNU_PROPERTY_TYPE_0, 4); // note type
+  OutStreamer.emitBytes(StringRef("GNU", 4));               // note name
+
+  // Emit the CFI(ZICFILP/ZICFISS) properties.
+  OutStreamer.emitIntValue(ELF::GNU_PROPERTY_RISCV_FEATURE_1_AND,
+                           4);        // and property
+  OutStreamer.emitIntValue(4, 4);     // data size
+  OutStreamer.emitIntValue(Flags, 4); // data
+  OutStreamer.emitIntValue(0, 4);     // pad
+
+  OutStreamer.endSection(Nt);
+  OutStreamer.switchSection(Cur);
+}
+
 void RISCVTargetELFStreamer::finish() {
   RISCVTargetStreamer::finish();
   MCAssembler &MCA = getStreamer().getAssembler();
@@ -118,6 +147,20 @@ void RISCVTargetELFStreamer::finish() {
   }
 
   MCA.setELFHeaderEFlags(EFlags);
+
+  unsigned GNUNoteFlags = 0;
+
+  // TODO check ZICFILP or ZICFISS with march
+  // should we check with codegen enable ex. -mllvm
+  // -riscv-hardware-shadow-stack=true ?
+  const FeatureBitset &Features = STI.getFeatureBits();
+  if (Features[RISCV::FeatureStdExtZicfilp])
+    GNUNoteFlags |= ELF::GNU_PROPERTY_RISCV_FEATURE_1_ZICFILP;
+
+  if (Features[RISCV::FeatureStdExtZicfiss])
+    GNUNoteFlags |= ELF::GNU_PROPERTY_RISCV_FEATURE_1_ZICFISS;
+
+  emitNoteSection(GNUNoteFlags);
 }
 
 void RISCVTargetELFStreamer::reset() {
diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.h b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.h
index a6f54bf67b5d2b..595dc777c30c4a 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.h
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.h
@@ -71,6 +71,7 @@ class RISCVTargetELFStreamer : public RISCVTargetStreamer {
   void emitDirectiveVariantCC(MCSymbol &Symbol) override;
 
   void finish() override;
+  void emitNoteSection(unsigned Flags);
 };
 
 MCELFStreamer *createRISCVELFStreamer(MCContext &C,
diff --git a/llvm/tools/llvm-readobj/ELFDumper.cpp b/llvm/tools/llvm-readobj/ELFDumper.cpp
index 50ea63e87a43be..a6840b6370ed5b 100644
--- a/llvm/tools/llvm-readobj/ELFDumper.cpp
+++ b/llvm/tools/llvm-readobj/ELFDumper.cpp
@@ -5094,7 +5094,7 @@ template <class ELFT> void GNUELFDumper<ELFT>::printAddrsig() {
 
 template <typename ELFT>
 static std::string getGNUProperty(uint32_t Type, uint32_t DataSize,
-                                  ArrayRef<uint8_t> Data) {
+                                  ArrayRef<uint8_t> Data, uint16_t Target) {
   std::string str;
   raw_string_ostream OS(str);
   uint32_t PrData;
@@ -5127,8 +5127,16 @@ static std::string getGNUProperty(uint32_t Type, uint32_t DataSize,
     return OS.str();
   case GNU_PROPERTY_AARCH64_FEATURE_1_AND:
   case GNU_PROPERTY_X86_FEATURE_1_AND:
-    OS << ((Type == GNU_PROPERTY_AARCH64_FEATURE_1_AND) ? "aarch64 feature: "
-                                                        : "x86 feature: ");
+
+    if (Type == GNU_PROPERTY_AARCH64_FEATURE_1_AND) {
+      if (Target == ELF::EM_RISCV)
+        OS << "riscv feature: ";
+      else
+        OS << "aarch64 feature: ";
+    } else {
+      OS << "x86 feature: ";
+    }
+
     if (DataSize != 4) {
       OS << format("<corrupt length: 0x%x>", DataSize);
       return OS.str();
@@ -5138,14 +5146,21 @@ static std::string getGNUProperty(uint32_t Type, uint32_t DataSize,
       OS << "<None>";
       return OS.str();
     }
+
     if (Type == GNU_PROPERTY_AARCH64_FEATURE_1_AND) {
-      DumpBit(GNU_PROPERTY_AARCH64_FEATURE_1_BTI, "BTI");
-      DumpBit(GNU_PROPERTY_AARCH64_FEATURE_1_PAC, "PAC");
-      DumpBit(GNU_PROPERTY_AARCH64_FEATURE_1_GCS, "GCS");
+      if (Target == ELF::EM_RISCV) {
+        DumpBit(GNU_PROPERTY_RISCV_FEATURE_1_ZICFILP, "ZICFILP");
+        DumpBit(GNU_PROPERTY_RISCV_FEATURE_1_ZICFISS, "ZICFISS");
+      } else {
+        DumpBit(GNU_PROPERTY_AARCH64_FEATURE_1_BTI, "BTI");
+        DumpBit(GNU_PROPERTY_AARCH64_FEATURE_1_PAC, "PAC");
+        DumpBit(GNU_PROPERTY_AARCH64_FEATURE_1_GCS, "GCS");
+      }
     } else {
       DumpBit(GNU_PROPERTY_X86_FEATURE_1_IBT, "IBT");
       DumpBit(GNU_PROPERTY_X86_FEATURE_1_SHSTK, "SHSTK");
     }
+
     if (PrData)
       OS << format("<unknown flags: 0x%x>", PrData);
     return OS.str();
@@ -5199,7 +5214,8 @@ static std::string getGNUProperty(uint32_t Type, uint32_t DataSize,
 }
 
 template <typename ELFT>
-static SmallVector<std::string, 4> getGNUPropertyList(ArrayRef<uint8_t> Arr) {
+static SmallVector<std::string, 4> getGNUPropertyList(ArrayRef<uint8_t> Arr,
+                                                      uint16_t Target) {
   using Elf_Word = typename ELFT::Word;
 
   SmallVector<std::string, 4> Properties;
@@ -5217,8 +5233,8 @@ static SmallVector<std::string, 4> getGNUPropertyList(ArrayRef<uint8_t> Arr) {
       Properties.push_back(OS.str());
       break;
     }
-    Properties.push_back(
-        getGNUProperty<ELFT>(Type, DataSize, Arr.take_front(PaddedSize)));
+    Properties.push_back(getGNUProperty<ELFT>(
+        Type, DataSize, Arr.take_front(PaddedSize), Target));
     Arr = Arr.drop_front(PaddedSize);
   }
 
@@ -5270,7 +5286,7 @@ static StringRef getDescAsStringRef(ArrayRef<uint8_t> Desc) {
 
 template <typename ELFT>
 static bool printGNUNote(raw_ostream &OS, uint32_t NoteType,
-                         ArrayRef<uint8_t> Desc) {
+                         ArrayRef<uint8_t> Desc, uint16_t Target) {
   // Return true if we were able to pretty-print the note, false otherwise.
   switch (NoteType) {
   default:
@@ -5292,7 +5308,7 @@ static bool printGNUNote(raw_ostream &OS, uint32_t NoteType,
     break;
   case ELF::NT_GNU_PROPERTY_TYPE_0:
     OS << "    Properties:";
-    for (const std::string &Property : getGNUPropertyList<ELFT>(Desc))
+    for (const std::string &Property : getGNUPropertyList<ELFT>(Desc, Target))
       OS << "    " << Property << "\n";
     break;
   }
@@ -6012,10 +6028,12 @@ template <class ELFT> void GNUELFDumper<ELFT>::printNotes() {
     else
       OS << "Unknown note type: (" << format_hex(Type, 10) << ")\n";
 
+    uint16_t Target = this->Obj.getHeader().e_machine;
+
     // Print the description, or fallback to printing raw bytes for unknown
     // owners/if we fail to pretty-print the contents.
     if (Name == "GNU") {
-      if (printGNUNote<ELFT>(OS, Type, Descriptor))
+      if (printGNUNote<ELFT>(OS, Type, Descriptor, Target))
         return Error::success();
     } else if (Name == "FreeBSD") {
       if (std::optional<FreeBSDNote> N =
@@ -7642,7 +7660,7 @@ template <class ELFT> void LLVMELFDumper<ELFT>::printAddrsig() {
 
 template <typename ELFT>
 static bool printGNUNoteLLVMStyle(uint32_t NoteType, ArrayRef<uint8_t> Desc,
-                                  ScopedPrinter &W) {
+                                  ScopedPrinter &W, uint16_t Target) {
   // Return true if we were able to pretty-print the note, false otherwise.
   switch (NoteType) {
   default:
@@ -7667,7 +7685,7 @@ static bool printGNUNoteLLVMStyle(uint32_t NoteType, ArrayRef<uint8_t> Desc,
     break;
   case ELF::NT_GNU_PROPERTY_TYPE_0:
     ListScope D(W, "Property");
-    for (const std::string &Property : getGNUPropertyList<ELFT>(Desc))
+    for (const std::string &Property : getGNUPropertyList<ELFT>(Desc, Target))
       W.printString(Property);
     break;
   }
@@ -7803,10 +7821,11 @@ template <class ELFT> void LLVMELFDumper<ELFT>::printNotes() {
       W.printString("Type",
                     "Unknown (" + to_string(format_hex(Type, 10)) + ")");
 
+    uint16_t Target = this->Obj.getHeader().e_machine;
     // Print the description, or fallback to printing raw bytes for unknown
     // owners/if we fail to pretty-print the contents.
     if (Name == "GNU") {
-      if (printGNUNote...
[truncated]

@SuHo-llrr
Copy link
Author

@yetingk
I have updated to the latest version. If you have any suggestions, please let me know.

@SuHo-llrr SuHo-llrr marked this pull request as ready for review May 10, 2024 06:35
@llvmbot llvmbot added clang Clang issues not falling into any other category lld clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' lld:ELF llvm:binary-utilities labels May 10, 2024
llvm/tools/llvm-readobj/ELFDumper.cpp Outdated Show resolved Hide resolved
lld/ELF/InputFiles.cpp Outdated Show resolved Hide resolved
@yetingk yetingk requested review from asb and kito-cheng May 10, 2024 08:30
lld/ELF/Config.h Outdated Show resolved Hide resolved
@yetingk
Copy link
Contributor

yetingk commented May 10, 2024

I am sorry that I missed the commit for a long time. Very sorry about it.

@topperc topperc requested a review from MaskRay May 10, 2024 17:21
lld/ELF/Driver.cpp Outdated Show resolved Hide resolved
OS << ((Type == GNU_PROPERTY_AARCH64_FEATURE_1_AND) ? "aarch64 feature: "
: "x86 feature: ");

if (Type == GNU_PROPERTY_AARCH64_FEATURE_1_AND) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a static_assert that GNU_PROPERTY_AARCH64_FEATURE_1_AND and GNU_PROPERTY_RISCV_FEATURE_1_AND have the same value.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add static_assert when I use GNU_PROPERTY_AARCH64_FEATURE_1_AND to check
Thanks

@MaskRay
Copy link
Member

MaskRay commented May 10, 2024

Unless lld/test/ELF tests will get a failure, the lld/ part should ideally be dropped from this PR.
This separation is a convention.

Otherwise (a) with just a [RISCV] tag and (b) with many RISCV-specific changes, lld folks (like I) might not notice this patch.

@@ -0,0 +1,29 @@
// When -march with zicfiss0p4 or zicfilp0p4 add GNU property to file object
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This directory is for driver tests. Codegen tests should use %clang_cc1 and be moved elsewhere (e.g. test/CodeGen)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I should only test in llvm-readobj not in Driver
Thanks

@topperc
Copy link
Collaborator

topperc commented May 10, 2024

Unless lld/test/ELF tests will get a failure, the lld/ part should ideally be dropped from this PR. This separation is a convention.

Otherwise (a) with just a [RISCV] tag and (b) with many RISCV-specific changes, lld folks (like I) might not notice this patch.

Are you saying that lld changes should be a different PR?

llvm/tools/llvm-readobj/ELFDumper.cpp Outdated Show resolved Hide resolved
llvm/tools/llvm-readobj/ELFDumper.cpp Outdated Show resolved Hide resolved
llvm/lib/Target/RISCV/MCTargetDesc/RISCVTargetStreamer.h Outdated Show resolved Hide resolved

unsigned GNUNoteFlags = 0;

// check ZICFILP or ZICFISS with features
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure this adds anything, but if it's staying please properly style it (e.g. start with a capital letter)

unsigned GNUNoteFlags = 0;

// check ZICFILP or ZICFISS with features
// TODO should we check with codegen enable ex. -mllvm
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like it should be resolved before merging?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think we shouldn't just check if there is CFI extension is on,
we should also check if the codegen options are really enabled.
Ex. following code in RISCVFrameLowering

@yetingk Do you think so?

if (!STI.hasForcedSWShadowStack() && STI.hasStdExtZicfiss()) {
  BuildMI(MBB, MI, DL, TII->get(RISCV::SSPUSH)).addReg(RAReg);
    return;
  }

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SuHo-llrr shadow stack depends on -fsanitize=shadow-call-stack. I think you need to also check this.

@@ -85,6 +85,35 @@ void RISCVTargetELFStreamer::finishAttributeSection() {
ELF::SHT_RISCV_ATTRIBUTES, AttributeSection);
}

void RISCVTargetELFStreamer::emitNoteSection(unsigned Flags) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know this is what AArch64 calls it, but it's a very vague name that could cover any number of note sections. Can we do better and make ours specific to what it's actually doing, i.e. something about it being a GNU properties note?

@SuHo-llrr SuHo-llrr force-pushed the upstream-main-cfi branch 2 times, most recently from 46e347d to 1fcb8b9 Compare May 13, 2024 13:01
@SuHo-llrr
Copy link
Author

Unless lld/test/ELF tests will get a failure, the lld/ part should ideally be dropped from this PR. This separation is a convention.
Otherwise (a) with just a [RISCV] tag and (b) with many RISCV-specific changes, lld folks (like I) might not notice this patch.

Are you saying that lld changes should be a different PR?

I think you are right, I should split lld Part to different PR, Thanks.

@@ -331,6 +333,8 @@ struct Config {
bool zText;
bool zRetpolineplt;
bool zWxneeded;
bool zForceZicfilp;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if using bool here is suitable now. Currently, we have a second PLT scheme for Zicfilp: the function-signature-based one at riscv-non-isa/riscv-elf-psabi-doc#434 , so in the foreseeable future, this field would likely to be changed to an enum.

@@ -85,6 +85,37 @@ void RISCVTargetELFStreamer::finishAttributeSection() {
ELF::SHT_RISCV_ATTRIBUTES, AttributeSection);
}

void RISCVTargetELFStreamer::emitGNUProgramProperties(unsigned Flags) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change the name of Flags to AndFlags: Although currently only the AND flag is used, the .note.gnu.property section actually defines more flags besides the AND one, so if you're emitting the whole section with this function (as indicated by the function name), you should be clear about what flag of the section you're emitting.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I should move emit program properties to TargetStreamer.
Thanks,

: GNU_PROPERTY_X86_FEATURE_1_AND;
uint32_t featureAndType = 0;
switch (config->emachine) {
default:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also change the default statement to EM_X86_64 ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:RISC-V clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang Clang issues not falling into any other category lld:ELF lld llvm:binary-utilities
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants