Skip to content

Enable sanitizers for test runs#67

Merged
tpressure merged 8 commits intomainfrom
sanitizer_enablement
Dec 1, 2025
Merged

Enable sanitizers for test runs#67
tpressure merged 8 commits intomainfrom
sanitizer_enablement

Conversation

@hertrste
Copy link
Copy Markdown
Collaborator

@hertrste hertrste commented Nov 27, 2025

This PR enables the sanitizers in our test runs and the test will fail if some memory issues were found during any test. This PR also fixes the nix debug build of libvirt so that debug infos are not stripped. This leads to proper line of code annotations e.g. when attaching GDB, sanitizer output or backtraces.

Open:

  • The sanitizers are now enabled all the time for every test run. I think this is the right way to go, but we should keep an eye on the execution times.
    • the time the CI runs does not go up at all, no issue here (see here)

@phip1611
Copy link
Copy Markdown
Member

The drawback currently is, that all tests fail after the first leak was
detected in the journal, as we currently cannot reliably get the journal
log only from the last test.

What it make sense to add #51 as optional behavior for that?

@hertrste
Copy link
Copy Markdown
Collaborator Author

The drawback currently is, that all tests fail after the first leak was
detected in the journal, as we currently cannot reliably get the journal
log only from the last test.

What it make sense to add #51 as optional behavior for that?

Yes that definitely makes sense. But I think with a little bit of creativity we are able to get the sanitizer logs just from the last test, so that we are able seamlessly integrate them in the test suite.

I also have to take a look if the sanitizer run takes significantly longer to run through. If yes, we might want to make them configurable and run them separately in the CI.

@hertrste hertrste force-pushed the sanitizer_enablement branch 3 times, most recently from 024bc78 to ae38de3 Compare November 28, 2025 13:34
@hertrste hertrste marked this pull request as ready for review November 28, 2025 13:35
Comment thread tests/common.nix
Comment thread tests/testscript.py Outdated
Comment thread tests/testscript.py
Comment thread tests/common.nix
Copy link
Copy Markdown
Member

@phip1611 phip1611 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very good work, thanks!

phip1611 and others added 7 commits December 1, 2025 08:51
On-behalf-of: SAP philipp.schuster@sap.com
On-behalf-of: SAP stefan.kober@sap.com
On-behalf-of: SAP stefan.kober@sap.com
On-behalf-of: SAP stefan.kober@sap.com
The leak sanitizer can only check for leaks when the program under
inspection is terminating. Therefore, we trigger a libvirtd restart in
the tearDown of a test. Further, we check the journal for 'ERROR'
messages from the sanitizer and let the test fail if we find one.

We check for the journal of only the recent test run by placing a 'Test
run: <test-name>' marker in the journalctl manually before every
testrun.

On-behalf-of: SAP stefan.kober@sap.com
On-behalf-of: SAP stefan.kober@sap.com
We silence the following warning about inlining cleanup methods of the
glib:

libvirt> /nix/store/0784w06097k9i46zipgjf05l87i19jj0-glib-2.84.3-dev/include/glib-2.0/glib/gmacros.h:1342:43: warning: inlining failed in call to 'glib_autoptr_cleanup_virNetlinkMsg': call is unlikely and code size would grow [-Winline]
libvirt>  1342 | #define _GLIB_AUTOPTR_FUNC_NAME(TypeName) glib_autoptr_cleanup_##TypeName
libvirt>       |                                           ^~~~~~~~~~~~~~~~~~~~~
libvirt> /nix/store/0784w06097k9i46zipgjf05l87i19jj0-glib-2.84.3-dev/include/glib-2.0/glib/gmacros.h:1362:36: note: in expansion of macro '_GLIB_AUTOPTR_FUNC_NAME'
libvirt>  1362 |   static G_GNUC_UNUSED inline void _GLIB_AUTOPTR_FUNC_NAME(TypeName) (TypeName **_ptr)                          \
libvirt>       |                                    ^~~~~~~~~~~~~~~~~~~~~~~
libvirt> /nix/store/0784w06097k9i46zipgjf05l87i19jj0-glib-2.84.3-dev/include/glib-2.0/glib/gmacros.h:1379:3: note: in expansion of macro '_GLIB_DEFINE_AUTOPTR_CLEANUP_FUNCS'
libvirt>  1379 |   _GLIB_DEFINE_AUTOPTR_CLEANUP_FUNCS(TypeName, TypeName, func)
libvirt>       |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
libvirt> ../src/util/virnetlink.h:31:1: note: in expansion of macro 'G_DEFINE_AUTOPTR_CLEANUP_FUNC'
libvirt>    31 | G_DEFINE_AUTOPTR_CLEANUP_FUNC(virNetlinkMsg, nlmsg_free);
libvirt>       | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
libvirt> ../src/util/virnetlink.c:791:30: note: called from here
libvirt>   791 |     g_autoptr(virNetlinkMsg) nl_msg = NULL;

On-behalf-of: SAP stefan.kober@sap.com
@hertrste hertrste force-pushed the sanitizer_enablement branch from ae38de3 to 0e063a5 Compare December 1, 2025 09:26
@tpressure tpressure enabled auto-merge (rebase) December 1, 2025 16:06
@tpressure tpressure merged commit 7eb83a0 into main Dec 1, 2025
4 checks passed
@tpressure tpressure deleted the sanitizer_enablement branch December 1, 2025 16:24
@phip1611 phip1611 mentioned this pull request Dec 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants