[BOLT] Handle internal calls in ValidateInternalCalls (#105736)

Move handling of all internal calls into the designated pass. Preserve
NOPs and mark functions as non-simple on non-X86 platforms.

Author: maksfb

bolt/include/bolt/Core/BinaryFunction.h

@@ -1692,6 +1692,8 @@ class BinaryFunction {
 
   void setPseudo(bool Pseudo) { IsPseudo = Pseudo; }
 
+  void setPreserveNops(bool Value) { PreserveNops = Value; }
+
   BinaryFunction &setUsesGnuArgsSize(bool Uses = true) {
     UsesGnuArgsSize = Uses;
     return *this;

bolt/lib/Core/BinaryFunction.cpp

@@ -1339,22 +1339,10 @@ Error BinaryFunction::disassemble() {
                   BC.getBinaryFunctionContainingAddress(TargetAddress))
             TargetFunc->setIgnored();
 
-        if (IsCall && containsAddress(TargetAddress)) {
-          if (TargetAddress == getAddress()) {
-            // Recursive call.
-            TargetSymbol = getSymbol();
-          } else {
-            if (BC.isX86()) {
-              // Dangerous old-style x86 PIC code. We may need to freeze this
-              // function, so preserve the function as is for now.
-              PreserveNops = true;
-            } else {
-              BC.errs() << "BOLT-WARNING: internal call detected at 0x"
-                        << Twine::utohexstr(AbsoluteInstrAddr)
-                        << " in function " << *this << ". Skipping.\n";
-              IsSimple = false;
-            }
-          }
+        if (IsCall && TargetAddress == getAddress()) {
+          // A recursive call. Calls to internal blocks are handled by
+          // ValidateInternalCalls pass.
+          TargetSymbol = getSymbol();
         }
 
         if (!TargetSymbol) {

bolt/lib/Passes/ValidateInternalCalls.cpp

@@ -302,24 +302,27 @@ bool ValidateInternalCalls::analyzeFunction(BinaryFunction &Function) const {
 }
 
 Error ValidateInternalCalls::runOnFunctions(BinaryContext &BC) {
-  if (!BC.isX86())
-    return Error::success();
-
   // Look for functions that need validation. This should be pretty rare.
   std::set<BinaryFunction *> NeedsValidation;
   for (auto &BFI : BC.getBinaryFunctions()) {
     BinaryFunction &Function = BFI.second;
     for (BinaryBasicBlock &BB : Function) {
       for (MCInst &Inst : BB) {
         if (getInternalCallTarget(Function, Inst)) {
+          BC.errs() << "BOLT-WARNING: internal call detected in function "
+                    << Function << '\n';
           NeedsValidation.insert(&Function);
           Function.setSimple(false);
+          Function.setPreserveNops(true);
           break;
         }
       }
     }
   }
 
+  if (!BC.isX86())
+    return Error::success();
+
   // Skip validation for non-relocation mode
   if (!BC.HasRelocations)
     return Error::success();

bolt/test/AArch64/internal-call.s

@@ -0,0 +1,65 @@
+## Test that llvm-bolt detects internal calls and marks the containing function
+## as non-simple.
+
+# RUN: llvm-mc -filetype=obj -triple aarch64-unknown-unknown %s -o %t.o
+# RUN: %clang %cflags %t.o -o %t.exe -Wl,-q -static
+# RUN: llvm-bolt %t.exe -o %t.null --print-all 2>&1 | FileCheck %s
+
+# CHECK: Binary Function "_start" after building cfg
+# CHECK: internal call detected in function _start
+# CHECK-NOT: Binary Function "_start" after validate-internal-calls
+
+	.text
+  .globl _start
+  .type _start, %function
+_start:
+	.cfi_startproc
+.LBB00:
+	mov	x11, #0x1fff
+	cmp	x1, x11
+	b.hi	.Ltmp1
+
+.entry1:
+	movi	v4.16b, #0x0
+	movi	v5.16b, #0x0
+	subs	x1, x1, #0x8
+	b.lo	.Ltmp2
+
+.entry2:
+	ld1	{ v2.2d, v3.2d }, [x0], #32
+	ld1	{ v0.2d, v1.2d }, [x0], #32
+
+.Ltmp2:
+	uaddlp	v4.4s, v4.8h
+	uaddlp	v4.2d, v4.4s
+	mov	x0, v4.d[0]
+	mov	x1, v4.d[1]
+	add	x0, x0, x1
+	ret	x30
+
+.Ltmp1:
+	mov	x8, x30
+
+.Lloop:
+	add	x5, x0, x9
+	mov	x1, #0xface
+	movi	v4.16b, #0x0
+	movi	v5.16b, #0x0
+	bl	.entry2
+	add	x4, x4, x0
+	mov	x0, x5
+	sub	x7, x7, x10
+	cmp	x7, x11
+	b.hi	.Lloop
+
+	mov	x1, x7
+	bl	.entry1
+	add	x0, x4, x0
+	mov	x30, x8
+	ret	x30
+
+	.cfi_endproc
+.size _start, .-_start
+
+## Force relocation mode.
+  .reloc 0, R_AARCH64_NONE