[PATCH] D70034: Fix include guard and properly order __deregister_frame_info.

Sterling Augustine via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Nov 11 12:21:42 PST 2019


saugustine added a comment.

In D70034#1739675 <https://reviews.llvm.org/D70034#1739675>, @phosek wrote:

> Thanks for these fixes! Personally, I'd be fine making a hard transition and making `EH_USE_FRAME_REGISTRY` independent from `CRT_HAS_INITFINI_ARRAY`, at least in our case it shouldn't cause any issue and I'd be surprised if anyone relied on the current behavior which is clearly a bug and not something I did intentionally.
>
> Regarding the test, I think it'd be still useful to include it, but the dependency on libunwind might be a problem for actually running it, we might need to include a special check to only run that test when libunwind is being available.


I figured out a more clever way on the test. It now just checks that register/deregister_frame_info is called at the proper time. That avoids the problem altogether.

EH_USE_FRAME_INFO is now completely separate from CRT_HAS_INITFINI_ARRAY, and on by default, as the original code always registered frames.

Thanks for the quick review. Will commit after running a few of the complicated cases that got me here, but I think this is right.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70034





More information about the llvm-commits mailing list