PG-1465 Add ASAN and UBSAN sanitizers #285

artemgavrilov · 2025-04-29T16:58:56Z

https://perconadev.atlassian.net/browse/PG-1465

This PR adds CI workflow with ASAN(including LSAN) and UBSAN sanitizers. To make this workflow pass it required to modify couple tests that were affected by sanitizers influence. Running check-world takes 46 minutes, so it looks like overkill for workflow that we will run for every PR. Because of that I ended with pg_tde tests + PG regression tests.

One real issue were found with sanitizers in tde_keyring.c file (access uninitialized pointer)

codecov-commenter · 2025-04-29T18:47:20Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.07%. Comparing base (7f2f26e) to head (5b7c987).

❌ Your project status has failed because the head coverage (80.07%) is below the target coverage (90.00%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files

@@                Coverage Diff                 @@
##           TDE_REL_17_STABLE     #285   +/-   ##
==================================================
  Coverage              80.07%   80.07%           
==================================================
  Files                     22       22           
  Lines                   2569     2570    +1     
  Branches                 402      403    +1     
==================================================
+ Hits                    2057     2058    +1     
  Misses                   434      434           
  Partials                  78       78

Components	Coverage Δ
access	`83.06% <ø> (ø)`
catalog	`87.57% <ø> (ø)`
common	`92.42% <ø> (ø)`
encryption	`71.90% <ø> (ø)`
keyring	`72.07% <ø> (ø)`
src	`64.60% <ø> (ø)`
smgr	`97.08% <ø> (+0.02%)`	⬆️
transam	`∅ <ø> (∅)`

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

AndersAstrand · 2025-05-07T13:57:18Z

contrib/pg_tde/src/catalog/tde_keyring.c

@@ -781,7 +781,10 @@ scan_key_provider_file(ProviderScanType scanType, void *scanKey, Oid dbOid)
 				providers_list = lappend(providers_list, keyring);
 #else
 				if (providers_list == NULL)
+				{
 					providers_list = palloc_object(SimplePtrList);


we should maybe use palloc0_object() here instead so that everything is null by default?

Yep, make sense. Changed.

dutow · 2025-05-07T18:06:51Z

ci_scripts/suppressions/lsan.supp

+leak:preproc.y
+leak:pgtypeslib/common.c
+leak:ecpglib/memory.c
+leak:kmip.c


can we create a ticket/tickets about this? Some of these supressions definitely seem like issues.

Most of them are not real issues, but "design features" or just allocation functions. I used timescale as a reference and they mute the same things: https://github.com/timescale/timescaledb/blob/main/scripts/suppressions/suppr_leak.txt

But I'm agree that this list should be reviewed. Probably some mutes scope can be reduced. Here is the ticket: https://perconadev.atlassian.net/browse/PG-1588

The kmip.c one is likely to be an actual bug. But, yeah, that can be outside the scope of this PR:

Yes, and strdup also seems to be a possible issue, or at least it can hide possible issues, as it suppresses any leaked strings created by strdup - even if it's a leak in postgres itself, we should look at that.

contrib/pg_tde/t/011_unlogged_tables.pl

jeltz · 2025-05-08T01:55:46Z

ci_scripts/meson-build.sh

+
+    sanitize)
+        echo "Building with sanitize option"
+        export CFLAGS="-fsanitize=address -fsanitize=undefined -fno-omit-frame-pointer -fno-inline-functions"


Is there a reason why you disable inlining? LLVM's documentation says nothing about that as far as I can see.

https://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html#stack-traces-and-report-symbolization

https://clang.llvm.org/docs/AddressSanitizer.html#usage

To get a reasonable performance add -O1 or higher. To get nicer stack traces in error messages add -fno-omit-frame-pointer. To get perfect stack traces you may need to disable inlining (just use -O1) and tail call elimination (-fno-optimize-sibling-calls).

This is for ASAN. BTW it looks like it's worth to add -fno-optimize-sibling-calls as well.

jeltz · 2025-05-08T02:00:12Z

ci_scripts/meson-build.sh

+    sanitize)
+        echo "Building with sanitize option"
+        export CFLAGS="-fsanitize=address -fsanitize=undefined -fno-omit-frame-pointer -fno-inline-functions"
+        BUILD_TYPE=debug


What is the best practices about optimization and sanitizers? Our production build will be -O2 but maybe that fucks up sanitizers.

I didn't see information that -O2 could harm sanitizer checks, however lower optimization levels simplify report reviews. I hope there should be no difference between -O0 and -O2 in terms of leaks and UBs, am I wrong?

Andriciuc

Approved for contribute and contributing changes to docs

Add CI workflow with address and undefined behaviour santitzers.

Use palloc0_object to initialize tail and head pointers to zero value.

artemgavrilov force-pushed the PG-1465-sanitizers branch 7 times, most recently from 6fa0c9f to bf3ad43 Compare May 6, 2025 15:38

nastena1606 deployed to PG-1465-sanitizers - INTERNAL-pg_tde docs PR #285 May 6, 2025 15:38 — with Render View deployment

nastena1606 deployed to PG-1465-sanitizers - pg-tde-docs PR #285 May 6, 2025 15:38 — with Render View deployment

artemgavrilov force-pushed the PG-1465-sanitizers branch from bf3ad43 to 5b2b56e Compare May 6, 2025 15:54

nastena1606 deployed to PG-1465-sanitizers - pg-tde-docs PR #285 May 6, 2025 15:54 — with Render View deployment

nastena1606 deployed to PG-1465-sanitizers - INTERNAL-pg_tde docs PR #285 May 6, 2025 15:54 — with Render View deployment

artemgavrilov force-pushed the PG-1465-sanitizers branch from 5b2b56e to 42bb329 Compare May 6, 2025 16:07

nastena1606 deployed to PG-1465-sanitizers - INTERNAL-pg_tde docs PR #285 May 6, 2025 16:07 — with Render View deployment

nastena1606 deployed to PG-1465-sanitizers - pg-tde-docs PR #285 May 6, 2025 16:07 — with Render View deployment

artemgavrilov force-pushed the PG-1465-sanitizers branch from 4299778 to b04f73e Compare May 7, 2025 13:05

nastena1606 deployed to PG-1465-sanitizers - pg-tde-docs PR #285 May 7, 2025 13:05 — with Render View deployment

nastena1606 deployed to PG-1465-sanitizers - INTERNAL-pg_tde docs PR #285 May 7, 2025 13:05 — with Render View deployment

artemgavrilov force-pushed the PG-1465-sanitizers branch from b04f73e to 4fcf202 Compare May 7, 2025 13:34

nastena1606 deployed to PG-1465-sanitizers - INTERNAL-pg_tde docs PR #285 May 7, 2025 13:34 — with Render View deployment

nastena1606 deployed to PG-1465-sanitizers - pg-tde-docs PR #285 May 7, 2025 13:34 — with Render View deployment

artemgavrilov changed the title ~~[WIP] PG-1465 Sanitizers~~ PG-1465 Add ASAN and UBSAN sanitizers May 7, 2025

artemgavrilov force-pushed the PG-1465-sanitizers branch from 4fcf202 to ef637b7 Compare May 7, 2025 13:52

nastena1606 deployed to PG-1465-sanitizers - pg-tde-docs PR #285 May 7, 2025 13:52 — with Render View deployment

nastena1606 deployed to PG-1465-sanitizers - INTERNAL-pg_tde docs PR #285 May 7, 2025 13:52 — with Render View deployment

artemgavrilov marked this pull request as ready for review May 7, 2025 13:54

artemgavrilov requested review from nastena1606, Andriciuc and dutow as code owners May 7, 2025 13:54

artemgavrilov requested a review from dAdAbird as a code owner May 7, 2025 13:54

artemgavrilov requested review from jeltz and AndersAstrand May 7, 2025 13:54

AndersAstrand reviewed May 7, 2025

View reviewed changes

artemgavrilov force-pushed the PG-1465-sanitizers branch from ef637b7 to 2fae6b3 Compare May 7, 2025 16:11

nastena1606 deployed to PG-1465-sanitizers - pg-tde-docs PR #285 May 7, 2025 16:11 — with Render View deployment

nastena1606 deployed to PG-1465-sanitizers - INTERNAL-pg_tde docs PR #285 May 7, 2025 16:11 — with Render View deployment

dutow reviewed May 7, 2025

View reviewed changes

jeltz reviewed May 8, 2025

View reviewed changes

contrib/pg_tde/t/011_unlogged_tables.pl Outdated Show resolved Hide resolved

jeltz reviewed May 8, 2025

View reviewed changes

Andriciuc approved these changes May 8, 2025

View reviewed changes

dutow approved these changes May 23, 2025

View reviewed changes

artemgavrilov added 2 commits May 23, 2025 12:03

PG-1465 Add CI workfolw with sanitizers

b7c42bc

Add CI workflow with address and undefined behaviour santitzers.

Fix uninitialized pointers

2cb644a

Use palloc0_object to initialize tail and head pointers to zero value.

artemgavrilov force-pushed the PG-1465-sanitizers branch from 2fae6b3 to 2cb644a Compare May 23, 2025 10:04

nastena1606 temporarily deployed to PG-1465-sanitizers - pg-tde-docs PR #285 May 23, 2025 10:04 — with Render Destroyed

nastena1606 temporarily deployed to PG-1465-sanitizers - INTERNAL-pg_tde docs PR #285 May 23, 2025 10:04 — with Render Destroyed

artemgavrilov merged commit 3aabe35 into TDE_REL_17_STABLE May 23, 2025
28 checks passed

artemgavrilov deleted the PG-1465-sanitizers branch May 23, 2025 10:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

PG-1465 Add ASAN and UBSAN sanitizers #285

PG-1465 Add ASAN and UBSAN sanitizers #285

Uh oh!

artemgavrilov commented Apr 29, 2025 •

edited

Loading

Uh oh!

codecov-commenter commented Apr 29, 2025 •

edited

Loading

Uh oh!

AndersAstrand May 7, 2025

Uh oh!

artemgavrilov May 7, 2025

Uh oh!

dutow May 7, 2025

Uh oh!

artemgavrilov May 7, 2025

Uh oh!

jeltz May 8, 2025

Uh oh!

dutow May 8, 2025

Uh oh!

Uh oh!

jeltz May 8, 2025

Uh oh!

dutow May 8, 2025

Uh oh!

artemgavrilov May 8, 2025

Uh oh!

jeltz May 8, 2025

Uh oh!

artemgavrilov May 8, 2025

Uh oh!

Andriciuc left a comment

Uh oh!

Uh oh!

Uh oh!

PG-1465 Add ASAN and UBSAN sanitizers #285

PG-1465 Add ASAN and UBSAN sanitizers #285

Uh oh!

Conversation

artemgavrilov commented Apr 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-commenter commented Apr 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Andriciuc left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

artemgavrilov commented Apr 29, 2025 •

edited

Loading

codecov-commenter commented Apr 29, 2025 •

edited

Loading