Skip to content

Commit 56aacb1

Browse files
MathiasVPMathiasVP
authored
Merge pull request #20145 from MathiasVP/fix-type-error-in-ir
C++: Fix missing `bool` -> `int` conversions in C code
2 parents 874f951 + 14345a8 commit 56aacb1

File tree

7 files changed

+133
-32
lines changed

7 files changed

+133
-32
lines changed

‎cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/InstructionTag.qll

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,8 @@ newtype TInstructionTag =
9696
exists(Expr e | exists(e.getImplicitDestructorCall(index))) or
9797
exists(Stmt s | exists(s.getImplicitDestructorCall(index)))
9898
} or
99-
CoAwaitBranchTag()
99+
CoAwaitBranchTag() or
100+
BoolToIntConversionTag()
100101

101102
class InstructionTag extends TInstructionTag {
102103
final string toString() { result = getInstructionTagId(this) }
@@ -286,4 +287,6 @@ string getInstructionTagId(TInstructionTag tag) {
286287
)
287288
or
288289
tag = CoAwaitBranchTag() and result = "CoAwaitBranch"
290+
or
291+
tag = BoolToIntConversionTag() and result = "BoolToIntConversion"
289292
}

‎cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/TranslatedElement.qll

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -509,6 +509,41 @@ predicate hasTranslatedSyntheticTemporaryObject(Expr expr) {
509509
not expr.hasLValueToRValueConversion()
510510
}
511511

512+
Opcode comparisonOpcode(ComparisonOperation expr) {
513+
expr instanceof EQExpr and result instanceof Opcode::CompareEQ
514+
or
515+
expr instanceof NEExpr and result instanceof Opcode::CompareNE
516+
or
517+
expr instanceof LTExpr and result instanceof Opcode::CompareLT
518+
or
519+
expr instanceof GTExpr and result instanceof Opcode::CompareGT
520+
or
521+
expr instanceof LEExpr and result instanceof Opcode::CompareLE
522+
or
523+
expr instanceof GEExpr and result instanceof Opcode::CompareGE
524+
}
525+
526+
private predicate parentExpectsBool(Expr child) {
527+
any(NotExpr notExpr).getOperand() = child
528+
or
529+
usedAsCondition(child)
530+
}
531+
532+
/**
533+
* Holds if `expr` should have a `TranslatedSyntheticBoolToIntConversion` on it.
534+
*/
535+
predicate hasTranslatedSyntheticBoolToIntConversion(Expr expr) {
536+
not ignoreExpr(expr) and
537+
not isIRConstant(expr) and
538+
not parentExpectsBool(expr) and
539+
expr.getUnspecifiedType() instanceof IntType and
540+
(
541+
expr instanceof NotExpr
542+
or
543+
exists(comparisonOpcode(expr))
544+
)
545+
}
546+
512547
class StaticInitializedStaticLocalVariable extends StaticLocalVariable {
513548
StaticInitializedStaticLocalVariable() {
514549
this.hasInitializer() and
@@ -647,6 +682,9 @@ newtype TTranslatedElement =
647682
// A temporary object that we had to synthesize ourselves, so that we could do a field access or
648683
// method call on a prvalue.
649684
TTranslatedSyntheticTemporaryObject(Expr expr) { hasTranslatedSyntheticTemporaryObject(expr) } or
685+
TTranslatedSyntheticBoolToIntConversion(Expr expr) {
686+
hasTranslatedSyntheticBoolToIntConversion(expr)
687+
} or
650688
// For expressions that would not otherwise generate an instruction.
651689
TTranslatedResultCopy(Expr expr) {
652690
not ignoreExpr(expr) and

‎cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/TranslatedExpr.qll

Lines changed: 43 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,8 @@ abstract class TranslatedCoreExpr extends TranslatedExpr {
216216
not hasTranslatedLoad(expr) and
217217
not hasTranslatedSyntheticTemporaryObject(expr) and
218218
// If there's a result copy, then this expression's result is the copy.
219-
not exprNeedsCopyIfNotLoaded(expr)
219+
not exprNeedsCopyIfNotLoaded(expr) and
220+
not hasTranslatedSyntheticBoolToIntConversion(expr)
220221
}
221222
}
222223

@@ -358,11 +359,12 @@ class TranslatedConditionValue extends TranslatedCoreExpr, ConditionContext,
358359
}
359360

360361
/**
361-
* The IR translation of a node synthesized to adjust the value category of its operand.
362+
* The IR translation of a node synthesized to adjust the value category or type of its operand.
362363
* One of:
363364
* - `TranslatedLoad` - Convert from glvalue to prvalue by loading from the location.
364365
* - `TranslatedSyntheticTemporaryObject` - Convert from prvalue to glvalue by storing to a
365366
* temporary variable.
367+
* - `TranslatedSyntheticBoolToIntConversion` - Convert a prvalue Boolean to a prvalue integer.
366368
*/
367369
abstract class TranslatedValueCategoryAdjustment extends TranslatedExpr {
368370
final override Instruction getFirstInstruction(EdgeKind kind) {
@@ -513,6 +515,45 @@ class TranslatedSyntheticTemporaryObject extends TranslatedValueCategoryAdjustme
513515
}
514516
}
515517

518+
class TranslatedSyntheticBoolToIntConversion extends TranslatedValueCategoryAdjustment,
519+
TTranslatedSyntheticBoolToIntConversion
520+
{
521+
TranslatedSyntheticBoolToIntConversion() { this = TTranslatedSyntheticBoolToIntConversion(expr) }
522+
523+
override string toString() { result = "Bool-to-int conversion of " + expr.toString() }
524+
525+
override predicate hasInstruction(Opcode opcode, InstructionTag tag, CppType resultType) {
526+
opcode instanceof Opcode::Convert and
527+
tag = BoolToIntConversionTag() and
528+
resultType = getIntType()
529+
}
530+
531+
override predicate isResultGLValue() { none() }
532+
533+
override Instruction getInstructionSuccessorInternal(InstructionTag tag, EdgeKind kind) {
534+
tag = BoolToIntConversionTag() and
535+
result = this.getParent().getChildSuccessor(this, kind)
536+
}
537+
538+
override Instruction getALastInstructionInternal() {
539+
result = this.getInstruction(BoolToIntConversionTag())
540+
}
541+
542+
override Instruction getChildSuccessorInternal(TranslatedElement child, EdgeKind kind) {
543+
child = this.getOperand() and
544+
result = this.getInstruction(BoolToIntConversionTag()) and
545+
kind instanceof GotoEdge
546+
}
547+
548+
override Instruction getResult() { result = this.getInstruction(BoolToIntConversionTag()) }
549+
550+
override Instruction getInstructionRegisterOperand(InstructionTag tag, OperandTag operandTag) {
551+
tag = BoolToIntConversionTag() and
552+
operandTag instanceof UnaryOperandTag and
553+
result = this.getOperand().getResult()
554+
}
555+
}
556+
516557
/**
517558
* IR translation of an expression that simply returns its result. We generate an otherwise useless
518559
* `CopyValue` instruction for these expressions so that there is at least one instruction
@@ -1794,20 +1835,6 @@ private Opcode binaryArithmeticOpcode(BinaryArithmeticOperation expr) {
17941835
expr instanceof PointerDiffExpr and result instanceof Opcode::PointerDiff
17951836
}
17961837

1797-
private Opcode comparisonOpcode(ComparisonOperation expr) {
1798-
expr instanceof EQExpr and result instanceof Opcode::CompareEQ
1799-
or
1800-
expr instanceof NEExpr and result instanceof Opcode::CompareNE
1801-
or
1802-
expr instanceof LTExpr and result instanceof Opcode::CompareLT
1803-
or
1804-
expr instanceof GTExpr and result instanceof Opcode::CompareGT
1805-
or
1806-
expr instanceof LEExpr and result instanceof Opcode::CompareLE
1807-
or
1808-
expr instanceof GEExpr and result instanceof Opcode::CompareGE
1809-
}
1810-
18111838
private Opcode spaceShipOpcode(SpaceshipExpr expr) {
18121839
exists(expr) and
18131840
result instanceof Opcode::Spaceship

‎cpp/ql/test/library-tests/ir/ir/PrintAST.expected

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4932,7 +4932,20 @@ ir.c:
49324932
# 103| Type = [IntType] int
49334933
# 103| ValueCategory = prvalue(load)
49344934
# 103| getThen(): [BlockStmt] { ... }
4935-
# 104| getStmt(16): [ReturnStmt] return ...
4935+
# 105| getStmt(16): [DeclStmt] declaration
4936+
# 105| getDeclarationEntry(0): [VariableDeclarationEntry] definition of double_negation
4937+
# 105| Type = [IntType] int
4938+
# 105| getVariable().getInitializer(): [Initializer] initializer for double_negation
4939+
# 105| getExpr(): [NotExpr] ! ...
4940+
# 105| Type = [IntType] int
4941+
# 105| ValueCategory = prvalue
4942+
# 105| getOperand(): [NotExpr] ! ...
4943+
# 105| Type = [IntType] int
4944+
# 105| ValueCategory = prvalue
4945+
# 105| getOperand(): [VariableAccess] x1
4946+
# 105| Type = [IntType] int
4947+
# 105| ValueCategory = prvalue(load)
4948+
# 106| getStmt(17): [ReturnStmt] return ...
49364949
ir.cpp:
49374950
# 1| [TopLevelFunction] void Constants()
49384951
# 1| <params>:

‎cpp/ql/test/library-tests/ir/ir/aliased_ir.expected

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3706,9 +3706,10 @@ ir.c:
37063706
# 88| r88_3(int) = Load[x1] : &:r88_2, m84_6
37073707
# 88| r88_4(int) = Constant[0] :
37083708
# 88| r88_5(bool) = CompareEQ : r88_3, r88_4
3709-
# 88| m88_6(int) = Store[y] : &:r88_1, r88_5
3709+
# 88| r88_6(int) = Convert : r88_5
3710+
# 88| m88_7(int) = Store[y] : &:r88_1, r88_6
37103711
# 89| r89_1(glval<int>) = VariableAddress[y] :
3711-
# 89| r89_2(int) = Load[y] : &:r89_1, m88_6
3712+
# 89| r89_2(int) = Load[y] : &:r89_1, m88_7
37123713
# 89| r89_3(int) = Constant[0] :
37133714
# 89| r89_4(bool) = CompareNE : r89_2, r89_3
37143715
# 89| v89_5(void) = ConditionalBranch : r89_4
@@ -3721,7 +3722,7 @@ ir.c:
37213722

37223723
# 90| Block 6
37233724
# 90| r90_1(glval<int>) = VariableAddress[y] :
3724-
# 90| r90_2(int) = Load[y] : &:r90_1, m88_6
3725+
# 90| r90_2(int) = Load[y] : &:r90_1, m88_7
37253726
# 90| r90_3(int) = Constant[0] :
37263727
# 90| r90_4(bool) = CompareEQ : r90_2, r90_3
37273728
# 90| v90_5(void) = ConditionalBranch : r90_4
@@ -3969,11 +3970,19 @@ ir.c:
39693970
# 103| v103_6(void) = NoOp :
39703971
#-----| Goto -> Block 40
39713972

3972-
# 104| Block 40
3973-
# 104| v104_1(void) = NoOp :
3974-
# 84| v84_9(void) = ReturnVoid :
3975-
# 84| v84_10(void) = AliasedUse : m84_3
3976-
# 84| v84_11(void) = ExitFunction :
3973+
# 105| Block 40
3974+
# 105| r105_1(glval<int>) = VariableAddress[double_negation] :
3975+
# 105| r105_2(glval<int>) = VariableAddress[x1] :
3976+
# 105| r105_3(int) = Load[x1] : &:r105_2, m84_6
3977+
# 105| r105_4(int) = Constant[0] :
3978+
# 105| r105_5(bool) = CompareEQ : r105_3, r105_4
3979+
# 105| r105_6(bool) = LogicalNot : r105_5
3980+
# 105| r105_7(int) = Convert : r105_6
3981+
# 105| m105_8(int) = Store[double_negation] : &:r105_1, r105_7
3982+
# 106| v106_1(void) = NoOp :
3983+
# 84| v84_9(void) = ReturnVoid :
3984+
# 84| v84_10(void) = AliasedUse : m84_3
3985+
# 84| v84_11(void) = ExitFunction :
39773986

39783987
ir.cpp:
39793988
# 1| void Constants()

‎cpp/ql/test/library-tests/ir/ir/ir.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,8 @@ void branch_on_integral_in_c(int x1, int x2) {
101101
int x_1_and_2 = x1 && x2;
102102
if(x_1_and_2) {}
103103
if(!x_1_and_2) {}
104+
105+
int double_negation = !!x1;
104106
}
105107

106108
// semmle-extractor-options: --microsoft

‎cpp/ql/test/library-tests/ir/ir/raw_ir.expected

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3343,7 +3343,8 @@ ir.c:
33433343
# 88| r88_3(int) = Load[x1] : &:r88_2, ~m?
33443344
# 88| r88_4(int) = Constant[0] :
33453345
# 88| r88_5(bool) = CompareEQ : r88_3, r88_4
3346-
# 88| mu88_6(int) = Store[y] : &:r88_1, r88_5
3346+
# 88| r88_6(int) = Convert : r88_5
3347+
# 88| mu88_7(int) = Store[y] : &:r88_1, r88_6
33473348
# 89| r89_1(glval<int>) = VariableAddress[y] :
33483349
# 89| r89_2(int) = Load[y] : &:r89_1, ~m?
33493350
# 89| r89_3(int) = Constant[0] :
@@ -3605,11 +3606,19 @@ ir.c:
36053606
# 103| v103_6(void) = NoOp :
36063607
#-----| Goto -> Block 40
36073608

3608-
# 104| Block 40
3609-
# 104| v104_1(void) = NoOp :
3610-
# 84| v84_8(void) = ReturnVoid :
3611-
# 84| v84_9(void) = AliasedUse : ~m?
3612-
# 84| v84_10(void) = ExitFunction :
3609+
# 105| Block 40
3610+
# 105| r105_1(glval<int>) = VariableAddress[double_negation] :
3611+
# 105| r105_2(glval<int>) = VariableAddress[x1] :
3612+
# 105| r105_3(int) = Load[x1] : &:r105_2, ~m?
3613+
# 105| r105_4(int) = Constant[0] :
3614+
# 105| r105_5(bool) = CompareEQ : r105_3, r105_4
3615+
# 105| r105_6(bool) = LogicalNot : r105_5
3616+
# 105| r105_7(int) = Convert : r105_6
3617+
# 105| mu105_8(int) = Store[double_negation] : &:r105_1, r105_7
3618+
# 106| v106_1(void) = NoOp :
3619+
# 84| v84_8(void) = ReturnVoid :
3620+
# 84| v84_9(void) = AliasedUse : ~m?
3621+
# 84| v84_10(void) = ExitFunction :
36133622

36143623
ir.cpp:
36153624
# 1| void Constants()

0 commit comments

Comments
 (0)