Skip to content

Conversation

mandlebug
Copy link
Member

The instructions are not supported on either 32-bit ELF (due to no redzone) or 32-bit AIX due to the instructions always using the full 64-bit width of the register inputs.

The instructions are not supported on either 32-bit ELF (due to no redzone)
or 32-bit AIX due to the instructions always using the full 64-bit
width of the register inputs.
@mandlebug mandlebug self-assigned this May 12, 2025
@llvmbot llvmbot added clang Clang issues not falling into any other category backend:PowerPC clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang:frontend Language frontend issues, e.g. anything involving "Sema" labels May 12, 2025
@llvmbot
Copy link
Member

llvmbot commented May 12, 2025

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-driver

Author: Sean Fertile (mandlebug)

Changes

The instructions are not supported on either 32-bit ELF (due to no redzone) or 32-bit AIX due to the instructions always using the full 64-bit width of the register inputs.


Full diff: https://github.com/llvm/llvm-project/pull/139619.diff

2 Files Affected:

  • (modified) clang/lib/Basic/Targets/PPC.cpp (+11-5)
  • (modified) clang/test/Driver/ppc-mrop-protection-support-check.c (+6)
diff --git a/clang/lib/Basic/Targets/PPC.cpp b/clang/lib/Basic/Targets/PPC.cpp
index 425ad68bb9098..e6ef0ecc526ba 100644
--- a/clang/lib/Basic/Targets/PPC.cpp
+++ b/clang/lib/Basic/Targets/PPC.cpp
@@ -679,11 +679,17 @@ bool PPCTargetInfo::initFeatureMap(
     }
   }
 
-  if (!(ArchDefs & ArchDefinePwr8) &&
-      llvm::is_contained(FeaturesVec, "+rop-protect")) {
-    // We can turn on ROP Protect on Power 8 and above.
-    Diags.Report(diag::err_opt_not_valid_with_opt) << "-mrop-protect" << CPU;
-    return false;
+  if (llvm::is_contained(FeaturesVec, "+rop-protect")) {
+    if (PointerWidth == 32) {
+      Diags.Report(diag::err_opt_not_valid_on_target) << "-mrop-protect";
+      return false;
+    }
+
+    if (!(ArchDefs & ArchDefinePwr8)) {
+      // We can turn on ROP Protect on Power 8 and above.
+      Diags.Report(diag::err_opt_not_valid_with_opt) << "-mrop-protect" << CPU;
+      return false;
+    }
   }
 
   if (!(ArchDefs & ArchDefinePwr8) &&
diff --git a/clang/test/Driver/ppc-mrop-protection-support-check.c b/clang/test/Driver/ppc-mrop-protection-support-check.c
index 50eaef3ed770b..9081c583de8bf 100644
--- a/clang/test/Driver/ppc-mrop-protection-support-check.c
+++ b/clang/test/Driver/ppc-mrop-protection-support-check.c
@@ -16,6 +16,11 @@
 // RUN: not %clang -target powerpc64le-unknown-linux-gnu -fsyntax-only \
 // RUN:   -mcpu=power7 -mrop-protect %s 2>&1 | FileCheck %s --check-prefix=NOROP
 
+// RUN: not %clang -target powerpc-unknown-linux -fsyntax-only \
+// RUN: -mcpu=pwr8 -mrop-protect %s 2>&1 | FileCheck %s --check-prefix=32BIT
+// RUN: not %clang -target powerpc-unknown-aix -fsyntax-only \
+// RUN: -mcpu=pwr8 -mrop-protect %s 2>&1 | FileCheck %s --check-prefix=32BIT
+
 #ifdef __ROP_PROTECT__
 static_assert(false, "ROP Protect enabled");
 #endif
@@ -24,3 +29,4 @@ static_assert(false, "ROP Protect enabled");
 // HASROP-NOT: option '-mrop-protect' cannot be specified with
 // NOROP: option '-mrop-protect' cannot be specified with
 
+// 32BIT: option '-mrop-protect' cannot be specified on this target

@llvmbot
Copy link
Member

llvmbot commented May 12, 2025

@llvm/pr-subscribers-backend-powerpc

Author: Sean Fertile (mandlebug)

Changes

The instructions are not supported on either 32-bit ELF (due to no redzone) or 32-bit AIX due to the instructions always using the full 64-bit width of the register inputs.


Full diff: https://github.com/llvm/llvm-project/pull/139619.diff

2 Files Affected:

  • (modified) clang/lib/Basic/Targets/PPC.cpp (+11-5)
  • (modified) clang/test/Driver/ppc-mrop-protection-support-check.c (+6)
diff --git a/clang/lib/Basic/Targets/PPC.cpp b/clang/lib/Basic/Targets/PPC.cpp
index 425ad68bb9098..e6ef0ecc526ba 100644
--- a/clang/lib/Basic/Targets/PPC.cpp
+++ b/clang/lib/Basic/Targets/PPC.cpp
@@ -679,11 +679,17 @@ bool PPCTargetInfo::initFeatureMap(
     }
   }
 
-  if (!(ArchDefs & ArchDefinePwr8) &&
-      llvm::is_contained(FeaturesVec, "+rop-protect")) {
-    // We can turn on ROP Protect on Power 8 and above.
-    Diags.Report(diag::err_opt_not_valid_with_opt) << "-mrop-protect" << CPU;
-    return false;
+  if (llvm::is_contained(FeaturesVec, "+rop-protect")) {
+    if (PointerWidth == 32) {
+      Diags.Report(diag::err_opt_not_valid_on_target) << "-mrop-protect";
+      return false;
+    }
+
+    if (!(ArchDefs & ArchDefinePwr8)) {
+      // We can turn on ROP Protect on Power 8 and above.
+      Diags.Report(diag::err_opt_not_valid_with_opt) << "-mrop-protect" << CPU;
+      return false;
+    }
   }
 
   if (!(ArchDefs & ArchDefinePwr8) &&
diff --git a/clang/test/Driver/ppc-mrop-protection-support-check.c b/clang/test/Driver/ppc-mrop-protection-support-check.c
index 50eaef3ed770b..9081c583de8bf 100644
--- a/clang/test/Driver/ppc-mrop-protection-support-check.c
+++ b/clang/test/Driver/ppc-mrop-protection-support-check.c
@@ -16,6 +16,11 @@
 // RUN: not %clang -target powerpc64le-unknown-linux-gnu -fsyntax-only \
 // RUN:   -mcpu=power7 -mrop-protect %s 2>&1 | FileCheck %s --check-prefix=NOROP
 
+// RUN: not %clang -target powerpc-unknown-linux -fsyntax-only \
+// RUN: -mcpu=pwr8 -mrop-protect %s 2>&1 | FileCheck %s --check-prefix=32BIT
+// RUN: not %clang -target powerpc-unknown-aix -fsyntax-only \
+// RUN: -mcpu=pwr8 -mrop-protect %s 2>&1 | FileCheck %s --check-prefix=32BIT
+
 #ifdef __ROP_PROTECT__
 static_assert(false, "ROP Protect enabled");
 #endif
@@ -24,3 +29,4 @@ static_assert(false, "ROP Protect enabled");
 // HASROP-NOT: option '-mrop-protect' cannot be specified with
 // NOROP: option '-mrop-protect' cannot be specified with
 
+// 32BIT: option '-mrop-protect' cannot be specified on this target

Copy link
Contributor

@amy-kwan amy-kwan left a comment

Choose a reason for hiding this comment

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

Makes sense and LGTM.

@mandlebug mandlebug merged commit 4ac8e90 into llvm:main May 14, 2025
16 checks passed
@mandlebug mandlebug deleted the sfertile/PPC_ROPProtect_32BitUnsupported branch May 14, 2025 17:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:PowerPC clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants