[libcxx-commits] [PATCH] D105968: [libunwind][CET] Support exception handling stack unwind in CET environment

[xiongji90 via Phabricator via libcxx-commits]

xiongji90 added inline comments.


================
Comment at: libunwind/CMakeLists.txt:55
 option(LIBUNWIND_BUILD_32_BITS "Build 32 bit libunwind" ${LLVM_BUILD_32_BITS})
+option(LIBUNWIND_BUILD_CET_ENABLE "Build libunwind with CET enabled" ${LLVM_BUILD_CET_ENABLE})
 option(LIBUNWIND_ENABLE_ASSERTIONS "Enable assertions independent of build mode." ON)
----------------
compnerd wrote:
> Please name this `LIBUNWIND_ENABLE_CET`
Done.

Thanks very much!


================
Comment at: libunwind/CMakeLists.txt:146
+  set(LIBUNWIND_CET_ON False)
+endif()
+
----------------
compnerd wrote:
> I don't think that this is needed.  You should be able to directly use `LIBUNWIND_BUILD_CET_ENABLE` at the sites using this local variable.
Removed the unnecessary local variable and use LIBUNWIND_ENABLE_CET directly.

Thanks very much!


================
Comment at: libunwind/src/UnwindLevel1.c:48
+#define __unw_phase2_resume(cursor, fn) __unw_resume((cursor))
+#elif _LIBUNWIND_CET_ENABLED == 1
+#define __unw_phase2_resume(cursor, fn)                                        \
----------------
compnerd wrote:
> Can we avoid the value here and use `#if defined(__x86_64__) || defined(_M_ARM64)` instead to select between the inline assembly?
Hi, @compnerd 
I have abandoned  usage of _LIBUNWIND_CET_ENABLED value and used "_LIBUNWIND_TARGET_I386" and "_LIBUNWIND_TARGET_X86_64" to select inline assembly. Currently CET is implemented in GNU Linux + Intel platform, so we don't need to check for ARM64.

Thanks very much.


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

https://reviews.llvm.org/D105968