Stream: compiler development

Topic: zig compiler valgrind


view this post on Zulip Anton (Jun 23 2025 at 12:13):

Is now a good time to add some valgrind tests for the new compiler? It's always best to add these things early. I know zig allocators can prevent issues as well, but maybe we should still test with valgrind? Please share your wisdom, experienced zig programmers :)

view this post on Zulip Brendan Hansknecht (Jun 23 2025 at 14:51):

Yeah, zigs gpa in debug mode should cache a ton of things, but it never hurts to test with valgrind as well

view this post on Zulip Anton (Jun 28 2025 at 12:46):

code run to generate snapshots is now checked by valgrind https://github.com/roc-lang/roc/pull/7893

view this post on Zulip Luke Boswell (Jul 09 2025 at 04:32):

@Anton adding this to CI has been very helpful. It picked up a few bugs in the module caching implementation that I couldn't on my mac. Thanks for setting this up :smiley:

view this post on Zulip Joshua Warner (Jul 10 2025 at 21:08):

I'm really struggling to make heads or tails of the current valgrind failures on this PR: https://github.com/roc-lang/roc/pull/7987

view this post on Zulip Joshua Warner (Jul 10 2025 at 21:09):

These look a hell of a lot like false-positives that need to be suppressed, but I'm rather confused as to why they would be appearing on this PR and not before.

view this post on Zulip Joshua Warner (Jul 10 2025 at 21:18):

These seem to all be cases where something deep in the allocator is reading theoretically uninitialized memory

view this post on Zulip Joshua Warner (Jul 10 2025 at 21:29):

It looks for all the world like this is a misconfiguration in zig / valgrind, perhaps one that's only triggered if we ever have a workload that ends up re-using memory that was freed previously.

view this post on Zulip Joshua Warner (Jul 10 2025 at 21:48):

Hmm, apparently this is an issue in main, and it bisects to:

82346e49b1e946b201ae3d7b2ea5a252f5d4429d is the first bad commit
commit 82346e49b1e946b201ae3d7b2ea5a252f5d4429d
Author: Joshua Warner <joshuawarner32@gmail.com>
Date:   Wed Jul 9 09:57:25 2025 -0700

    Switch to using the c_allocator by default for snapshots

    This leads to a 4-6x speed increase in snapshot generation

 .github/workflows/ci_zig.yml |  4 ++--
 src/snapshot.zig             | 26 +++++++++++++++++++-------
 2 files changed, 21 insertions(+), 9 deletions(-)

view this post on Zulip Joshua Warner (Jul 10 2025 at 21:48):

I feel pretty firmly that that's NOT an indication of an actual bug, but rather some sort of misconfiguration in valgrind or zig

view this post on Zulip Joshua Warner (Jul 10 2025 at 22:00):

Calling all valgrind experts?

view this post on Zulip Joshua Warner (Jul 10 2025 at 22:03):

I suppose as a temporary workaround I can switch to using couple arena allocators or something

view this post on Zulip Brendan Hansknecht (Jul 11 2025 at 02:48):

Yeah, I was gonna guess either allocator change or some weird multithreaded complaint.

view this post on Zulip Brendan Hansknecht (Jul 11 2025 at 02:49):

Out of curiousity does it have an issue if we run valgrind in the single threaded mode (probably, but worth double checking)

view this post on Zulip Brendan Hansknecht (Jul 11 2025 at 02:49):

If not, I think we run valgrind on debug builds, so could always use gpa in debug builds.

view this post on Zulip Brendan Hansknecht (Jul 11 2025 at 02:49):

Otherwise, I'm sure @Anton or I can take a more serious look.

view this post on Zulip Anton (Jul 11 2025 at 10:29):

so could always use gpa in debug builds.

I think we currently do this @Joshua Warner?

view this post on Zulip Anton (Jul 11 2025 at 10:35):

Joshua Warner said:

I suppose as a temporary workaround I can switch to using couple arena allocators or something

I see we went with arena allocators, that may be even faster, why should we only use it temporarily?

view this post on Zulip Joshua Warner (Jul 11 2025 at 14:53):

So technically what's in main right now has us using the c allocator for arg parsing and such regardless - then we switch to the GPA for actual snapshot processing.

view this post on Zulip Joshua Warner (Jul 11 2025 at 14:53):

Based on local testing, arena allocators are something like 20% slower than the C allocator :/

view this post on Zulip Anton (Jul 11 2025 at 15:10):

Ok, I'll check out the valgrind errors

view this post on Zulip Brendan Hansknecht (Jul 11 2025 at 15:30):

We probably should make the main compiler use gpa in debug

view this post on Zulip Brendan Hansknecht (Jul 11 2025 at 15:30):

Turn on the flags to help catch leaks and bugs and stuff

view this post on Zulip Anton (Jul 11 2025 at 15:41):

Brendan Hansknecht said:

We probably should make the main compiler use gpa in debug

So, using e.g. comptime if (builtin.mode == .Debug) {to set the allocator?

view this post on Zulip Brendan Hansknecht (Jul 11 2025 at 16:01):

Yeah


Last updated: Jul 26 2025 at 12:14 UTC