Skip to content
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

lib/ukrust: Fix helloworld-rust application compilation error #1163

Closed
wants to merge 3 commits into from

Conversation

cocodery
Copy link

@cocodery cocodery commented Nov 20, 2023

Description of changes

This change, first manually move awk-generation commands to UK_PREPARE, because the bindgen command need uk/bits/libid but it's generated later than bindgen command. Secondly, fix command-line argument used by bindgen, and comment redefinition symbol. Then, helloworld-rust can successfully run.

Prerequisite checklist

  • Read the contribution guidelines regarding submitting new changes to the project;
  • Tested your changes against relevant architectures and platforms;
  • Ran the checkpatch.uk on your commit series before opening this PR;
  • Updated relevant documentation.

Base target

  • Architecture(s): [e.g. x86_64 or N/A]
  • Platform(s): [e.g. kvm, xen or N/A]
  • Application(s): [e.g. app-python3 or N/A]

Additional configuration

Maybe you need a high version bindgen, >= 0.60.0
You can download latest version through bindgen

Closes #1147

Fix sub-problem of issue#1147 when enable libukrust, which will throw an error that
`fatal error: 'uk/bits/libid.h' file not found` because of wrong commands order.
this header file is needed by `bindings_helper.h` but command generates
'uk/bits/libid/h' is ahead from the bindgen command.

Signed-off-by: Chris Wu <cocodery@outlook.com>
This commit aims to fix issue#1147 which compile apps with enabled
libukrust

1. ukrust/Makefile.uk: fix command line argument with high-version
   bindgen, from 'size_t-is-usize' to 'no-size_t-is-usize'.
2. ukrust/src/allocator.rs: fix previous defined symbol by comment
   them, becasue rustc will help to resolve, and add new symbol
'__rust_no_alloc_shim_is_unstable' for locate.

Signed-off-by: Chris Wu <cocodery@outlook.com>
@cocodery cocodery requested review from a team as code owners November 20, 2023 15:55
@cocodery cocodery changed the title lib/ukrust: Fix helloworld-rust appilication compilation error lib/ukrust: Fix helloworld-rust application compilation error Nov 24, 2023
unsafe { bindings::__ukrust_sys_malloc(size as u64) as *mut u8 }
}
// #[no_mangle]
// pub extern "Rust" fn __rust_alloc(size: usize, _align: usize) -> *mut u8 {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this intended in this PR? I think without __rust_alloc we won't be able to allocate anything on the heap.

Copy link
Author

@cocodery cocodery Nov 27, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In fact this is another question need to solve later.
This is a multiple definition problem. If enable these functions, rustc will report these errors.
And the target of this PR aims to run helloworld-rust at least becasue ukrust cannot be used yet. Maybe the upgrade of rust cause this problem.
In issue#1147 is more detailed.

UK_SRCS-y += $(2)
UK_PREPROCESS-y += $(3)
$(eval $(call vprefix_lib,$(1),CLEAN-y) += $(call build_clean,$(3)))
# $(3): $(2) \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We used this rule to gather all the rust sources, but I'm now sure how it would work if you remove them.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, this part only related with AWK file. And I enable all uklibs to compile unikraft, no error is reported. And I manually move this part into lib/uklibid/Makefile.uk.
The reason of removing them is mentioned in my commit message. Rust-bindgen file need uk/include/libid.h which generated from an awk file, but because UK_PREPARE is executed before UK_PREPROCESS, so one error that not find uk/include/libid.h occurs.
In issue#1147 is more detailed.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be instead solved by stating the dependencies with the bingen rule. Please note that also external libraries are using these AWK rules. I assume bindgen should run just before a rust unit is compiled, so the rust compile rule should have bindgen as dependency (or actually the files it created) and bindgen should run after UK_PREPROCESS target was reached. So I think the bingen rule should have an order-only pre-requisite to the preprocess target: <...> | preprocess
Would something like this work?

@razvand
Copy link
Contributor

razvand commented Dec 6, 2023

@mkroening , does it make sense to have this PR in? Or would your catalog PR deprecate this?

@razvand razvand removed request for a team December 6, 2023 11:17
@mkroening
Copy link
Member

@mkroening , does it make sense to have this PR in? Or would your catalog PR deprecate this?

Maybe? That depends on the exact issue that @cocodery is trying to solve. My catalog PR describes building full Rust applications. This PR fixes @vladandrew's approach for building Rust libraries together with a C application, that I don't know much of. I could acquaint myself with the approach if necessary, though. :)

@mkroening mkroening assigned vladandrew and unassigned mkroening Dec 6, 2023
@cocodery
Copy link
Author

cocodery commented Dec 6, 2023

This PR is trying to solve compilation failure when runing rust libraries with C application.
I don't know whether failure will happen when building full rust application, but you can try it if you're avaliable @mkroening. I guess it will maybe because of the problem mentioned in related issue.
This PR havn't finish yet, but can make rust application without heap allocation build and run successfully currently.

@mkroening
Copy link
Member

With my approach for full Rust applications, ukrust is not used. So this issue is orthogonal to my approach and will appear as well.

Is there a special reason you are going for Rust libraries and C apps, @cocodery? You can try having the whole app in Rust. That way, Rust libraries work, as well as std, so allocations, network, etc.

@cocodery
Copy link
Author

cocodery commented Dec 6, 2023

Aha, not any special reason, but want to try solve this issue.
As a optional library, ukrust can't be compiled is not normal.
I'm also interested in runnning whole rust app.

@cocodery
Copy link
Author

cocodery commented Dec 7, 2023

Hmmm, I'm wondering when develop ukrust, only test stack program without heap structure like vec or anything else?

I roll back lib/ukrust to all its 4 previous commit one by one, and set rust toolchain nightly-2022-02-04.
It can be successfully compiled without heap structure. Also, latest nightly rustc can do. But will have another multiple definition on __rust_alloc, etc. That's why I comment __rustc_alloc, etc.
Also, can run successfully.

When use heap, they have the similar output, multiple definition of __rdl_oom, and undefined reference to _Unwind_Resume.
So, I guess when app use crate alloc, cargo choose local crate instead of unikraft compiled one, which cause two main problem above.

But I'm not very familier with rustc command arguments. Is there someone availiable to work together?

@Mihnea27
Copy link

@mkroening I think the main reason ukrust is needed is to have Rust libraries that you can use as replacements for Unikraft internal libraries. For applications I think it is quite a particular case when you want to mix Rust and C libraries.

@mkroening
Copy link
Member

But I'm not very familier with rustc command arguments. Is there someone availiable to work together?

I am sure you are aware of the official docs. If you need to discuss things or need help with rustc, let's chat on Discord. :)

@razvand razvand added this to the v0.17.0 (Calypso) milestone Dec 30, 2023
@razvand razvand added area/lib Internal Unikraft Microlibrary kind/quick-fix Issue is a quick fix topic/build Topics to do with the build system bug/compile-time Bug occurs during compile-time lib/ukrust Rust support for internal libraries labels Dec 30, 2023
@mkroening mkroening self-assigned this Apr 28, 2024
@mkroening
Copy link
Member

I am closing this in favor of #1350.

@cocodery, thank you so much for your PR. If you are still interested in this issue, we are discussing details on #1350 on Discord. :)

@mkroening mkroening closed this Apr 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/lib Internal Unikraft Microlibrary bug/compile-time Bug occurs during compile-time kind/quick-fix Issue is a quick fix lib/ukrust Rust support for internal libraries topic/build Topics to do with the build system
Projects
Status: Done!
Development

Successfully merging this pull request may close these issues.

Apps fail to compile when using lib-ukrust
6 participants