Testing Arenas under ASan

[nathany]

While poking around the arena.odin code, I noticed sanitizer.address_poison lines that are commented out. They were from #5344 with the comment:

"...after doing some testing of the newly-poisonable allocators, I found that ASan doesn't actually support sub-8-byte ranges"
"So, I have disabled ASan everywhere until each allocator can be reviewed whether or not it should actually use ASan."

So I thought I'd ask what it would take to get that working? But I also wanted to throw out another idea:

• What if the virtual.Arena, etc. had a different implementation guarded behind the when .Address in ODIN_SANITIZER_FLAGS check?
• Individual allocations (malloc) while tracking them on a stack, free as a no-op, with free_all freeing them, and arena_temp_begin/end still exhibiting the same behaviour.
• Since it's running under ASan anyway, equivalent performance probably isn't a concern, but behaviour should match

My theory is that this would give ASan visibility into the goings on. But I'm new to Odin and have never written an allocator, so I don't know what downsides there may be? I also haven't thought through the effects of layering allocators with this.

[corleypc]

Yes, the malloc pseudoarena can be done and is often the debug version of arena allocators. Sanitization falls out from this for free without manual instrumentation + you get the use-after-reset checks you can't reliably get from the arena since arenas reuse memory by construction. You lose contiguity though, so arena behavior isn't fully matched. This isn't a deal breaker but something to be aware of; some application code may actually depend on contiguity for static/buffer arenas for whatever reasons. Debug allocators almost universally introduce redzone blocks to catch overflow access, thus breaking contiguity, so this is expected.

The existing manual instrumentation in the arena code is about detecting use-after-reset only (no redzones), so it doesn't actually depend on the ASan sub-8 byte rules. Poisoning may underpoison and unpoisoning may overunpoison, so, crucially, this can't introduce false positives. But there's missing instrumentation (e.g. arena_temp_end), so it won't work correctly by simply uncommenting, it needs review and fixes.

Note that it is also possible to introduce working redzones manually in a debug version of the arenas by aligning all allocations to 8 bytes + special handling for allocation sizes not multiple of 8: combine unpoisoning of the full allocation (which will overunpoison) with a followup poisoning of the appropriately sized suffix of the last unpoisoned 8-byte region (which is exactly the overunpoisoned part).

[nathany]

Thanks for the explanation. I guess it depends if redzones are wanted in the later case. Since we have language-level bounds checking, use after reset may be the more common bug.

The debug version (malloc pseudoarena) seems more straightforward to me than trying to get all the alignment and poisoning right, and I think the alignment/redzone/poison version also impacts contiguity and potentially performance (if it's always on).

Of course there are other allocators too. Even if the debug malloc pseudoarena idea worked for all cases, maintaining debug variants of every allocator may be a pain. The sanitizer.address_poison approach may be a more general solution.

But one complication we haven't talked about is "layered poison state management" mentioned in #5344.

The nice thing is that arenas can exist in user code. I might try writing a malloc pseudoarena proof of concept at some point. But I'm still just learning Odin, so it won't be right away. If someone solves this first, I'll just adopt that. 👍🏼