Skip to content

Commit 3c8c458

Browse files
jketemajketema
authored
Merge pull request github#16060 from jketema/qual-fix
C++: Output destructor calls for delete expressions
2 parents 9409d7f + 0118380 commit 3c8c458

File tree

10 files changed

+405
-178
lines changed

10 files changed

+405
-178
lines changed

‎cpp/ql/lib/semmle/code/cpp/PrintAST.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -862,7 +862,7 @@ private predicate namedExprChildPredicates(Expr expr, Element ele, string pred)
862862
or
863863
expr.(DeleteOrDeleteArrayExpr).getDestructorCall() = ele and pred = "getDestructorCall()"
864864
or
865-
expr.(DeleteOrDeleteArrayExpr).getExpr() = ele and pred = "getExpr()"
865+
expr.(DeleteOrDeleteArrayExpr).getExprWithReuse() = ele and pred = "getExprWithReuse()"
866866
or
867867
expr.(DestructorFieldDestruction).getExpr() = ele and pred = "getExpr()"
868868
or

‎cpp/ql/lib/semmle/code/cpp/exprs/Expr.qll

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1015,8 +1015,33 @@ class DeleteOrDeleteArrayExpr extends Expr, TDeleteOrDeleteArrayExpr {
10151015
Expr getExpr() {
10161016
// If there is a destructor call, the object being deleted is the qualifier
10171017
// otherwise it is the third child.
1018-
result = this.getChild(3) or result = this.getDestructorCall().getQualifier()
1018+
exists(Expr exprWithReuse | exprWithReuse = this.getExprWithReuse() |
1019+
if not exprWithReuse instanceof ReuseExpr
1020+
then result = exprWithReuse
1021+
else result = this.getDestructorCall().getQualifier()
1022+
)
10191023
}
1024+
1025+
/**
1026+
* Gets the object or array being deleted, and gets a `ReuseExpr` when there
1027+
* is a destructor call and the object is also the qualifier of the call.
1028+
*
1029+
* For example, given:
1030+
* ```
1031+
* struct HasDestructor { ~HasDestructor(); };
1032+
* struct PlainOldData { int x, char y; };
1033+
*
1034+
* void f(HasDestructor* hasDestructor, PlainOldData* pod) {
1035+
* delete hasDestructor;
1036+
* delete pod;
1037+
* }
1038+
* ```
1039+
* This predicate yields a `ReuseExpr` for `delete hasDestructor`, as the
1040+
* the deleted expression has a destructor, and that expression is also
1041+
* the qualifier of the destructor call. In the case of `delete pod` the
1042+
* predicate does not yield a `ReuseExpr`, as there is no destructor call.
1043+
*/
1044+
Expr getExprWithReuse() { result = this.getChild(3) }
10201045
}
10211046

10221047
/**
@@ -1340,7 +1365,17 @@ class ReuseExpr extends Expr, @reuseexpr {
13401365
/**
13411366
* Gets the expression that is being re-used.
13421367
*/
1343-
Expr getReusedExpr() { expr_reuse(underlyingElement(this), unresolveElement(result), _) }
1368+
Expr getReusedExpr() {
1369+
// In the case of a prvalue, the extractor outputs the expression
1370+
// before conversion, but the converted expression is intended.
1371+
if this.isPRValueCategory()
1372+
then result = this.getBaseReusedExpr().getFullyConverted()
1373+
else result = this.getBaseReusedExpr()
1374+
}
1375+
1376+
private Expr getBaseReusedExpr() {
1377+
expr_reuse(underlyingElement(this), unresolveElement(result), _)
1378+
}
13441379

13451380
override Type getType() { result = this.getReusedExpr().getType() }
13461381

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

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -150,11 +150,6 @@ private predicate ignoreExprOnly(Expr expr) {
150150
or
151151
not translateFunction(getEnclosingFunction(expr)) and
152152
not Raw::varHasIRFunc(getEnclosingVariable(expr))
153-
or
154-
exists(DeleteOrDeleteArrayExpr deleteExpr |
155-
// Ignore the destructor call, because the duplicated qualifier breaks control flow.
156-
deleteExpr.getDestructorCall() = expr
157-
)
158153
}
159154

160155
/**

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

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2245,7 +2245,11 @@ class TranslatedDeleteOrDeleteArrayExpr extends TranslatedNonConstantExpr, Trans
22452245

22462246
final override Type getCallResultType() { result = expr.getType() }
22472247

2248-
final override TranslatedExpr getQualifier() { none() }
2248+
final override TranslatedExpr getQualifier() {
2249+
result = getTranslatedExpr(expr.getDestructorCall())
2250+
}
2251+
2252+
final override Instruction getQualifierResult() { none() }
22492253

22502254
final override predicate hasArguments() {
22512255
// All deallocator calls have at least one argument.
@@ -2260,7 +2264,7 @@ class TranslatedDeleteOrDeleteArrayExpr extends TranslatedNonConstantExpr, Trans
22602264
final override TranslatedExpr getArgument(int index) {
22612265
// The only argument we define is the pointer to be deallocated.
22622266
index = 0 and
2263-
result = getTranslatedExpr(expr.getExpr().getFullyConverted())
2267+
result = getTranslatedExpr(expr.getExprWithReuse().getFullyConverted())
22642268
}
22652269

22662270
final override predicate mayThrowException() {

‎cpp/ql/test/examples/expressions/PrintAST.expected

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -426,11 +426,14 @@ DestructorCall.cpp:
426426
# 12| getQualifier(): [VariableAccess] c
427427
# 12| Type = [PointerType] C *
428428
# 12| ValueCategory = prvalue(load)
429+
# 12| getExprWithReuse(): [ReuseExpr] reuse of c
430+
# 12| Type = [PointerType] C *
431+
# 12| ValueCategory = prvalue
429432
# 13| getStmt(1): [ExprStmt] ExprStmt
430433
# 13| getExpr(): [DeleteExpr] delete
431434
# 13| Type = [VoidType] void
432435
# 13| ValueCategory = prvalue
433-
# 13| getExpr(): [VariableAccess] d
436+
# 13| getExprWithReuse(): [VariableAccess] d
434437
# 13| Type = [PointerType] D *
435438
# 13| ValueCategory = prvalue(load)
436439
# 14| getStmt(2): [ReturnStmt] return ...

‎cpp/ql/test/library-tests/compiler_generated/compilerGenerated.expected

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
| cpp.cpp:10:7:10:7 | operator= | Function |
1515
| cpp.cpp:10:7:10:7 | ~MyClass | Function |
1616
| cpp.cpp:15:5:15:12 | call to ~MyClass | Expr |
17+
| cpp.cpp:15:12:15:12 | reuse of m | Expr |
1718
| cpp.cpp:16:1:16:1 | return ... | Stmt |
1819
| file://:0:0:0:0 | operator delete | Function |
1920
| file://:0:0:0:0 | operator new | Function |

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

Lines changed: 35 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -9068,11 +9068,11 @@ ir.cpp:
90689068
# 1016| getExpr(): [DeleteExpr] delete
90699069
# 1016| Type = [VoidType] void
90709070
# 1016| ValueCategory = prvalue
9071-
# 1016| getExpr(): [Literal] 0
9071+
# 1016| getExprWithReuse(): [Literal] 0
90729072
# 1016| Type = [NullPointerType] decltype(nullptr)
90739073
# 1016| Value = [Literal] 0
90749074
# 1016| ValueCategory = prvalue
9075-
# 1016| getExpr().getFullyConverted(): [StaticCast] static_cast<int *>...
9075+
# 1016| getExprWithReuse().getFullyConverted(): [StaticCast] static_cast<int *>...
90769076
# 1016| Conversion = [PointerConversion] pointer conversion
90779077
# 1016| Type = [IntPointerType] int *
90789078
# 1016| Value = [StaticCast] 0
@@ -9093,18 +9093,21 @@ ir.cpp:
90939093
# 1017| Type = [PointerType] String *
90949094
# 1017| Value = [StaticCast] 0
90959095
# 1017| ValueCategory = prvalue
9096+
# 1017| getExprWithReuse(): [ReuseExpr] reuse of static_cast<String *>...
9097+
# 1017| Type = [PointerType] String *
9098+
# 1017| ValueCategory = prvalue
90969099
# 1018| getStmt(2): [ExprStmt] ExprStmt
90979100
# 1018| getExpr(): [DeleteExpr] delete
90989101
# 1018| Type = [VoidType] void
90999102
# 1018| ValueCategory = prvalue
91009103
# 1018| getDeallocatorCall(): [FunctionCall] call to operator delete
91019104
# 1018| Type = [VoidType] void
91029105
# 1018| ValueCategory = prvalue
9103-
# 1018| getExpr(): [Literal] 0
9106+
# 1018| getExprWithReuse(): [Literal] 0
91049107
# 1018| Type = [NullPointerType] decltype(nullptr)
91059108
# 1018| Value = [Literal] 0
91069109
# 1018| ValueCategory = prvalue
9107-
# 1018| getExpr().getFullyConverted(): [StaticCast] static_cast<SizedDealloc *>...
9110+
# 1018| getExprWithReuse().getFullyConverted(): [StaticCast] static_cast<SizedDealloc *>...
91089111
# 1018| Conversion = [PointerConversion] pointer conversion
91099112
# 1018| Type = [PointerType] SizedDealloc *
91109113
# 1018| Value = [StaticCast] 0
@@ -9113,11 +9116,11 @@ ir.cpp:
91139116
# 1019| getExpr(): [DeleteExpr] delete
91149117
# 1019| Type = [VoidType] void
91159118
# 1019| ValueCategory = prvalue
9116-
# 1019| getExpr(): [Literal] 0
9119+
# 1019| getExprWithReuse(): [Literal] 0
91179120
# 1019| Type = [NullPointerType] decltype(nullptr)
91189121
# 1019| Value = [Literal] 0
91199122
# 1019| ValueCategory = prvalue
9120-
# 1019| getExpr().getFullyConverted(): [StaticCast] static_cast<Overaligned *>...
9123+
# 1019| getExprWithReuse().getFullyConverted(): [StaticCast] static_cast<Overaligned *>...
91219124
# 1019| Conversion = [PointerConversion] pointer conversion
91229125
# 1019| Type = [PointerType] Overaligned *
91239126
# 1019| Value = [StaticCast] 0
@@ -9138,6 +9141,9 @@ ir.cpp:
91389141
# 1020| Type = [PointerType] PolymorphicBase *
91399142
# 1020| Value = [StaticCast] 0
91409143
# 1020| ValueCategory = prvalue
9144+
# 1020| getExprWithReuse(): [ReuseExpr] reuse of static_cast<PolymorphicBase *>...
9145+
# 1020| Type = [PointerType] PolymorphicBase *
9146+
# 1020| ValueCategory = prvalue
91419147
# 1021| getStmt(5): [ReturnStmt] return ...
91429148
# 1024| [TopLevelFunction] void OperatorDeleteArray()
91439149
# 1024| <params>:
@@ -9146,11 +9152,11 @@ ir.cpp:
91469152
# 1025| getExpr(): [DeleteArrayExpr] delete[]
91479153
# 1025| Type = [VoidType] void
91489154
# 1025| ValueCategory = prvalue
9149-
# 1025| getExpr(): [Literal] 0
9155+
# 1025| getExprWithReuse(): [Literal] 0
91509156
# 1025| Type = [NullPointerType] decltype(nullptr)
91519157
# 1025| Value = [Literal] 0
91529158
# 1025| ValueCategory = prvalue
9153-
# 1025| getExpr().getFullyConverted(): [StaticCast] static_cast<int *>...
9159+
# 1025| getExprWithReuse().getFullyConverted(): [StaticCast] static_cast<int *>...
91549160
# 1025| Conversion = [PointerConversion] pointer conversion
91559161
# 1025| Type = [IntPointerType] int *
91569162
# 1025| Value = [StaticCast] 0
@@ -9171,18 +9177,21 @@ ir.cpp:
91719177
# 1026| Type = [PointerType] String *
91729178
# 1026| Value = [StaticCast] 0
91739179
# 1026| ValueCategory = prvalue
9180+
# 1026| getExprWithReuse(): [ReuseExpr] reuse of static_cast<String *>...
9181+
# 1026| Type = [PointerType] String *
9182+
# 1026| ValueCategory = prvalue
91749183
# 1027| getStmt(2): [ExprStmt] ExprStmt
91759184
# 1027| getExpr(): [DeleteArrayExpr] delete[]
91769185
# 1027| Type = [VoidType] void
91779186
# 1027| ValueCategory = prvalue
91789187
# 1027| getDeallocatorCall(): [FunctionCall] call to operator delete[]
91799188
# 1027| Type = [VoidType] void
91809189
# 1027| ValueCategory = prvalue
9181-
# 1027| getExpr(): [Literal] 0
9190+
# 1027| getExprWithReuse(): [Literal] 0
91829191
# 1027| Type = [NullPointerType] decltype(nullptr)
91839192
# 1027| Value = [Literal] 0
91849193
# 1027| ValueCategory = prvalue
9185-
# 1027| getExpr().getFullyConverted(): [StaticCast] static_cast<SizedDealloc *>...
9194+
# 1027| getExprWithReuse().getFullyConverted(): [StaticCast] static_cast<SizedDealloc *>...
91869195
# 1027| Conversion = [PointerConversion] pointer conversion
91879196
# 1027| Type = [PointerType] SizedDealloc *
91889197
# 1027| Value = [StaticCast] 0
@@ -9191,11 +9200,11 @@ ir.cpp:
91919200
# 1028| getExpr(): [DeleteArrayExpr] delete[]
91929201
# 1028| Type = [VoidType] void
91939202
# 1028| ValueCategory = prvalue
9194-
# 1028| getExpr(): [Literal] 0
9203+
# 1028| getExprWithReuse(): [Literal] 0
91959204
# 1028| Type = [NullPointerType] decltype(nullptr)
91969205
# 1028| Value = [Literal] 0
91979206
# 1028| ValueCategory = prvalue
9198-
# 1028| getExpr().getFullyConverted(): [StaticCast] static_cast<Overaligned *>...
9207+
# 1028| getExprWithReuse().getFullyConverted(): [StaticCast] static_cast<Overaligned *>...
91999208
# 1028| Conversion = [PointerConversion] pointer conversion
92009209
# 1028| Type = [PointerType] Overaligned *
92019210
# 1028| Value = [StaticCast] 0
@@ -9216,6 +9225,9 @@ ir.cpp:
92169225
# 1029| Type = [PointerType] PolymorphicBase *
92179226
# 1029| Value = [StaticCast] 0
92189227
# 1029| ValueCategory = prvalue
9228+
# 1029| getExprWithReuse(): [ReuseExpr] reuse of static_cast<PolymorphicBase *>...
9229+
# 1029| Type = [PointerType] PolymorphicBase *
9230+
# 1029| ValueCategory = prvalue
92199231
# 1030| getStmt(5): [ReturnStmt] return ...
92209232
# 1032| [CopyAssignmentOperator] EmptyStruct& EmptyStruct::operator=(EmptyStruct const&)
92219233
# 1032| <params>:
@@ -16699,7 +16711,7 @@ ir.cpp:
1669916711
# 2085| getExpr(): [DeleteExpr] delete
1670016712
# 2085| Type = [VoidType] void
1670116713
# 2085| ValueCategory = prvalue
16702-
# 2085| getExpr(): [VariableAccess] x
16714+
# 2085| getExprWithReuse(): [VariableAccess] x
1670316715
# 2085| Type = [IntPointerType] int *
1670416716
# 2085| ValueCategory = prvalue(load)
1670516717
# 2086| getStmt(3): [ReturnStmt] return ...
@@ -16783,6 +16795,9 @@ ir.cpp:
1678316795
# 2108| getQualifier(): [VariableAccess] b1
1678416796
# 2108| Type = [PointerType] Base2 *
1678516797
# 2108| ValueCategory = prvalue(load)
16798+
# 2108| getExprWithReuse(): [ReuseExpr] reuse of b1
16799+
# 2108| Type = [PointerType] Base2 *
16800+
# 2108| ValueCategory = prvalue
1678616801
# 2110| getStmt(2): [DeclStmt] declaration
1678716802
# 2110| getDeclarationEntry(0): [VariableDeclarationEntry] definition of b2
1678816803
# 2110| Type = [PointerType] Base2 *
@@ -16810,6 +16825,9 @@ ir.cpp:
1681016825
# 2111| getQualifier(): [VariableAccess] b2
1681116826
# 2111| Type = [PointerType] Base2 *
1681216827
# 2111| ValueCategory = prvalue(load)
16828+
# 2111| getExprWithReuse(): [ReuseExpr] reuse of b2
16829+
# 2111| Type = [PointerType] Base2 *
16830+
# 2111| ValueCategory = prvalue
1681316831
# 2113| getStmt(4): [DeclStmt] declaration
1681416832
# 2113| getDeclarationEntry(0): [VariableDeclarationEntry] definition of d
1681516833
# 2113| Type = [PointerType] Derived2 *
@@ -16833,6 +16851,9 @@ ir.cpp:
1683316851
# 2114| getQualifier(): [VariableAccess] d
1683416852
# 2114| Type = [PointerType] Derived2 *
1683516853
# 2114| ValueCategory = prvalue(load)
16854+
# 2114| getExprWithReuse(): [ReuseExpr] reuse of d
16855+
# 2114| Type = [PointerType] Derived2 *
16856+
# 2114| ValueCategory = prvalue
1683616857
# 2115| getStmt(6): [ReturnStmt] return ...
1683716858
# 2117| [TopLevelFunction] void test_constant_folding_use(int)
1683816859
# 2117| <params>:
@@ -17168,7 +17189,7 @@ ir.cpp:
1716817189
# 2176| getExpr(): [DeleteExpr] delete
1716917190
# 2176| Type = [VoidType] void
1717017191
# 2176| ValueCategory = prvalue
17171-
# 2176| getExpr(): [ImplicitThisFieldAccess,PointerFieldAccess] x
17192+
# 2176| getExprWithReuse(): [ImplicitThisFieldAccess,PointerFieldAccess] x
1717217193
# 2176| Type = [CharPointerType] char *
1717317194
# 2176| ValueCategory = prvalue(load)
1717417195
# 2176| getQualifier(): [ThisExpr] this

0 commit comments

Comments
 (0)