remove need for -fno-pie and -no-pie by solskogen · Pull Request #1817 · BlitterStudio/amiberry

solskogen · 2026-03-04T07:00:14Z

Copilot

Pull request overview

This PR aims to stop forcing non-PIE builds for JIT and removes the ARM-side compile-time guard that rejected PIE builds.

Changes:

Removed the __pie__/__PIE__ compile-time error from the ARM JIT support file.
Removed the -fno-pie and -no-pie compile/link flags from the CMake target (but left behind empty target_*_options() calls).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
`src/jit/arm/compemu_support_arm.cpp`	Allows building the ARM JIT code even when the toolchain defines PIE (`__pie__/__PIE__`).
`cmake/SourceFiles.cmake`	Stops explicitly overriding PIE in non-Android/non-Windows builds (currently leaving empty options calls).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-04T07:04:40Z

cmake/SourceFiles.cmake

 if(NOT ANDROID AND NOT WIN32)
-    target_compile_options(${PROJECT_NAME} PRIVATE -fno-pie)
-    target_link_options(${PROJECT_NAME} PRIVATE -no-pie)
+    target_compile_options(${PROJECT_NAME} PRIVATE )
+    target_link_options(${PROJECT_NAME} PRIVATE )


The if(NOT ANDROID AND NOT WIN32) block now calls target_compile_options / target_link_options with no options, which is a no-op at best and makes the build logic harder to understand. Remove these empty calls (and likely the whole if branch) rather than leaving placeholder commands behind.

@copilot update pull request to apply changes based on this feedback

cmake/SourceFiles.cmake

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/jit/x86/compemu_support_x86.cpp

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

cmake/SourceFiles.cmake:568

Dropping the global -fno-pie / -no-pie override means Linux/BSD toolchains that default to PIE will now build the main executable as PIE. The x86_64 JIT exception-handler path still contains logic that relies on non-PIE global placement (it allocates veccode with UAE_VM_32BIT/MAP_32BIT specifically because it expects globals to be within range of low 2GB mappings). This can cause runtime failures when RIP-relative references from veccode can’t reach PIE-loaded globals. Consider either (1) keeping non-PIE only for the affected JIT configurations/architectures, or (2) making the exception handler’s veccode allocation anchor near the .data segment so PIE builds remain safe.

if(WIN32 AND CMAKE_SIZEOF_VOID_P EQUAL 8)
    # x86_64 JIT is now 64-bit pointer-clean: no longer needs ASLR-disabling
    # linker flags or forced low image base.
    # Mark as GUI application in Release builds to suppress the console
    # window. Debug builds keep the console for convenience.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

midwan

@solskogen did you test this on x86-64? It seems likely that it would break JIT there. It's probably safe for ARM64 JIT however.

ARM64 — The PIE guard removal is safe. ARM64 JIT is already fully 64-bit pointer-clean: UAE_VM_32BIT is dropped for natmem, vm_acquire()/vm_acquire_code() clear the 32-bit flag on CPU_AARCH64, and there's no RIP-relative addressing.

x86-64 — The PIE guard removal is unsafe. The x86-64 JIT generates RIP-relative [rip+disp32] encodings (via _r_X macro in codegen_x86.h:423) to reference globals like regs.pc_p, specflags, live.state[].mem, and bi->pc_p. These 32-bit displacements require all targets within ±2GB of the JIT code. With PIE, ASLR moves globals to high addresses while JIT code sits in the low 2GB (via MAP_32BIT), breaking the ±2GB invariant.

Additionally, the exception handler (exception_handler.cpp:890-894) explicitly documents its dependency on non-PIE globals being in range.

Recommended approach:

Split the PR — accept ARM64 PIE support, keep x86-64 restrictions
Or scope the CMake flags to x86-64 only:

if(NOT ANDROID AND NOT WIN32 AND CMAKE_SYSTEM_PROCESSOR MATCHES "x86_64|amd64")
    target_compile_options(${PROJECT_NAME} PRIVATE -fno-pie)
    target_link_options(${PROJECT_NAME} PRIVATE -no-pie)
endif()

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-04T13:28:54Z

cmake/SourceFiles.cmake

+if(NOT ANDROID AND NOT WIN32 AND CMAKE_SYSTEM_PROCESSOR MATCHES "x86_64|amd64")
    target_compile_options(${PROJECT_NAME} PRIVATE -fno-pie)
    target_link_options(${PROJECT_NAME} PRIVATE -no-pie)


The PR title indicates we no longer need to force non-PIE builds, but SourceFiles.cmake still applies -fno-pie/-no-pie for non-Android/non-Windows x86_64/amd64. If PIE is now supported (as implied by removing the __pie__/__PIE__ hard-error guards), these options should be removed; otherwise the PR still disables PIE/ASLR on those builds and doesn’t match the stated goal.

midwan · 2026-03-04T13:37:47Z

cmake/SourceFiles.cmake

-# come after static libs that depend on them (enet, etc.).

-if(NOT ANDROID AND NOT WIN32)
+if(NOT ANDROID AND NOT WIN32 AND CMAKE_SYSTEM_PROCESSOR MATCHES "x86_64|amd64")


CMAKE_SYSTEM_PROCESSOR matching is case-sensitive. "x86_64|amd64" covers the standard Linux and FreeBSD values, but some cross-compilation setups or toolchains might report X86_64 or AMD64. A safer pattern would be:

if(NOT ANDROID AND NOT WIN32 AND CMAKE_SYSTEM_PROCESSOR MATCHES "[xX]86_64|[aA][mM][dD]64")

Low risk in practice since native builds always report lowercase, but worth considering for robustness.

midwan · 2026-03-04T13:38:55Z

src/jit/x86/compemu_support_x86.cpp

 #endif
 #include "uae/log.h"

-#if defined(__pie__) || defined (__PIE__)


This is a problematic change. The x86-64 JIT fundamentally depends on non-PIE:

RIP-relative [rip+disp32] encodings require globals within ±2GB of JIT code
UAE_VM_32BIT (MAP_32BIT) anchors natmem allocations in the low 4GB
There are 432 uses of JITPTR and related pointer arithmetic across 8 files
The exception handler (exception_handler.cpp:890-894) explicitly documents this dependency
The CMake flag change protects standard Linux/FreeBSD builds, but the #error guard is a defense-in-depth mechanism that catches:

Custom build systems (Makefiles, Meson, etc.) that don't use CMake
CMake misconfigurations or overrides (-DCMAKE_POSITION_INDEPENDENT_CODE=ON)
Distro packaging scripts that force PIE globally
Recommendation: Keep the guard in the x86 JIT but scope it to x86-64 only:

#if defined(CPU_x86_64) && (defined(__pie__) || defined(__PIE__)) #error Position-independent code (PIE) cannot be used with x86-64 JIT #endif

This lets ARM64 builds proceed with PIE while still catching dangerous x86-64 PIE builds at compile time.

Or ideally we'll need to do the work to make x86-64 JIT truly 64-bit clean, not needing PIE as well (like the work I did on the ARM64 JIT).

midwan · 2026-03-04T18:52:32Z

This PR is obsolete with the changes in #1820

solskogen requested a review from midwan as a code owner March 4, 2026 07:00

Copilot AI review requested due to automatic review settings March 4, 2026 07:00

Copilot started reviewing on behalf of solskogen March 4, 2026 07:00 View session

Copilot AI reviewed Mar 4, 2026

View reviewed changes

solskogen force-pushed the pie-patch branch from 321c55f to 66cf7af Compare March 4, 2026 07:20

solskogen requested a review from Copilot March 4, 2026 07:21

Copilot started reviewing on behalf of solskogen March 4, 2026 07:22 View session

Copilot AI reviewed Mar 4, 2026

View reviewed changes

src/jit/x86/compemu_support_x86.cpp Show resolved Hide resolved

solskogen requested a review from Copilot March 4, 2026 11:23

Copilot started reviewing on behalf of solskogen March 4, 2026 11:24 View session

Copilot AI reviewed Mar 4, 2026

View reviewed changes

midwan requested changes Mar 4, 2026

View reviewed changes

remove need for -fno-pie and -no-pie on aarch64

525a517

solskogen force-pushed the pie-patch branch from bb922b6 to 525a517 Compare March 4, 2026 13:21

Merge branch 'master' into pie-patch

963962b

solskogen requested review from Copilot and midwan March 4, 2026 13:23

Copilot started reviewing on behalf of solskogen March 4, 2026 13:24 View session

Copilot AI reviewed Mar 4, 2026

View reviewed changes

midwan requested changes Mar 4, 2026

View reviewed changes

midwan closed this Mar 4, 2026

Uh oh!

Conversation

solskogen commented Mar 4, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

solskogen Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

midwan left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

midwan Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

midwan Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

midwan Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

midwan commented Mar 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants