Skip to content
Open
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
4 changes: 4 additions & 0 deletions cpp/ql/lib/change-notes/2022-08-01-gvn-functions.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* Multiple function calls made in the same function may now receive the same global value number when the analysis can show they produce the same results.
Original file line number Diff line number Diff line change
Expand Up @@ -1712,6 +1712,11 @@ class SideEffectInstruction extends Instruction {
*/
class CallSideEffectInstruction extends SideEffectInstruction {
CallSideEffectInstruction() { this.getOpcode() instanceof Opcode::CallSideEffect }

/** Gets the operand for the value that will be read by this instruction */
final SideEffectOperand getSideEffectOperand() { result = this.getAnOperand() }

final Instruction getSideEffect() { result = this.getAnOperand().getDef() }
}

/**
Expand All @@ -1723,6 +1728,11 @@ class CallSideEffectInstruction extends SideEffectInstruction {
*/
class CallReadSideEffectInstruction extends SideEffectInstruction {
CallReadSideEffectInstruction() { this.getOpcode() instanceof Opcode::CallReadSideEffect }

/** Gets the operand for the value that will be read by this instruction */
final SideEffectOperand getSideEffectOperand() { result = this.getAnOperand() }

final Instruction getSideEffect() { result = this.getAnOperand().getDef() }
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import semmle.code.cpp.ir.implementation.aliased_ssa.IR
import semmle.code.cpp.ir.internal.Overlap
import semmle.code.cpp.ir.internal.IRCppLanguage as Language
import semmle.code.cpp.models.interfaces.SideEffect as SideEffect
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,92 @@ newtype TValueNumber =
) {
loadTotalOverlapValueNumber(_, irFunc, type, memOperand, operand)
} or
TCallValueNumber(TCallPartialValueNumber vn) { callValueNumber(_, _, vn) } or
TUniqueValueNumber(IRFunction irFunc, Instruction instr) { uniqueValueNumber(instr, irFunc) }

private class NumberableCallInstruction extends CallInstruction {
NumberableCallInstruction() {
not this.getResultIRType() instanceof IRVoidType and
exists(SideEffect::SideEffectFunction sideEffectFunc |
sideEffectFunc = this.getStaticCallTarget()
|
sideEffectFunc.hasOnlySpecificReadSideEffects() and
sideEffectFunc.hasOnlySpecificWriteSideEffects()
)
}
}

private newtype TCallPartialValueNumber =
TNilArgument() or
TArgument(TCallPartialValueNumber head, TValueNumber arg) {
exists(NumberableCallInstruction call, int index |
callArgValueNumber(call, index, arg) and
callPartialValueNumber(call, index, head)
)
}

private predicate callValueNumber(
NumberableCallInstruction call, int index, TCallPartialValueNumber vn
) {
index = max(int n | callArgRank(call, n, _) | n) and
exists(TCallPartialValueNumber head, TValueNumber arg |
callPartialValueNumber(call, index, pragma[only_bind_out](head)) and
callArgValueNumber(call, index, pragma[only_bind_into](arg)) and
vn = TArgument(head, arg)
)
or
not exists(int n | callArgRank(call, n, _)) and
index = -1 and
vn = TNilArgument()
}

private predicate callPartialValueNumber(
NumberableCallInstruction call, int index, TCallPartialValueNumber head
) {
exists(call) and
index = 1 and
head = TNilArgument()
or
exists(TCallPartialValueNumber prev, TValueNumber prevVN |
callPartialValueNumber(call, index - 1, pragma[only_bind_out](prev)) and
callArgValueNumber(call, index - 1, pragma[only_bind_into](prevVN)) and
head = TArgument(prev, prevVN)
)
}

/**
*/
private predicate callArgValueNumber(NumberableCallInstruction call, int index, TValueNumber arg) {
exists(Instruction instr |
callArgRank(call, index, instr) and
arg = tvalueNumber(instr)
)
}

/**
* Holds if `arg` is the `index`th element in `call`'s extended argument list, including the `this`
* argument and side-effect reads.
*/
private predicate callArgRank(NumberableCallInstruction call, int index, Instruction arg) {
arg =
rank[index](int argIndex, boolean isEffect, Instruction instr |
// There is no need to include the call's read and write side effects on
// all-aliased-memory as `NumberableCallInstruction`s do not read or write
// to all-aliased-memory.
instr = call.getArgument(argIndex) and
isEffect = false
or
exists(ReadSideEffectInstruction read |
read.getPrimaryInstruction() = call and
read.getSideEffectOperand().getAnyDef() = instr and
read.getIndex() = argIndex and
isEffect = true
)
|
instr order by argIndex, isEffect
)
}

/**
* A `CopyInstruction` whose source operand's value is congruent to the definition of that source
* operand.
Expand Down Expand Up @@ -93,6 +177,8 @@ private predicate numberableInstruction(Instruction instr) {
instr instanceof CongruentCopyInstruction
or
instr instanceof LoadTotalOverlapInstruction
or
instr instanceof NumberableCallInstruction
}

private predicate filteredNumberableInstruction(Instruction instr) {
Expand Down Expand Up @@ -309,6 +395,11 @@ private TValueNumber nonUniqueValueNumber(Instruction instr) {
or
// The value number of a copy is just the value number of its source value.
result = tvalueNumber(instr.(CongruentCopyInstruction).getSourceValue())
or
exists(TCallPartialValueNumber pvn |
callValueNumber(instr, _, pvn) and
result = TCallValueNumber(pvn)
)
)
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,6 @@ private import Imports::RawIR as RawIR
private import SsaInstructions
private import SsaOperands
private import NewIR

private class OldBlock = Reachability::ReachableBlock;

private class OldInstruction = Reachability::ReachableInstruction;

import Cached

cached
Expand Down Expand Up @@ -58,7 +53,9 @@ private module Cached {

cached
predicate hasInstruction(TStageInstruction instr) {
instr instanceof TRawInstruction and instr instanceof OldInstruction
instr instanceof TRawInstruction and
instr instanceof OldInstruction and
not removedInstruction(instr)
or
instr = phiInstruction(_, _)
or
Expand Down Expand Up @@ -382,7 +379,14 @@ private module Cached {
(
if Reachability::isInfeasibleInstructionSuccessor(oldInstruction, kind)
then result = unreachedInstruction(instruction.getEnclosingIRFunction())
else result = getNewInstruction(oldInstruction.getSuccessor(kind))
else
if removedInstruction(oldInstruction.getSuccessor(kind))
then
// the only removed nodes are side-effect writes, but those may have Chi nodes
// skip to the following instruction in the old IR, which won't be removed
// if we start skipping specific side effects, this may no longer hold
result = getNewInstruction(oldInstruction.getSuccessor(kind).getSuccessor(kind))
else result = getNewInstruction(oldInstruction.getSuccessor(kind))
)
)
or
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,15 @@ import semmle.code.cpp.ir.implementation.internal.TOperand::AliasedSsaOperands a

/** DEPRECATED: Alias for SsaOperands */
deprecated module SSAOperands = SsaOperands;

private import SideEffectElimination as Elim
Comment thread Fixed

predicate removedInstruction(Reachability::ReachableInstruction instr) {
Elim::removeableSideEffect(instr)
Comment thread Fixed
Comment thread Fixed
}

class OldBlock = Reachability::ReachableBlock;

class OldInstruction extends Reachability::ReachableInstruction {
OldInstruction() { not Elim::removeableSideEffect(this) }
Comment thread Fixed
Comment thread Fixed
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
import cpp as CPP
import semmle.code.cpp.ir.implementation.unaliased_ssa.IR as Unaliased
import AliasAnalysis as Alias
import AliasConfiguration as Conf
import semmle.code.cpp.models.interfaces.SideEffect as SideEffect

private predicate noLocalSideEffectWrite(CPP::Function func) {
forall(Unaliased::AddressOperand addr | addr.getUse().getEnclosingFunction() = func |
not (
Alias::getAddressOperandAllocation(addr) instanceof Conf::VariableAllocation and
Alias::getAddressOperandAllocation(addr).(Conf::VariableAllocation).alwaysEscapes()
)
)
}

language[monotonicAggregates]
private predicate noTransitiveSideEffectWrite(CPP::Function func) {
exists(Unaliased::IRFunction irFunc | irFunc.getFunction() = func) and
noLocalSideEffectWrite(func) and
forall(Unaliased::CallInstruction call | call.getEnclosingFunction() = func |
exists(call.getStaticCallTarget())
) and
forall(Unaliased::CallInstruction call, CPP::Function callee |
call.getStaticCallTarget() = callee and
call.getEnclosingFunction() = func
|
noTransitiveSideEffectWrite(callee)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm always scared of recursion through forall as it's very easy to get a quadratic blowup. There's a general performance trick to this described in this internal issue: https://github.com/github/codeql-core/issues/416. We might want to apply that here.

or
callee.(SideEffect::SideEffectFunction).hasOnlySpecificWriteSideEffects()
)
}

predicate removeableSideEffect(Unaliased::SideEffectInstruction instr) {
(
instr instanceof Unaliased::CallSideEffectInstruction or
instr instanceof Unaliased::CallReadSideEffectInstruction
) and
noTransitiveSideEffectWrite(instr
.getPrimaryInstruction()
.(Unaliased::CallInstruction)
.getStaticCallTarget())
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ newtype TInstruction =
AliasedSsa::SSA::hasPhiInstruction(blockStartInstr, memoryLocation)
} or
TAliasedSsaChiInstruction(TRawInstruction primaryInstruction) {
not AliasedSsa::removedInstruction(primaryInstruction) and
AliasedSsa::SSA::hasChiInstruction(primaryInstruction)
} or
TAliasedSsaUnreachedInstruction(IRFunctionBase irFunc) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,5 @@ import semmle.code.cpp.ir.implementation.aliased_ssa.internal.SSAConstruction as

/** DEPRECATED: Alias for AliasedSsa */
deprecated module AliasedSSA = AliasedSsa;

import semmle.code.cpp.ir.implementation.aliased_ssa.internal.SideEffectElimination as Elim
Comment thread Fixed

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think you've checked SideEffectElimination.qll in yet. That explains all the consistency errors in ql-for-ql.

Original file line number Diff line number Diff line change
Expand Up @@ -1712,6 +1712,11 @@ class SideEffectInstruction extends Instruction {
*/
class CallSideEffectInstruction extends SideEffectInstruction {
CallSideEffectInstruction() { this.getOpcode() instanceof Opcode::CallSideEffect }

/** Gets the operand for the value that will be read by this instruction */
final SideEffectOperand getSideEffectOperand() { result = this.getAnOperand() }

final Instruction getSideEffect() { result = this.getAnOperand().getDef() }
}

/**
Expand All @@ -1723,6 +1728,11 @@ class CallSideEffectInstruction extends SideEffectInstruction {
*/
class CallReadSideEffectInstruction extends SideEffectInstruction {
CallReadSideEffectInstruction() { this.getOpcode() instanceof Opcode::CallReadSideEffect }

/** Gets the operand for the value that will be read by this instruction */
final SideEffectOperand getSideEffectOperand() { result = this.getAnOperand() }

final Instruction getSideEffect() { result = this.getAnOperand().getDef() }
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import semmle.code.cpp.ir.implementation.aliased_ssa.IR
import semmle.code.cpp.ir.internal.Overlap
import semmle.code.cpp.ir.internal.IRCppLanguage as Language
import semmle.code.cpp.models.interfaces.SideEffect as SideEffect
Loading