[PATCH] D130617: [BOLT] Update split landing pad check for stripped binaries

[Huan Nguyen via Phabricator via llvm-commits]

nhuhuan created this revision.
Herald added subscribers: ayermolo, JDevlieghere.
Herald added a reviewer: rafauler.
Herald added a reviewer: Amir.
Herald added a reviewer: maksfb.
Herald added a project: All.
nhuhuan requested review of this revision.
Herald added subscribers: llvm-commits, yota9.
Herald added a project: LLVM.

Landing pad represents catch block, so it needs to be in same function
with callsite. If both reside in two different fragments, we check if
these fragments are siblings.

In nonstripped binaries, we rely on symbol name to verify sibling
relationship. In stripped binaries, we don't have symbol name.

Unlike jump table where it is challenging to detect the size, LSDA uses
an encoding format that reveals its size, even in stripped mode. It
makes sense to trust the correctness of input binaries, and skip the
sibling check only for stripped binaries.

Test Plan:

  ninja check-bolt


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D130617

Files:
  bolt/lib/Core/Exceptions.cpp
  bolt/test/X86/split-landing-pad.s


Index: bolt/test/X86/split-landing-pad.s
===================================================================
--- bolt/test/X86/split-landing-pad.s
+++ bolt/test/X86/split-landing-pad.s
@@ -8,6 +8,8 @@
 # This test is written to ensure BOLT safely handle these targets, e.g., by
 # marking them as non-simple.
 #
+# This test is for both stripped and non-stripped binaries
+#
 # Steps to write this test:
 # - Create a copy of Inputs/src/unreachable.cpp
 # - Simplify bar(), focus on throw an exception
@@ -27,6 +29,10 @@
 # RUN: %clang++ %cxxflags %t.o -o %t.exe -Wl,-q
 # RUN: llvm-bolt -v=3 %t.exe -o %t.out -print-exceptions 2>&1 | FileCheck %s
 
+# RUN: cp %t.exe %t.tmp.exe
+# RUN: llvm-strip -s %t.tmp.exe
+# RUN: llvm-bolt -v=3 %t.tmp.exe -o %t.out -print-exceptions 2>&1 | FileCheck %s
+
 # CHECK: BOLT-INFO: marking [[FOO_COLD:.+]] as a fragment of [[FOO:.+]]
 # CHECK: landing pad label: [[TMP2:.+]]
 # CHECK: landing pad label: [[TMP5:.+]]
Index: bolt/lib/Core/Exceptions.cpp
===================================================================
--- bolt/lib/Core/Exceptions.cpp
+++ bolt/lib/Core/Exceptions.cpp
@@ -180,19 +180,36 @@
     BinaryFunction *Fragment = BC.getBinaryFunctionContainingAddress(LPAddress);
 
     // Verify if landing pad code is located outside current function
-    // Support landing pad to builtin_unreachable
+    // Skip case where landing pad targets builtin_unreachable
     if (LPAddress < Address || LPAddress > Address + getSize()) {
+      // Assume landing pad not target another fragment's builtin_unreachable
+      // If this assumption is violated, must run a global check first
       assert(Fragment != nullptr &&
              "BOLT-ERROR: cannot find landing pad fragment");
+
+      // Update IsFragment:
+      // In stripped mode, always trust LSDA, consider the function that
+      // contains LP as a fragment
+      // In non-stripped mode, use pattern matching (adjustFunctionBoundaries)
+      if (BC.IsStripped)
+        Fragment->IsFragment = true;
+
+      // Update parent-fragment relation, add Fragment as secondary entry of
+      // the current function, not an independent function
       BC.addInterproceduralReference(this, Fragment->getAddress());
       BC.processInterproceduralReferences();
-      auto isFragmentOf = [](BinaryFunction *Fragment,
-                             BinaryFunction *Parent) -> bool {
-        return (Fragment->isFragment() && Fragment->isParentFragment(Parent));
-      };
-      assert((isFragmentOf(this, Fragment) || isFragmentOf(Fragment, this)) &&
-             "BOLT-ERROR: cannot have landing pads in different "
-             "functions");
+
+      // In stripped mode, parent-fragment is always established --> skip check
+      // In non-stripped mode, parent-fragment depends on symbol name --> check
+      if (!BC.IsStripped) {
+        auto isFragmentOf = [](BinaryFunction *Fragment,
+                               BinaryFunction *Parent) -> bool {
+          return (Fragment->isFragment() && Fragment->isParentFragment(Parent));
+        };
+        assert((isFragmentOf(this, Fragment) || isFragmentOf(Fragment, this)) &&
+               "BOLT-ERROR: cannot have landing pads in different functions");
+      }
+      // Mark that this fragment reaches LP in another fragment of same function
       setHasIndirectTargetToSplitFragment(true);
       BC.addFragmentsToSkip(this);
     }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D130617.447965.patch
Type: text/x-patch
Size: 3413 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220727/2600a7d8/attachment.bin>