Page MenuHomePhabricator

[secp256k1] ci: Run ASan/LSan and reorganize sanitizer and Valgrind jobs
ClosedPublic

Authored by Fabien on Jan 6 2023, 16:36.

Details

Summary
This enables asan (including lsan), and restructures the sanitizer builds. It also removes -fno-omit-frame-pointer again. This is debatable. The main reason for removing this is that GCC (but not clang) fails to build (runs out of registers) when combining asan, -fno-omit-frame-pointer, and the x86_64 asm. But I believe without -fno-omit-frame-pointer, the instrumented binary may be closer to the real binary, which is good. If we get unusable debugging output on CI without frame pointers, we can still reproduce the build locally.

Backport of secp256k1#846.
Completes backport of secp256k1#969:
https://github.com/bitcoin-core/secp256k1/pull/969/commits/3d2f492ceb76eea93d3a9f85f80baec7b5842160

Depends on D12957.

Test Plan
ninja check-secp256k1

Tested the cirrus build against my personal github forked repo.

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Fabien requested review of this revision.Jan 6 2023, 16:36
sdulfari requested changes to this revision.Jan 6 2023, 18:00
sdulfari added a subscriber: sdulfari.
sdulfari added inline comments.
src/secp256k1/.cirrus.yml
189 ↗(On Diff #37482)

No need to disable openssl-tests?

192–193 ↗(On Diff #37482)

missing comment

209 ↗(On Diff #37482)

why is ASM removed?

This revision now requires changes to proceed.Jan 6 2023, 18:00
src/secp256k1/.cirrus.yml
189 ↗(On Diff #37482)

We removed them already

209 ↗(On Diff #37482)

We don't support "auto" and this runs on both 32 and 64 bits arch

This revision is now accepted and ready to land.Jan 9 2023, 16:37