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

Unsupported operation sqlite3_open_v2 #2325

Closed
weiznich opened this issue Jul 4, 2022 · 7 comments
Closed

Unsupported operation sqlite3_open_v2 #2325

weiznich opened this issue Jul 4, 2022 · 7 comments
Labels
A-docs Area: affects documentation A-shims Area: This affects the external function shims C-proposal Category: a proposal for something we might want to do, or maybe not; details still being worked out

Comments

@weiznich
Copy link

weiznich commented Jul 4, 2022

Motivated by Ralf Jung's blog post about the last 2 years of miri development I've tried to run diesels test suit using miri. Especially the sqlite backend does a lot of nasty pointer things, so another layer of checks beside asan and friends would be helpful. I run into the following error message:

error: unsupported operation: can't call foreign function: sqlite3_open_v2
   --> diesel/src/sqlite/connection/raw.rs:47:13
    |
47  |             ffi::sqlite3_open_v2(database_url.as_ptr(), &mut conn_pointer, flags, ptr::null())
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ can't call foreign function: sqlite3_open_v2
    |
    = help: this is likely not a bug in the program; it indicates that the program performed an operation that the interpreter does not support

    = note: inside `sqlite::connection::raw::RawConnection::establish` at diesel/src/sqlite/connection/raw.rs:47:13
note: inside `<sqlite::connection::SqliteConnection as connection::Connection>::establish` at diesel/src/sqlite/connection/mod.rs:92:30
   --> diesel/src/sqlite/connection/mod.rs:92:30
    |
92  |         let raw_connection = RawConnection::establish(database_url)?;
    |                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
note: inside `test_helpers::connection` at diesel/src/test_helpers.rs:10:13
   --> diesel/src/test_helpers.rs:10:13
    |
10  |             SqliteConnection::establish(":memory:").unwrap()
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
note: inside `connection::transaction_manager::test::sqlite_transaction_is_rolled_back_upon_deferred_constraint_failure` at diesel/src/connection/transaction_manager.rs:896:25
   --> diesel/src/connection/transaction_manager.rs:896:25
    |
896 |         let conn = &mut crate::test_helpers::connection();
    |                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
note: inside closure at diesel/src/connection/transaction_manager.rs:889:5
   --> diesel/src/connection/transaction_manager.rs:889:5
    |
887 |       #[test]
    |       ------- in this procedural macro expansion
888 |       #[cfg(feature = "sqlite")]
889 | /     fn sqlite_transaction_is_rolled_back_upon_deferred_constraint_failure() {
890 | |         use crate::connection::transaction_manager::AnsiTransactionManager;
891 | |         use crate::connection::transaction_manager::TransactionManager;
892 | |         use crate::result::Error;
...   |
923 | |         );
924 | |     }
    | |_____^
    = note: this error originates in the attribute macro `test` (in Nightly builds, run with -Z macro-backtrace for more info)

I should note that I kind of expected that. I mainly raise this issue as the blog post explicitly calls for opening issues for unsupported operations.

Environment information:

  • Ubuntu 22.04
  • rustc --version:
rustc 1.63.0-nightly (c06728704 2022-05-19)
binary: rustc
commit-hash: c0672870491e84362f76ddecd50fa229f9b06dff
commit-date: 2022-05-19
host: x86_64-unknown-linux-gnu
release: 1.63.0-nightly
LLVM version: 14.0.4

Steps to reproduce:

git clone https://github.com/diesel-rs/diesel
cd diesel
git checkout cbdfee30b206fff0aba62fa482fd837c0edd2cea
cd diesel
cargo miri test --features "sqlite" --no-default-features
@oli-obk
Copy link
Contributor

oli-obk commented Jul 4, 2022

I think the only way forward for this would be to change libsqlite3-sys to use #[cfg(miri)] to do a pure-Rust in-memory implementation (beyond compiling the C library to Rust via that c2rust converter, I forget its name).

I mean, we do have #11 which would allow these things directly, but considering it has been untouched for years, it may take more years.

@RalfJung
Copy link
Member

RalfJung commented Jul 4, 2022

Yeah, I should probably have worded this a bit more carefully in the blog post... we're not going to be able to implement Python, PHP, or sqlite in Miri. :/

@RalfJung RalfJung added A-shims Area: This affects the external function shims C-proposal Category: a proposal for something we might want to do, or maybe not; details still being worked out labels Jul 4, 2022
@weiznich
Copy link
Author

weiznich commented Jul 5, 2022

That's totally understandable and that is what I've assumed anyway. I'm fine with closing this issue as out of scope ❤️

@oli-obk
Copy link
Contributor

oli-obk commented Jul 5, 2022

I guess to resolve this issue properly we should add something to https://github.com/rust-lang/miri/blob/master/CONTRIBUTING.md that says what things are in scope of miri and what are out of scope

@RalfJung RalfJung added the A-docs Area: affects documentation label Jul 5, 2022
@LegNeato
Copy link
Contributor

LegNeato commented Jul 8, 2022

👀 maurer#1

@RalfJung
Copy link
Member

RalfJung commented Jul 8, 2022

Oh wow. oO

@RalfJung
Copy link
Member

RalfJung commented May 4, 2024

Closing as we'll not implement this shim in Miri.

@RalfJung RalfJung closed this as completed May 4, 2024
bors added a commit that referenced this issue May 4, 2024
update 'unsupported' message

Instead of "the interpreter", just say Miri.

Also be a more more clear about what is expected to be supported and what not (Cc #2325).
RalfJung pushed a commit to RalfJung/rust that referenced this issue May 4, 2024
update 'unsupported' message

Instead of "the interpreter", just say Miri.

Also be a more more clear about what is expected to be supported and what not (Cc rust-lang/miri#2325).
RalfJung pushed a commit to RalfJung/rust that referenced this issue May 4, 2024
update 'unsupported' message

Instead of "the interpreter", just say Miri.

Also be a more more clear about what is expected to be supported and what not (Cc rust-lang/miri#2325).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-docs Area: affects documentation A-shims Area: This affects the external function shims C-proposal Category: a proposal for something we might want to do, or maybe not; details still being worked out
Projects
None yet
Development

No branches or pull requests

4 participants