[PATCH] D107784: Fix SEH table addresses for Windows

Daniel Paoliello via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Aug 11 12:54:43 PDT 2021


dpaoliello marked 3 inline comments as done.
dpaoliello added inline comments.


================
Comment at: llvm/lib/CodeGen/AsmPrinter/WinException.cpp:670
     AddComment("LabelEnd");
-    OS.emitValue(getLabel(EndLabel), 4);
+    OS.emitValue(getLabelPlusOne(EndLabel), 4);
     AddComment(UME.IsFinally ? "FinallyFunclet" : UME.Filter ? "FilterFunction"
----------------
rnk wrote:
> So, this *will* be a behavior change for ARM. Is that desired? Are there issues with adding one to ARM PCs? It sets the thumb bit, but maybe that doesn't matter to `__C_specific_handler` on ARM. Neither ARM ISA has single byte instructions (right?), so adding one isn't likely to accidentally cause an exception from the next instruction to go to the wrong handler.
`__C_specific_handler` has the same call-return-address compensation on ARM/ARM64 as `FrameFromIp`. That being said, this change is desired as the end address is exclusive, so we should +1 it to make sure that the comparison works correctly.


================
Comment at: llvm/lib/CodeGen/AsmPrinter/WinException.cpp:955-958
+      // NOTE: On ARM architectures, the StateFromIp automatically takes into
+      // account that the return address is after the call instruction (whose EH
+      // state we should be using), but on other platforms we need to +1 to the
+      // label so that we are using the correct EH state.
----------------
rnk wrote:
> Truthfully, it feels to me like this is an x64-specific bug. If Windows were to be ported to a new architecture in five years, I bet they'd stick with the ARM behavior, and we wouldn't do the +1. However, that's all very hypothetical, I don't see a need for any code changes.
Sorry, I don't really have any explanation here either ¯\_(ツ)_/¯


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

https://reviews.llvm.org/D107784



More information about the llvm-commits mailing list