From 3ea09af490c2063460a36b7c611f661230f89fc2 Mon Sep 17 00:00:00 2001 From: usamoi Date: Tue, 8 Oct 2024 08:32:41 +0800 Subject: [PATCH] fix compilation on some CPUs and Windows (#1901) 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 --- pgrx-bindgen/src/build.rs | 20 +++---- pgrx-macros/src/rewriter.rs | 32 ++++++++++-- pgrx-pg-sys/Cargo.toml | 1 + pgrx-pg-sys/pgrx-cshim.c | 42 +++++---------- pgrx-pg-sys/src/cshim.rs | 14 ++--- pgrx-pg-sys/src/submodules/ffi.rs | 61 +++++++++++++++++++++- pgrx-pg-sys/src/submodules/panic.rs | 4 ++ pgrx-pg-sys/src/submodules/thread_check.rs | 3 ++ pgrx/src/bgworkers.rs | 8 +-- pgrx/src/guc.rs | 10 ++-- pgrx/src/lib.rs | 2 +- pgrx/src/list/old_list.rs | 22 ++++---- 12 files changed, 146 insertions(+), 73 deletions(-) diff --git a/pgrx-bindgen/src/build.rs b/pgrx-bindgen/src/build.rs index 708b98067a..fed8dcac0b 100644 --- a/pgrx-bindgen/src/build.rs +++ b/pgrx-bindgen/src/build.rs @@ -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(); @@ -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 { @@ -1116,18 +1125,11 @@ fn apply_pg_guard(items: &Vec) -> eyre::Result { 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()); diff --git a/pgrx-macros/src/rewriter.rs b/pgrx-macros/src/rewriter.rs index 0ce5a4cc3a..45a5842204 100644 --- a/pgrx-macros/src/rewriter.rs +++ b/pgrx-macros/src/rewriter.rs @@ -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 { @@ -126,6 +126,7 @@ fn foreign_item(item: ForeignItem, abi: &syn::Abi) -> syn::Result foreign_item_static(&variable, abi), _ => Ok(quote! { #abi { #item } }), } } @@ -135,19 +136,44 @@ fn foreign_item_fn(func: &ForeignItemFn, abi: &syn::Abi) -> syn::Result { + 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 { + 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 { let mut arg_list = proc_macro2::TokenStream::new(); diff --git a/pgrx-pg-sys/Cargo.toml b/pgrx-pg-sys/Cargo.toml index 85531d6352..f49b761065 100644 --- a/pgrx-pg-sys/Cargo.toml +++ b/pgrx-pg-sys/Cargo.toml @@ -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" diff --git a/pgrx-pg-sys/pgrx-cshim.c b/pgrx-pg-sys/pgrx-cshim.c index aedb4c2d9a..2fc397d99a 100644 --- a/pgrx-pg-sys/pgrx-cshim.c +++ b/pgrx-pg-sys/pgrx-cshim.c @@ -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; + } +} diff --git a/pgrx-pg-sys/src/cshim.rs b/pgrx-pg-sys/src/cshim.rs index d766aa350d..35126495e4 100644 --- a/pgrx-pg-sys/src/cshim.rs +++ b/pgrx-pg-sys/src/cshim.rs @@ -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; } diff --git a/pgrx-pg-sys/src/submodules/ffi.rs b/pgrx-pg-sys/src/submodules/ffi.rs index f69aacad18..9aaa1a0c72 100644 --- a/pgrx-pg-sys/src/submodules/ffi.rs +++ b/pgrx-pg-sys/src/submodules/ffi.rs @@ -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(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( + 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::, + ) + } + } +} + +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] @@ -113,7 +170,7 @@ unsafe fn pg_guard_ffi_boundary_impl 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 = 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); diff --git a/pgrx-pg-sys/src/submodules/panic.rs b/pgrx-pg-sys/src/submodules/panic.rs index 189631ddfd..355c35f5e6 100644 --- a/pgrx-pg-sys/src/submodules/panic.rs +++ b/pgrx-pg-sys/src/submodules/panic.rs @@ -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() -> !; } @@ -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; @@ -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); @@ -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, ...); diff --git a/pgrx-pg-sys/src/submodules/thread_check.rs b/pgrx-pg-sys/src/submodules/thread_check.rs index beaf9df268..e4230f7602 100644 --- a/pgrx-pg-sys/src/submodules/thread_check.rs +++ b/pgrx-pg-sys/src/submodules/thread_check.rs @@ -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. @@ -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(); } diff --git a/pgrx/src/bgworkers.rs b/pgrx/src/bgworkers.rs index bf6a1a1304..74e3f0cf86 100644 --- a/pgrx/src/bgworkers.rs +++ b/pgrx/src/bgworkers.rs @@ -343,7 +343,7 @@ impl DynamicBackgroundWorker { /// return [`BackgroundWorkerStatus::Untracked`] error pub fn wait_for_startup(&self) -> Result { 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 }); } } @@ -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 }); } } @@ -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 } @@ -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, diff --git a/pgrx/src/guc.rs b/pgrx/src/guc.rs index 975bdfb2ea..4617a2b0f9 100644 --- a/pgrx/src/guc.rs +++ b/pgrx/src/guc.rs @@ -224,7 +224,7 @@ impl GucRegistry { PgMemoryContexts::TopMemoryContext.pstrdup(long_description), setting.as_ptr(), setting.get(), - context as isize as u32, + context as isize as _, flags.bits(), None, None, @@ -252,7 +252,7 @@ impl GucRegistry { setting.get(), min_value, max_value, - context as isize as u32, + context as isize as _, flags.bits(), None, None, @@ -278,7 +278,7 @@ impl GucRegistry { PgMemoryContexts::TopMemoryContext.pstrdup(long_description), setting.as_ptr(), boot_val, - context as isize as u32, + context as isize as _, flags.bits(), None, None, @@ -306,7 +306,7 @@ impl GucRegistry { setting.boot_val, min_value, max_value, - context as isize as u32, + context as isize as _, flags.bits(), None, None, @@ -335,7 +335,7 @@ impl GucRegistry { setting.as_ptr(), boot_val, setting.get().config_matrix(), - context as isize as u32, + context as isize as _, flags.bits(), None, None, diff --git a/pgrx/src/lib.rs b/pgrx/src/lib.rs index 9cacbf2f6c..46696c54d3 100644 --- a/pgrx/src/lib.rs +++ b/pgrx/src/lib.rs @@ -271,7 +271,7 @@ macro_rules! pg_magic_func { pub(crate) static UTF8DATABASE: Lazy = Lazy::new(|| { use pg_sys::pg_enc::*; let encoding_int = unsafe { pg_sys::GetDatabaseEncoding() }; - match encoding_int as core::ffi::c_uint { + match encoding_int as _ { PG_UTF8 => Utf8Compat::Yes, // The 0 encoding. It... may be UTF-8 PG_SQL_ASCII => Utf8Compat::Maybe, diff --git a/pgrx/src/list/old_list.rs b/pgrx/src/list/old_list.rs index 53c88c2c56..2231acea43 100644 --- a/pgrx/src/list/old_list.rs +++ b/pgrx/src/list/old_list.rs @@ -53,7 +53,7 @@ impl PgList { if self.list.is_null() { None } else { - Some(unsafe { pg_sys::pgrx_list_nth(self.list, 0) } as *mut T) + Some(unsafe { pg_sys::list_nth(self.list, 0) } as *mut T) } } @@ -62,7 +62,7 @@ impl PgList { if self.list.is_null() { None } else { - Some(unsafe { pg_sys::pgrx_list_nth(self.list, (self.len() - 1) as i32) } as *mut T) + Some(unsafe { pg_sys::list_nth(self.list, (self.len() - 1) as i32) } as *mut T) } } @@ -76,7 +76,7 @@ impl PgList { if self.list.is_null() || i >= self.len() { None } else { - Some(unsafe { pg_sys::pgrx_list_nth(self.list, i as i32) } as *mut T) + Some(unsafe { pg_sys::list_nth(self.list, i as i32) } as *mut T) } } @@ -91,7 +91,7 @@ impl PgList { if self.list.is_null() || i >= self.len() { None } else { - Some(unsafe { pg_sys::pgrx_list_nth_int(self.list, i as i32) }) + Some(unsafe { pg_sys::list_nth_int(self.list, i as i32) }) } } @@ -106,14 +106,14 @@ impl PgList { if self.list.is_null() || i >= self.len() { None } else { - Some(unsafe { pg_sys::pgrx_list_nth_oid(self.list, i as i32) }) + Some(unsafe { pg_sys::list_nth_oid(self.list, i as i32) }) } } #[cfg(feature = "pg12")] #[inline] pub unsafe fn replace_ptr(&mut self, i: usize, with: *mut T) { - let cell = pg_sys::pgrx_list_nth_cell(self.list, i as i32); + let cell = pg_sys::list_nth_cell(self.list, i as i32); cell.as_mut().expect("cell is null").data.ptr_value = with as void_mut_ptr; } @@ -126,7 +126,7 @@ impl PgList { ))] #[inline] pub unsafe fn replace_ptr(&mut self, i: usize, with: *mut T) { - let cell = pg_sys::pgrx_list_nth_cell(self.list, i as i32); + let cell = pg_sys::list_nth_cell(self.list, i as i32); cell.as_mut().expect("cell is null").ptr_value = with as void_mut_ptr; } @@ -134,7 +134,7 @@ impl PgList { #[inline] pub fn replace_int(&mut self, i: usize, with: i32) { unsafe { - let cell = pg_sys::pgrx_list_nth_cell(self.list, i as i32); + let cell = pg_sys::list_nth_cell(self.list, i as i32); cell.as_mut().expect("cell is null").data.int_value = with; } } @@ -149,7 +149,7 @@ impl PgList { #[inline] pub fn replace_int(&mut self, i: usize, with: i32) { unsafe { - let cell = pg_sys::pgrx_list_nth_cell(self.list, i as i32); + let cell = pg_sys::list_nth_cell(self.list, i as i32); cell.as_mut().expect("cell is null").int_value = with; } } @@ -158,7 +158,7 @@ impl PgList { #[inline] pub fn replace_oid(&mut self, i: usize, with: pg_sys::Oid) { unsafe { - let cell = pg_sys::pgrx_list_nth_cell(self.list, i as i32); + let cell = pg_sys::list_nth_cell(self.list, i as i32); cell.as_mut().expect("cell is null").data.oid_value = with; } } @@ -173,7 +173,7 @@ impl PgList { #[inline] pub fn replace_oid(&mut self, i: usize, with: pg_sys::Oid) { unsafe { - let cell = pg_sys::pgrx_list_nth_cell(self.list, i as i32); + let cell = pg_sys::list_nth_cell(self.list, i as i32); cell.as_mut().expect("cell is null").oid_value = with; } }