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

Correctly import foreign statics #115

Merged
merged 2 commits into from Jan 25, 2022
Merged

Conversation

bjorn3
Copy link
Member

@bjorn3 bjorn3 commented Jan 24, 2022

This is a fix for the environment variable issue I reported in a PM.

Previously foreign statics would actually cause a local static to be defined and exported. This issue was found because std::env::vars() was found to return no env vars despite many being defined. This was caused by libstd importing environ as foreign static. The accidental definition of environ caused libstd to read a null pointer which was interpreted as there being no environment variables at all.

I also move rustup component installation to rust-toolchain This allows cargo check to function correctly without having to first
run prepare_build.sh. cg_clif has been using rust-toolchain too for a while now.

Copy link
Contributor

@antoyo antoyo left a comment

Choose a reason for hiding this comment

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

Thanks a lot!

src/declare.rs Outdated Show resolved Hide resolved
@antoyo
Copy link
Contributor

antoyo commented Jan 24, 2022

This seems to break STDOUT, according to the CI.

@bjorn3
Copy link
Member Author

bjorn3 commented Jan 24, 2022

pub static STDOUT: *mut i32;

This is trying to import a global named STDOUT and then call fflush on it. This is not a thing libc exports. There is an export called stdout though. Previously the code would pass a null pointer to fflush. Will fix the test code tomorrow.

Previously foreign statics would actually cause a local static to be
defined and exported. This issue was found because std::env::vars() was
found to return no env vars despite many being defined. This was caused
by libstd importing environ as foreign static. The accidental definition
of environ caused libstd to read a null pointer which was interpreted as
there being no environment variables at all.

Also fix tests. STDOUT is not defined by libc. The correct name is stdout.
This previously worked as STDOUT was incorrectly defined as null pointer
during codegen.
This allows cargo check to function correctly without having to first
run prepare_build.sh. cg_clif has been using rust-toolchain too for a
while now.
@bjorn3
Copy link
Member Author

bjorn3 commented Jan 25, 2022

Fixed tests and squashed.

@antoyo antoyo merged commit fc23678 into rust-lang:master Jan 25, 2022
@antoyo
Copy link
Contributor

antoyo commented Jan 25, 2022

Thanks a lot!

@bjorn3 bjorn3 deleted the foreign_statics branch January 25, 2022 14:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants