higan-emu / libco

libco is a cooperative multithreading library written in C89.
Other
125 stars 25 forks source link

Teach libco about valgrind #24

Closed Screwtapello closed 1 year ago

Screwtapello commented 3 years ago

Valgrind is a great way to diagnose memory-safety issues, but it doesn't like libco very much - when libco switches to a new stack, Valgrind assumes that the entire span of memory from the old stack pointer to the new stack pointer is stack memory, and gets very confused.

Apparently Valgrind has a client request API that lets you tell valgrind "this region of memory is a new, unconnected stack" which should make valgrind much more reliable.

Here and here are examples of the API in use.

To integrate this properly with libco, we would need to:

I don't think we need to care about including the header only on supported platforms (the header is pretty good about platform detection as-is) or about including it only in debug builds (apparently the overhead is ~7 instructions, and creating/destroying coroutines shouldn't be on any hot path).

The Valgrind docs for the stack annotations say "Warning: Unfortunately, this client request is unreliable and best avoided" but they don't suggest any real alternative.

namandixit commented 3 years ago

This webpage contains some more details about making it work with various debuggers and sanitizers, in addition to Valgrind.

zuiderkwast commented 1 year ago

Fixed by #39.

Screwtapello commented 1 year ago

To be clear, #39 tells Valgrind about stacks when they are created.

I'm calling this issue done, we can open new issues about specific issues if and when they occur.