Skip to content

Commit fa4ac19

Browse files
authored
[BOLT] Accept PLT fall-throughs as valid traces (#129481)
We used to report PLT traces as invalid (mismatching disassembled function contents) because PLT functions are marked as pseudo and ignored, thus missing CFG. However, such traces are not mismatching the function contents. Accept them without attaching the profile. Test Plan: updated callcont-fallthru.s
1 parent 3a2d9a7 commit fa4ac19

File tree

3 files changed

+34
-6
lines changed

3 files changed

+34
-6
lines changed

bolt/lib/Profile/DataAggregator.cpp

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -871,18 +871,27 @@ DataAggregator::getFallthroughsInTrace(BinaryFunction &BF,
871871

872872
BinaryContext &BC = BF.getBinaryContext();
873873

874-
if (!BF.isSimple())
875-
return std::nullopt;
876-
877-
assert(BF.hasCFG() && "can only record traces in CFG state");
878-
879874
// Offsets of the trace within this function.
880875
const uint64_t From = FirstLBR.To - BF.getAddress();
881876
const uint64_t To = SecondLBR.From - BF.getAddress();
882877

883878
if (From > To)
884879
return std::nullopt;
885880

881+
// Accept fall-throughs inside pseudo functions (PLT/thunks).
882+
// This check has to be above BF.empty as pseudo functions would pass it:
883+
// pseudo => ignored => CFG not built => empty.
884+
// If we return nullopt, trace would be reported as mismatching disassembled
885+
// function contents which it is not. To avoid this, return an empty
886+
// fall-through list instead.
887+
if (BF.isPseudo())
888+
return Branches;
889+
890+
if (!BF.isSimple())
891+
return std::nullopt;
892+
893+
assert(BF.hasCFG() && "can only record traces in CFG state");
894+
886895
const BinaryBasicBlock *FromBB = BF.getBasicBlockContainingOffset(From);
887896
const BinaryBasicBlock *ToBB = BF.getBasicBlockContainingOffset(To);
888897

bolt/test/X86/callcont-fallthru.s

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
# RUN: link_fdata %s %t %t.pa3 PREAGG3
1010
# RUN: link_fdata %s %t %t.pat PREAGGT1
1111
# RUN: link_fdata %s %t %t.pat2 PREAGGT2
12+
# RUN: link_fdata %s %t %t.patplt PREAGGPLT
1213

1314
## Check normal case: fallthrough is not LP or secondary entry.
1415
# RUN: llvm-strip --strip-unneeded %t -o %t.strip
@@ -42,6 +43,12 @@
4243
# RUN: llvm-bolt %t.strip --pa -p %t.pat2 -o %t.out \
4344
# RUN: --print-cfg --print-only=main | FileCheck %s --check-prefix=CHECK3
4445

46+
## Check pre-aggregated traces don't report zero-sized PLT fall-through as
47+
## invalid trace
48+
# RUN: llvm-bolt %t.strip --pa -p %t.patplt -o %t.out | FileCheck %s \
49+
# RUN: --check-prefix=CHECK-PLT
50+
# CHECK-PLT: traces mismatching disassembled function contents: 0
51+
4552
.globl foo
4653
.type foo, %function
4754
foo:
@@ -65,7 +72,10 @@ main:
6572
movl $0x0, -0x4(%rbp)
6673
movl %edi, -0x8(%rbp)
6774
movq %rsi, -0x10(%rbp)
75+
Ltmp0_br:
6876
callq puts@PLT
77+
## Check PLT traces are accepted
78+
# PREAGGPLT: T #Ltmp0_br# #puts@plt# #puts@plt# 3
6979
## Target is an external-origin call continuation
7080
# PREAGG1: B X:0 #Ltmp1# 2 0
7181
# PREAGGT1: T X:0 #Ltmp1# #Ltmp4_br# 2

bolt/test/link_fdata.py

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
"""
99

1010
import argparse
11+
import os
1112
import subprocess
1213
import sys
1314
import re
@@ -84,8 +85,16 @@
8485
exit("ERROR: unexpected input:\n%s" % line)
8586

8687
# Read nm output: <symbol value> <symbol type> <symbol name>
88+
is_llvm_nm = os.path.basename(args.nmtool) == "llvm-nm"
8789
nm_output = subprocess.run(
88-
[args.nmtool, "--defined-only", args.objfile], text=True, capture_output=True
90+
[
91+
args.nmtool,
92+
"--defined-only",
93+
"--special-syms" if is_llvm_nm else "--synthetic",
94+
args.objfile,
95+
],
96+
text=True,
97+
capture_output=True,
8998
).stdout
9099
# Populate symbol map
91100
symbols = {}

0 commit comments

Comments
 (0)