Skip to content

[BOLT] Accept PLT fall-throughs as valid traces #129481

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 14 additions & 5 deletions bolt/lib/Profile/DataAggregator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -871,18 +871,27 @@ DataAggregator::getFallthroughsInTrace(BinaryFunction &BF,

BinaryContext &BC = BF.getBinaryContext();

if (!BF.isSimple())
return std::nullopt;

assert(BF.hasCFG() && "can only record traces in CFG state");

// Offsets of the trace within this function.
const uint64_t From = FirstLBR.To - BF.getAddress();
const uint64_t To = SecondLBR.From - BF.getAddress();

if (From > To)
return std::nullopt;

// Accept fall-throughs inside pseudo functions (PLT/thunks).
// This check has to be above BF.empty as pseudo functions would pass it:
// pseudo => ignored => CFG not built => empty.
// If we return nullopt, trace would be reported as mismatching disassembled
// function contents which it is not. To avoid this, return an empty
// fall-through list instead.
if (BF.isPseudo())
return Branches;

if (!BF.isSimple())
return std::nullopt;

assert(BF.hasCFG() && "can only record traces in CFG state");

const BinaryBasicBlock *FromBB = BF.getBasicBlockContainingOffset(From);
const BinaryBasicBlock *ToBB = BF.getBasicBlockContainingOffset(To);

Expand Down
10 changes: 10 additions & 0 deletions bolt/test/X86/callcont-fallthru.s
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
# RUN: link_fdata %s %t %t.pa3 PREAGG3
# RUN: link_fdata %s %t %t.pat PREAGGT1
# RUN: link_fdata %s %t %t.pat2 PREAGGT2
# RUN: link_fdata %s %t %t.patplt PREAGGPLT

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

## Check pre-aggregated traces don't report zero-sized PLT fall-through as
## invalid trace
# RUN: llvm-bolt %t.strip --pa -p %t.patplt -o %t.out | FileCheck %s \
# RUN: --check-prefix=CHECK-PLT
# CHECK-PLT: traces mismatching disassembled function contents: 0

.globl foo
.type foo, %function
foo:
Expand All @@ -65,7 +72,10 @@ main:
movl $0x0, -0x4(%rbp)
movl %edi, -0x8(%rbp)
movq %rsi, -0x10(%rbp)
Ltmp0_br:
callq puts@PLT
## Check PLT traces are accepted
# PREAGGPLT: T #Ltmp0_br# #puts@plt# #puts@plt# 3
## Target is an external-origin call continuation
# PREAGG1: B X:0 #Ltmp1# 2 0
# PREAGGT1: T X:0 #Ltmp1# #Ltmp4_br# 2
Expand Down
11 changes: 10 additions & 1 deletion bolt/test/link_fdata.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
"""

import argparse
import os
import subprocess
import sys
import re
Expand Down Expand Up @@ -84,8 +85,16 @@
exit("ERROR: unexpected input:\n%s" % line)

# Read nm output: <symbol value> <symbol type> <symbol name>
is_llvm_nm = os.path.basename(args.nmtool) == "llvm-nm"
nm_output = subprocess.run(
[args.nmtool, "--defined-only", args.objfile], text=True, capture_output=True
[
args.nmtool,
"--defined-only",
"--special-syms" if is_llvm_nm else "--synthetic",
args.objfile,
],
text=True,
capture_output=True,
).stdout
# Populate symbol map
symbols = {}
Expand Down
Loading