-
-
Notifications
You must be signed in to change notification settings - Fork 222
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
towards mingw/msys2 windows support: type has conflicting packed and align representation hints #1132
Comments
lets ping @thomcc on this one. It's unfortunate this is around setjmp, b/c I'd otherwise suggest just blocklisting the |
blocklisting? Not following where would I blocklist? |
in I think you could |
Sadly that seems to have done nothing. I also tried: explicit
What's the reason for the line below. Have those caused problems?
Does order of items matter? I assumed not. |
What did you do next to determine it didn't do anything?
we declare our own in
naw |
oh, you can always force We use that only when publishing a release, but it can be handy for the type of stuff you're doing right now. |
I did a
It looked like it was rebuilding the pgx-pg-sys from scratch I just did a and rebuild and still failing on those two. |
Not sure it matters, but I had fiddled with the logging on run_bindgen to see the error around 752.
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: |
On the bright side, this seems to work.
outputs
|
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. |
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
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 and showing this RUST backtrace:
I'm going to add CARGO_PROFILE_TEST_BUILD_OVERRIDE_DEBUG=true to see if I can get more detail. |
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). |
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
The pgrx-pg-sys/src/pg15*.rs are overwritten and pg15.rs looks pretty different from what was there before 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
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. 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. |
Yes, the committed entries are mostly for the sake of docs.rs and linking and such.
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. |
They appear to have some sort of wrapper to compensate for absense of sigsetjmp on windows. Their src/include/c.h include has this:
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. |
Thanks, I see, hmm... That's tricky since certainly 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. |
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. |
This doesn't work for Rust, as it doesn't have equivalent intrinsics. |
But you think setjmp/longjmp could work at least for regular old VC build. 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 |
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 😅. |
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. |
@eeeebbbbrrrr you were close. I managed to get past this by using blocklist_type instead of blocklist_item.
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. |
I think I'm getting a bit warmer here. Now I'm battling this error:
The code from 56286 to 56297 looks like
and also
Code from 56706 to 56737 looks like
this I am assuming is autogenerated bindgen code, which sounds like it might be this issue:
rust-lang/rust#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.
The text was updated successfully, but these errors were encountered: