Merge pull request #21227 from MathiasVP/postfix-fix

C++: Get rid of an ugly workaround in dataflow
This commit is contained in:
Mathias Vorreiter Pedersen
2026-01-29 12:25:02 +00:00
committed by GitHub
3 changed files with 355 additions and 80 deletions

View File

@@ -15,17 +15,79 @@ private import DataFlowPrivate
import SsaImplCommon
private module SourceVariables {
/**
* Holds if `store` is the `StoreInstruction` generated by a postfix
* increment or decrement operation `e`, and `postCrement` is the operand
* that represents the use of the evaluated value of `e`.
*/
private predicate isUseAfterPostfixCrement0(StoreInstruction store, Operand postCrement) {
exists(
BinaryInstruction binary, IRBlock b, int iPre, int iPost, int iStore, Operand preCrement,
Instruction left
|
binary instanceof AddInstruction
or
binary instanceof PointerAddInstruction
or
binary instanceof SubInstruction
or
binary instanceof PointerSubInstruction
|
store.getSourceValue() = binary and
left = binary.getLeft() and
strictcount(left.getAUse()) = 2 and
left.getAUse() = preCrement and
left.getAUse() = postCrement and
b.getInstruction(iPre) = preCrement.getUse() and
b.getInstruction(iPost) = postCrement.getUse() and
b.getInstruction(iStore) = store and
iPre < iStore and
iStore < iPost
)
}
/**
* Holds if `store` is the `StoreInstruction` generated by an postfix
* increment or decrement operation `e`, and `postCrement` is the fully
* converted operand that represents the use of the evaluated value of `e`.
*/
private predicate isUseAfterPostfixCrement(StoreInstruction store, Operand postCrement) {
isUseAfterPostfixCrement0(store, postCrement) and
conversionFlow(postCrement, _, false, _)
or
exists(Instruction instr, Operand postCrement0 |
isUseAfterPostfixCrement(store, postCrement0) and
conversionFlow(postCrement0, instr, false, _) and
instr = postCrement.getDef()
)
}
private predicate hasSavedPostfixCrementSourceVariable(
BaseSourceVariable base, StoreInstruction store, int ind
) {
exists(BaseSourceVariableInstruction inst, int ind0 |
isUseAfterPostfixCrement(store, _) and
inst.getBaseSourceVariable() = base and
isDef(_, _, store.getDestinationAddressOperand(), inst, ind0, 0) and
ind = [ind0 .. countIndirectionsForCppType(base.getLanguageType()) + 1]
)
}
cached
private newtype TSourceVariable =
TMkSourceVariable(BaseSourceVariable base, int ind) {
TNormalSourceVariable(BaseSourceVariable base, int ind) {
ind = [0 .. countIndirectionsForCppType(base.getLanguageType()) + 1]
} or
TSavedPostfixCrementSourceVariable(StoreInstruction store, int ind) {
hasSavedPostfixCrementSourceVariable(_, store, ind)
}
class SourceVariable extends TSourceVariable {
abstract private class AbstractSourceVariable extends TSourceVariable {
BaseSourceVariable base;
int ind;
SourceVariable() { this = TMkSourceVariable(base, ind) }
bindingset[ind]
AbstractSourceVariable() { any() }
/** Gets the IR variable associated with this `SourceVariable`, if any. */
IRVariable getIRVariable() { result = base.(BaseIRVariable).getIRVariable() }
@@ -37,7 +99,7 @@ private module SourceVariables {
BaseSourceVariable getBaseVariable() { result = base }
/** Gets a textual representation of this element. */
string toString() { result = repeatStars(this.getIndirection()) + base.toString() }
abstract string toString();
/**
* Gets the number of loads performed on the base source variable
@@ -62,6 +124,53 @@ private module SourceVariables {
/** Gets the location of this variable. */
Location getLocation() { result = this.getBaseVariable().getLocation() }
}
final class SourceVariable = AbstractSourceVariable;
/**
* A regular source variable. Most source variables are instances of this
* class.
*/
class NormalSourceVariable extends AbstractSourceVariable, TNormalSourceVariable {
NormalSourceVariable() { this = TNormalSourceVariable(base, ind) }
final override string toString() {
result = repeatStars(this.getIndirection()) + base.toString()
}
}
/**
* Before a value is postfix incremented (or decremented) we "save" its
* current value so that the pre-incremented value can be returned to the
* enclosing expression. We use the source variables represented by this
* class to represent the "saved value".
*/
class SavedPostfixCrementSourceVariable extends AbstractSourceVariable,
TSavedPostfixCrementSourceVariable
{
StoreInstruction store;
SavedPostfixCrementSourceVariable() {
this = TSavedPostfixCrementSourceVariable(store, ind) and
hasSavedPostfixCrementSourceVariable(base, store, ind)
}
final override string toString() {
result = repeatStars(this.getIndirection()) + base.toString() + " [before crement]"
}
/**
* Gets the `StoreInstruction` that writes the incremented (or decremented)
* value.
*/
StoreInstruction getStoreInstruction() { result = store }
/**
* Gets the fully converted `Operand` that represents the use of the
* value before the increment.
*/
Operand getOperand() { isUseAfterPostfixCrement(store, result) }
}
}
import SourceVariables
@@ -109,17 +218,43 @@ private newtype TDefImpl =
TDirectDefImpl(Operand address, int indirectionIndex) {
isDef(_, _, address, _, _, indirectionIndex)
} or
TSavedPostfixCrementDefImpl(SavedPostfixCrementSourceVariable sv, int indirectionIndex) {
isDef(_, _, sv.getStoreInstruction().getDestinationAddressOperand(), _, sv.getIndirection(),
indirectionIndex)
} or
TGlobalDefImpl(GlobalLikeVariable v, IRFunction f, int indirectionIndex) {
// Represents the initial "definition" of a global variable when entering
// a function body.
isGlobalDefImpl(v, f, _, indirectionIndex)
}
pragma[nomagic]
private predicate hasOperandAndIndirection(
SavedPostfixCrementSourceVariable sv, Operand operand, int indirection
) {
sv.getOperand() = operand and
sv.getIndirection() = indirection
}
private predicate hasBeforePostCrementUseImpl(
SavedPostfixCrementSourceVariable sv, Operand operand, int indirectionIndex
) {
not isDef(true, _, operand, _, _, _) and
exists(int indirection |
hasOperandAndIndirection(sv, operand, indirection) and
isUse(_, operand, _, indirection, indirectionIndex)
)
}
cached
private newtype TUseImpl =
TDirectUseImpl(Operand operand, int indirectionIndex) {
isUse(_, operand, _, _, indirectionIndex) and
not isDef(true, _, operand, _, _, _)
not isDef(true, _, operand, _, _, _) and
not hasBeforePostCrementUseImpl(_, operand, indirectionIndex)
} or
TSavedPostfixCrementUseImpl(SavedPostfixCrementSourceVariable sv, int indirectionIndex) {
hasBeforePostCrementUseImpl(sv, _, indirectionIndex)
} or
TGlobalUse(GlobalLikeVariable v, IRFunction f, int indirectionIndex) {
// Represents a final "use" of a global variable to ensure that
@@ -223,19 +358,8 @@ abstract class DefImpl extends TDefImpl {
*/
abstract int getIndirection();
/**
* Gets the base source variable (i.e., the variable without
* any indirection) of this definition or use.
*/
abstract BaseSourceVariable getBaseSourceVariable();
/** Gets the variable that is defined or used. */
SourceVariable getSourceVariable() {
exists(BaseSourceVariable v, int indirection |
sourceVariableHasBaseAndIndex(result, v, indirection) and
defHasSourceVariable(this, v, indirection)
)
}
abstract SourceVariable getSourceVariable();
/**
* Holds if this definition is guaranteed to totally overwrite the
@@ -243,8 +367,8 @@ abstract class DefImpl extends TDefImpl {
*/
abstract predicate isCertain();
/** Gets the value written to the destination variable by this definition. */
abstract Node0Impl getValue();
/** Gets the value written to the destination variable by this definition, if any. */
Node0Impl getValue() { none() }
/** Gets the operand that represents the address of this definition, if any. */
Operand getAddressOperand() { none() }
@@ -293,19 +417,8 @@ abstract class UseImpl extends TUseImpl {
/** Gets the indirection index of this use. */
final int getIndirectionIndex() { result = indirectionIndex }
/**
* Gets the base source variable (i.e., the variable without
* any indirection) of this definition or use.
*/
abstract BaseSourceVariable getBaseSourceVariable();
/** Gets the variable that is defined or used. */
SourceVariable getSourceVariable() {
exists(BaseSourceVariable v, int indirection |
sourceVariableHasBaseAndIndex(result, v, indirection) and
useHasSourceVariable(this, v, indirection)
)
}
abstract SourceVariable getSourceVariable();
/**
* Holds if this use is guaranteed to read the
@@ -314,18 +427,6 @@ abstract class UseImpl extends TUseImpl {
abstract predicate isCertain();
}
pragma[noinline]
private predicate defHasSourceVariable(DefImpl def, BaseSourceVariable bv, int ind) {
bv = def.getBaseSourceVariable() and
ind = def.getIndirection()
}
pragma[noinline]
private predicate useHasSourceVariable(UseImpl use, BaseSourceVariable bv, int ind) {
bv = use.getBaseSourceVariable() and
ind = use.getIndirection()
}
pragma[noinline]
private predicate sourceVariableHasBaseAndIndex(SourceVariable v, BaseSourceVariable bv, int ind) {
v.getBaseVariable() = bv and
@@ -358,16 +459,12 @@ abstract private class DefAddressImpl extends DefImpl, TDefAddressImpl {
final override predicate isCertain() { any() }
final override Node0Impl getValue() { none() }
override Cpp::Location getLocation() { result = v.getLocation() }
final override SourceVariable getSourceVariable() {
final override NormalSourceVariable getSourceVariable() {
result.getBaseVariable() = v and
result.getIndirection() = 0
}
final override BaseSourceVariable getBaseSourceVariable() { result = v }
}
private class DefVariableAddressImpl extends DefAddressImpl {
@@ -413,8 +510,17 @@ private class DirectDef extends DefImpl, TDirectDefImpl {
isDef(_, _, address, result, _, indirectionIndex)
}
override BaseSourceVariable getBaseSourceVariable() {
result = this.getBase().getBaseSourceVariable()
pragma[nomagic]
private predicate hasBaseSourceVariableAndIndirection(BaseSourceVariable v, int indirection) {
v = this.getBase().getBaseSourceVariable() and
indirection = this.getIndirection()
}
final override NormalSourceVariable getSourceVariable() {
exists(BaseSourceVariable v, int indirection |
sourceVariableHasBaseAndIndex(result, v, indirection) and
this.hasBaseSourceVariableAndIndirection(v, indirection)
)
}
override int getIndirection() { isDef(_, _, address, _, result, indirectionIndex) }
@@ -424,6 +530,32 @@ private class DirectDef extends DefImpl, TDirectDefImpl {
override predicate isCertain() { isDef(true, _, address, _, _, indirectionIndex) }
}
/**
* A definition that "saves" the value of a variable before it is incremented
* or decremented.
*/
private class SavedPostfixCrementDefImpl extends DefImpl, TSavedPostfixCrementDefImpl {
SavedPostfixCrementSourceVariable sv;
SavedPostfixCrementDefImpl() { this = TSavedPostfixCrementDefImpl(sv, indirectionIndex) }
override Cpp::Location getLocation() { result = sv.getStoreInstruction().getLocation() }
final override predicate hasIndexInBlock(IRBlock block, int index) {
sv.getStoreInstruction() = block.getInstruction(index)
}
override string toString() { result = "Def of " + this.getSourceVariable() }
override SourceVariable getSourceVariable() { result = sv }
override int getIndirection() { result = sv.getIndirection() }
override predicate isCertain() {
isDef(true, _, sv.getStoreInstruction().getDestinationAddressOperand(), _, _, indirectionIndex)
}
}
private class DirectUseImpl extends UseImpl, TDirectUseImpl {
Operand operand;
@@ -432,29 +564,22 @@ private class DirectUseImpl extends UseImpl, TDirectUseImpl {
override string toString() { result = "Use of " + this.getSourceVariable() }
final override predicate hasIndexInBlock(IRBlock block, int index) {
// See the comment in `ssa0`'s `OperandBasedUse` for an explanation of this
// predicate's implementation.
if this.getBase().getAst() = any(Cpp::PostfixCrementOperation c).getOperand()
then
exists(Operand op, int indirection, Instruction base |
indirection = this.getIndirection() and
base = this.getBase() and
op =
min(Operand cand, int i |
isUse(_, cand, base, indirection, indirectionIndex) and
block.getInstruction(i) = cand.getUse()
|
cand order by i
) and
block.getInstruction(index) = op.getUse()
)
else operand.getUse() = block.getInstruction(index)
operand.getUse() = block.getInstruction(index)
}
private BaseSourceVariableInstruction getBase() { isUse(_, operand, result, _, indirectionIndex) }
override BaseSourceVariable getBaseSourceVariable() {
result = this.getBase().getBaseSourceVariable()
pragma[nomagic]
private predicate hasBaseSourceVariableAndIndirection(BaseSourceVariable bv, int indirection) {
this.getBase().getBaseSourceVariable() = bv and
this.getIndirection() = indirection
}
override NormalSourceVariable getSourceVariable() {
exists(BaseSourceVariable v, int indirection |
sourceVariableHasBaseAndIndex(result, v, indirection) and
this.hasBaseSourceVariableAndIndirection(v, indirection)
)
}
final Operand getOperand() { result = operand }
@@ -468,6 +593,34 @@ private class DirectUseImpl extends UseImpl, TDirectUseImpl {
override Node getNode() { nodeHasOperand(result, operand, indirectionIndex) }
}
/**
* The use of the original "saved" variable after the variable has been incremented
* or decremented.
*/
private class SavedPostfixCrementUseImpl extends UseImpl, TSavedPostfixCrementUseImpl {
SavedPostfixCrementSourceVariable sv;
SavedPostfixCrementUseImpl() { this = TSavedPostfixCrementUseImpl(sv, indirectionIndex) }
override string toString() { result = "Use of " + this.getSourceVariable() }
final override predicate hasIndexInBlock(IRBlock block, int index) {
this.getOperand().getUse() = block.getInstruction(index)
}
override SourceVariable getSourceVariable() { result = sv }
final Operand getOperand() { result = sv.getOperand() }
final override Cpp::Location getLocation() { result = this.getOperand().getLocation() }
override int getIndirection() { result = sv.getIndirection() }
override predicate isCertain() { isUse(true, this.getOperand(), _, _, indirectionIndex) }
override Node getNode() { nodeHasOperand(result, this.getOperand(), indirectionIndex) }
}
pragma[nomagic]
private predicate finalParameterNodeHasParameterAndIndex(
FinalParameterNode n, Parameter p, int indirectionIndex
@@ -532,7 +685,18 @@ class FinalParameterUse extends UseImpl, TFinalParameterUse {
result instanceof UnknownLocation
}
override BaseIRVariable getBaseSourceVariable() { result.getIRVariable().getAst() = p }
pragma[nomagic]
private predicate hasBaseSourceVariableAndIndirection(BaseIRVariable v, int indirection) {
v.getIRVariable().getAst() = p and
indirection = this.getIndirection()
}
override NormalSourceVariable getSourceVariable() {
exists(BaseIRVariable v, int indirection |
sourceVariableHasBaseAndIndex(result, v, indirection) and
this.hasBaseSourceVariableAndIndirection(v, indirection)
)
}
}
/**
@@ -612,8 +776,17 @@ class GlobalUse extends UseImpl, TGlobalUse {
hasReturnPosition(f, block, index)
}
override BaseSourceVariable getBaseSourceVariable() {
baseSourceVariableIsGlobal(result, global, f)
pragma[nomagic]
private predicate hasBaseSourceVariableAndIndirection(BaseIRVariable v, int indirection) {
baseSourceVariableIsGlobal(v, global, f) and
indirection = this.getIndirection()
}
override NormalSourceVariable getSourceVariable() {
exists(BaseIRVariable v, int indirection |
sourceVariableHasBaseAndIndex(result, v, indirection) and
this.hasBaseSourceVariableAndIndirection(v, indirection)
)
}
final override Cpp::Location getLocation() { result = f.getLocation() }
@@ -658,15 +831,15 @@ class GlobalDefImpl extends DefImpl, TGlobalDefImpl {
)
}
/** Gets the global variable associated with this definition. */
override BaseSourceVariable getBaseSourceVariable() {
baseSourceVariableIsGlobal(result, global, f)
final override NormalSourceVariable getSourceVariable() {
exists(BaseSourceVariable v |
sourceVariableHasBaseAndIndex(result, v, indirectionIndex) and
baseSourceVariableIsGlobal(v, global, f)
)
}
override int getIndirection() { result = indirectionIndex }
override Node0Impl getValue() { none() }
override predicate isCertain() { any() }
/**
@@ -704,9 +877,15 @@ predicate defToNode(Node node, Definition def, SourceVariable sv) {
}
private predicate defToNode(Node node, Definition def) {
nodeHasOperand(node, def.getValue().asOperand(), def.getIndirectionIndex())
or
nodeHasInstruction(node, def.getValue().asInstruction(), def.getIndirectionIndex())
// Only definitions of `NormalSourceVariable` need to be converted into
// dataflow nodes. The other case, `SavedPostfixCrementSourceVariable`,
// are internal definitions that don't have a dataflow node representation.
def.getSourceVariable() instanceof NormalSourceVariable and
(
nodeHasOperand(node, def.getValue().asOperand(), def.getIndirectionIndex())
or
nodeHasInstruction(node, def.getValue().asInstruction(), def.getIndirectionIndex())
)
or
node.(InitialGlobalValue).getGlobalDef() = def
}

View File

@@ -147,6 +147,29 @@ astFlow
| test.cpp:1165:10:1165:15 | call to source | test.cpp:1239:10:1239:26 | * ... |
| test.cpp:1195:10:1195:24 | call to indirect_source | test.cpp:1200:19:1200:36 | global_int_ptr_ptr |
| test.cpp:1195:10:1195:24 | call to indirect_source | test.cpp:1201:10:1201:27 | global_int_ptr_ptr |
| test.cpp:1258:11:1258:16 | call to source | test.cpp:1259:8:1259:10 | ... ++ |
| test.cpp:1262:7:1262:12 | call to source | test.cpp:1263:8:1263:10 | ... -- |
| test.cpp:1266:7:1266:12 | call to source | test.cpp:1267:8:1267:10 | ++ ... |
| test.cpp:1266:7:1266:12 | call to source | test.cpp:1268:8:1268:8 | x |
| test.cpp:1270:7:1270:12 | call to source | test.cpp:1271:8:1271:10 | -- ... |
| test.cpp:1270:7:1270:12 | call to source | test.cpp:1272:8:1272:8 | x |
| test.cpp:1274:7:1274:12 | call to source | test.cpp:1275:8:1275:14 | ... += ... |
| test.cpp:1274:7:1274:12 | call to source | test.cpp:1276:8:1276:8 | x |
| test.cpp:1278:7:1278:12 | call to source | test.cpp:1279:8:1279:14 | ... -= ... |
| test.cpp:1278:7:1278:12 | call to source | test.cpp:1280:8:1280:8 | x |
| test.cpp:1284:11:1284:16 | call to source | test.cpp:1285:8:1285:20 | ... ? ... : ... |
| test.cpp:1288:7:1288:12 | call to source | test.cpp:1289:8:1289:20 | ... ++ |
| test.cpp:1288:7:1288:12 | call to source | test.cpp:1290:8:1290:8 | x |
| test.cpp:1292:7:1292:12 | call to source | test.cpp:1294:8:1294:8 | x |
| test.cpp:1296:7:1296:12 | call to source | test.cpp:1297:8:1297:18 | ... ? ... : ... |
| test.cpp:1296:7:1296:12 | call to source | test.cpp:1298:8:1298:8 | x |
| test.cpp:1300:7:1300:12 | call to source | test.cpp:1301:8:1301:18 | ... ? ... : ... |
| test.cpp:1300:7:1300:12 | call to source | test.cpp:1302:8:1302:8 | x |
| test.cpp:1304:7:1304:12 | call to source | test.cpp:1305:8:1305:18 | ... ? ... : ... |
| test.cpp:1304:7:1304:12 | call to source | test.cpp:1306:8:1306:8 | x |
| test.cpp:1308:7:1308:12 | call to source | test.cpp:1309:14:1309:16 | ... ++ |
| test.cpp:1312:7:1312:12 | call to source | test.cpp:1313:8:1313:24 | ... ? ... : ... |
| test.cpp:1312:7:1312:12 | call to source | test.cpp:1314:8:1314:8 | x |
| true_upon_entry.cpp:17:11:17:16 | call to source | true_upon_entry.cpp:21:8:21:8 | x |
| true_upon_entry.cpp:27:9:27:14 | call to source | true_upon_entry.cpp:29:8:29:8 | x |
| true_upon_entry.cpp:33:11:33:16 | call to source | true_upon_entry.cpp:39:8:39:8 | x |
@@ -354,6 +377,19 @@ irFlow
| test.cpp:1195:10:1195:24 | *call to indirect_source | test.cpp:1218:19:1218:36 | **global_int_ptr_ptr |
| test.cpp:1195:10:1195:24 | *call to indirect_source | test.cpp:1224:19:1224:37 | ** ... |
| test.cpp:1195:10:1195:24 | *call to indirect_source | test.cpp:1227:10:1227:29 | * ... |
| test.cpp:1258:11:1258:16 | call to source | test.cpp:1259:8:1259:10 | ... ++ |
| test.cpp:1262:7:1262:12 | call to source | test.cpp:1263:8:1263:10 | ... -- |
| test.cpp:1284:11:1284:16 | call to source | test.cpp:1285:8:1285:20 | ... ? ... : ... |
| test.cpp:1288:7:1288:12 | call to source | test.cpp:1290:8:1290:8 | x |
| test.cpp:1292:7:1292:12 | call to source | test.cpp:1294:8:1294:8 | x |
| test.cpp:1296:7:1296:12 | call to source | test.cpp:1297:8:1297:18 | ... ? ... : ... |
| test.cpp:1296:7:1296:12 | call to source | test.cpp:1298:8:1298:8 | x |
| test.cpp:1300:7:1300:12 | call to source | test.cpp:1301:8:1301:18 | ... ? ... : ... |
| test.cpp:1300:7:1300:12 | call to source | test.cpp:1302:8:1302:8 | x |
| test.cpp:1304:7:1304:12 | call to source | test.cpp:1306:8:1306:8 | x |
| test.cpp:1308:7:1308:12 | call to source | test.cpp:1309:8:1309:16 | ... ++ |
| test.cpp:1312:7:1312:12 | call to source | test.cpp:1313:8:1313:24 | ... ? ... : ... |
| test.cpp:1312:7:1312:12 | call to source | test.cpp:1314:8:1314:8 | x |
| true_upon_entry.cpp:9:11:9:16 | call to source | true_upon_entry.cpp:13:8:13:8 | x |
| true_upon_entry.cpp:17:11:17:16 | call to source | true_upon_entry.cpp:21:8:21:8 | x |
| true_upon_entry.cpp:27:9:27:14 | call to source | true_upon_entry.cpp:29:8:29:8 | x |

View File

@@ -1252,4 +1252,64 @@ namespace globals_without_explicit_def {
calls_set_array();
sink(*global_int_array); // $ ir MISSING: ast
}
}
void crement_test1() {
int x = source();
sink(x++); // $ ir ast
sink(x);
x = source();
sink(x--); // $ ir ast
sink(x);
x = source();
sink(++x); // $ SPURIOUS: ast
sink(x); // $ SPURIOUS: ast
x = source();
sink(--x); // $ SPURIOUS: ast
sink(x); // $ SPURIOUS: ast
x = source();
sink(x += 10); // $ SPURIOUS: ast
sink(x); // $ SPURIOUS: ast
x = source();
sink(x -= 10); // $ SPURIOUS: ast
sink(x); // $ SPURIOUS: ast
}
void crement_test2(bool b, int y) {
int x = source();
sink(b ? x++ : x--); // $ ir ast
sink(x);
x = source();
sink((b ? x : y)++); // $ ast MISSING: ir
sink(x); // $ ir ast
x = source();
sink(++(b ? x : y));
sink(x); // $ ir ast
x = source();
sink(b ? x++ : y); // $ ir ast
sink(x); // $ ir ast
x = source();
sink(b ? x : y++); // $ ir ast
sink(x); // $ ir ast
x = source();
sink(b ? ++x : y); // $ SPURIOUS: ast
sink(x); // $ ir ast
x = source();
sink((long)x++); // $ ir ast
sink(x);
x = source();
sink(b ? (long)x++ : 0); // $ ir ast
sink(x); // $ ir ast
}