Skip to content

Navigation Menu

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

[PPC] Disable rop-protect for 32-bit OS targets. #139619

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

Merged

Conversation

mandlebug
Copy link
Contributor

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
Morty Proxy This is a proxified and sanitized view of the page, visit original site.