[llvm-dev] [RFC] [Windows SEH][-EHa] Support Hardware Exception Handling

Ten Tzen via llvm-dev llvm-dev at lists.llvm.org
Thu Apr 30 23:34:33 PDT 2020


Hi,
Is there other concerns or suggestions about this proposal?

I have added some details about Reid’s earlier feedback regarding control-flow “region”.  The new doc for -EHa is also span off here:  https://github.com/tentzen/llvm-project/wiki/Windows-SEH:-HARDWARE-EXCEPTION-HANDLING-(EHa)

Since last time I also added several changes in my prototype to more robustly compute State at block-level in SEME (Single-Entry-Multiple-Exits) region. I also fixed the bug brought up by Eli regarding memcpy intrinsic.  The patch can be read here: https://github.com/tentzen/llvm-project/compare/SEH-EHa-base...SEH-EHa?expand=1.

The implementation is actually very simple, isolated and very easy to read.  It adds very little complexity into existing code.
Major changes are in four files:

  *   [Clang] CGException.cpp and CGCleanup.cpp:  Add scope_begin&scope_end intrinsic at entry and exits of seh-_try/cpp-cleanup SEME region.
  *   [LLVM] WinEHPrepare.coo & SelectionDAGISel.cpp: Compute and report EH State at block-level.
Other charges are trivial and straight-forward helper code.

I believe I have answered and addressed all concerns or questions from you guys.  Please let me know if I miss anything.
Thanks,

--Ten

From: Ten Tzen <tentzen at microsoft.com>
Sent: Thursday, April 16, 2020 1:52 PM
To: Eli Friedman <efriedma at quicinc.com>; llvm-dev at lists.llvm.org
Cc: rnk at google.com; Aaron Smith <aaron.smith at microsoft.com>; Joseph Tremoulet <jotrem at microsoft.com>
Subject: RE: [RFC] [Windows SEH][-EHa] Support Hardware Exception Handling


As stated in the design paragraph, this design does not intend to model precise CFG at instruction level since it’s complicated and unnecessary.
As long as we comply C and C++ rules listed below, we achieve -EHa semantic.  There is NO need to precisely model HW exception control flow at instruction-level.

Your example about memcpy() is just a bug in current implementation.  I will fix it so that it’s volatilized in some manner.  We are not in Code Review stage yet. let’s focus on design.

There is one seh_try_begin() invoke at the beginning of _try and one seh_try_end() invoke at the end of _try.  Their EH edge will point to _except handler.  So
your example below will not happen.  Compiler should not generate code like that.

Thanks,

--Ten

From: Eli Friedman <efriedma at quicinc.com<mailto:efriedma at quicinc.com>>
Sent: Thursday, April 16, 2020 2:10 AM
To: Ten Tzen <tentzen at microsoft.com<mailto:tentzen at microsoft.com>>; llvm-dev at lists.llvm.org<mailto:llvm-dev at lists.llvm.org>
Cc: rnk at google.com<mailto:rnk at google.com>; Aaron Smith <aaron.smith at microsoft.com<mailto:aaron.smith at microsoft.com>>; Joseph Tremoulet <jotrem at microsoft.com<mailto:jotrem at microsoft.com>>
Subject: [EXTERNAL] RE: [RFC] [Windows SEH][-EHa] Support Hardware Exception Handling



From: Ten Tzen <tentzen at microsoft.com<mailto:tentzen at microsoft.com>>
Sent: Wednesday, April 15, 2020 9:15 PM
To: Eli Friedman <efriedma at quicinc.com<mailto:efriedma at quicinc.com>>; llvm-dev at lists.llvm.org<mailto:llvm-dev at lists.llvm.org>
Cc: rnk at google.com<mailto:rnk at google.com>; aaron.smith at microsoft.com<mailto:aaron.smith at microsoft.com>; Joseph Tremoulet <jotrem at microsoft.com<mailto:jotrem at microsoft.com>>
Subject: [EXT] RE: [RFC] [Windows SEH][-EHa] Support Hardware Exception Handling

Hi, Eli,

Why are you under the impression that threw_exception() will not be called if optimizations are enabled?  I don’t know if the -EHa Spec is clearly described in MSFT Webs.  At least this proposal has described the rules for both C & C++ code.

The very first rule clearly said that “no exception can move in or out of _try region., i.e., no potential faulty instruction can be moved across _try boundary”.  As such the dereference of statement return *(C*)0  must be kept in _try scope and the access-violation fault will be caught in _except handler where threw_exception() will be called.

There is no exception with your current implementation.

LLVM IR load and store instructions are marked volatile.  But *(C*)0 doesn’t generate an LLVM IR load or store instruction; it gets lowered to a llvm.memcpy.  The LLVM optimizer will erase that, because dereferencing a null pointer is undefined in C.  Therefore, threw_exception will never be called.  We didn’t move a fault-generating  instruction across the boundary; the binary never contained a fault-generating instruction in the first place.

From your response, clearly you think one of those steps shouldn’t be happening, but I’m not sure which step, or why.  I’m assuming “potential faulty instruction” is referring to an assembly instruction, but maybe that’s not right?

I don’t see why Register allocation plays a part in this topic. I do see a serious problem in LLVM SJLJ today (All tests in MSVC’s Setjmp suite fail with -O2 that I will look into it soon).  But I failed to see why HW exception is corelated to setjmp/longjmp.  These are two totally different features and the approaches employed are also totally different.
It would be helpful if you can give one example why this proposal need to care about how registers are allocated.

The point doesn’t have anything to with the specific underlying technique used to handle control flow; the point is that you’re violating the normal LLVM CFG rules in essentially the same way.  The code inside the __try will execute, and then control flow will jump back before the try block, without that jump getting included in the CFG.  Therefore, you’ll run into the same classes of issues with optimizations as we do with setjmp.

For example, suppose you have something roughly like the following in your code:

__declspec(dllimport) extern int a[10];
void f() {
__try {
   a[0] = 10;
   a[1] = 20;
   *(volatile int*)0 = 0;
} __except (1) {
  a[0] = 30;
}
}

Now, when the code is generated here, we need to compute the address of “a”, and store it somewhere, so we can then do a store.  (Well, we’ll assume it does, for the purpose of describing this example.)   So the compiler could generate something like the following sequence of operations:

Before the try block:
a_address = &a;

In the try block:
a_val = 10;
*a_address = a_val;
a_address += 4;
a_val = 20;
*a_address = a_val
trigger exception

In the except block:
a_val = 30;
*a_address = a_val;

(This is sort of pseudo-assembly using C syntax, so I don’t dive too deeply into target details; hopefully the intent is clear.)

Note that the compiler is assuming that either the try block or the except block will execute, not both, since that’s what the CFG indicates.  (At least, that’s what I understand from your proposal.)

Now suppose a_address ends up in a spill slot.  So the compiler has a space on the stack dedicated to storing a_address.  When we actually trigger an exception, the compiler runs the except block, and loads the value which is supposed to be &a[0]… but at that point, it actually contains &a[1], so it writes the value “30” to the wrong address.  That’s a miscompile, similar to the one described in the link I gave you.

This is skipping over a lot of details about how the compiler makes various decisions about where to compute and store data (you’d need somewhat more complicated example to make the compiler actually spill this way), but this is a rough outline of why your choice not to model the CFG correctly matters.

-Eli

Again what we intend to do in this feature is to achieve these two points below.  Please take a moment to read through it.  Let me know if there is anything unclear.
Thanks

--Ten

**** The rules for C code: ****
    For C-code, one way (MSVC approach) to achieve SEH -EHa semantic is to
    follow three rules. First, no exception can move in or out of _try
    region., i.e., no "potential faulty instruction can be moved across _try
    boundary. Second, the order of exceptions for instructions 'directly'
    under a _try must be preserved (not applied to those in callees).
    Finally, global states (local/global/heap variables) that can be read
    outside of _try region must be updated in memory (not just in register)
    before the subsequent exception occurs.

    **** The impact to C++ code: ****
    Although SEH is a feature for C code, -EHa does have a profound effect
    on C++ side. When a C++ function (in the same compilation unit with
    option -EHa ) is called by a SEH C function, a hardware exception occurs
    in C++ code can also be handled properly by an upstream SEH _try-handler
    or a C++ catch(...). As such, when that happens in the middle of an
    object's life scope, the dtor must be invoked the same way as C++
    Synchronous Exception during unwinding process.


From: Eli Friedman <efriedma at quicinc.com<mailto:efriedma at quicinc.com>>
Sent: Wednesday, April 15, 2020 4:29 PM
To: Ten Tzen <tentzen at microsoft.com<mailto:tentzen at microsoft.com>>; llvm-dev at lists.llvm.org<mailto:llvm-dev at lists.llvm.org>
Cc: rnk at google.com<mailto:rnk at google.com>; Aaron Smith <aaron.smith at microsoft.com<mailto:aaron.smith at microsoft.com>>; Joseph Tremoulet <jotrem at microsoft.com<mailto:jotrem at microsoft.com>>
Subject: [EXTERNAL] RE: [RFC] [Windows SEH][-EHa] Support Hardware Exception Handling

I still have basically the same concerns.  I’ll try to give more concrete examples for what I’m concerned about.

Suppose I have something like the following:

typedef struct C { int x[2]; } C;
void threw_exception();
void z();
C f() {
    __try {
        z();
        return *(C*)0;
    } __except(1) {
        threw_exception();
    }
    C c = {0};
    return c;
}


Currently, under your proposal, this won’t call threw_exception() if optimization is enabled, as far as I can tell.  I have no idea if this is intentional: your proposal and your patch don’t contain or point to any documentation, and I can’t find any documentation that describes this on Microsoft’s website.  (I don’t really care what the answer is here; I care that there’s some documented answer to this question, and other questions like it.)


Constructing a testcase for the register allocation issues I mentioned before is hard because it’s sort of “random” based on the register allocation heuristics, but see https://reviews.llvm.org/D77767<https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Freviews.llvm.org%2FD77767&data=02%7C01%7Ctentzen%40microsoft.com%7C1bd3a45727ee4b6b532d08d7e247fc6c%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637226671113780572&sdata=NG1xqVwCc1FQ5zEvtq3rsfDBSr3ZvX6XyZjHaDaBYrs%3D&reserved=0> for the sort of issues that come up.  Note that we mark setjmp returns_twice, which turns off certain optimizations.  I don’t really like extending the usage of this sort of construct further, but if we are going to, we should at least mark the new intrinsics returns_twice, so they get the same protection as setjmp.

-Eli

From: Ten Tzen <tentzen at microsoft.com<mailto:tentzen at microsoft.com>>
Sent: Wednesday, April 15, 2020 1:51 PM
To: llvm-dev at lists.llvm.org<mailto:llvm-dev at lists.llvm.org>
Cc: rnk at google.com<mailto:rnk at google.com>; Eli Friedman <efriedma at quicinc.com<mailto:efriedma at quicinc.com>>; aaron.smith at microsoft.com<mailto:aaron.smith at microsoft.com>; Joseph Tremoulet <jotrem at microsoft.com<mailto:jotrem at microsoft.com>>
Subject: [EXT] [RFC] [Windows SEH][-EHa] Support Hardware Exception Handling

Hi,
This is a spin-off of previous Windows SEH RFC below. This RFC only focus on supporting HW Exception Handling.

A detailed implementation can be seen in here: https://github.com/tentzen/llvm-project/commit/8a2421c274b683051e456cbe12c177e3b934fb5e<https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Ftentzen%2Fllvm-project%2Fcommit%2F8a2421c274b683051e456cbe12c177e3b934fb5e&data=02%7C01%7Ctentzen%40microsoft.com%7C1bd3a45727ee4b6b532d08d7e247fc6c%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637226671113780572&sdata=aBJJzyZJLMnoAB73ELT8yXPnn8zcVJ4F5McvpwEe7oE%3D&reserved=0>
It passes all MSVC SEH suite (excluding those with “Jumping out of _finally” ( _Local_Unwind)).

Thanks,

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20200501/b905f3d4/attachment.html>


More information about the llvm-dev mailing list