Skip to content

Commit

Permalink
Stop SpiClient soundness from regressing
Browse files Browse the repository at this point in the history
...by adding a regression test with trybuild!
Adds ui tests using trybuild's test runner.
  • Loading branch information
workingjubilee committed Jul 14, 2023
1 parent ec8c42a commit d0796ab
Show file tree
Hide file tree
Showing 7 changed files with 136 additions and 0 deletions.
34 changes: 34 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions pgrx-tests/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ rand = "0.8.5"

[dev-dependencies]
eyre = "0.6.8" # testing functions that return `eyre::Result`
trybuild = "1"

[dependencies.pgrx]
path = "../pgrx"
Expand Down
8 changes: 8 additions & 0 deletions pgrx-tests/tests/ui.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
use trybuild::TestCases;

#[test]
fn ui() {
let t = TestCases::new();
t.compile_fail("tests/ui/*.rs");
}

27 changes: 27 additions & 0 deletions pgrx-tests/tests/ui/escaping-spiclient-1209-cursor.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
use pgrx::prelude::*;
use std::error::Error;

#[pg_test]
fn issue1209() -> Result<Option<String>, Box<dyn Error>> {
// create the cursor we actually care about
let mut res = Spi::connect(|c| {
c.open_cursor("select 'hello' from generate_series(1, 10000)", None)
.fetch(10000)
.unwrap()
});

// here we just perform some allocations to make sure that the previous cursor gets invalidated
for _ in 0..1000 {
Spi::connect(|c| c.open_cursor("select 1", None).fetch(1).unwrap());
}

// later elements are probably more likely to point to deallocated memory
for _ in 0..1000 {
res.next();
}

// segfault
Ok(res.next().unwrap().get::<String>(1)?)
}

fn main() {}
45 changes: 45 additions & 0 deletions pgrx-tests/tests/ui/escaping-spiclient-1209-cursor.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
error: lifetime may not live long enough
--> tests/ui/escaping-spiclient-1209-cursor.rs:8:9
|
7 | let mut res = Spi::connect(|c| {
| -- return type of closure is SpiTupleTable<'2>
| |
| has type `SpiClient<'1>`
8 | / c.open_cursor("select 'hello' from generate_series(1, 10000)", None)
9 | | .fetch(10000)
10 | | .unwrap()
| |_____________________^ returning this value requires that `'1` must outlive `'2`

error[E0515]: cannot return value referencing temporary value
--> tests/ui/escaping-spiclient-1209-cursor.rs:8:9
|
8 | c.open_cursor("select 'hello' from generate_series(1, 10000)", None)
| ^-------------------------------------------------------------------
| |
| _________temporary value created here
| |
9 | | .fetch(10000)
10 | | .unwrap()
| |_____________________^ returns a value referencing data owned by the current function
|
= help: use `.collect()` to allocate the iterator

error: lifetime may not live long enough
--> tests/ui/escaping-spiclient-1209-cursor.rs:15:26
|
15 | Spi::connect(|c| c.open_cursor("select 1", None).fetch(1).unwrap());
| -- ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ returning this value requires that `'1` must outlive `'2`
| ||
| |return type of closure is SpiTupleTable<'2>
| has type `SpiClient<'1>`

error[E0515]: cannot return value referencing temporary value
--> tests/ui/escaping-spiclient-1209-cursor.rs:15:26
|
15 | Spi::connect(|c| c.open_cursor("select 1", None).fetch(1).unwrap());
| -------------------------------^^^^^^^^^^^^^^^^^^
| |
| returns a value referencing data owned by the current function
| temporary value created here
|
= help: use `.collect()` to allocate the iterator
13 changes: 13 additions & 0 deletions pgrx-tests/tests/ui/escaping-spiclient-1209-prep-stmt.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
use pgrx::prelude::*;
use std::error::Error;

#[pg_extern]
fn issue1209_prepared_stmt(q: &str) -> Result<Option<String>, Box<dyn Error>> {
use pgrx::spi::Query;

let prepared = { Spi::connect(|c| c.prepare(q, None))? };

Ok(Spi::connect(|c| prepared.execute(&c, Some(1), None)?.first().get(1))?)
}

fn main() {}
8 changes: 8 additions & 0 deletions pgrx-tests/tests/ui/escaping-spiclient-1209-prep-stmt.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
error: lifetime may not live long enough
--> tests/ui/escaping-spiclient-1209-prep-stmt.rs:8:39
|
8 | let prepared = { Spi::connect(|c| c.prepare(q, None))? };
| -- ^^^^^^^^^^^^^^^^^^ returning this value requires that `'1` must outlive `'2`
| ||
| |return type of closure is std::result::Result<PreparedStatement<'2>, pgrx::spi::Error>
| has type `SpiClient<'1>`

0 comments on commit d0796ab

Please sign in to comment.