[PATCH] D79760: [WinEH64] Fix a crush issue when c++ exception nested in a particular form.

Pengfei Wang via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed May 13 19:07:24 PDT 2020


pengfei added a comment.

In D79760#2034640 <https://reviews.llvm.org/D79760#2034640>, @andrew.w.kaylor wrote:

> In D79760#2033081 <https://reviews.llvm.org/D79760#2033081>, @pengfei wrote:
>
> > @andrew.w.kaylor I tested for below 3 cases, and get the runtime result for them in different compilers.
>
>
> What I was trying to say is that you should add a new LIT test case that corresponds to the failure in PR42266. But also, I think we need stronger checking of the correlation between the try map and the handler map. Currently the test is looking for a specific order of the states in the try map, but the handler map is using a wild card check -- "CHECK-NEXT:   .long   "?catch**${{[0-9]+}}**@?0?try_in_catch at 4HA"" We should be verifying somehow that the states in the try map get connected to the correct handler, which involves matching the specific handler. Your change reversed the order of the try map, but appeared not to change the handler map. That's a problem. If the handler map didn't also change, the code would be wrong.
>
> Now that I've seen the patch from @tentzen I am not convinced that there is a required difference between 64-bit and 32-bit. I could be wrong, but it looks like the 64-bit case is different because newer runtimes require it. It isn't clear to me whether the 32-bit runtime will work with the new format.
>
> If D79474 <https://reviews.llvm.org/D79474> works for the cases you're trying to fix (including the 32-bit case), I think it's a better solution.


I tested @tentzen 's patch and it failed on 32-bit in my case. I wonder if the 32-bit still using the old runtime.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79760





More information about the llvm-commits mailing list