search

timescale / timescaledb-toolkit

Extension for more hyperfunctions, fully compatible with TimescaleDB and PostgreSQL 📈
https://www.timescale.com
Other
361 stars 46 forks source link

Consider running tests with ASAN #341

Open epgts opened 2 years ago

epgts commented 2 years ago

In order to interact with our great C host PostgreSQL, we must write lots of unsafe code. Some of it may have memory errors. This isn't just theoretical: we recently got lucky and spotted memory bugs during code review.

Since Rust is built on LLVM, surely it can use ASAN? It seems it's supposed to work with RUSTFLAGS='-Z sanitizer=address' (either with nightly or with RUSTC_BOOTSTRAP=1), but my naive attempt failed (see logs). Probably I've just done something wrong. We should look into it.

Some of the crates actually do work. Others fail to link. The extension fails to link, too. See attached logs:

crates.asan.log extension.asan.log

JLockerman commented 2 years ago

Definitely looks like linker errors.

I wonder if the environment variable RUSTFLAGS gets overridden by rustflags in the .cargo/config during some passes, so we don't pass the ASAN arguments for linking. Doctests are apparently kinda weird from a build standpoint, so I wouldn't at all be surprised if something like that occurs. That suggests two things we could try for the first issues: 1. edit the .cargo/config to add sanitizers, 2. disable doctests while trying to run with sanitizers.

The second one is a failure of this script that pgx uses to discover exported SQL objects. While linking it's failing to discover some postgres symbols. I'm even less certain of this one 🤔

JLockerman commented 2 years ago

(my gut wants to say that the second one is caused by postgres not being compiled with santizers, but I have no reason to actually think that...)

workingjubilee commented 9 months ago

We have recently been successful in configuring and enabling using valgrind with pgrx.