]> granicus.if.org Git - llvm/commitdiff
[AArch64][Windows] Compute function length correctly in unwind tables.
authorEli Friedman <efriedma@quicinc.com>
Fri, 3 May 2019 00:10:45 +0000 (00:10 +0000)
committerEli Friedman <efriedma@quicinc.com>
Fri, 3 May 2019 00:10:45 +0000 (00:10 +0000)
The primary fix here is to WinException.cpp: we need to exclude jump
tables when computing the length of a function, or else we fail to
correctly compute the length. (We can only compute the number of bytes
consumed by certain assembler directives after the entire file is
parsed. ".p2align" is one of those directives, and is used by jump table
generation.)

The secondary fix, to MCWin64EH, is to make sure we don't silently
miscompile if we hit a similar situation in the future.

It's possible we could extend ARM64EmitUnwindInfo so it allows function
bodies that contain assembler directives, but that's a lot more
complicated; see the FIXME in MCWin64EH.cpp.

Fixes https://bugs.llvm.org/show_bug.cgi?id=41581 .

Differential Revision: https://reviews.llvm.org/D61095

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@359849 91177308-0d34-0410-b5e6-96231b3b80d8

lib/CodeGen/AsmPrinter/WinException.cpp
lib/CodeGen/AsmPrinter/WinException.h
lib/MC/MCWin64EH.cpp
test/CodeGen/AArch64/win64-jumptable.ll

index c870419056ba669971a376c9753d39b44390bb8b..155e91ce61a1b2fd8c6101e8ac881521d457bcce 100644 (file)
@@ -109,6 +109,12 @@ void WinException::beginFunction(const MachineFunction *MF) {
   beginFunclet(MF->front(), Asm->CurrentFnSym);
 }
 
+void WinException::markFunctionEnd() {
+  if (isAArch64 && CurrentFuncletEntry &&
+      (shouldEmitMoves || shouldEmitPersonality))
+    Asm->OutStreamer->EmitWinCFIFuncletOrFuncEnd();
+}
+
 /// endFunction - Gather and emit post-function exception information.
 ///
 void WinException::endFunction(const MachineFunction *MF) {
@@ -128,7 +134,7 @@ void WinException::endFunction(const MachineFunction *MF) {
     NonConstMF->tidyLandingPads();
   }
 
-  endFunclet();
+  endFuncletImpl();
 
   // endFunclet will emit the necessary .xdata tables for x64 SEH.
   if (Per == EHPersonality::MSVC_Win64SEH && MF->hasEHFunclets())
@@ -231,6 +237,15 @@ void WinException::beginFunclet(const MachineBasicBlock &MBB,
 }
 
 void WinException::endFunclet() {
+  if (isAArch64 && CurrentFuncletEntry &&
+      (shouldEmitMoves || shouldEmitPersonality)) {
+    Asm->OutStreamer->SwitchSection(CurrentFuncletTextSection);
+    Asm->OutStreamer->EmitWinCFIFuncletOrFuncEnd();
+  }
+  endFuncletImpl();
+}
+
+void WinException::endFuncletImpl() {
   // No funclet to process?  Great, we have nothing to do.
   if (!CurrentFuncletEntry)
     return;
@@ -246,8 +261,6 @@ void WinException::endFunclet() {
     // to EmitWinEHHandlerData below can calculate the size of the funclet or
     // function.
     if (isAArch64) {
-      Asm->OutStreamer->SwitchSection(CurrentFuncletTextSection);
-      Asm->OutStreamer->EmitWinCFIFuncletOrFuncEnd();
       MCSection *XData = Asm->OutStreamer->getAssociatedXDataSection(
           Asm->OutStreamer->getCurrentSectionOnly());
       Asm->OutStreamer->SwitchSection(XData);
index 9491d7d69c2a4addaae77d6912988a4c7cc3c069..dc5036302131369b0795b36e424ad4b59be37f0c 100644 (file)
@@ -85,6 +85,7 @@ class LLVM_LIBRARY_VISIBILITY WinException : public EHStreamer {
   /// only), it is relative to the frame pointer.
   int getFrameIndexOffset(int FrameIndex, const WinEHFuncInfo &FuncInfo);
 
+  void endFuncletImpl();
 public:
   //===--------------------------------------------------------------------===//
   // Main entry points.
@@ -99,6 +100,8 @@ public:
   /// immediately after the function entry point.
   void beginFunction(const MachineFunction *MF) override;
 
+  void markFunctionEnd() override;
+
   /// Gather and emit post-function exception information.
   void endFunction(const MachineFunction *) override;
 
index 93c115970e55d806e18fe6cf50e9546be6bac1ef..4e9a29667097b57259edc3bd59a30d5deee5e96c 100644 (file)
@@ -255,8 +255,12 @@ static int64_t GetAbsDifference(MCStreamer &Streamer, const MCSymbol *LHS,
       MCBinaryExpr::createSub(MCSymbolRefExpr::create(LHS, Context),
                               MCSymbolRefExpr::create(RHS, Context), Context);
   MCObjectStreamer *OS = (MCObjectStreamer *)(&Streamer);
+  // It should normally be possible to calculate the length of a function
+  // at this point, but it might not be possible in the presence of certain
+  // unusual constructs, like an inline asm with an alignment directive.
   int64_t value;
-  Diff->evaluateAsAbsolute(value, OS->getAssembler());
+  if (!Diff->evaluateAsAbsolute(value, OS->getAssembler()))
+    report_fatal_error("Failed to evaluate function length in SEH unwind info");
   return value;
 }
 
@@ -498,11 +502,44 @@ static void ARM64EmitUnwindInfo(MCStreamer &streamer, WinEH::FrameInfo *info) {
   streamer.EmitLabel(Label);
   info->Symbol = Label;
 
-  uint32_t FuncLength = 0x0;
-  if (info->FuncletOrFuncEnd)
-    FuncLength = (uint32_t)GetAbsDifference(streamer, info->FuncletOrFuncEnd,
-                                            info->Begin);
-  FuncLength /= 4;
+  int64_t RawFuncLength;
+  if (!info->FuncletOrFuncEnd) {
+    // FIXME: This is very wrong; we emit SEH data which covers zero bytes
+    // of code. But otherwise test/MC/AArch64/seh.s crashes.
+    RawFuncLength = 0;
+  } else {
+    // FIXME: GetAbsDifference tries to compute the length of the function
+    // immediately, before the whole file is emitted, but in general
+    // that's impossible: the size in bytes of certain assembler directives
+    // like .align and .fill is not known until the whole file is parsed and
+    // relaxations are applied. Currently, GetAbsDifference fails with a fatal
+    // error in that case. (We mostly don't hit this because inline assembly
+    // specifying those directives is rare, and we don't normally try to
+    // align loops on AArch64.)
+    //
+    // There are two potential approaches to delaying the computation. One,
+    // we could emit something like ".word (endfunc-beginfunc)/4+0x10800000",
+    // as long as we have some conservative estimate we could use to prove
+    // that we don't need to split the unwind data. Emitting the constant
+    // is straightforward, but there's no existing code for estimating the
+    // size of the function.
+    //
+    // The other approach would be to use a dedicated, relaxable fragment,
+    // which could grow to accommodate splitting the unwind data if
+    // necessary. This is more straightforward, since it automatically works
+    // without any new infrastructure, and it's consistent with how we handle
+    // relaxation in other contexts.  But it would require some refactoring
+    // to move parts of the pdata/xdata emission into the implementation of
+    // a fragment. We could probably continue to encode the unwind codes
+    // here, but we'd have to emit the pdata, the xdata header, and the
+    // epilogue scopes later, since they depend on whether the we need to
+    // split the unwind data.
+    RawFuncLength = GetAbsDifference(streamer, info->FuncletOrFuncEnd,
+                                     info->Begin);
+  }
+  if (RawFuncLength > 0xFFFFF)
+    report_fatal_error("SEH unwind data splitting not yet implemented");
+  uint32_t FuncLength = (uint32_t)RawFuncLength / 4;
   uint32_t PrologCodeBytes = ARM64CountOfUnwindCodes(info->Instructions);
   uint32_t TotalCodeBytes = PrologCodeBytes;
 
index 1caba458400d36d567e2eff80c76862ad31c18ba..6a9752687aaf501e4782965d478f2660f52b1674 100644 (file)
@@ -1,4 +1,5 @@
 ; RUN: llc -o - %s -mtriple=aarch64-windows -aarch64-enable-compress-jump-tables=0 | FileCheck %s
+; RUN: llc -o - %s -mtriple=aarch64-windows -aarch64-enable-compress-jump-tables=0 -filetype=obj | llvm-readobj -unwind | FileCheck %s -check-prefix=UNWIND
 
 define void @f(i32 %x) {
 entry:
@@ -46,3 +47,6 @@ declare void @g(i32, i32)
 ; CHECK:    .seh_handlerdata
 ; CHECK:    .text
 ; CHECK:    .seh_endproc
+
+; Check that we can emit an object file with correct unwind info.
+; UNWIND: FunctionLength: {{[1-9][0-9]*}}