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

[xiongji90 via Phabricator via libcxx-commits]

xiongji90 marked an inline comment as done.
xiongji90 added a comment.

In D105968#2963188 <https://reviews.llvm.org/D105968#2963188>, @MaskRay wrote:

> In D105968#2962009 <https://reviews.llvm.org/D105968#2962009>, @xiongji90 wrote:
>
>> In D105968#2961508 <https://reviews.llvm.org/D105968#2961508>, @MaskRay wrote:
>>
>>> There are still many unresolved comments. Please mark resolved ones as done and then click "Submit".
>>
>> Hi, @MaskRay 
>> Left unresolved comments are resolved and I also rebased the patch to latest llvm libunwind code.
>>
>> Thanks very much.
>
> You marked your own comments as "Done" but not reviewers' comments as "Done".
>
> So I currently see 35 among 79 comments are done, while the rest are not.
>
> Please mark resolved ones as done.
>
> If you need a reviewer to commit this patch on your behalf, please provide `git commit --amend --author="name <email>"`.

Hi, @MaskRay

Sorry for the mistake, I just marked all reviewers' comments as "Done". I just got the commit access and will commit the code soon.

Thanks very much for your kind help.



================
Comment at: libunwind/src/cet_unwind.h:1
+//===--------------------------- cet_unwind.h -----------------------------===//
+//
----------------
MaskRay wrote:
> This file probably should just be moved into config.h, then other files don't need to add a new `#include`.
> 
> 
Hi, @MaskRay 
I suggest to keep cet_unwind.h currently since CET is still a narrow used hardware feature right now and all macro definitions in config.h seems to have general usage. In the long term, if CET is enabled by default in x86/x86_64 platforms, we can move the contents in cet_unwind.h into config.h.

Thanks very much.


================
Comment at: libunwind/src/cet_unwind.h:19
+#if defined(_LIBUNWIND_TARGET_LINUX) && defined(__CET__) && defined(__SHSTK__)
+#define _LIBUNWIND_CET_ENABLED 1
+#endif
----------------
MaskRay wrote:
> Perhaps rename to `_LIBUNWIND_USE_CET`
Done.
Thanks very much.


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

https://reviews.llvm.org/D105968