<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
<meta name="Generator" content="Microsoft Word 15 (filtered medium)">
<style><!--
/* Font Definitions */
@font-face
        {font-family:PMingLiU;
        panose-1:2 2 5 0 0 0 0 0 0 0;}
@font-face
        {font-family:"Cambria Math";
        panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
        {font-family:Calibri;
        panose-1:2 15 5 2 2 2 4 3 2 4;}
@font-face
        {font-family:"Segoe UI Emoji";
        panose-1:2 11 5 2 4 2 4 2 2 3;}
@font-face
        {font-family:"\@PMingLiU";
        panose-1:2 1 6 1 0 1 1 1 1 1;}
@font-face
        {font-family:Consolas;
        panose-1:2 11 6 9 2 2 4 3 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
        {margin:0in;
        margin-bottom:.0001pt;
        font-size:11.0pt;
        font-family:"Calibri",sans-serif;}
a:link, span.MsoHyperlink
        {mso-style-priority:99;
        color:blue;
        text-decoration:underline;}
span.EmailStyle19
        {mso-style-type:personal-reply;
        font-family:"Calibri",sans-serif;
        color:windowtext;}
.MsoChpDefault
        {mso-style-type:export-only;
        font-size:10.0pt;}
@page WordSection1
        {size:8.5in 11.0in;
        margin:1.0in 1.0in 1.0in 1.0in;}
div.WordSection1
        {page:WordSection1;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]-->
</head>
<body lang="EN-US" link="blue" vlink="purple">
<div class="WordSection1">
<p class="MsoNormal">Hi,<o:p></o:p></p>
<p class="MsoNormal">Per Reid’s feedback, I have separated two SEH missing features. 
<o:p></o:p></p>
<p class="MsoNormal">This thread now is only focusing on _<i>local</i>_unwind(), Jumping out of _finally.<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">The design is documented in Wiki here: <a href="https://github.com/tentzen/llvm-project/wiki/Windows-SEH:-Local_Unwind-(aka:-Jumping-Out-of-_Finally)">
https://github.com/tentzen/llvm-project/wiki/Windows-SEH:-Local_Unwind-(aka:-Jumping-Out-of-_Finally)</a>
<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">The implementation can be seen here: <a href="https://github.com/tentzen/llvm-project/compare/SEH-LU-base...SEH-LU?expand=1">
https://github.com/tentzen/llvm-project/compare/SEH-LU-base...SEH-LU?expand=1</a>
<o:p></o:p></p>
<p class="MsoNormal">There are only 13 files changed that is much less complicated than previously when it’s combined with -EHa. 
<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">Major code is surrounding at SehTryStmt and _Finally in CGexception.cpp that is the place to house Windows SEH specific code anyways.  <o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">The implementation chose to lever Parser/Semantic phase (SemaStmt.cpp, JumpDiagnostics.cpp, scope.h, etc) to identify the right _Try scope for LU dispatching because that is the place “<span style="font-size:9.0pt;font-family:Consolas;color:#24292E;background:white">warn_jump_out_of_seh_finally”
</span> is diagnosed and reported.  I feel this is the most robust approach.  <o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">Local_unwind is an important feature, broadly used in Windows Kernel for all architectures.<o:p></o:p></p>
<p class="MsoNormal">Without LU, Windows SEH is incomplete! <o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">Thanks,<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">--Ten <o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<div style="border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0in 0in 0in">
<p class="MsoNormal"><b>From:</b> Ten Tzen <tentzen@microsoft.com> <br>
<b>Sent:</b> Friday, April 3, 2020 9:43 PM<br>
<b>To:</b> rnk@google.com<br>
<b>Cc:</b> llvm-dev@lists.llvm.org; Aaron Smith <aaron.smith@microsoft.com><br>
<b>Subject:</b> RE: [EXTERNAL] Re: [llvm-dev] [RFC] [Windows SEH] Local_Unwind (Jumping out of a _finally) and -EHa (Hardware Exception Handling)<o:p></o:p></p>
</div>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">Hi, Reid,<o:p></o:p></p>
<p class="MsoNormal">Nice to finally meet you<span style="font-family:"Segoe UI Emoji",sans-serif">😊</span>.  <o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">Thank you for reading through the doc and providing insightful feedbacks.<o:p></o:p></p>
<p class="MsoNormal">Yes I definitely can separate these two features if it’s more convenient for everyone. 
<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">For now, the local_unwind specific changes can be separated and reviewed between these two commits:<o:p></o:p></p>
<p class="MsoNormal">      git diff 9b48ea90f4c9ae7ef030719d6c0b49b00861cdde 06c81a4b6262445432a4166627b87bf595f5291b  
<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">the -EHa changes can be read :<o:p></o:p></p>
<p class="MsoNormal">     git diff e943329ba00772f96fbc1fe5dec836cfd0707a38   9b48ea90f4c9ae7ef030719d6c0b49b00861cdde<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">My reply inline below in [Ten] lines.<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">--Ten<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<div style="border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0in 0in 0in">
<p class="MsoNormal"><b>From:</b> Reid Kleckner <<a href="mailto:rnk@google.com">rnk@google.com</a>>
<br>
<b>Sent:</b> Friday, April 3, 2020 3:36 PM<br>
<b>To:</b> Ten Tzen <<a href="mailto:tentzen@microsoft.com">tentzen@microsoft.com</a>><br>
<b>Cc:</b> <a href="mailto:llvm-dev@lists.llvm.org">llvm-dev@lists.llvm.org</a>; Aaron Smith <<a href="mailto:aaron.smith@microsoft.com">aaron.smith@microsoft.com</a>><br>
<b>Subject:</b> [EXTERNAL] Re: [llvm-dev] [RFC] [Windows SEH] Local_Unwind (Jumping out of a _finally) and -EHa (Hardware Exception Handling)<o:p></o:p></p>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<div>
<p class="MsoNormal">UHi Ten,<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><br>
Thanks for the writeup and implementation, nice to meet you.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">I wonder if it would be best to try to discuss the features separately. My view is that catching hardware exceptions (/EHa) is critical functionality, but it's not clear to me if local unwind is truly worth implementing. Having looked at
 the code briefly, it seemed like a large portion of the complexity comes from local unwind. Today, clang crashes on this small example that jumps out of a __finally block, but the intention was to reject the code and avoid implementing the functionality. Clang
 does, in fact, emit a warning:<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">$ clang -c t.cpp<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">t.cpp:7:7: warning: jump out of __finally block has undefined behavior [-Wjump-seh-finally]<br>
      goto lu1;<br>
      ^<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">Local unwind, in my view, is the user saying, "I wrote __finally, but actually I decided I wanted to catch the exception, so let's transfer to normal control flow now." It seems to me that the user already has a way to express this: __except.
 I know the mapping isn't trivial and it's not exactly the same, but it seems feasible to rewrite most uses of local unwind this way.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">[Ten] Right, I agree that to some degree a local_unwind can be viewed as another type of _except handler in the middle of unwinding. And true that  some usage patterns can be worked around by rewriting SEH hierarchy. But I believe the work
 can be substantial and risky, especially in an OS Kernel.  Furthermore, to broaden the interpretation, local_unwind can also serve as  a _filter (or even rethrow-like handler in C++ EH), and the target block is the final handler.  See the multi-local-unwind
  example in the doc. <o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">Can you estimate the prevalence of local unwind? What percent of __finally blocks in your experience use non-local control flow? I see a lot of value in supporting catching hardware exceptions, but if we can avoid carrying over the complexity
 of this local unwind feature, it seems to me that future generations of compiler engineers will thank us.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">[Ten] I don’t have this data in hand. But what I know is that local_unwind is an essential feature to build Windows Kernel.  One most important SEH test (the infamous xcpt4u.c) is composed of 88 tests; among them there are 25 jumping-out-of-finally
 occurrences.  Of course this does not translate to a percentage of local_unwind, but it does show us the significance of this feature to Windows. FYI Passing xcpt4u.c is the very first fundamental requirement before building Windows Kernel.<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">---<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">Regarding trap / non-call / hardware exception handling, I guess I am a bit more blase about precisely modeling the control flow. As Eli mentions, we already have setjmp, and we already don't model it. Users file bugs about problems with
 setjmp, and we essentially close them as "wontfix" and tell them to put more "volatile" on the problem until it stops hurting.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">One thing that I am very concerned about is the implications for basic block layout. Right now, machine basic block layout is very free-handed. Today, CodeGen puts labels around every potentially-throwing call, does block layout without
 considering try regions, and then collapses adjacent label regions with the same landingpad during AsmPrinting. For MSVC C++ EH, state number stores and the ip2state table achieve the same goal.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">[Ten] Yes, I saw that (pretty nice implementation actually).  This design and implementation completely inherits current mechanism except that now it’s allowed to report EH state ranges that only contain memory/computation instructions
  (for obvious reason). I’m not sure which part of that concerns you.   <o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">I think we need rules about how LLVM is allowed to transform the following code:<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">void foo(volatile int *pv) {<br>
  __try {<br>
    if (cond()) {<br>
      ++*pv;<br>
      __builtin_unreachable();<br>
    }<br>
  } __except(1) { }<br>
  __try {<br>
    if (cond()) {<br>
      ++*pv;<br>
      __builtin_unreachable();<br>
    }<br>
  } __except(1) { }<br>
}<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">In this case, the *pv operation may throw, but I believe it would be semantics preserving to merge the two identical if-then blocks. The call.setup proposal I sent not long ago runs into the same issue. I have written a patch to tail merge
 such similar blocks, but I have not landed it:<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><a href="https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Freviews.llvm.org%2FD29428&data=02%7C01%7Ctentzen%40microsoft.com%7Cac3ebdd6804a46bedefd08d7d852ac14%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637215721902320037&sdata=gnc5zhiNpq1Cv2Of0VSl7nwcS8F6uPBprFT4ffQgDx0%3D&reserved=0" target="_blank">https://reviews.llvm.org/D29428</a> <o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">Even though it's not yet landed, I think we need to know if the transform is valid. If it is, then we need to do more than volatilize the try region to make EHa work.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">[Ten] The merging should not happen.  Per C-standard, a volatile must be read (or write) ONCE and only once (as long as it’s naturally aligned and can be accessed in one operation by HW).  So merging two volatiles violates the standard. 
 I’m sure it’s currently well-protected in LLVM today.   <o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">For a long time I've wanted regions of some kind in LLVM IR, and this use case has made me want to pick it up again. However, assuming that you want to land support for hardware exceptions without some kind of generalized region support
 in the IR, I think we do need to do something about these blocks ending in unreachable in __try regions. The simplest thing that could possibly work is to make clang end the try region before unreachable. This would mean ending the block and adding `invoke
 void @seh_try_end` after every unreachable. It would be redundant for noreturn calls, since those will already have an unwind edge, ensuring they remain in the try region.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">[Ten] it’s interesting you mentioned this “blocks ending in unreachable in __try regions" here.  With these two features supported, two remaining bugs in my ToDo list are; one setjmp() and one nested EH throw.  The second one seems caused
 by a _try_block ended with an unreachable.   Yes, this is on my list.  Will discuss with you guys further when I look into it.     <o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">---<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">Another interesting aspect of /EHa is how it affects C++ destructor cleanups. I am personally comfortable with the requirement that LLVM avoid moving around volatile instructions in __try blocks. LLVM is already required to leave volatile
 operations in order. But I *am* concerned about C++ destructor scopes, which are much more frequent than __try. As you have described it, clang would invoke eha_scope_begin() / eha_scope_end() around the object lifetime, but are you proposing to volatilize
 all memory operations in the region? If not, I see nothing that would prevent LLVM from moving potentially faulting operations in or out of this scope. We cannot require passes to look for non-local EH regions before doing code motion. Would that be acceptable
 behavior? It could lead to some strange behavior, where a load is sunk to the point of use outside the cleanup region, but maybe users don't care about this in practice.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">[Ten] No, memory operations in C++ need not be volatilized.  The order of exception in C++ code does not matter for -EHa.  Potential trap instructions are free to move in/out of any EH region.  The only criteria is that when a HW exception
 is caught and handled, local live objects must be dtored gracefully, the same manner as C++ Synchronous exception.  By reporting the EH state of those trap instructions, this is automatically done in LLVM today.
<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">---<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">To summarize, my feedback would be:<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">1. Focus on __try and hardware exceptions first, the value proposition is clear and large. In particular, something has to be done about unreachable. Clang should already thread other abnormal control flow through the region exit.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">2. Please gather some data on prevalence of local unwind to motivate the feature<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">3. Please elaborate on the design for /EHa C++ destructor cleanups and code motion<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">I hope that helps, and I'm sorry if I'm slow to respond, this is a tricky problem, and it's not my first priority.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">Reid<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<div>
<p class="MsoNormal">On Wed, Apr 1, 2020 at 8:22 AM Ten Tzen via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> wrote:<o:p></o:p></p>
</div>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-top:5.0pt;margin-right:0in;margin-bottom:5.0pt">
<div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">Hi, all,<o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"> <o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">The intend of this thread is to complete the support for Windows SEH. 
<o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">Currently there are two major missing features:  Jumping out of a _finally and Hardware exception handling.    <o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"> <o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">The document below is my proposed design and implementation to fully support SEH on LLVM. 
<o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">I have completely implemented this design on a branch in repo:  <a href="https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Ftentzen%2Fllvm-project&data=02%7C01%7Ctentzen%40microsoft.com%7Cac3ebdd6804a46bedefd08d7d852ac14%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637215721902330030&sdata=%2BC8CO9VQMv6DZk0HabsMOswQ8YFvqjdZ%2B9dUhKtjsMo%3D&reserved=0" target="_blank">https://github.com/tentzen/llvm-project</a>.
<o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">It now passes MSVC’s in-house SEH suite.
<o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"> <o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">Sorry for this long write-up.  For better readability, please read it on
<a href="https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Ftentzen%2Fllvm-project%2Fwiki&data=02%7C01%7Ctentzen%40microsoft.com%7Cac3ebdd6804a46bedefd08d7d852ac14%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637215721902330030&sdata=XukIQtEqSpgL13dk57%2FV2gHUw7YOwseyPy7212U7uDM%3D&reserved=0" target="_blank">
https://github.com/tentzen/llvm-project/wiki</a><o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"> <o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">Special thanks to Joseph Tremoulet for his earlier comments and suggestions.<o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"> <o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">Note: I just subscribed llvm-dev, probably not in the list yet.  So please reply with my email address (<a href="mailto:tentzen@microsoft.com" target="_blank">tentzen@microsoft.com</a>)
 explicitly in To-list.  <o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">Thanks,<o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"> <o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">--Ten<o:p></o:p></p>
</div>
</div>
</blockquote>
</div>
</div>
</div>
</body>
</html>