Merge pull request #8525 from MathiasVP/more-precise-is-before

C++: Consider columns in `Location.isBefore`

Author: MathiasVP

cpp/ql/lib/semmle/code/cpp/Location.qll

@@ -73,8 +73,24 @@ class Location extends @location {
 
   /** Holds if `this` comes on a line strictly before `l`. */
   pragma[inline]
-  predicate isBefore(Location l) {
-    this.getFile() = l.getFile() and this.getEndLine() < l.getStartLine()
+  predicate isBefore(Location l) { this.isBefore(l, false) }
+
+  /**
+   * Holds if `this` comes strictly before `l`. The boolean `sameLine` is
+   * true if `l` is on the same line as `this`, but starts at a later column.
+   * Otherwise, `sameLine` is false.
+   */
+  pragma[inline]
+  predicate isBefore(Location l, boolean sameLine) {
+    this.getFile() = l.getFile() and
+    (
+      sameLine = false and
+      this.getEndLine() < l.getStartLine()
+      or
+      sameLine = true and
+      this.getEndLine() = l.getStartLine() and
+      this.getEndColumn() < l.getStartColumn()
+    )
   }
 
   /** Holds if location `l` is completely contained within this one. */

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

@@ -349,7 +349,7 @@ Instruction getInstructionBackEdgeSuccessor(Instruction instruction, EdgeKind ki
 
 /** Holds if `goto` jumps strictly forward in the program text. */
 private predicate isStrictlyForwardGoto(GotoStmt goto) {
-  goto.getLocation().isBefore(goto.getTarget().getLocation())
+  goto.getLocation().isBefore(goto.getTarget().getLocation(), _)
 }
 
 Locatable getInstructionAst(TStageInstruction instr) {

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

@@ -13035,6 +13035,23 @@ ir.cpp:
 # 1689|   getEntryPoint(): [BlockStmt] { ... }
 # 1689|     getStmt(0): [EmptyStmt] ;
 # 1689|     getStmt(1): [ReturnStmt] return ...
+# 1693| [TopLevelFunction] int goto_on_same_line()
+# 1693|   <params>: 
+# 1693|   getEntryPoint(): [BlockStmt] { ... }
+# 1694|     getStmt(0): [DeclStmt] declaration
+# 1694|       getDeclarationEntry(0): [VariableDeclarationEntry] definition of x
+# 1694|           Type = [IntType] int
+# 1694|         getVariable().getInitializer(): [Initializer] initializer for x
+# 1694|           getExpr(): [Literal] 42
+# 1694|               Type = [IntType] int
+# 1694|               Value = [Literal] 42
+# 1694|               ValueCategory = prvalue
+# 1695|     getStmt(1): [GotoStmt] goto ...
+# 1695|     getStmt(2): [LabelStmt] label ...:
+# 1696|     getStmt(3): [ReturnStmt] return ...
+# 1696|       getExpr(): [VariableAccess] x
+# 1696|           Type = [IntType] int
+# 1696|           ValueCategory = prvalue(load)
 perf-regression.cpp:
 #    4| [CopyAssignmentOperator] Big& Big::operator=(Big const&)
 #    4|   <params>: 

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

@@ -1690,4 +1690,10 @@ void captured_lambda(int x, int &y, int &&z)
     };
 }
 
+int goto_on_same_line() {
+  int x = 42;
+  goto next; next:
+  return x;
+}
+
 // semmle-extractor-options: -std=c++17 --clang

cpp/ql/test/library-tests/ir/ir/operand_locations.expected

@@ -7527,6 +7527,17 @@
 | ir.cpp:1689:50:1689:50 | Load | m1689_6 |
 | ir.cpp:1689:50:1689:50 | SideEffect | m1689_3 |
 | ir.cpp:1689:50:1689:50 | SideEffect | m1689_8 |
+| ir.cpp:1693:5:1693:21 | Address | &:r1693_5 |
+| ir.cpp:1693:5:1693:21 | ChiPartial | partial:m1693_3 |
+| ir.cpp:1693:5:1693:21 | ChiTotal | total:m1693_2 |
+| ir.cpp:1693:5:1693:21 | Load | m1696_4 |
+| ir.cpp:1693:5:1693:21 | SideEffect | m1693_3 |
+| ir.cpp:1694:7:1694:7 | Address | &:r1694_1 |
+| ir.cpp:1694:10:1694:12 | StoreValue | r1694_2 |
+| ir.cpp:1696:3:1696:11 | Address | &:r1696_1 |
+| ir.cpp:1696:10:1696:10 | Address | &:r1696_2 |
+| ir.cpp:1696:10:1696:10 | Load | m1694_3 |
+| ir.cpp:1696:10:1696:10 | StoreValue | r1696_3 |
 | perf-regression.cpp:6:3:6:5 | Address | &:r6_5 |
 | perf-regression.cpp:6:3:6:5 | Address | &:r6_5 |
 | perf-regression.cpp:6:3:6:5 | Address | &:r6_7 |

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

@@ -8842,6 +8842,25 @@ ir.cpp:
 # 1689|     v1689_12(void)                             = AliasedUse                   : ~m?
 # 1689|     v1689_13(void)                             = ExitFunction                 : 
 
+# 1693| int goto_on_same_line()
+# 1693|   Block 0
+# 1693|     v1693_1(void)       = EnterFunction            : 
+# 1693|     mu1693_2(unknown)   = AliasedDefinition        : 
+# 1693|     mu1693_3(unknown)   = InitializeNonLocal       : 
+# 1694|     r1694_1(glval<int>) = VariableAddress[x]       : 
+# 1694|     r1694_2(int)        = Constant[42]             : 
+# 1694|     mu1694_3(int)       = Store[x]                 : &:r1694_1, r1694_2
+# 1695|     v1695_1(void)       = NoOp                     : 
+# 1695|     v1695_2(void)       = NoOp                     : 
+# 1696|     r1696_1(glval<int>) = VariableAddress[#return] : 
+# 1696|     r1696_2(glval<int>) = VariableAddress[x]       : 
+# 1696|     r1696_3(int)        = Load[x]                  : &:r1696_2, ~m?
+# 1696|     mu1696_4(int)       = Store[#return]           : &:r1696_1, r1696_3
+# 1693|     r1693_4(glval<int>) = VariableAddress[#return] : 
+# 1693|     v1693_5(void)       = ReturnValue              : &:r1693_4, ~m?
+# 1693|     v1693_6(void)       = AliasedUse               : ~m?
+# 1693|     v1693_7(void)       = ExitFunction             : 
+
 perf-regression.cpp:
 #    6| void Big::Big()
 #    6|   Block 0