pgcentralfoundation / pgrx

Build Postgres Extensions with Rust!
Other
3.7k stars 249 forks source link

towards mingw/msys2 windows support: type has conflicting packed and align representation hints #1132

Open robe2 opened 1 year ago

robe2 commented 1 year ago

I think I'm getting a bit warmer here. Now I'm battling this error:

error[E0587]: type has conflicting packed and align representation hints
     --> C:\ming64\projects\rust\pgrx\target\debug\build\pgrx-pg-sys-a88a8e0a957dfe55\out/pg15.rs:56286:1
      |
56286 | pub union __mingw_ldbl_type_t {
      | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

The code from 56286 to 56297 looks like

#[repr(C, packed(8))]
#[repr(align(8))]
#[derive(Copy, Clone)]
pub union __mingw_ldbl_type_t {
    pub x: u128,
    pub lh: __mingw_ldbl_type_t__bindgen_ty_1,
}
#[repr(C)]
#[derive(Debug, Default, Copy, Clone)]
pub struct __mingw_ldbl_type_t__bindgen_ty_1 {
    pub low: ::std::os::raw::c_uint,
    pub high: ::std::os::raw::c_uint,
    pub _bitfield_align_1: [u32; 0],
    pub _bitfield_1: __BindgenBitfieldUnit<[u8; 8usize]>,
}

and also

error[E0587]: type has conflicting packed and align representation hints
     --> C:\ming64\projects\rust\pgrx\target\debug\build\pgrx-pg-sys-a88a8e0a957dfe55\out/pg15.rs:56711:1
      |
56711 | pub struct _JUMP_BUFFER {
      | ^^^^^^^^^^^^^^^^^^^^^^^

Code from 56706 to 56737 looks like

pub type SETJMP_FLOAT128 = _SETJMP_FLOAT128;
pub type _JBTYPE = SETJMP_FLOAT128;
#[repr(C, packed(8))]
#[repr(align(8))]
#[derive(Debug, Default, Copy, Clone)]
pub struct _JUMP_BUFFER {
    pub Frame: ::std::os::raw::c_ulonglong,
    pub Rbx: ::std::os::raw::c_ulonglong,
    pub Rsp: ::std::os::raw::c_ulonglong,
    pub Rbp: ::std::os::raw::c_ulonglong,
    pub Rsi: ::std::os::raw::c_ulonglong,
    pub Rdi: ::std::os::raw::c_ulonglong,
    pub R12: ::std::os::raw::c_ulonglong,
    pub R13: ::std::os::raw::c_ulonglong,
    pub R14: ::std::os::raw::c_ulonglong,
    pub R15: ::std::os::raw::c_ulonglong,
    pub Rip: ::std::os::raw::c_ulonglong,
    pub MxCsr: ::std::os::raw::c_ulong,
    pub FpCsr: ::std::os::raw::c_ushort,
    pub Spare: ::std::os::raw::c_ushort,
    pub Xmm6: SETJMP_FLOAT128,
    pub Xmm7: SETJMP_FLOAT128,
    pub Xmm8: SETJMP_FLOAT128,
    pub Xmm9: SETJMP_FLOAT128,
    pub Xmm10: SETJMP_FLOAT128,
    pub Xmm11: SETJMP_FLOAT128,
    pub Xmm12: SETJMP_FLOAT128,
    pub Xmm13: SETJMP_FLOAT128,
    pub Xmm14: SETJMP_FLOAT128,
    pub Xmm15: SETJMP_FLOAT128,
}
pub type jmp_buf = [_JBTYPE; 16usize];

this I am assuming is autogenerated bindgen code, which sounds like it might be this issue: https://github.com/rust-lang/rust/issues/59154

I tried rolling back to a bindgen < than the 0.58 discussed in that ticket to see if it would fix the issue, but then the pgrx code requires features introduced after that. Any thoughts how I could disable this so I can move further thru to see what other issues there are holding back windows building? If you have a thought on how to fix, that would be even better.

FWIW in case it's related, I had remarked out all the jump error stuff and was planning to revisit that. I tried windows _setjmp3 which is the closest proxy to the unix sigsetjmp I could find but ran into casting issue. So I'm thinking maybe that is what triggered this too.

eeeebbbbrrrr commented 1 year ago

lets ping @thomcc on this one. It's unfortunate this is around setjmp, b/c I'd otherwise suggest just blocklisting the __mingw* stuff.

robe2 commented 1 year ago

blocklisting? Not following where would I blocklist?

eeeebbbbrrrr commented 1 year ago

in pgrx-pg-sys/build.rs in the run_bindgen() function (where you added the clang_args) there's a whole list of .blocklist_xxx() calls.

I think you could .blocklist_item('__mingw.*)` and see what happens next. Those functions take a regex and match against the "thing" from the C headers.

robe2 commented 1 year ago

Sadly that seems to have done nothing. I also tried: explicit

blocklist_item("__mingw_ldbl_type_t")

What's the reason for the line below. Have those caused problems?

.blocklist_function(".*(?:set|long)jmp")

Does order of items matter? I assumed not.

eeeebbbbrrrr commented 1 year ago

Sadly that seems to have done nothing. I also tried: explicit

blocklist_item("__mingw_ldbl_type_t")

What did you do next to determine it didn't do anything? build.rs should re-run on the next build after being modified, but a cargo clean will ensure that.

What's the reason for the line below. Have those caused problems?

.blocklist_function(".*(?:set|long)jmp")

we declare our own in pgrx-pg-sys/src/submodules/mod.rs. Probably something you'd have to handle for mingw too.

Does order of items matter? I assumed not.

naw

eeeebbbbrrrr commented 1 year ago

oh, you can always force build.rs to re-gen the bidnings by setting PGRX_PG_SYS_GENERATE_BINDINGS_FOR_RELEASE=1 in your environment.

We use that only when publishing a release, but it can be handy for the type of stuff you're doing right now.

robe2 commented 1 year ago

Sadly that seems to have done nothing. I also tried: explicit blocklist_item("__mingw_ldbl_type_t")

What did you do next to determine it didn't do anything? build.rs should re-run on the next build after being modified, but a cargo clean will ensure that.

I did a git clean -fdx which seemed to clear out the compile directory and then a

export PG_VER=15
cargo install --path cargo-pgrx/ --debug --force --locked
cargo pgrx init --pg${PG_VER} pg_config
cargo test \
  --features "pg$PG_VER cshim" \
  --package pgrx-tests

It looked like it was rebuilding the pgx-pg-sys from scratch

I just did a cargo clean

and rebuild and still failing on those two.

robe2 commented 1 year ago

Not sure it matters, but I had fiddled with the logging on run_bindgen to see the error around 752.
I wasn't seeing the above error at all before.

        .wrap_err_with(|| format!("Unable to generate bindings for pg{} {}", major_version, pg_target_include_flags(major_version, pg_config).expect(" ").join(" ")))?;

The GHA runner, is still running the code before I put in any of the blocklist_item and .wrap_err_with and shows this error: https://github.com/robe2/pgrx/actions/runs/4866115611/jobs/8677275422#step:7:279

robe2 commented 1 year ago

On the bright side, this seems to work.

cargo pgrx new test_extension
 tree test_extension

outputs

test_extension
├── Cargo.toml
├── sql
├── src
│   └── lib.rs
└── test_extension.control

2 directories, 3 files
thomcc commented 1 year ago

Hm, so it works even though you have that error? I'm a bit confused.

Sadly, I don't think we support blocklisting setjmp/longjmp types and functions. That said, this looks like a straightforward bindgen bug. #[repr(C, packed(8))] and #[repr(align(8))] shouldn't both be emitted.

robe2 commented 1 year ago

regarding Sadly, I don't think we support blocklisting setjmp/longjmp types and functions

The block of setjmp/longjmp I was seeing in the code already. I didn't touch that part, but I think I tried taking it out and it made no difference so maybe that line is just ignored. Was just wondering why it was there in the first place.

I still need to try @eeeebbbbrrrr suggestion of PGRX_PG_SYS_GENERATE_BINDINGS_FOR_RELEASE=1

Yah cargo pgrx compiled fine (basic help and create works). It's when it gets to the custom build pgrx-pg-sys

It also crashes when I do

cargo pgrx run

Though now I'm beginning to wonder if the errors I am getting locally are same as what I'm seeing on the GHA runner I had set up as I had put in some extra statements in mine to trace further.

The latest GHA logs are here - https://github.com/robe2/pgrx/actions/runs/4866750660/jobs/8678596644#step:7:224 Which does this - https://github.com/robe2/pgrx/actions/runs/4866750660/workflow

and showing this RUST backtrace:

   Compiling clap_builder v4.2.4
   Compiling time-macros v0.2.8
   Compiling clap_derive v4.2.0
   Compiling wyz v0.5.1
   Compiling pgrx-macros v0.8.2 (D:\a\pgrx\pgrx\pgrx-macros)
   Compiling tokio-util v0.7.8
   Compiling parking_lot v0.12.1
   Compiling phf v0.11.1
   Compiling postgres-types v0.2.5
error: failed to run custom build command for `pgrx-pg-sys v0.8.2 (D:\a\pgrx\pgrx\pgrx-pg-sys)`
note: To improve backtraces for build dependencies, set the CARGO_PROFILE_TEST_BUILD_OVERRIDE_DEBUG=true environment variable to enable debug information generation.

Caused by:
  process didn't exit successfully: `D:\a\pgrx\pgrx\target\debug\build\pgrx-pg-sys-a7b0f58a20a1e84c\build-script-build` (exit code: 101)
  --- stdout
  cargo:rerun-if-env-changed=PGRX_BUILD_VERBOSE
  cargo:rerun-if-env-changed=PGRX_PG_SYS_GENERATE_BINDINGS_FOR_RELEASE
  cargo:rerun-if-env-changed=PGRX_PG_CONFIG_PATH
  cargo:rerun-if-env-changed=PGRX_PG_CONFIG_AS_ENV
  cargo:rerun-if-env-changed=LLVM_CONFIG_PATH
  cargo:rerun-if-env-changed=LIBCLANG_PATH
  cargo:rerun-if-env-changed=LIBCLANG_STATIC_PATH
  cargo:rerun-if-env-changed=BINDGEN_EXTRA_CLANG_ARGS
  cargo:rerun-if-env-changed=BINDGEN_EXTRA_CLANG_ARGS_x86_64-pc-windows-gnu
  cargo:rerun-if-env-changed=BINDGEN_EXTRA_CLANG_ARGS_x86_64_pc_windows_gnu
  cargo:rerun-if-env-changed=PGRX_PG_SYS_GENERATE_BINDINGS_FOR_RELEASE
  cargo:rerun-if-changed=include
  cargo:rerun-if-changed=cshim
  cargo:rerun-if-changed=C:\Users\runneradmin\.pgrx\config.toml
  cargo:rerun-if-env-changed=PGRX_PG_SYS_GENERATE_BINDINGS_FOR_RELEASE
  cargo:rerun-if-env-changed=PGRX_TARGET_INFO_PATH_PG15_x86_64-pc-windows-gnu
  cargo:rerun-if-env-changed=PGRX_TARGET_INFO_PATH_PG15
  cargo:rerun-if-env-changed=PGRX_INCLUDEDIR_SERVER_PG15_x86_64-pc-windows-gnu
  cargo:rerun-if-env-changed=PGRX_INCLUDEDIR_SERVER_PG15
  cargo:rerun-if-env-changed=PGRX_INCLUDEDIR_SERVER_x86_64-pc-windows-gnu
  cargo:rerun-if-env-changed=PGRX_INCLUDEDIR_SERVER
  cargo:rerun-if-env-changed=PGRX_BINDGEN_NO_DETECT_INCLUDES_x86_64-pc-windows-gnu
  cargo:rerun-if-env-changed=PGRX_BINDGEN_NO_DETECT_INCLUDES

  --- stderr
  build_paths=BuildPaths { manifest_dir: "D:\\a\\pgrx\\pgrx\\pgrx-pg-sys", out_dir: "D:\\a\\pgrx\\pgrx\\target\\debug\\build\\pgrx-pg-sys-3d3842e67d194a31\\out", src_dir: "D:\\a\\pgrx\\pgrx\\pgrx-pg-sys\\src", shim_src: "D:\\a\\pgrx\\pgrx\\pgrx-pg-sys\\cshim", shim_dst: "D:\\a\\pgrx\\pgrx\\target\\debug\\build\\pgrx-pg-sys-3d3842e67d194a31\\out\\cshim" }
  Generating bindings for pg15
  clang diag: D:/a/pgrx/pgrx/pgsql/include/server/port.h:212:70: warning: 'format' attribute argument not supported: gnu_printf [-Wignored-attributes]
  clang diag: D:/a/pgrx/pgrx/pgsql/include/server/port.h:214:55: warning: 'format' attribute argument not supported: gnu_printf [-Wignored-attributes]
  clang diag: D:/a/pgrx/pgrx/pgsql/include/server/port.h:216:58: warning: 'format' attribute argument not supported: gnu_printf [-Wignored-attributes]
  clang diag: D:/a/pgrx/pgrx/pgsql/include/server/port.h:218:43: warning: 'format' attribute argument not supported: gnu_printf [-Wignored-attributes]
  clang diag: D:/a/pgrx/pgrx/pgsql/include/server/utils/elog.h:173:40: warning: 'format' attribute argument not supported: gnu_printf [-Wignored-attributes]
  clang diag: D:/a/pgrx/pgrx/pgsql/include/server/utils/elog.h:174:49: warning: 'format' attribute argument not supported: gnu_printf [-Wignored-attributes]
  clang diag: D:/a/pgrx/pgrx/pgsql/include/server/utils/elog.h:177:30: warning: 'format' attribute argument not supported: gnu_printf [-Wignored-attributes]
  clang diag: D:/a/pgrx/pgrx/pgsql/include/server/utils/elog.h:177:56: warning: 'format' attribute argument not supported: gnu_printf [-Wignored-attributes]
  clang diag: D:/a/pgrx/pgrx/pgsql/include/server/utils/elog.h:179:43: warning: 'format' attribute argument not supported: gnu_printf [-Wignored-attributes]
  clang diag: D:/a/pgrx/pgrx/pgsql/include/server/utils/elog.h:180:52: warning: 'format' attribute argument not supported: gnu_printf [-Wignored-attributes]
  clang diag: D:/a/pgrx/pgrx/pgsql/include/server/utils/elog.h:182:47: warning: 'format' attribute argument not supported: gnu_printf [-Wignored-attributes]
  clang diag: D:/a/pgrx/pgrx/pgsql/include/server/utils/elog.h:186:31: warning: 'format' attribute argument not supported: gnu_printf [-Wignored-attributes]
  clang diag: D:/a/pgrx/pgrx/pgsql/include/server/utils/elog.h:186:57: warning: 'format' attribute argument not supported: gnu_printf [-Wignored-attributes]
  clang diag: D:/a/pgrx/pgrx/pgsql/include/server/utils/elog.h:189:30: warning: 'format' attribute argument not supported: gnu_printf [-Wignored-attributes]
  clang diag: D:/a/pgrx/pgrx/pgsql/include/server/utils/elog.h:189:56: warning: 'format' attribute argument not supported: gnu_printf [-Wignored-attributes]
  clang diag: D:/a/pgrx/pgrx/pgsql/include/server/utils/elog.h:191:41: warning: 'format' attribute argument not supported: gnu_printf [-Wignored-attributes]
  clang diag: D:/a/pgrx/pgrx/pgsql/include/server/utils/elog.h:194:31: warning: 'format' attribute argument not supported: gnu_printf [-Wignored-attributes]
  clang diag: D:/a/pgrx/pgrx/pgsql/include/server/utils/elog.h:194:57: warning: 'format' attribute argument not supported: gnu_printf [-Wignored-attributes]
  clang diag: D:/a/pgrx/pgrx/pgsql/include/server/utils/elog.h:208:48: warning: 'format' attribute argument not supported: gnu_printf [-Wignored-attributes]
  clang diag: D:/a/pgrx/pgrx/pgsql/include/server/utils/elog.h:239:54: warning: 'format' attribute argument not supported: gnu_printf [-Wignored-attributes]
  clang diag: D:/a/pgrx/pgrx/pgsql/include/server/utils/elog.h:468:47: warning: 'format' attribute argument not supported: gnu_printf [-Wignored-attributes]
  clang diag: D:/a/pgrx/pgrx/pgsql/include/server/utils/palloc.h:155:44: warning: 'format' attribute argument not supported: gnu_printf [-Wignored-attributes]
  clang diag: D:/a/pgrx/pgrx/pgsql/include/server/utils/palloc.h:156:80: warning: 'format' attribute argument not supported: gnu_printf [-Wignored-attributes]
  clang diag: D:/a/pgrx/pgrx/pgsql/include/server/lib/stringinfo.h:96:67: warning: 'format' attribute argument not supported: gnu_printf [-Wignored-attributes]
  clang diag: D:/a/pgrx/pgrx/pgsql/include/server/lib/stringinfo.h:107:78: warning: 'format' attribute argument not supported: gnu_printf [-Wignored-attributes]
  thread '<unnamed>' panicked at 'Non floating-type complex? Type(_Complex _Float16, kind: Complex, cconv: 100, decl: Cursor( kind: NoDeclFound, loc: builtin definitions, usr: None), canon: Cursor( kind: NoDeclFound, loc: builtin definitions, usr: None)), Type(_Float16, kind: Float16, cconv: 100, decl: Cursor( kind: NoDeclFound, loc: builtin definitions, usr: None), canon: Cursor( kind: NoDeclFound, loc: builtin definitions, usr: None))', C:\Users\runneradmin\.cargo\registry\src\github.com-1ecc6299db9ec823\bindgen-0.60.1\src\ir\context.rs:1975:26
  stack backtrace:
     0:     0x7ff6cbbd80d6 - std::backtrace_rs::backtrace::trace_unsynchronized::hf5069f7c0e16fbed
     1:     0x7ff6cbc159c1 - <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt::hcf7b3d6f069f1f47
     2:     0x7ff6cbc7cacb - core::fmt::write::he405dcefc30248de
     3:     0x7ff6cbc04ce5 - std::io::Write::write_fmt::h12a14834ed1271ae
     4:     0x7ff6cbc15789 - std::sys_common::backtrace::print::h8d0eac42a9604cee
     5:     0x7ff6cbbeb8e0 - std::panicking::default_hook::{{closure}}::h61413103b9ea6ee3
     6:     0x7ff6cbbeb54c - std::panicking::default_hook::h0627ac56f54f3375
     7:     0x7ff6cbbec79b - std::panicking::rust_panic_with_hook::hca7add4a78942f74
     8:     0x7ff6cbc17136 - std::panicking::begin_panic_handler::{{closure}}::h1cd18720b142de82
     9:     0x7ff6cbc15cd9 - std::sys_common::backtrace::__rust_end_short_backtrace::ha89913ed57dde6a1
    10:     0x7ff6cbbec320 - rust_begin_unwind
    11:     0x7ff6cbc5f9d5 - core::panicking::panic_fmt::hd998973ddd4646bc
    12:     0x7ff6cb9ee425 - bindgen::ir::context::BindgenContext::builtin_or_resolved_ty::h20b05f6cc29b2f10
    13:     0x7ff6cb9ce560 - <bindgen::ir::item::Item as bindgen::parse::ClangItemParser>::from_ty_or_ref_with_id::h9f69acc5f6463ad2
    14:     0x7ff6cb988fc8 - core::ops::function::impls::<impl core::ops::function::FnOnce<A> for &mut F>::call_once::h7f3721f6bbd1259f
    15:     0x7ff6cb99bcce - <core::iter::adapters::map::Map<I,F> as core::iter::traits::iterator::Iterator>::next::he85f7615ecb166ac
    16:     0x7ff6cb9a6a76 - <alloc::vec::Vec<T> as alloc::vec::spec_from_iter::SpecFromIter<T,I>>::from_iter::hb218bac7622437d1
    17:     0x7ff6cb993213 - bindgen::ir::function::args_from_ty_and_cursor::h72582990012bc9d0
    18:     0x7ff6cb9933e8 - bindgen::ir::function::FunctionSig::from_ty::h26bb27b30c1e1f43
    19:     0x7ff6cb996d92 - bindgen::ir::ty::Type::from_clang_ty::h80df572e86740bf5
    20:     0x7ff6cb9ceff4 - <bindgen::ir::item::Item as bindgen::parse::ClangItemParser>::from_ty_with_id::hb1948cdc11f85391
    21:     0x7ff6cb9ce7c3 - <bindgen::ir::item::Item as bindgen::parse::ClangItemParser>::from_ty::h2d1398427d5d4f83
    22:     0x7ff6cb9ce039 - <bindgen::ir::item::Item as bindgen::parse::ClangItemParser>::parse::h5404ad398d110482
    23:     0x7ff6cba2f2b3 - bindgen::clang::visit_children::hc685c2125f7a03b3
    24:     0x7ff88aa5321f - <unknown>
    25:     0x7ff88aa5148e - <unknown>
    26:     0x7ff88aa4d6a8 - <unknown>
    27:     0x7ff88aa47d76 - <unknown>
    28:     0x7ff88ae6d3ca - <unknown>
    29:     0x7ff6cbaef4af - clang_sys::clang_visitChildren::h74b068b72fc7fe2b
    30:     0x7ff6cb9836e0 - bindgen::Builder::generate::h925460a07dda91db
    31:     0x7ff6cb6e4a64 - build_script_build::generate_bindings::h83f58e6cfec5e73e
    32:     0x7ff6cb6d4f61 - std::sys_common::backtrace::__rust_begin_short_backtrace::h5048818af276990f
    33:     0x7ff6cb6f3b71 - core::ops::function::FnOnce::call_once{{vtable.shim}}::h920bee0412f69f7f
    34:     0x7ff6cbc231f7 - std::sys::windows::thread::Thread::new::thread_start::hd8be633433792143
    35:     0x7ff8bed24dd0 - <unknown>
    36:     0x7ff8c019e3db - <unknown>
  thread 'main' panicked at 'thread panicked while generating bindings: Any { .. }', pgrx-pg-sys\build.rs:192:41
  stack backtrace:
     0:     0x7ff6cbbd80d6 - std::backtrace_rs::backtrace::trace_unsynchronized::hf5069f7c0e16fbed
     1:     0x7ff6cbc159c1 - <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt::hcf7b3d6f069f1f47
     2:     0x7ff6cbc7cacb - core::fmt::write::he405dcefc30248de
     3:     0x7ff6cbc04ce5 - std::io::Write::write_fmt::h12a14834ed1271ae
     4:     0x7ff6cbc15789 - std::sys_common::backtrace::print::h8d0eac42a9604cee
     5:     0x7ff6cbbeb8e0 - std::panicking::default_hook::{{closure}}::h61413103b9ea6ee3
     6:     0x7ff6cbbeb54c - std::panicking::default_hook::h0627ac56f54f3375
     7:     0x7ff6cbbec79b - std::panicking::rust_panic_with_hook::hca7add4a78942f74
     8:     0x7ff6cbc17136 - std::panicking::begin_panic_handler::{{closure}}::h1cd18720b142de82
     9:     0x7ff6cbc15cd9 - std::sys_common::backtrace::__rust_end_short_backtrace::ha89913ed57dde6a1
    10:     0x7ff6cbbec320 - rust_begin_unwind
    11:     0x7ff6cbc5f9d5 - core::panicking::panic_fmt::hd998973ddd4646bc
    12:     0x7ff6cbc60016 - core::result::unwrap_failed::h68e87c1b7205d8f1
    13:     0x7ff6cb6c4a85 - <core::iter::adapters::map::Map<I,F> as core::iter::traits::iterator::Iterator>::fold::h8245a3e30885b2f8
    14:     0x7ff6cb6d22ed - alloc::vec::in_place_collect::<impl alloc::vec::spec_from_iter::SpecFromIter<T,I> for alloc::vec::Vec<T>>::from_iter::h7c77f93be923df6d
    15:     0x7ff6cb6ec47c - <core::panic::unwind_safe::AssertUnwindSafe<F> as core::ops::function::FnOnce<()>>::call_once::h05304a6527fdcd94
    16:     0x7ff6cb6d968b - std::thread::scoped::scope::he5dd49f8996b372f
    17:     0x7ff6cb6e1b68 - build_script_build::main::hed62962b4d0f136e
    18:     0x7ff6cb6d4f76 - std::sys_common::backtrace::__rust_begin_short_backtrace::h6272915cb4744f68
    19:     0x7ff6cb6d4d8d - std::rt::lang_start::{{closure}}::h8fd8edc00adbd4ef
    20:     0x7ff6cbbe9aa8 - std::rt::lang_start_internal::h5dae0a62d308d052
    21:     0x7ff6cb6ebd0d - main
    22:     0x7ff6cb6c12ee - __tmainCRTStartup
                                 at C:\M\B\src\build-MINGW64\C:/M/B/src/mingw-w64/mingw-w64-crt/crt\crtexe.c:272:15
    23:     0x7ff6cb6c1406 - mainCRTStartup
                                 at C:\M\B\src\build-MINGW64\C:/M/B/src/mingw-w64/mingw-w64-crt/crt\crtexe.c:193:9
    24:     0x7ff8bed24dd0 - <unknown>
    25:     0x7ff8c019e3db - <unknown>
warning: build failed, waiting for other jobs to finish...

I'm going to add CARGO_PROFILE_TEST_BUILD_OVERRIDE_DEBUG=true to see if I can get more detail.

thomcc commented 1 year ago

In general if you have the wrong ABI for the setjmp buffer, we'll likely crash. It might be enough to have an overlarge buffer with the right alignment, but it's pretty annoying to make bindgen generate such things (when it's possible at all).

robe2 commented 1 year ago

Okay this may be a dumb question or something I'm doing wrong.

I see that pgrx-pg-sys/src/pg15*.rs is committed to the repo.

However when I run

export PG_VER=15
cargo test \
  --features "pg$PG_VER" \
  --package pgrx-tests

The pgrx-pg-sys/src/pg15*.rs are overwritten and pg15.rs looks pretty different from what was there before Is that supposed to happen?

The pgrx-pg-sys/src/pg15_oids.rs doesn't change too much. My regenerated for pg15_oids.rs has extra entries:

In the pub enum BuiltinOid section

DMPAPER_TABLOID = 3,
IMAGE_SYM_TYPE_VOID = 1,
IOC_VOID = 536870912,
LOCALE_IGEOID = 91,
PC_TRAPEZOID = 4,

and similar changes in the impl BuiltinOid {

FWIW, I suspect my jump stuff is pretty broken, so that might be the cause of all these issues. That was where I had left off in trying to make things work in msys2/mingw64 since jmp calls I was trying to replace with what I thought was the windows equivalent _setjmp3 and then gave up and started remarking all those out to see what bug would come out next.

For PostGIS building when we were working on windows 64-bit, we settled on Structured exception handling (SEH) because trying to use long jump / short jump model would cause PostGIS to crash whenever it hit an exception.

I'm wondering if this issue might be related. Error handling stuff I never understood much, so what I'm saying might sound like jibberish.

thomcc commented 1 year ago

The pgrx-pg-sys/src/pg15*.rs are overwritten and pg15.rs looks pretty different from what was there before Is that supposed to happen?

Yes, the committed entries are mostly for the sake of docs.rs and linking and such.

For PostGIS building when we were working on windows 64-bit, we settled on Structured exception handling (SEH) because trying to use long jump / short jump model would cause PostGIS to crash whenever it hit an exception.

Hm, do you know what postgres itself uses for error handling on windows? Our setjmp/longjmp stuff is just to catch errors thrown by postgres, since forced unwinding across Rust frames would be UB.

robe2 commented 1 year ago

They appear to have some sort of wrapper to compensate for absense of sigsetjmp on windows.

Their src/include/c.h include has this:

/*
 * When there is no sigsetjmp, its functionality is provided by plain
 * setjmp.  We now support the case only on Windows.  However, it seems
 * that MinGW-64 has some longstanding issues in its setjmp support,
 * so on that toolchain we cheat and use gcc's builtins.
 */
#ifdef WIN32
#ifdef __MINGW64__
typedef intptr_t sigjmp_buf[5];
#define sigsetjmp(x,y) __builtin_setjmp(x)
#define siglongjmp __builtin_longjmp
#else                           /* !__MINGW64__ */
#define sigjmp_buf jmp_buf
#define sigsetjmp(x,y) setjmp(x)
#define siglongjmp longjmp
#endif                          /* __MINGW64__ */
#endif                          /* WIN32 */

In case of postgis, we have a lot of C++ brought in for GEOS and GDAL so maybe the issue was mostly when errors happened in GEOS or GDAL.

thomcc commented 1 year ago

Thanks, I see, hmm... That's tricky since certainly __builtin_setjmp won't work (Rust has no such builtin, nor any direct equivalent -- even using setjmp/longjmp at all is... pretty dubious in Rust), but probably something else will.

I think it's pretty likely we'd have to just use normal setjmp/longjmp (not the sig versions, although that is... plausibly fine on windows, since signal delivery works a different way).

That said, that probably doesn't get you unstuck, hm.

robe2 commented 1 year ago

Why wouldn't __builtin_setjmp work? I thought you can call that with an unsafe since that I think is provided by mingw64 itself. I think mingw64 just prefixes all the functions it is not relegating to the MS VC runtime implementation as __builtin to distinguish it from the MS VC otherwise same named ones.

thomcc commented 1 year ago

__builtin_foo functions generally aren't real functions, they're direct compiler intrinsics. They aren't declared anywhere, although they are often lowered into the same underlying function as the other versions (especially if their address is taken).

This doesn't work for Rust, as it doesn't have equivalent intrinsics.

robe2 commented 1 year ago

But you think setjmp/longjmp could work at least for regular old VC build. That note was written a while ago I think so things might have changed.

Also mingw64 came out with UCRT64 https://www.msys2.org/docs/environments/ which uses the VS universal C library that ships with newer VS and is included with Windows 10 and above. The only downside with using that is it requires extra install for windows < 10, but I think it works fine for Windows 10 and above, which is probably all of the realistic audience today anyway. Let me see if I can fumble my way thru putting in conditionals for the sig -> without sigs. I'll enable the UCRT64 one for testing that if it doesn't work on the regular old mingw64

thomcc commented 1 year ago

Yeah, I think using the ucrt should be fine in principal (hopefully it works too!).

A problem can come of postgres is compiled against one CRT and then Rust compiled against another, but I think that shouldn't cause issues here (perhaps they're using the same one, even!). Fingers crossed 😅.

robe2 commented 1 year ago

I suspect it won't be an issue even with different CRT once we get thru this hurdle. I for example build all PostGIS and pgRouting and other spatial extensions and their dependencies under mingw64 (with a mingw64 compiled PostgreSQL), with plans to use UCRT. These are all deployed via the EDB stackbuilder that is VC++ compiled PostgreSQL. But can't remember the last time I've had an issue with these crashing that wasn't an issue in the underlying extension code even when we have dlls that overlap each other. E.g. PostGIS works fine if it's using the mingw compiled libxml or the EnterpriseDB VC++ compiled one.

robe2 commented 1 year ago

@eeeebbbbrrrr you were close. I managed to get past this by using blocklist_type instead of blocklist_item. in the pgrx-pg-sys/build.rs in the run_bindgen() function where the rest of the whole list of .blocklist_xxx() calls.

 .blocklist_type("__mingw_ldbl_type_t")
 .blocklist_type("_JUMP_BUFFER")

Now I have a whole bunch of threading errors showing in bgworkers.rs. I'll put in another ticket if I can't figure that one out.