Skip to content

Commit 2494b7d

Browse files
rdmarsh2rdmarsh2
committed
C++: fix for IR CFG problem with return in if
1 parent 2c8ed64 commit 2494b7d

11 files changed

+529
-166
lines changed

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

Lines changed: 61 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -413,7 +413,23 @@ class TranslatedReturnValueStmt extends TranslatedReturnStmt, TranslatedVariable
413413
TranslatedReturnValueStmt() { stmt.hasExpr() and hasReturnValue(stmt.getEnclosingFunction()) }
414414

415415
final override Instruction getInitializationSuccessor(EdgeKind kind) {
416-
result = this.getEnclosingFunction().getReturnSuccessorInstruction(kind)
416+
if this.hasAnImplicitDestructorCall()
417+
then result = this.getChild(1).getFirstInstruction(kind)
418+
else result = this.getEnclosingFunction().getReturnSuccessorInstruction(kind)
419+
}
420+
421+
override Instruction getChildSuccessorInternal(TranslatedElement child, EdgeKind kind) {
422+
result = TranslatedVariableInitialization.super.getChildSuccessorInternal(child, kind)
423+
or
424+
exists(int id |
425+
this.getChild(id) = child and
426+
(
427+
result = this.getChild(id + 1).getFirstInstruction(kind)
428+
or
429+
not exists(this.getChild(id + 1)) and
430+
result = this.getEnclosingFunction().getReturnSuccessorInstruction(kind)
431+
)
432+
)
417433
}
418434

419435
final override TranslatedElement getChildInternal(int id) {
@@ -429,6 +445,8 @@ class TranslatedReturnValueStmt extends TranslatedReturnStmt, TranslatedVariable
429445
final override IRVariable getIRVariable() {
430446
result = this.getEnclosingFunction().getReturnVariable()
431447
}
448+
449+
override predicate handlesDestructorsExplicitly() { any() }
432450
}
433451

434452
/**
@@ -449,7 +467,10 @@ class TranslatedReturnVoidExpressionStmt extends TranslatedReturnStmt {
449467
}
450468

451469
override Instruction getALastInstructionInternal() {
452-
result = this.getInstruction(OnlyInstructionTag())
470+
if this.hasAnImplicitDestructorCall()
471+
then
472+
result = this.getChildInternal(max(int id | exists(this.getChild(id)))).getALastInstruction()
473+
else result = this.getInstruction(OnlyInstructionTag())
453474
}
454475

455476
override predicate hasInstruction(Opcode opcode, InstructionTag tag, CppType resultType) {
@@ -460,16 +481,34 @@ class TranslatedReturnVoidExpressionStmt extends TranslatedReturnStmt {
460481

461482
override Instruction getInstructionSuccessorInternal(InstructionTag tag, EdgeKind kind) {
462483
tag = OnlyInstructionTag() and
463-
result = this.getEnclosingFunction().getReturnSuccessorInstruction(kind)
484+
if this.hasAnImplicitDestructorCall()
485+
then result = this.getChildInternal(1).getFirstInstruction(kind)
486+
else result = this.getEnclosingFunction().getReturnSuccessorInstruction(kind)
464487
}
465488

466489
override Instruction getChildSuccessorInternal(TranslatedElement child, EdgeKind kind) {
467490
child = this.getExpr() and
468491
result = this.getInstruction(OnlyInstructionTag()) and
469492
kind instanceof GotoEdge
493+
or
494+
exists(int id |
495+
child = this.getChild(id) and
496+
id >= this.getFirstDestructorCallIndex() and
497+
result = this.getChild(id + 1).getFirstInstruction(kind)
498+
)
499+
or
500+
exists(int id |
501+
child = this.getChild(id) and
502+
id >= this.getFirstDestructorCallIndex() and
503+
exists(this.getChild(id)) and
504+
not exists(this.getChild(id + 1)) and
505+
result = this.getEnclosingFunction().getReturnSuccessorInstruction(kind)
506+
)
470507
}
471508

472509
private TranslatedExpr getExpr() { result = getTranslatedExpr(stmt.getExpr()) }
510+
511+
override predicate handlesDestructorsExplicitly() { any() }
473512
}
474513

475514
/**
@@ -489,7 +528,9 @@ class TranslatedReturnVoidStmt extends TranslatedReturnStmt {
489528
}
490529

491530
override Instruction getALastInstructionInternal() {
492-
result = this.getInstruction(OnlyInstructionTag())
531+
if this.hasAnImplicitDestructorCall()
532+
then result = this.getChild(max(int id | exists(this.getChild(id)))).getALastInstruction()
533+
else result = this.getInstruction(OnlyInstructionTag())
493534
}
494535

495536
override predicate hasInstruction(Opcode opcode, InstructionTag tag, CppType resultType) {
@@ -500,10 +541,24 @@ class TranslatedReturnVoidStmt extends TranslatedReturnStmt {
500541

501542
override Instruction getInstructionSuccessorInternal(InstructionTag tag, EdgeKind kind) {
502543
tag = OnlyInstructionTag() and
503-
result = this.getEnclosingFunction().getReturnSuccessorInstruction(kind)
544+
if this.hasAnImplicitDestructorCall()
545+
then result = this.getChild(0).getFirstInstruction(kind)
546+
else result = this.getEnclosingFunction().getReturnSuccessorInstruction(kind)
504547
}
505548

506-
override Instruction getChildSuccessorInternal(TranslatedElement child, EdgeKind kind) { none() }
549+
override Instruction getChildSuccessorInternal(TranslatedElement child, EdgeKind kind) {
550+
exists(int id |
551+
this.getChild(id) = child and
552+
(
553+
result = this.getChild(id + 1).getFirstInstruction(kind)
554+
or
555+
not exists(this.getChild(id + 1)) and
556+
result = this.getEnclosingFunction().getReturnSuccessorInstruction(kind)
557+
)
558+
)
559+
}
560+
561+
override predicate handlesDestructorsExplicitly() { any() }
507562
}
508563

509564
/**

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

Lines changed: 91 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -4316,8 +4316,6 @@ ir.cpp:
43164316
# 365| ValueCategory = prvalue
43174317
# 361| getStmt(2): [LabelStmt] label ...:
43184318
# 367| getStmt(1): [ReturnStmt] return ...
4319-
# 369| [TopLevelFunction] void VoidFunc()
4320-
# 369| <params>:
43214319
# 370| [TopLevelFunction] int Add(int, int)
43224320
# 370| <params>:
43234321
# 370| getParameter(0): [Parameter] x
@@ -17001,41 +16999,99 @@ ir.cpp:
1700116999
# 2234| Type = [Class] Bool
1700217000
# 2234| ValueCategory = lvalue
1700317001
# 2236| getStmt(2): [ReturnStmt] return ...
17004-
# 2238| [TopLevelFunction] void IfReturnDestructors(bool)
17002+
# 2238| [TopLevelFunction] void VoidFunc()
1700517003
# 2238| <params>:
17006-
# 2238| getParameter(0): [Parameter] b
17007-
# 2238| Type = [BoolType] bool
1700817004
# 2238| getEntryPoint(): [BlockStmt] { ... }
17009-
# 2239| getStmt(0): [DeclStmt] declaration
17010-
# 2239| getDeclarationEntry(0): [VariableDeclarationEntry] definition of s
17011-
# 2239| Type = [Struct] String
17012-
# 2239| getVariable().getInitializer(): [Initializer] initializer for s
17013-
# 2239| getExpr(): [ConstructorCall] call to String
17014-
# 2239| Type = [VoidType] void
17015-
# 2239| ValueCategory = prvalue
17016-
# 2240| getStmt(1): [IfStmt] if (...) ...
17017-
# 2240| getCondition(): [VariableAccess] b
17018-
# 2240| Type = [BoolType] bool
17019-
# 2240| ValueCategory = prvalue(load)
17020-
# 2240| getThen(): [BlockStmt] { ... }
17021-
# 2241| getStmt(0): [ReturnStmt] return ...
17022-
# 2244| getImplicitDestructorCall(0): [DestructorCall] call to ~String
17023-
# 2244| Type = [VoidType] void
17024-
# 2244| ValueCategory = prvalue
17025-
# 2244| getQualifier(): [VariableAccess] s
17026-
# 2244| Type = [Struct] String
17027-
# 2244| ValueCategory = lvalue
17028-
# 2243| getStmt(2): [ExprStmt] ExprStmt
17029-
# 2243| getExpr(): [VariableAccess] s
17030-
# 2243| Type = [Struct] String
17031-
# 2243| ValueCategory = lvalue
17032-
# 2244| getStmt(3): [ReturnStmt] return ...
17033-
# 2244| getImplicitDestructorCall(0): [DestructorCall] call to ~String
17034-
# 2244| Type = [VoidType] void
17035-
# 2244| ValueCategory = prvalue
17036-
# 2244| getQualifier(): [VariableAccess] s
17037-
# 2244| Type = [Struct] String
17038-
# 2244| ValueCategory = lvalue
17005+
# 2238| getStmt(0): [ReturnStmt] return ...
17006+
# 2240| [TopLevelFunction] void IfReturnDestructors(bool)
17007+
# 2240| <params>:
17008+
# 2240| getParameter(0): [Parameter] b
17009+
# 2240| Type = [BoolType] bool
17010+
# 2240| getEntryPoint(): [BlockStmt] { ... }
17011+
# 2241| getStmt(0): [DeclStmt] declaration
17012+
# 2241| getDeclarationEntry(0): [VariableDeclarationEntry] definition of s
17013+
# 2241| Type = [Struct] String
17014+
# 2241| getVariable().getInitializer(): [Initializer] initializer for s
17015+
# 2241| getExpr(): [ConstructorCall] call to String
17016+
# 2241| Type = [VoidType] void
17017+
# 2241| ValueCategory = prvalue
17018+
# 2242| getStmt(1): [IfStmt] if (...) ...
17019+
# 2242| getCondition(): [VariableAccess] b
17020+
# 2242| Type = [BoolType] bool
17021+
# 2242| ValueCategory = prvalue(load)
17022+
# 2242| getThen(): [BlockStmt] { ... }
17023+
# 2243| getStmt(0): [ReturnStmt] return ...
17024+
# 2249| getImplicitDestructorCall(0): [DestructorCall] call to ~String
17025+
# 2249| Type = [VoidType] void
17026+
# 2249| ValueCategory = prvalue
17027+
# 2249| getQualifier(): [VariableAccess] s
17028+
# 2249| Type = [Struct] String
17029+
# 2249| ValueCategory = lvalue
17030+
# 2245| getStmt(2): [IfStmt] if (...) ...
17031+
# 2245| getCondition(): [VariableAccess] b
17032+
# 2245| Type = [BoolType] bool
17033+
# 2245| ValueCategory = prvalue(load)
17034+
# 2245| getThen(): [BlockStmt] { ... }
17035+
# 2246| getStmt(0): [ReturnStmt] return ...
17036+
# 2246| getExpr(): [FunctionCall] call to VoidFunc
17037+
# 2246| Type = [VoidType] void
17038+
# 2246| ValueCategory = prvalue
17039+
# 2249| getImplicitDestructorCall(0): [DestructorCall] call to ~String
17040+
# 2249| Type = [VoidType] void
17041+
# 2249| ValueCategory = prvalue
17042+
# 2249| getQualifier(): [VariableAccess] s
17043+
# 2249| Type = [Struct] String
17044+
# 2249| ValueCategory = lvalue
17045+
# 2248| getStmt(3): [ExprStmt] ExprStmt
17046+
# 2248| getExpr(): [VariableAccess] s
17047+
# 2248| Type = [Struct] String
17048+
# 2248| ValueCategory = lvalue
17049+
# 2249| getStmt(4): [ReturnStmt] return ...
17050+
# 2249| getImplicitDestructorCall(0): [DestructorCall] call to ~String
17051+
# 2249| Type = [VoidType] void
17052+
# 2249| ValueCategory = prvalue
17053+
# 2249| getQualifier(): [VariableAccess] s
17054+
# 2249| Type = [Struct] String
17055+
# 2249| ValueCategory = lvalue
17056+
# 2251| [TopLevelFunction] int IfReturnDestructors3(bool)
17057+
# 2251| <params>:
17058+
# 2251| getParameter(0): [Parameter] b
17059+
# 2251| Type = [BoolType] bool
17060+
# 2251| getEntryPoint(): [BlockStmt] { ... }
17061+
# 2252| getStmt(0): [DeclStmt] declaration
17062+
# 2252| getDeclarationEntry(0): [VariableDeclarationEntry] definition of s
17063+
# 2252| Type = [Struct] String
17064+
# 2252| getVariable().getInitializer(): [Initializer] initializer for s
17065+
# 2252| getExpr(): [ConstructorCall] call to String
17066+
# 2252| Type = [VoidType] void
17067+
# 2252| ValueCategory = prvalue
17068+
# 2253| getStmt(1): [IfStmt] if (...) ...
17069+
# 2253| getCondition(): [VariableAccess] b
17070+
# 2253| Type = [BoolType] bool
17071+
# 2253| ValueCategory = prvalue(load)
17072+
# 2253| getThen(): [BlockStmt] { ... }
17073+
# 2254| getStmt(0): [ReturnStmt] return ...
17074+
# 2254| getExpr(): [Literal] 1
17075+
# 2254| Type = [IntType] int
17076+
# 2254| Value = [Literal] 1
17077+
# 2254| ValueCategory = prvalue
17078+
# 2257| getImplicitDestructorCall(0): [DestructorCall] call to ~String
17079+
# 2257| Type = [VoidType] void
17080+
# 2257| ValueCategory = prvalue
17081+
# 2257| getQualifier(): [VariableAccess] s
17082+
# 2257| Type = [Struct] String
17083+
# 2257| ValueCategory = lvalue
17084+
# 2256| getStmt(2): [ReturnStmt] return ...
17085+
# 2256| getExpr(): [Literal] 0
17086+
# 2256| Type = [IntType] int
17087+
# 2256| Value = [Literal] 0
17088+
# 2256| ValueCategory = prvalue
17089+
# 2257| getImplicitDestructorCall(0): [DestructorCall] call to ~String
17090+
# 2257| Type = [VoidType] void
17091+
# 2257| ValueCategory = prvalue
17092+
# 2257| getQualifier(): [VariableAccess] s
17093+
# 2257| Type = [Struct] String
17094+
# 2257| ValueCategory = lvalue
1703917095
perf-regression.cpp:
1704017096
# 4| [CopyAssignmentOperator] Big& Big::operator=(Big const&)
1704117097
# 4| <params>:

0 commit comments

Comments
 (0)