Skip to content

Commit

Permalink
fix compilation on some CPUs and Windows (#1901)
Browse files Browse the repository at this point in the history
This patch contains two part:

* use cshim for sigsetjmp on CPUs/OSes that are not supported by
`cee-scape`'s asm (needed by #1897 and Windows)
* fix linking error on Windows in #1133
  • Loading branch information
usamoi authored Oct 8, 2024
1 parent 9ed73d5 commit 3ea09af
Show file tree
Hide file tree
Showing 12 changed files with 146 additions and 73 deletions.
20 changes: 11 additions & 9 deletions pgrx-bindgen/src/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -794,6 +794,7 @@ fn run_bindgen(
.default_non_copy_union_style(NonCopyUnionStyle::ManuallyDrop)
.wrap_static_fns(enable_cshim)
.wrap_static_fns_path(out_path.join("pgrx-cshim-static"))
.wrap_static_fns_suffix("__pgrx_cshim")
.generate()
.wrap_err_with(|| format!("Unable to generate bindings for pg{major_version}"))?;
let mut binding_str = bindings.to_string();
Expand Down Expand Up @@ -877,6 +878,14 @@ fn add_blocklists(bind: bindgen::Builder) -> bindgen::Builder {
.blocklist_function("range_table_walker")
.blocklist_function("raw_expression_tree_walker")
.blocklist_function("type_is_array")
// These structs contains array that is larger than 32
// so that `derive(Default)` would fail.
.blocklist_type("tagMONITORINFOEXA")
.blocklist_type("MONITORINFOEXA")
.blocklist_type("LPMONITORINFOEXA")
.blocklist_type("MONITORINFOEX")
.blocklist_type("LPMONITORINFOEX")
.blocklist_function("ua_.*") // this should be Windows's names
}

fn add_derives(bind: bindgen::Builder) -> bindgen::Builder {
Expand Down Expand Up @@ -1116,18 +1125,11 @@ fn apply_pg_guard(items: &Vec<syn::Item>) -> eyre::Result<proc_macro2::TokenStre
match item {
Item::ForeignMod(block) => {
let abi = &block.abi;
let (mut extern_funcs, mut others) = (Vec::new(), Vec::new());
block.items.iter().filter(|&item| !is_blocklisted_item(item)).cloned().for_each(
|item| match item {
ForeignItem::Fn(func) => extern_funcs.push(func),
item => others.push(item),
},
);
let items = block.items.iter().filter(|&item| !is_blocklisted_item(item));
out.extend(quote! {
#[pgrx_macros::pg_guard]
#abi { #(#extern_funcs)* }
#abi { #(#items)* }
});
out.extend(quote! { #abi { #(#others)* } });
}
_ => {
out.extend(item.into_token_stream());
Expand Down
32 changes: 29 additions & 3 deletions pgrx-macros/src/rewriter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ use std::str::FromStr;
use syn::punctuated::Punctuated;
use syn::spanned::Spanned;
use syn::{
FnArg, ForeignItem, ForeignItemFn, GenericParam, ItemFn, ItemForeignMod, Pat, Signature, Token,
Visibility,
Expr, ExprLit, FnArg, ForeignItem, ForeignItemFn, ForeignItemStatic, GenericParam, ItemFn,
ItemForeignMod, Lit, Pat, Signature, Token, Visibility,
};

macro_rules! format_ident {
Expand Down Expand Up @@ -126,6 +126,7 @@ fn foreign_item(item: ForeignItem, abi: &syn::Abi) -> syn::Result<proc_macro2::T

foreign_item_fn(&func, abi)
}
ForeignItem::Static(variable) => foreign_item_static(&variable, abi),
_ => Ok(quote! { #abi { #item } }),
}
}
Expand All @@ -135,19 +136,44 @@ fn foreign_item_fn(func: &ForeignItemFn, abi: &syn::Abi) -> syn::Result<proc_mac
let arg_list = rename_arg_list(&func.sig)?;
let arg_list_with_types = rename_arg_list_with_types(&func.sig)?;
let return_type = func.sig.output.clone();
let link_with_cshim = func.attrs.iter().any(|attr| match &attr.meta {
syn::Meta::NameValue(kv) if kv.path.get_ident().filter(|x| *x == "link_name").is_some() => {
if let Expr::Lit(ExprLit { lit: Lit::Str(value), .. }) = &kv.value {
value.value().ends_with("__pgrx_cshim")
} else {
false
}
}
_ => false,
});
let link = if link_with_cshim {
quote! {}
} else {
quote! { #[cfg_attr(target_os = "windows", link(name = "postgres"))] }
};

Ok(quote! {
#[inline]
#[track_caller]
pub unsafe fn #func_name ( #arg_list_with_types ) #return_type {
crate::ffi::pg_guard_ffi_boundary(move || {
#abi { #func }
#link #abi { #func }
#func_name(#arg_list)
})
}
})
}

fn foreign_item_static(
variable: &ForeignItemStatic,
abi: &syn::Abi,
) -> syn::Result<proc_macro2::TokenStream> {
let link = quote! { #[cfg_attr(target_os = "windows", link(name = "postgres"))] };
Ok(quote! {
#link #abi { #variable }
})
}

#[allow(clippy::cmp_owned)]
fn build_arg_list(sig: &Signature, suffix_arg_name: bool) -> syn::Result<proc_macro2::TokenStream> {
let mut arg_list = proc_macro2::TokenStream::new();
Expand Down
1 change: 1 addition & 0 deletions pgrx-pg-sys/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ serde.workspace = true # impls on pub types
# polyfill until #![feature(strict_provenance)] stabilizes
sptr = "0.3"

[target.'cfg(all(any(target_os = "linux", target_os = "macos"), any(target_arch = "x86_64", target_arch = "aarch64")))'.dependencies]
# Safer `sigsetjmp` and `siglongjmp`
cee-scape = "0.2"

Expand Down
42 changes: 14 additions & 28 deletions pgrx-pg-sys/pgrx-cshim.c
Original file line number Diff line number Diff line change
Expand Up @@ -10,42 +10,28 @@

#include "pgrx-cshim-static.c"

PGDLLEXPORT void *pgrx_list_nth(List *list, int nth);
void *pgrx_list_nth(List *list, int nth) {
return list_nth(list, nth);
}

PGDLLEXPORT int pgrx_list_nth_int(List *list, int nth);
int pgrx_list_nth_int(List *list, int nth) {
return list_nth_int(list, nth);
}

PGDLLEXPORT Oid pgrx_list_nth_oid(List *list, int nth);
Oid pgrx_list_nth_oid(List *list, int nth) {
return list_nth_oid(list, nth);
}

PGDLLEXPORT ListCell *pgrx_list_nth_cell(List *list, int nth);
ListCell *pgrx_list_nth_cell(List *list, int nth) {
return list_nth_cell(list, nth);
}

PGDLLEXPORT void pgrx_SpinLockInit(volatile slock_t *lock);
void pgrx_SpinLockInit(volatile slock_t *lock) {
void SpinLockInit__pgrx_cshim(volatile slock_t *lock) {
SpinLockInit(lock);
}

PGDLLEXPORT void pgrx_SpinLockAcquire(volatile slock_t *lock);
void pgrx_SpinLockAcquire(volatile slock_t *lock) {
void SpinLockAcquire__pgrx_cshim(volatile slock_t *lock) {
SpinLockAcquire(lock);
}

PGDLLEXPORT void pgrx_SpinLockRelease(volatile slock_t *lock);
void pgrx_SpinLockRelease(volatile slock_t *lock) {
void SpinLockRelease__pgrx_cshim(volatile slock_t *lock) {
SpinLockRelease(lock);
}

PGDLLEXPORT bool pgrx_SpinLockFree(slock_t *lock);
bool pgrx_SpinLockFree(slock_t *lock) {
bool SpinLockFree__pgrx_cshim(slock_t *lock) {
return SpinLockFree(lock);
}

int call_closure_with_sigsetjmp(int savemask, void* closure_env_ptr, int (*closure_code)(sigjmp_buf jbuf, void *env_ptr)) {
sigjmp_buf jbuf;
int val;
if (0 == (val = sigsetjmp(jbuf, savemask))) {
return closure_code(jbuf, closure_env_ptr);
} else {
return val;
}
}
14 changes: 4 additions & 10 deletions pgrx-pg-sys/src/cshim.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,15 @@
#![allow(deprecated)]

use crate as pg_sys;
use core::ffi;

#[pgrx_macros::pg_guard]
extern "C" {
pub fn pgrx_list_nth(list: *mut pg_sys::List, nth: i32) -> *mut ffi::c_void;
pub fn pgrx_list_nth_int(list: *mut pg_sys::List, nth: i32) -> i32;
pub fn pgrx_list_nth_oid(list: *mut pg_sys::List, nth: i32) -> pg_sys::Oid;
pub fn pgrx_list_nth_cell(list: *mut pg_sys::List, nth: i32) -> *mut pg_sys::ListCell;

#[link_name = "pgrx_SpinLockInit"]
#[link_name = "SpinLockInit__pgrx_cshim"]
pub fn SpinLockInit(lock: *mut pg_sys::slock_t);
#[link_name = "pgrx_SpinLockAcquire"]
#[link_name = "SpinLockAcquire__pgrx_cshim"]
pub fn SpinLockAcquire(lock: *mut pg_sys::slock_t);
#[link_name = "pgrx_SpinLockRelease"]
#[link_name = "SpinLockRelease__pgrx_cshim"]
pub fn SpinLockRelease(lock: *mut pg_sys::slock_t);
#[link_name = "pgrx_SpinLockFree"]
#[link_name = "SpinLockFree__pgrx_cshim"]
pub fn SpinLockFree(lock: *mut pg_sys::slock_t) -> bool;
}
61 changes: 59 additions & 2 deletions pgrx-pg-sys/src/submodules/ffi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,64 @@
//LICENSE Use of this source code is governed by the MIT license that can be found in the LICENSE file.
#![deny(unsafe_op_in_unsafe_fn)]

use cee_scape::SigJmpBufFields;
#[cfg(not(all(
any(target_os = "linux", target_os = "macos"),
any(target_arch = "x86_64", target_arch = "aarch64")
)))]
mod cee_scape {
#[cfg(not(feature = "cshim"))]
compile_error!("target platform cannot work without feature cshim");

use libc::{c_int, c_void};
use std::marker::PhantomData;

#[repr(C)]
pub struct SigJmpBufFields {
_internal: [u8; 0],
_neither_send_nor_sync: PhantomData<*const u8>,
}

pub fn call_with_sigsetjmp<F>(savemask: bool, mut callback: F) -> c_int
where
F: for<'a> FnOnce(&'a SigJmpBufFields) -> c_int,
{
extern "C" {
fn call_closure_with_sigsetjmp(
savemask: c_int,
closure_env_ptr: *mut c_void,
closure_code: extern "C" fn(
jbuf: *const SigJmpBufFields,
env_ptr: *mut c_void,
) -> c_int,
) -> c_int;
}

extern "C" fn call_from_c_to_rust<F>(
jbuf: *const SigJmpBufFields,
closure_env_ptr: *mut c_void,
) -> c_int
where
F: for<'a> FnOnce(&'a SigJmpBufFields) -> c_int,
{
let closure_env_ptr: *mut F = closure_env_ptr as *mut F;
unsafe { (closure_env_ptr.read())(&*jbuf) }
}

let savemask: libc::c_int = if savemask { 1 } else { 0 };

unsafe {
let closure_env_ptr = core::ptr::addr_of_mut!(callback);
core::mem::forget(callback);
call_closure_with_sigsetjmp(
savemask,
closure_env_ptr as *mut libc::c_void,
call_from_c_to_rust::<F>,
)
}
}
}

use cee_scape::{call_with_sigsetjmp, SigJmpBufFields};

/**
Given a closure that is assumed to be a wrapped Postgres `extern "C"` function, [pg_guard_ffi_boundary]
Expand Down Expand Up @@ -113,7 +170,7 @@ unsafe fn pg_guard_ffi_boundary_impl<T, F: FnOnce() -> T>(f: F) -> T {
let prev_exception_stack = pg_sys::PG_exception_stack;
let prev_error_context_stack = pg_sys::error_context_stack;
let mut result: std::mem::MaybeUninit<T> = MaybeUninit::uninit();
let jump_value = cee_scape::call_with_sigsetjmp(false, |jump_buffer| {
let jump_value = call_with_sigsetjmp(false, |jump_buffer| {
// Make Postgres' error-handling system aware of our new
// setjmp/longjmp restore point.
pg_sys::PG_exception_stack = std::mem::transmute(jump_buffer as *const SigJmpBufFields);
Expand Down
4 changes: 4 additions & 0 deletions pgrx-pg-sys/src/submodules/panic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -395,6 +395,7 @@ where
match unsafe { run_guarded(AssertUnwindSafe(f)) } {
GuardAction::Return(r) => r,
GuardAction::ReThrow => {
#[cfg_attr(target_os = "windows", link(name = "postgres"))]
extern "C" /* "C-unwind" */ {
fn pg_re_throw() -> !;
}
Expand Down Expand Up @@ -509,6 +510,7 @@ fn do_ereport(ereport: ErrorReportWithLevel) {
// `build.rs` and we'd prefer pgrx users not have access to them at all
//

#[cfg_attr(target_os = "windows", link(name = "postgres"))]
extern "C" {
fn errcode(sqlerrcode: ::std::os::raw::c_int) -> ::std::os::raw::c_int;
fn errmsg(fmt: *const ::std::os::raw::c_char, ...) -> ::std::os::raw::c_int;
Expand All @@ -524,6 +526,7 @@ fn do_ereport(ereport: ErrorReportWithLevel) {
#[cfg(any(feature = "pg13", feature = "pg14", feature = "pg15", feature = "pg16", feature = "pg17"))]
fn do_ereport_impl(ereport: ErrorReportWithLevel) {

#[cfg_attr(target_os = "windows", link(name = "postgres"))]
extern "C" {
fn errstart(elevel: ::std::os::raw::c_int, domain: *const ::std::os::raw::c_char) -> bool;
fn errfinish(filename: *const ::std::os::raw::c_char, lineno: ::std::os::raw::c_int, funcname: *const ::std::os::raw::c_char);
Expand Down Expand Up @@ -588,6 +591,7 @@ fn do_ereport(ereport: ErrorReportWithLevel) {
#[cfg(feature = "pg12")]
fn do_ereport_impl(ereport: ErrorReportWithLevel) {

#[cfg_attr(target_os = "windows", link(name = "postgres"))]
extern "C" {
fn errstart(elevel: ::std::os::raw::c_int, filename: *const ::std::os::raw::c_char, lineno: ::std::os::raw::c_int, funcname: *const ::std::os::raw::c_char, domain: *const ::std::os::raw::c_char) -> bool;
fn errfinish(dummy: ::std::os::raw::c_int, ...);
Expand Down
3 changes: 3 additions & 0 deletions pgrx-pg-sys/src/submodules/thread_check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ fn init_active_thread(tid: NonZeroUsize) {
panic!("Attempt to initialize `pgrx` active thread from a thread other than the main");
}
match ACTIVE_THREAD.compare_exchange(0, tid.get(), Ordering::Relaxed, Ordering::Relaxed) {
#[cfg(not(target_os = "windows"))]
Ok(_) => unsafe {
// We won the race. Register an atfork handler to clear the atomic
// in any child processes we spawn.
Expand All @@ -102,6 +103,8 @@ fn init_active_thread(tid: NonZeroUsize) {
}
libc::pthread_atfork(None, None, Some(clear_in_child));
},
#[cfg(target_os = "windows")]
Ok(_) => (),
Err(_) => {
thread_id_check_failed();
}
Expand Down
8 changes: 4 additions & 4 deletions pgrx/src/bgworkers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,7 @@ impl DynamicBackgroundWorker {
/// return [`BackgroundWorkerStatus::Untracked`] error
pub fn wait_for_startup(&self) -> Result<Pid, BackgroundWorkerStatus> {
unsafe {
if self.notify_pid != pg_sys::MyProcPid {
if self.notify_pid != pg_sys::MyProcPid as pg_sys::pid_t {
return Err(BackgroundWorkerStatus::Untracked { notify_pid: self.notify_pid });
}
}
Expand Down Expand Up @@ -382,7 +382,7 @@ impl TerminatingDynamicBackgroundWorker {
/// return [`BackgroundWorkerStatus::Untracked`] error
pub fn wait_for_shutdown(self) -> Result<(), BackgroundWorkerStatus> {
unsafe {
if self.notify_pid != pg_sys::MyProcPid {
if self.notify_pid != pg_sys::MyProcPid as pg_sys::pid_t {
return Err(BackgroundWorkerStatus::Untracked { notify_pid: self.notify_pid });
}
}
Expand Down Expand Up @@ -580,7 +580,7 @@ impl BackgroundWorkerBuilder {
/// to wait for the worker to start up. Otherwise, it should be initialized to
/// `pgrx::pg_sys::MyProcPid`
pub fn set_notify_pid(mut self, input: i32) -> Self {
self.bgw_notify_pid = input;
self.bgw_notify_pid = input as pg_sys::pid_t;
self
}

Expand Down Expand Up @@ -634,7 +634,7 @@ impl<'a> From<&'a BackgroundWorkerBuilder> for pg_sys::BackgroundWorker {
bgw_name: RpgffiChar::from(&builder.bgw_name[..]).0,
bgw_type: RpgffiChar::from(&builder.bgw_type[..]).0,
bgw_flags: builder.bgw_flags.bits(),
bgw_start_time: builder.bgw_start_time as u32,
bgw_start_time: builder.bgw_start_time as _,
bgw_restart_time: match builder.bgw_restart_time {
None => -1,
Some(d) => d.as_secs() as i32,
Expand Down
Loading

0 comments on commit 3ea09af

Please sign in to comment.