Skip to content

Commit 6b1cd17

Browse files
committed
C++: Fix FPs due to data flow Conversion handling
Since we cannot track data flow from a fully-converted expression but only the unconverted expression, we should check whether the address initially escapes into the unconverted expression, not the fully-converted one. This fixes most of the false positives observed on lgtm.com.
1 parent 1a7351e commit 6b1cd17

File tree

3 files changed

+3
-5
lines changed

3 files changed

+3
-5
lines changed

cpp/ql/src/Likely Bugs/Memory Management/ReturnStackAllocatedMemory.ql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ where
4141
// the address escapes into some expression `pointerToLocal`, which flows
4242
// in a non-trivial way (one or more steps) to a returned expression.
4343
exists(Expr pointerToLocal |
44-
variableAddressEscapesTree(va, pointerToLocal.getFullyConverted()) and
44+
variableAddressEscapesTree(va, pointerToLocal) and
4545
conservativeDataFlowStep+(
4646
DataFlow::exprNode(pointerToLocal),
4747
DataFlow::exprNode(r.getExpr())

cpp/ql/test/query-tests/Likely Bugs/Memory Management/ReturnStackAllocatedMemory/ReturnStackAllocatedMemory.expected

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,5 +6,3 @@
66
| test.cpp:112:2:112:12 | return ... | May return stack-allocated memory from $@. | test.cpp:112:9:112:11 | arr | arr |
77
| test.cpp:119:2:119:19 | return ... | May return stack-allocated memory from $@. | test.cpp:119:11:119:13 | arr | arr |
88
| test.cpp:149:3:149:22 | return ... | May return stack-allocated memory from $@. | test.cpp:149:11:149:21 | threadLocal | threadLocal |
9-
| test.cpp:155:3:155:18 | return ... | May return stack-allocated memory from $@. | test.cpp:154:19:154:26 | localInt | localInt |
10-
| test.cpp:165:3:165:19 | return ... | May return stack-allocated memory from $@. | test.cpp:164:21:164:28 | localBuf | localBuf |

cpp/ql/test/query-tests/Likely Bugs/Memory Management/ReturnStackAllocatedMemory/test.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,7 @@ int *returnThreadLocal() {
152152
int returnDereferenced() {
153153
int localInt = 2;
154154
int &localRef = localInt;
155-
return localRef; // GOOD [FALSE POSITIVE]
155+
return localRef; // GOOD
156156
}
157157

158158
typedef unsigned long size_t;
@@ -162,5 +162,5 @@ char *returnAfterCopy() {
162162
char localBuf[] = "Data";
163163
static char staticBuf[sizeof(localBuf)];
164164
memcpy(staticBuf, localBuf, sizeof(staticBuf));
165-
return staticBuf; // GOOD [FALSE POSITIVE]
165+
return staticBuf; // GOOD
166166
}

0 commit comments

Comments
 (0)