hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2025-12-22 19:56:32 +01:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #9952 from MathiasVP/speedup-return-stack-allocated-memory

Browse Source 
C++: Speedup `cpp/return-stack-allocated-memory`
This commit is contained in:
Mathias Vorreiter Pedersen
2022-08-03 09:41:38 +01:00
committed by GitHub
parent 03fa5d8ed0 f385041ab3
commit c582d17350
4 changed files with 36 additions and 18 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
cpp/ql/lib/change-notes/2022-08-02-must-flow-local-only-flow.md Normal file
View File

@@ -0,0 +1,4 @@
---
category: feature
---
* A new class predicate `MustFlowConfiguration::allowInterproceduralFlow` has been added to the `semmle.code.cpp.ir.dataflow.MustFlow` library. The new predicate can be overridden to disable interprocedural flow.

20
cpp/ql/lib/semmle/code/cpp/ir/dataflow/MustFlow.qll
View File

@@ -38,6 +38,9 @@ abstract class MustFlowConfiguration extends string {
*/
predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { none() }
/** Holds if this configuration allows flow from arguments to parameters. */
predicate allowInterproceduralFlow() { any() }
/**
* Holds if data must flow from `source` to `sink` for this configuration.
*
@@ -204,10 +207,25 @@ private module Cached {
}
}
/**
* Gets the enclosing callable of `n`. Unlike `n.getEnclosingCallable()`, this
* predicate ensures that joins go from `n` to the result instead of the other
* way around.
*/
pragma[inline]
private Declaration getEnclosingCallable(DataFlow::Node n) {
pragma[only_bind_into](result) = pragma[only_bind_out](n).getEnclosingCallable()
}
/** Holds if `nodeFrom` flows to `nodeTo`. */
private predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo, MustFlowConfiguration config) {
exists(config) and
Cached::step(nodeFrom, nodeTo)
Cached::step(pragma[only_bind_into](nodeFrom), pragma[only_bind_into](nodeTo)) and
(
config.allowInterproceduralFlow()
or
getEnclosingCallable(nodeFrom) = getEnclosingCallable(nodeTo)
)
or
config.isAdditionalFlowStep(nodeFrom, nodeTo)
}

17
cpp/ql/src/Likely Bugs/Memory Management/ReturnStackAllocatedMemory.ql
View File

@@ -52,6 +52,18 @@ class ReturnStackAllocatedMemoryConfig extends MustFlowConfiguration {
)
}
// We disable flow into callables in this query as we'd otherwise get a result on this piece of code:
// ```cpp
// int* id(int* px) {
// return px; // this returns the local variable `x`, but it's fine as the local variable isn't declared in this scope.
// }
// void f() {
// int x;
// int* px = id(&x);
// }
// ```
override predicate allowInterproceduralFlow() { none() }
/**
* This configuration intentionally conflates addresses of fields and their object, and pointer offsets
* with their base pointer as this allows us to detect cases where an object's address flows to a
@@ -77,9 +89,6 @@ from
ReturnStackAllocatedMemoryConfig conf
where
conf.hasFlowPath(pragma[only_bind_into](source), pragma[only_bind_into](sink)) and
source.getNode().asInstruction() = var and
// Only raise an alert if we're returning from the _same_ callable as the on that
// declared the stack variable.
var.getEnclosingFunction() = sink.getNode().getEnclosingCallable()
source.getNode().asInstruction() = var
select sink.getNode(), source, sink, "May return stack-allocated memory from $@.", var.getAst(),
var.getAst().toString()

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

@@ -100,12 +100,6 @@ edges
| test.cpp:190:10:190:13 | Unary | test.cpp:190:10:190:13 | (reference dereference) |
| test.cpp:190:10:190:13 | Unary | test.cpp:190:10:190:13 | (reference to) |
| test.cpp:190:10:190:13 | pRef | test.cpp:190:10:190:13 | Unary |
| test.cpp:225:14:225:15 | px | test.cpp:226:10:226:11 | Load |
| test.cpp:226:10:226:11 | Load | test.cpp:226:10:226:11 | px |
| test.cpp:226:10:226:11 | px | test.cpp:226:10:226:11 | StoreValue |
| test.cpp:231:16:231:17 | & ... | test.cpp:225:14:225:15 | px |
| test.cpp:231:17:231:17 | Unary | test.cpp:231:16:231:17 | & ... |
| test.cpp:231:17:231:17 | x | test.cpp:231:17:231:17 | Unary |
nodes
| test.cpp:17:9:17:11 | & ... | semmle.label | & ... |
| test.cpp:17:9:17:11 | StoreValue | semmle.label | StoreValue |
@@ -221,13 +215,6 @@ nodes
| test.cpp:190:10:190:13 | Unary | semmle.label | Unary |
| test.cpp:190:10:190:13 | Unary | semmle.label | Unary |
| test.cpp:190:10:190:13 | pRef | semmle.label | pRef |
| test.cpp:225:14:225:15 | px | semmle.label | px |
| test.cpp:226:10:226:11 | Load | semmle.label | Load |
| test.cpp:226:10:226:11 | StoreValue | semmle.label | StoreValue |
| test.cpp:226:10:226:11 | px | semmle.label | px |
| test.cpp:231:16:231:17 | & ... | semmle.label | & ... |
| test.cpp:231:17:231:17 | Unary | semmle.label | Unary |
| test.cpp:231:17:231:17 | x | semmle.label | x |
#select
| test.cpp:17:9:17:11 | StoreValue | test.cpp:17:10:17:11 | mc | test.cpp:17:9:17:11 | StoreValue | May return stack-allocated memory from $@. | test.cpp:17:10:17:11 | mc | mc |
| test.cpp:25:9:25:11 | StoreValue | test.cpp:23:18:23:19 | mc | test.cpp:25:9:25:11 | StoreValue | May return stack-allocated memory from $@. | test.cpp:23:18:23:19 | mc | mc |