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

[OFFLOAD] Update ffi_cif structure to match libffi #128756

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

adurang
Copy link
Contributor

@adurang adurang commented Feb 25, 2025

The ffi_cif structure defined in the wrapper header is smaller than the actual structure in libffi which results in other structures being overwritten when libffi is called, and finally in a segfault.

The patch updates the structure to the correct layout as specified in ffi.h

The ffi_cif structure defined in the wrapper header is smaller than
the actual structure in libffi which results in other structures
being overwritten when libffi is called, and finally in a segfault.

The patch updates the structure to the correct layout as specified in
ffi.h
@llvmbot
Copy link
Member

llvmbot commented Feb 25, 2025

@llvm/pr-subscribers-offload

Author: Alex (adurang)

Changes

The ffi_cif structure defined in the wrapper header is smaller than the actual structure in libffi which results in other structures being overwritten when libffi is called, and finally in a segfault.

The patch updates the structure to the correct layout as specified in ffi.h


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

1 Files Affected:

  • (modified) offload/plugins-nextgen/host/dynamic_ffi/ffi.h (+10)
diff --git a/offload/plugins-nextgen/host/dynamic_ffi/ffi.h b/offload/plugins-nextgen/host/dynamic_ffi/ffi.h
index 80aa512236d28..33285b4aef402 100644
--- a/offload/plugins-nextgen/host/dynamic_ffi/ffi.h
+++ b/offload/plugins-nextgen/host/dynamic_ffi/ffi.h
@@ -53,6 +53,16 @@ typedef enum ffi_abi {
 #else
 #error "Unknown ABI"
 #endif
+} ffi_abi;
+
+typedef struct {
+  ffi_abi abi;
+  unsigned nargs;
+  ffi_type **arg_types;
+  ffi_type *rtype;
+  unsigned bytes;
+  unsigned flags;
+  long long extra_fields; // Longest extra field defined in the FFI sources
 } ffi_cif;
 
 #ifdef __cplusplus

Copy link
Contributor

@jhuber6 jhuber6 left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this, but I really wish we could delete this and resolve #91264. The confounding issue is the fact that the OpenMP implicit arguments are eagerly prepended instead of appended late.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants