Borrowck: Polonius FFI #2716

jdupak · 2023-10-29T17:05:25Z

FFI gccrs -> Polonius and build of Polonius (via FFI-Polonius bindigs) with gcc build system.

Cargo supports --outdir to copy final library to, which would be nice here, but right now it is unstable. :(

@tschwinge Is the clean alright? Should we somehow use autoconf to find cargo? Should/can we pass down the the information whether debug symbols are to be created to Rust?

jdupak · 2023-10-29T18:02:24Z

To solve:

explain cargo dependency in readme
resolve dependencies for GH actions
??? Configure option to enable/disable borrowchecker

ChangeLog: * .github/workflows/ccpp.yml: Install cargo. Signed-off-by: Jakub Dupak <dev@jakubdupak.com>

jdupak · 2023-10-29T19:28:06Z

I have added cargo install to GH actions.

P-E-P · 2023-10-30T08:52:43Z

Should we somehow use autoconf to find cargo?

We should at least ensure it is available during configuration.

gcc/rust/checks/errors/borrowck/ffi-polonius/src/gccrs_ffi.rs

gcc/rust/checks/errors/borrowck/polonius/rust-polonius.h

tschwinge · 2023-10-30T11:42:42Z

gcc/rust/Make-lang.in

+rust/libffi_polonius.a: \
+	rust/checks/errors/borrowck/ffi-polonius/Cargo.toml \
+	$(wildcard $(srcdir)/rust/checks/errors/borrowck/ffi-polonius/src/*)
+	cargo build --manifest-path $(srcdir)/rust/checks/errors/borrowck/ffi-polonius/Cargo.toml --release --target-dir rust/ffi-polonius


I assume that requiring cargo as part of GCC/Rust build process is not exactly what upstream GCC was looking for... 🙃

So, there is not simply a shared library to link against, or executable to call (at GCC/Rust invocation time)?

That said, cargo is "just" a build tool -- so assumingly we can get FFI-Polonius built in another "standard" (for GCC practices) way?

We would need to compile polonius with gccrs, which probably won't be possible for a long time. But in the long run it would probably be better to remove this dependency.

We could probably use direct rustc invocation instead of using cargo. The current polonius-engine only has 3 deps:

polonius-engine v0.13. ├── datafrog v2.0.1 ├── log v0.4.14 │ └── cfg-if v1.0.0 └── rustc-hash v1.1.0

but this doesn't scale very well if more dependencies are added. Would having a dep on rustc to build gccrs something we want in GCC until gccrs can build polonius? If not, then this can stay in gccrs tree (for a long time...).

Is there already a plan for this?

We could probably use direct rustc invocation instead of using cargo. The current polonius-engine only has 3 deps:

polonius-engine v0.13. ├── datafrog v2.0.1 ├── log v0.4.14 │ └── cfg-if v1.0.0 └── rustc-hash v1.1.0

but this doesn't scale very well if more dependencies are added. Would having a dep on rustc to build gccrs something we want in GCC until gccrs can build polonius? If not, then this can stay in gccrs tree (for a long time...).

Is there already a plan for this?

Do you really think using plain rustc is better for the gcc folks? (I would feel somewhat ashamed to build Rust with make 😢, but it is not really important here.)

Have no idea. I feel like this change won't hit upstream gcc until...some time. So whatever is best for you, but this needs to not get in the way of upstreaming the rest of the compiler. If that's the plan (we commit things we know are not fit for upstream gcc), then you should probably have separate commits for changes that will be upstreamed and the changes that we know won't ever reach gcc in this state. Meaning that some code in GCC would be unused until we can build with gccrs and enable the build...?

If you start requiring rustc/cargo, you'll break all platforms that are not supported by rustc (exit sh-*, s390x-*, ...)

I clearly wasn't really awake yet, when posting my comments yesterday Monday morning...

requiring cargo in not really worse then rustc

Point taken. ;-)

compile polonius with gccrs

That also won't solve all problems immediately, given that we need Polonius compiled for the host architecture (in order to link it into GCC), and build, host, target architectures may all be different. So we cannot rely on being able to use the GCC/Rust being built (for the target architecture) for building any Rust code for host usage. (Building for target is fine, of course, like other GCC target libraries.)

A multi-stage build process would resolve that issue; either manual or even scripted into the GCC's top-level build system (non-trivial).

My original assumption was:

So, there is not simply a shared library to link against, or executable to call (at GCC/Rust invocation time)?

I agree to what Marc just said:

whatever is best for you, but this needs to not get in the way of upstreaming the rest of the compiler

..., so we have to make use of Polonius conditional on its availability; and otherwise continue to require -frust-incomplete-and-experimental-compiler-do-not-use being specified, for instance.

Such a GCC/Rust mode generally will be necessary to continue supporting GCC/Rust in configurations where standard Rust is not available, and thus Polonius cannot be built at all, currently. (Standard bootstrapping problem.) ..., unless Polonius can be cross-compiled for the respective host architecture, of course.

compile polonius with gccrs

That also won't solve all problems immediately, given that we need Polonius compiled for the host architecture (in order to link it into GCC), and build, host, target architectures may all be different. So we cannot rely on being able to use the GCC/Rust being built (for the target architecture) for building any Rust code for host usage. (Building for target is fine, of course, like other GCC target libraries.)

Kind of similar issue with proc macros that need to be compiled for host (but not only during the build of gcc)

If you start requiring rustc/cargo, you'll break all platforms that are not supported by rustc (exit sh-*, s390x-*, ...)

Not 100% correct, only where rustc is not available for host, not target (e.g. sh is usually cross compiled, so rustc doesn't need to be available for the sh target)

Could ffi-polonius be loaded as a shared library by gccrs during compilation? That way it could be kept out of tree for now.

jdupak · 2023-10-30T13:18:16Z

Btw could anyone help me with the gcc4.8. It builds on my machine and I am not sure what is wrong there with pthreads linking (different then elsewhere)?

tschwinge · 2023-10-31T09:58:19Z

gcc4.8 [...] what is wrong there with pthreads linking

Possibly in the old system, pthreads symbols are still in a separate library, before that was all moved into libc (in the glibc context -- may still be separate on other architectures), and thus -lpthread needs to be specified explicitly.

Generally there should be a way to query cargo for link flags that need to be specified when linking the Polonius library that it has built, which should return -lpthread for old glibc vs. empty string for new glibc, approximately.

gcc/rust/ChangeLog: * Make-lang.in: Build FFI-Polonius. * checks/errors/borrowck/rust-borrow-checker.cc (BorrowChecker::go): Invoke Polonius. * checks/errors/borrowck/ffi-polonius/Cargo.toml: New file. * checks/errors/borrowck/ffi-polonius/src/gccrs_ffi.rs: New file. * checks/errors/borrowck/ffi-polonius/src/lib.rs: New file. * checks/errors/borrowck/polonius/rust-polonius-ffi.h: New file. * checks/errors/borrowck/polonius/rust-polonius.h: New file. Signed-off-by: Jakub Dupak <dev@jakubdupak.com>

jdupak · 2023-11-03T08:30:56Z

I don't see why not.

…

On Thu, 2 Nov 2023, at 23:06, Owen Avery wrote: ***@***.**** commented on this pull request. In gcc/rust/Make-lang.in <#2716 (comment)>: > @@ -461,3 +464,9 @@ rust/%.o: rust/checks/errors/borrowck/%.cc rust/%.o: rust/metadata/%.cc $(COMPILE) $(RUST_CXXFLAGS) $(RUST_INCLUDES) $< $(POSTCOMPILE) + +rust/libffi_polonius.a: \ + rust/checks/errors/borrowck/ffi-polonius/Cargo.toml \ + $(wildcard $(srcdir)/rust/checks/errors/borrowck/ffi-polonius/src/*) + cargo build --manifest-path $(srcdir)/rust/checks/errors/borrowck/ffi-polonius/Cargo.toml --release --target-dir rust/ffi-polonius Could polonius be loaded as a shared library by the compiler? The actual implementation, as opposed to the bindings, could be kept out-of-tree for now. — Reply to this email directly, view it on GitHub <#2716 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AFNB76E4M7H4RQPWGILTUADYCQKNDAVCNFSM6AAAAAA6U67LSGVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTOMJRGMYTKMJTG4>. You are receiving this because you authored the thread.Message ID: ***@***.***>

jdupak · 2024-03-22T19:41:54Z

Replaced with #2889

jdupak force-pushed the borrowck-polonius-ffi branch from 4129af8 to 2cf9912 Compare October 29, 2023 17:22

jdupak marked this pull request as draft October 29, 2023 18:00

jdupak force-pushed the borrowck-polonius-ffi branch 2 times, most recently from 6fa6aa2 to 9b8581e Compare October 29, 2023 18:44

CI: Add Cargo to build Polonius

2ec81de

ChangeLog: * .github/workflows/ccpp.yml: Install cargo. Signed-off-by: Jakub Dupak <dev@jakubdupak.com>

jdupak force-pushed the borrowck-polonius-ffi branch from 9b8581e to 2f89ed0 Compare October 29, 2023 19:27

P-E-P reviewed Oct 30, 2023

View reviewed changes

gcc/rust/checks/errors/borrowck/ffi-polonius/src/gccrs_ffi.rs Show resolved Hide resolved

dkm reviewed Oct 30, 2023

View reviewed changes

gcc/rust/checks/errors/borrowck/ffi-polonius/src/gccrs_ffi.rs Outdated Show resolved Hide resolved

gcc/rust/checks/errors/borrowck/polonius/rust-polonius.h Outdated Show resolved Hide resolved

tschwinge reviewed Oct 30, 2023

View reviewed changes

jdupak force-pushed the borrowck-polonius-ffi branch from 2f89ed0 to 8a2dd90 Compare October 30, 2023 13:38

jdupak force-pushed the borrowck-polonius-ffi branch 2 times, most recently from 3a0d489 to b34dd80 Compare October 31, 2023 20:24

jdupak force-pushed the borrowck-polonius-ffi branch from b34dd80 to b87f621 Compare October 31, 2023 20:31

jdupak mentioned this pull request Feb 28, 2024

Borrowck tests and build #2889

Merged

jdupak closed this Mar 22, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Borrowck: Polonius FFI #2716

Borrowck: Polonius FFI #2716

jdupak commented Oct 29, 2023

jdupak commented Oct 29, 2023 •

edited

Loading

jdupak commented Oct 29, 2023

P-E-P commented Oct 30, 2023

tschwinge Oct 30, 2023

tschwinge Oct 30, 2023

tschwinge Oct 30, 2023

P-E-P Oct 30, 2023 •

edited

Loading

dkm Oct 30, 2023

jdupak Oct 30, 2023

dkm Oct 31, 2023 •

edited

Loading

tschwinge Oct 31, 2023

dkm Nov 1, 2023

powerboat9 Nov 2, 2023 •

edited

Loading

jdupak commented Oct 30, 2023 •

edited

Loading

tschwinge commented Oct 31, 2023

jdupak commented Nov 3, 2023 via email

jdupak commented Mar 22, 2024

Borrowck: Polonius FFI #2716

Borrowck: Polonius FFI #2716

Conversation

jdupak commented Oct 29, 2023

jdupak commented Oct 29, 2023 • edited Loading

jdupak commented Oct 29, 2023

P-E-P commented Oct 30, 2023

tschwinge Oct 30, 2023

Choose a reason for hiding this comment

tschwinge Oct 30, 2023

Choose a reason for hiding this comment

tschwinge Oct 30, 2023

Choose a reason for hiding this comment

P-E-P Oct 30, 2023 • edited Loading

Choose a reason for hiding this comment

dkm Oct 30, 2023

Choose a reason for hiding this comment

jdupak Oct 30, 2023

Choose a reason for hiding this comment

dkm Oct 31, 2023 • edited Loading

Choose a reason for hiding this comment

tschwinge Oct 31, 2023

Choose a reason for hiding this comment

dkm Nov 1, 2023

Choose a reason for hiding this comment

powerboat9 Nov 2, 2023 • edited Loading

Choose a reason for hiding this comment

jdupak commented Oct 30, 2023 • edited Loading

tschwinge commented Oct 31, 2023

jdupak commented Nov 3, 2023 via email

jdupak commented Mar 22, 2024

jdupak commented Oct 29, 2023 •

edited

Loading

P-E-P Oct 30, 2023 •

edited

Loading

dkm Oct 31, 2023 •

edited

Loading

powerboat9 Nov 2, 2023 •

edited

Loading

jdupak commented Oct 30, 2023 •

edited

Loading