Skip to content

Commit d626745

Browse files
authored
fix ThisArgumentOperand location
The correct check to do to choose between using `getAnyDef` and `getUse` is to check whether the location is an instance of UknonwnLocation.
1 parent e99a040 commit d626745

File tree

7 files changed

+32
-38
lines changed

7 files changed

+32
-38
lines changed

cpp/ql/lib/semmle/code/cpp/ir/implementation/aliased_ssa/Operand.qll

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -416,10 +416,9 @@ class ThisArgumentOperand extends ArgumentOperand {
416416
// in most cases the def location makes more sense, but in some corner cases it
417417
// does not have a location: in those cases we fall back to the use location
418418
override Language::Location getLocation() {
419-
result = this.getAnyDef().getLocation()
420-
or
421-
not exists(this.getAnyDef().getLocation()) and
422-
result = this.getUse().getLocation()
419+
if not this.getAnyDef().getLocation() instanceof Language::UnknownLocation
420+
then result = this.getAnyDef().getLocation()
421+
else result = this.getUse().getLocation()
423422
}
424423
}
425424

cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/Operand.qll

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -416,10 +416,9 @@ class ThisArgumentOperand extends ArgumentOperand {
416416
// in most cases the def location makes more sense, but in some corner cases it
417417
// does not have a location: in those cases we fall back to the use location
418418
override Language::Location getLocation() {
419-
result = this.getAnyDef().getLocation()
420-
or
421-
not exists(this.getAnyDef().getLocation()) and
422-
result = this.getUse().getLocation()
419+
if not this.getAnyDef().getLocation() instanceof Language::UnknownLocation
420+
then result = this.getAnyDef().getLocation()
421+
else result = this.getUse().getLocation()
423422
}
424423
}
425424

cpp/ql/lib/semmle/code/cpp/ir/implementation/unaliased_ssa/Operand.qll

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -416,10 +416,9 @@ class ThisArgumentOperand extends ArgumentOperand {
416416
// in most cases the def location makes more sense, but in some corner cases it
417417
// does not have a location: in those cases we fall back to the use location
418418
override Language::Location getLocation() {
419-
result = this.getAnyDef().getLocation()
420-
or
421-
not exists(this.getAnyDef().getLocation()) and
422-
result = this.getUse().getLocation()
419+
if not this.getAnyDef().getLocation() instanceof Language::UnknownLocation
420+
then result = this.getAnyDef().getLocation()
421+
else result = this.getUse().getLocation()
423422
}
424423
}
425424

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

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -671,18 +671,6 @@
671671
| file://:0:0:0:0 | Arg(0) | 0:r0_8 |
672672
| file://:0:0:0:0 | Arg(0) | 0:r0_15 |
673673
| file://:0:0:0:0 | Arg(0) | 0:r0_15 |
674-
| file://:0:0:0:0 | Arg(this) | this:r0_1 |
675-
| file://:0:0:0:0 | Arg(this) | this:r0_1 |
676-
| file://:0:0:0:0 | Arg(this) | this:r0_3 |
677-
| file://:0:0:0:0 | Arg(this) | this:r0_5 |
678-
| file://:0:0:0:0 | Arg(this) | this:r0_5 |
679-
| file://:0:0:0:0 | Arg(this) | this:r0_5 |
680-
| file://:0:0:0:0 | Arg(this) | this:r0_7 |
681-
| file://:0:0:0:0 | Arg(this) | this:r0_9 |
682-
| file://:0:0:0:0 | Arg(this) | this:r0_11 |
683-
| file://:0:0:0:0 | Arg(this) | this:r0_11 |
684-
| file://:0:0:0:0 | Arg(this) | this:r0_13 |
685-
| file://:0:0:0:0 | Arg(this) | this:r0_15 |
686674
| file://:0:0:0:0 | CallTarget | func:r0_1 |
687675
| file://:0:0:0:0 | ChiPartial | partial:m0_2 |
688676
| file://:0:0:0:0 | ChiPartial | partial:m0_3 |
@@ -3265,6 +3253,7 @@
32653253
| ir.cpp:754:8:754:8 | Address | &:r754_22 |
32663254
| ir.cpp:754:8:754:8 | Address | &:r754_24 |
32673255
| ir.cpp:754:8:754:8 | Address | &:r754_34 |
3256+
| ir.cpp:754:8:754:8 | Arg(this) | this:r0_5 |
32683257
| ir.cpp:754:8:754:8 | Arg(this) | this:r754_22 |
32693258
| ir.cpp:754:8:754:8 | CallTarget | func:r754_11 |
32703259
| ir.cpp:754:8:754:8 | CallTarget | func:r754_23 |
@@ -3356,6 +3345,7 @@
33563345
| ir.cpp:763:8:763:8 | Address | &:r763_22 |
33573346
| ir.cpp:763:8:763:8 | Address | &:r763_24 |
33583347
| ir.cpp:763:8:763:8 | Address | &:r763_34 |
3348+
| ir.cpp:763:8:763:8 | Arg(this) | this:r0_5 |
33593349
| ir.cpp:763:8:763:8 | Arg(this) | this:r763_22 |
33603350
| ir.cpp:763:8:763:8 | CallTarget | func:r763_11 |
33613351
| ir.cpp:763:8:763:8 | CallTarget | func:r763_23 |
@@ -5166,6 +5156,10 @@
51665156
| ir.cpp:1078:18:1078:18 | Address | &:r1078_42 |
51675157
| ir.cpp:1078:18:1078:18 | Address | &:r1078_42 |
51685158
| ir.cpp:1078:18:1078:18 | Arg(0) | 0:r1078_28 |
5159+
| ir.cpp:1078:18:1078:18 | Arg(this) | this:r0_1 |
5160+
| ir.cpp:1078:18:1078:18 | Arg(this) | this:r0_3 |
5161+
| ir.cpp:1078:18:1078:18 | Arg(this) | this:r0_5 |
5162+
| ir.cpp:1078:18:1078:18 | Arg(this) | this:r0_7 |
51695163
| ir.cpp:1078:18:1078:18 | Arg(this) | this:r1078_42 |
51705164
| ir.cpp:1078:18:1078:18 | CallTarget | func:r1078_10 |
51715165
| ir.cpp:1078:18:1078:18 | CallTarget | func:r1078_18 |
@@ -5227,6 +5221,10 @@
52275221
| ir.cpp:1084:25:1084:25 | Address | &:r1084_33 |
52285222
| ir.cpp:1084:25:1084:25 | Address | &:r1084_33 |
52295223
| ir.cpp:1084:25:1084:25 | Arg(0) | 0:r1084_28 |
5224+
| ir.cpp:1084:25:1084:25 | Arg(this) | this:r0_9 |
5225+
| ir.cpp:1084:25:1084:25 | Arg(this) | this:r0_11 |
5226+
| ir.cpp:1084:25:1084:25 | Arg(this) | this:r0_13 |
5227+
| ir.cpp:1084:25:1084:25 | Arg(this) | this:r0_15 |
52305228
| ir.cpp:1084:25:1084:25 | Arg(this) | this:r1084_33 |
52315229
| ir.cpp:1084:25:1084:25 | CallTarget | func:r1084_10 |
52325230
| ir.cpp:1084:25:1084:25 | CallTarget | func:r1084_18 |
@@ -6622,6 +6620,7 @@
66226620
| ir.cpp:1447:16:1447:39 | ChiTotal | total:m1446_5 |
66236621
| ir.cpp:1447:16:1447:39 | SideEffect | ~m1446_5 |
66246622
| ir.cpp:1447:16:1447:39 | StoreValue | r1447_3 |
6623+
| ir.cpp:1447:44:1447:44 | Arg(this) | this:r0_11 |
66256624
| ir.cpp:1447:44:1447:44 | CallTarget | func:r1447_7 |
66266625
| ir.cpp:1447:44:1447:44 | ChiPartial | partial:m1447_9 |
66276626
| ir.cpp:1447:44:1447:44 | ChiTotal | total:m1447_5 |

cpp/ql/test/library-tests/syntax-zoo/dataflow-ir-consistency.expected

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1613,6 +1613,8 @@ postWithInFlow
16131613
| cpp11.cpp:28:21:28:21 | (__begin) [post update] | PostUpdateNode should not be the target of local flow. |
16141614
| cpp11.cpp:28:21:28:21 | (__range) [post update] | PostUpdateNode should not be the target of local flow. |
16151615
| cpp11.cpp:28:21:28:21 | (__range) [post update] | PostUpdateNode should not be the target of local flow. |
1616+
| cpp11.cpp:28:21:28:21 | (__range) [post update] | PostUpdateNode should not be the target of local flow. |
1617+
| cpp11.cpp:28:21:28:21 | (__range) [post update] | PostUpdateNode should not be the target of local flow. |
16161618
| cpp11.cpp:28:21:28:21 | VariableAddress [post update] | PostUpdateNode should not be the target of local flow. |
16171619
| cpp11.cpp:28:21:28:21 | VariableAddress [post update] | PostUpdateNode should not be the target of local flow. |
16181620
| cpp11.cpp:28:21:28:34 | temporary object [post update] | PostUpdateNode should not be the target of local flow. |
@@ -1689,10 +1691,6 @@ postWithInFlow
16891691
| fieldaccess.cpp:9:2:9:2 | i [post update] | PostUpdateNode should not be the target of local flow. |
16901692
| file://:0:0:0:0 | (Base *)... [post update] | PostUpdateNode should not be the target of local flow. |
16911693
| file://:0:0:0:0 | (Middle *)... [post update] | PostUpdateNode should not be the target of local flow. |
1692-
| file://:0:0:0:0 | (__range) [post update] | PostUpdateNode should not be the target of local flow. |
1693-
| file://:0:0:0:0 | (__range) [post update] | PostUpdateNode should not be the target of local flow. |
1694-
| file://:0:0:0:0 | (__range) [post update] | PostUpdateNode should not be the target of local flow. |
1695-
| file://:0:0:0:0 | (__range) [post update] | PostUpdateNode should not be the target of local flow. |
16961694
| file://:0:0:0:0 | (reference dereference) [post update] | PostUpdateNode should not be the target of local flow. |
16971695
| file://:0:0:0:0 | (reference dereference) [post update] | PostUpdateNode should not be the target of local flow. |
16981696
| file://:0:0:0:0 | (reference dereference) [post update] | PostUpdateNode should not be the target of local flow. |
@@ -1704,8 +1702,6 @@ postWithInFlow
17041702
| file://:0:0:0:0 | VariableAddress [post update] | PostUpdateNode should not be the target of local flow. |
17051703
| file://:0:0:0:0 | VariableAddress [post update] | PostUpdateNode should not be the target of local flow. |
17061704
| file://:0:0:0:0 | VariableAddress [post update] | PostUpdateNode should not be the target of local flow. |
1707-
| file://:0:0:0:0 | this [post update] | PostUpdateNode should not be the target of local flow. |
1708-
| file://:0:0:0:0 | this [post update] | PostUpdateNode should not be the target of local flow. |
17091705
| forstmt.cpp:2:14:2:14 | VariableAddress [post update] | PostUpdateNode should not be the target of local flow. |
17101706
| forstmt.cpp:2:29:2:29 | VariableAddress [post update] | PostUpdateNode should not be the target of local flow. |
17111707
| forstmt.cpp:9:14:9:14 | VariableAddress [post update] | PostUpdateNode should not be the target of local flow. |
@@ -2067,6 +2063,7 @@ postWithInFlow
20672063
| ir.cpp:754:8:754:8 | VariableAddress [post update] | PostUpdateNode should not be the target of local flow. |
20682064
| ir.cpp:754:8:754:8 | middle_s [post update] | PostUpdateNode should not be the target of local flow. |
20692065
| ir.cpp:754:8:754:8 | this [post update] | PostUpdateNode should not be the target of local flow. |
2066+
| ir.cpp:754:8:754:8 | this [post update] | PostUpdateNode should not be the target of local flow. |
20702067
| ir.cpp:757:3:757:8 | this [post update] | PostUpdateNode should not be the target of local flow. |
20712068
| ir.cpp:757:12:757:12 | Argument this [post update] | PostUpdateNode should not be the target of local flow. |
20722069
| ir.cpp:757:12:757:12 | Argument this [post update] | PostUpdateNode should not be the target of local flow. |
@@ -2078,6 +2075,7 @@ postWithInFlow
20782075
| ir.cpp:763:8:763:8 | VariableAddress [post update] | PostUpdateNode should not be the target of local flow. |
20792076
| ir.cpp:763:8:763:8 | derived_s [post update] | PostUpdateNode should not be the target of local flow. |
20802077
| ir.cpp:763:8:763:8 | this [post update] | PostUpdateNode should not be the target of local flow. |
2078+
| ir.cpp:763:8:763:8 | this [post update] | PostUpdateNode should not be the target of local flow. |
20812079
| ir.cpp:766:3:766:9 | this [post update] | PostUpdateNode should not be the target of local flow. |
20822080
| ir.cpp:766:13:766:13 | Argument this [post update] | PostUpdateNode should not be the target of local flow. |
20832081
| ir.cpp:766:13:766:13 | Argument this [post update] | PostUpdateNode should not be the target of local flow. |
@@ -2642,6 +2640,8 @@ postWithInFlow
26422640
| stream_it.cpp:11:10:11:12 | VariableAddress [post update] | PostUpdateNode should not be the target of local flow. |
26432641
| stream_it.cpp:11:16:11:16 | (__range) [post update] | PostUpdateNode should not be the target of local flow. |
26442642
| stream_it.cpp:11:16:11:16 | (__range) [post update] | PostUpdateNode should not be the target of local flow. |
2643+
| stream_it.cpp:11:16:11:16 | (__range) [post update] | PostUpdateNode should not be the target of local flow. |
2644+
| stream_it.cpp:11:16:11:16 | (__range) [post update] | PostUpdateNode should not be the target of local flow. |
26452645
| stream_it.cpp:11:16:11:16 | VariableAddress [post update] | PostUpdateNode should not be the target of local flow. |
26462646
| stream_it.cpp:11:16:11:16 | VariableAddress [post update] | PostUpdateNode should not be the target of local flow. |
26472647
| stream_it.cpp:19:13:19:14 | (reference to) [post update] | PostUpdateNode should not be the target of local flow. |

csharp/ql/src/experimental/ir/implementation/raw/Operand.qll

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -416,10 +416,9 @@ class ThisArgumentOperand extends ArgumentOperand {
416416
// in most cases the def location makes more sense, but in some corner cases it
417417
// does not have a location: in those cases we fall back to the use location
418418
override Language::Location getLocation() {
419-
result = this.getAnyDef().getLocation()
420-
or
421-
not exists(this.getAnyDef().getLocation()) and
422-
result = this.getUse().getLocation()
419+
if not this.getAnyDef().getLocation() instanceof Language::UnknownLocation
420+
then result = this.getAnyDef().getLocation()
421+
else result = this.getUse().getLocation()
423422
}
424423
}
425424

csharp/ql/src/experimental/ir/implementation/unaliased_ssa/Operand.qll

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -416,10 +416,9 @@ class ThisArgumentOperand extends ArgumentOperand {
416416
// in most cases the def location makes more sense, but in some corner cases it
417417
// does not have a location: in those cases we fall back to the use location
418418
override Language::Location getLocation() {
419-
result = this.getAnyDef().getLocation()
420-
or
421-
not exists(this.getAnyDef().getLocation()) and
422-
result = this.getUse().getLocation()
419+
if not this.getAnyDef().getLocation() instanceof Language::UnknownLocation
420+
then result = this.getAnyDef().getLocation()
421+
else result = this.getUse().getLocation()
423422
}
424423
}
425424

0 commit comments

Comments
 (0)