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

[LuoYuanke via Phabricator via llvm-commits]

LuoYuanke marked an inline comment as done.
LuoYuanke 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
----------------
MaskRay wrote:
> 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.
Sorry. I don't know getcontext() is unsupported in other libc libraries on linux. I will apply your suggestion to use __GLIBC__.


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

https://reviews.llvm.org/D60039