[PATCH] D60039: Fix the bug of garbage collection of siod.

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: MultiSource/Applications/siod/slib.c:1273
     ++heap;}
+#if defined(linux)
+ // On linux, some register is mangled in jmp buffer, so object that
----------------
LuoYuanke wrote:
> MaskRay wrote:
> > LuoYuanke wrote:
> > > MaskRay wrote:
> > > > `#ifdef __GLIBC__`
> > > > 
> > > > musl doesn't mangle RIP,RSP,RBP.
> > > Thank you for the hint. But I don't find macro__GLIBC__ with "cpp -dM". Is __GLIBC__ an internal macro of compiler? 
> > > I assume on linux getcontext() is supported, so whenever getcontext() is supported we can use it.
> > > So how about just fix the comments to tell developer on glibc (instead of on linux) register is mangled in jmp buffer?
> > > 
> > glibc `stdio.h` includes `features.h` which defines `__GLIBC__`.
> I think we prefer getcontext() to setjmp() whenever getcontext() is available, because getcontext() get more register values and it is more conservetive for garbage collection. Agree?
Using getcontext on Linux will make it uncompilable with musl.

musl doesn't implement makecontext/getcontext/swapcontext/setcontext. (The rationale was that the ucontext API saves/restores the signal mask as part of context switching, which inherently requires a syscall (SYS_rt_sigprocmask), making it unclear whether it outperforms threads. POSIX obsoleted and then removed it because the function prototype makes use of an obsolescent feature (function declarators with empty parentheses) of ISO C).

> each test-suite application is a representative of a class of applications with similar code regions, and that class of applications has known size

Since you found the bug. Do you know if it is severe enough to make it less representative on Linux. If yes, it has probably been unnoticed for a decade. Maybe we should find something to replace siod.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60039/new/

https://reviews.llvm.org/D60039