Skip to content

Conversation

@jakemas
Copy link
Contributor

@jakemas jakemas commented Dec 19, 2025

Service Indicator: Add error call trampoline to avoid delocator issue

Problem

On AArch64, the delocator can patch up PC-relative addressing only if the references can be computed with a PC-relative offset in the range (-1MB, 1MB).

For the OPENSSL_PUT_ERROR macro calls in crypto/fipsmodule/service_indicator/service_indicator.c, the __FILE__ string literal references were exceeding the ARM64 ADR instruction's ±1MB range limit as the FIPS module grew larger. This manifested as build failures in ARM64 FIPS builds:

error: fixup value out of range
 adr x3, .L.str.192
 ^

The error originated from string literals being placed too far from their usage sites (e.g., string at line ~365493, first use at line ~2597 in bcm-delocated.S).

Solution

This commit fixes the issue by adding a non-inlined trampoline function for error calls:

#if defined(_MSC_VER)
__declspec(noinline)
#else
__attribute__((noinline))
#endif
static void put_set_thread_local_error(void) {
  OPENSSL_PUT_ERROR(CRYPTO, ERR_R_INTERNAL_ERROR);
}

This ensures the __FILE__ string literal stays close to its usage site and can be addressed using a PC-relative offset within the allowed range.

Key Distinction from Function Pointer Trampolines

This fix differs from previous function pointer trampolines (e.g., #2165), which wrapped assembly functions and used branch instructions to call them. Here, the pointer is not to a function but to a string literal (__FILE__ from error macros), so instead we wrap the function that uses the pointer to keep the string reference close to its usage. The noinline directive is critical - without it, the compiler will inline the function, placing the __FILE__ reference back at the original call sites and recreating the distance problem.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license.

@jakemas jakemas mentioned this pull request Dec 19, 2025
@jakemas jakemas marked this pull request as ready for review December 19, 2025 20:16
@jakemas jakemas requested a review from a team as a code owner December 19, 2025 20:16
@codecov-commenter
Copy link

codecov-commenter commented Dec 19, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.23%. Comparing base (989f64c) to head (05f899e).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2920   +/-   ##
=======================================
  Coverage   78.22%   78.23%           
=======================================
  Files         690      690           
  Lines      118745   118745           
  Branches    16679    16679           
=======================================
+ Hits        92894    92899    +5     
+ Misses      24961    24958    -3     
+ Partials      890      888    -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment on lines +27 to +28
#if defined(_MSC_VER)
__declspec(noinline)
Copy link
Contributor

Choose a reason for hiding this comment

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

NP: this line is not strictly necessary since the delocator is not used on Windows; but I think it's fine for consistency.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants