[libunwind] Implement the Get/SetTopOfFunctionStack functions via a __thread TLS variable

[mstorsjo]

Summary

When targeting apple platforms, these functions are implemented in
Unwind_AppleExtras.cpp, but there's previously no implementation for
other platforms.

[compnerd]

I have a complete implementation for this which is known to work on Windows at least. The only thing that __thread requires is a working linker (which does not include binutils -- I do have a local patch to make that work though). Are these two the only ones that are missing? At the very least, the implementation of SetTopOfFunctionStack is incorrect.

[mstorsjo]

In D38250#880871, @compnerd wrote:

I have a complete implementation for this which is known to work on Windows at least.

Patches for libunwind, or standalone? In any case, please do share if you think it's relevant.

The only thing that __thread requires is a working linker (which does not include binutils -- I do have a local patch to make that work though).

Ok, thanks for clarifying. (If I read wikipedia right, it requires Vista if built into a DLL that isn't linked to by an executable but loaded via LoadLibrary though.)

Are these two the only ones that are missing?

As far as I've seen in my tests, these are the only ones missing yes. Most of the rest of Unwind_AppleExtras.cpp contains things not relating to SJLJ.

At the very least, the implementation of SetTopOfFunctionStack is incorrect.

In which way? My implementation does pretty much exactly the same as the apple version of SetTopOfFunctionStack, but using a __thread variable instead of _pthread_setspecific_direct(__PTK_LIBC_DYLD_Unwind_SjLj_Key, fc);.

[compnerd]

Ugh, I was mixing up _Unwind_SjLj_Register with this function.

src/Unwind-sjlj.c

468

I would prefer:

#if !defined(__APPLE__)

469

Please make this static.

481

Can't both of these also be static?

[mstorsjo]

src/Unwind-sjlj.c

468

Sure, I can change that.

469

Oh, indeed, yes, I'll change that.

481

No, since they're declared earlier as non-static.

[mstorsjo]

Made the tls variable static, changed ifndef into !defined().

[compnerd]

src/Unwind-sjlj.c

481

Yeah, that is a bug :-). __Unwind_SjLj_GetTopOfFunctionStack and __Unwind_SjLj_SetToOfFunctionStack are implementation details of LLVM's libunwind. They are not part of the public interfaces, and are not used outside of this TU, and should be marked as static as such. I think that changing the prototype declaration to indicate this is reasonable. I suppose that I can make that change separately.

[compnerd]

Actually, Ill go ahead and make the more invasive change and that will provide this properly.

[mstorsjo]

src/Unwind-sjlj.c

481

Yes, except that in the apple case, those functions are provided by another TU (Unwind_AppleExtras.cpp). So in that case, those functions only should be static in the non-apple case.

[compnerd]

I believe that SVN r314632 should address the issue more thorougly.

[mstorsjo]

Works fine with the version from r314632 (with a minor fixup in r314635).

src/Unwind-sjlj.c

if (context != NULL) {
     _Unwind_FunctionContext_t ufc = (_Unwind_FunctionContext_t) context;
     // Setjmp/longjmp based exceptions don't have a true CFA.
     // Instead, the SP in the jmpbuf is the closest approximation.
     return (uintptr_t) ufc->jbuf[2];
   }
   return 0;
 }
 
+#if !defined(__APPLE__)
+static __thread struct _Unwind_FunctionContext *stack = NULL;
+
+_LIBUNWIND_HIDDEN
+struct _Unwind_FunctionContext *__Unwind_SjLj_GetTopOfFunctionStack() {
+  return stack;
+}
+
+_LIBUNWIND_HIDDEN
+void __Unwind_SjLj_SetTopOfFunctionStack(struct _Unwind_FunctionContext *fc) {
+  stack = fc;
+}
+#endif // !defined(__APPLE__)
+
 #endif // defined(_LIBUNWIND_BUILD_SJLJ_APIS)