SEH for Windows aarch64: invalid Packed unwind data detection with H=1 #54879
Comments
|
@llvm/issue-subscribers-backend-aarch64 |
|
Hello, @egelke I'm not familiar with Win64EH and unwinding. Can you confirm if below is correct packed unwind data or not for your test case 2 (homing int params but no mirrored nop in epilog)? |
|
To be honest I'm still quite novice, but yes that seems to right (the assembly , not the expected opcodes). |
|
I pushed a fix for this: https://reviews.llvm.org/D125433 |
|
While I do agree that it wouldn't make sense to have the corresponding nops in the epilogue, the Windows unwinder actually does seem to behave as if it expects the epilogue to have them. That's why I implemented it this way originally. (I hadn't noticed dumpbin's So here, the docs would imply that the epilogue shouldn't have nops: https://docs.microsoft.com/en-us/cpp/build/arm64-exception-handling?view=msvc-170#packed-unwind-data
And the dumpbin output also seems to agree with this. However, if you create a function with packed unwind info with the Dumpbin for this function thinks that the epilogue should look like this: Unwinding from different offsets adjusts the Sp register like this: According to dumpbin, the epilogue should start at offset 0x34 - thus unwinding from offset 0x30 should definitely increment Sp by the full amount 0x70. But in practice, Sp is increased by 0x70 only if unwinding from 0x24 or earlier. So this does make it look like the implementation of I'm not sure if MSVC actually ever emits packed unwind info with the #include <stdarg.h>
void other(va_list ap, void* ptr);
void func(int a, ...) {
va_list ap;
va_start(ap, a);
char buf[32];
other(ap, buf);
va_end(ap);
}Compiling this with MSVC (I tested 2017, 2019 and 2022) I don't get packed unwind info with any of them: So it would seem to me that MSVC actually doesn't ever generate packed unwind info with So in this case, if we change LLVM's code generation to match the docs and dumpbin, we'd actually get incorrect unwinding results in practice, near the start of the epilogue. Therefore, I guess the only practical thing we can do in LLVM would be to stop generating packed unwind info with (I noticed a similar issue when initially working on optimizing the ARM64 unwind info; |
I wouldn't be shocked if dumpbin is wrong. See also #54900 .
Umm, what? Really? That's seems like a pretty big oversight if the "H" bit is simply broken.
That's kind of ugly. But if that's how the unwinder actually behaves, I guess H=1 is completely useless in practice.
We don't have any code to insert nops in the epilogue, no. :) If we know the Microsoft documentation is wrong, can we try to fix it? |
Indeed - but I guess it can happen if you don't have through unit tests that test unwinding from every single instruction location in prologues/epilogues, if the compiler doesn't happen to ever emit it.
I guess that could be done... I have gotten fixes for some other cases merged there before, but those were bugs in the documentation (while the implementation was sensible and correct). And for other bugs that are found, like e.g. |
|
So there's inconsistency between MS documentation and windows unwinder ( |
|
I posted https://reviews.llvm.org/D125876 to make us stop producing packed unwind info, for this inconsistent state. Our current code does work correctly on current Windows unwinders, but if they fix their implementation to match the docs, it would break in the future. So let's stop producing such code. The current code doesn't ever match for compiler generated code anyway, only for handcrafted cases. |
…meters There's an inconsistency regarding the epilogs of packed ARM64 unwind info with homed parameters; according to the documentation (and according to common sense), the epilog wouldn't have a series of nop instructions matching the stp x0-x7 in the prolog - however in practice, RtlVirtualUnwind still seems to behave as if the epilog does have the mirrored nops from the prolog. In practice, MSVC doesn't seem to produce packed unwind info with homed parameters, which might be why this inconsistency hasn't been noticed. Thus, to play it safe, avoid creating such packed unwind info with homed parameters. (LLVM's current behaviour matches the current runtime behaviour of RtlVirtualUnwind, but if it later is bug fixed to match the documentation, such unwind information would be incorrect.) See #54879 for further discussion on the matter. Differential Revision: https://reviews.llvm.org/D125876
…meters There's an inconsistency regarding the epilogs of packed ARM64 unwind info with homed parameters; according to the documentation (and according to common sense), the epilog wouldn't have a series of nop instructions matching the stp x0-x7 in the prolog - however in practice, RtlVirtualUnwind still seems to behave as if the epilog does have the mirrored nops from the prolog. In practice, MSVC doesn't seem to produce packed unwind info with homed parameters, which might be why this inconsistency hasn't been noticed. Thus, to play it safe, avoid creating such packed unwind info with homed parameters. (LLVM's current behaviour matches the current runtime behaviour of RtlVirtualUnwind, but if it later is bug fixed to match the documentation, such unwind information would be incorrect.) See llvm/llvm-project#54879 for further discussion on the matter. Differential Revision: https://reviews.llvm.org/D125876
There is an issue with the the detection of "packed unwind data" when compiling aaarch64 asm with SEH directives on Windows, e.g.:
Everything works fine as long as you don't home the integer parameter registers. The below example results in a correctly packed unwind data.
However, when you home the integer parameter registers, i.e. try to generate packed unwind data with H=1, it fails recognize that it's a packed format and generates an unpacked version instead:
If you however, repeat the "nop" seh directives with instructions into the unwind code, then you get an incorrect packed unwind data as shows by the following example:
(please ignore the bug in MS dumpbin.exe, the opcode is correct it's just the check that is wrong since it expect x0 and x1 instead of x2 and x3)
Obviously, a NOP should not be repeated in the unwind code. Instead, when clang encounters a .seh_nop directive in the prologue it should not expect any matching instruction in the epilogue in order for it to to generate a packed unwind data.
The text was updated successfully, but these errors were encountered: