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

UB and usage of deprecated function #59

Closed
Dylan-DPC-zz opened this issue Jul 9, 2020 · 10 comments
Closed

UB and usage of deprecated function #59

Dylan-DPC-zz opened this issue Jul 9, 2020 · 10 comments

Comments

@Dylan-DPC-zz
Copy link

Found this crate via a a crater run (crater is a tool that runs a specific branch of rustc against locked dependencies of specific crates to figure out regressions in the compiler or in potential usages in applications).

This crate uses the std::mem:uninitliazed method which is not only deprecated but also could be unsound as per a similar advisory for another crate.

If you wish to continue maintaining it, it will be adisable to fix this issue and publish a new release. Else you could possibly hand it over to a new set of maintainers (can help you with that) or publish a note that it is abandoned/deprecated/won't be maintained in the future since it has around 10 open source crates that depend on it.

If you need any help in going about this, you can let us know

Thanks :)

@RalfJung
Copy link

RalfJung commented Aug 31, 2020

Specifically, the bug is here:

let mut res: Context = uninitialized();

The Context struct contains a field of type Rounding which is an enum; leaving an enum uninitialized is Undefined Behavior. rust-lang/rust#71274 will detect this and turn it into a panic.

@alkis
Copy link
Owner

alkis commented Oct 1, 2020

I think #47 fixes this. I asked @jonathanstrong to make it apply without conflicts and I will merge it.

@probablykasper
Copy link

2.0.4 now panics in Rust 1.48.0. The uninit-fix branch seems to work, though. Any plans on merging it and releasing a new version, @alkis / @jonathanstrong?

@CastilloDel
Copy link

The master version still panics when running the tests. The uninit-fix branch seems to pass all of them as @probablykasper said. Are there any plans to address this? @alkis

@jonathanstrong
Copy link
Collaborator

earlier this week I spoke to @alkis about this and plan to be taking a bigger role on the project. I have some time slotted to work on this next week and merging uninit-fix is first on the list. thanks for your patience.

@CastilloDel
Copy link

Those are great news! and thanks for the quick reply

@probablykasper
Copy link

@jonathanstrong Is anything missing from uninit-fix? I went and just published that branch as it's own crate so I could use it in cpc, and that seems to be working perfectly.

@jonathanstrong
Copy link
Collaborator

@probablykasper I don't think so - I've also been using in production for months. just wanted to give it a good once-over before merging it here.

@jonathanstrong
Copy link
Collaborator

@probablykasper @CastilloDel quick update on this: I merged uninit-fix and released a new version on github earlier this week. however, at that time I realized I do not have permissions to publish a new crate version on crates.io. I emailed @alkis about this and am awaiting his reply.

@jonathanstrong
Copy link
Collaborator

@probablykasper @CastilloDel just published version 2.1.0 of the crate, which fixes the uninitialized issue.

probablykasper added a commit to probablykasper/cpc that referenced this issue Mar 14, 2021
Switch back to official `decimal` because alkis/decimal#59 is fixed
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

No branches or pull requests

6 participants