From 69417e150ae7bbb6ad1f909fbb46e95bb39c1dd2 Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Tue, 8 Mar 2022 13:15:02 +0000 Subject: [PATCH] C++: Address review comments. --- cpp/ql/lib/semmle/code/cpp/ir/dataflow/MustFlow.qll | 5 +++++ .../ReturnStackAllocatedMemory.expected | 13 +++++++++++++ .../ReturnStackAllocatedMemory/test.cpp | 9 +++++++++ 3 files changed, 27 insertions(+) diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/MustFlow.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/MustFlow.qll index 80dd57b5a75..1f3ea2a4d3d 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/MustFlow.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/MustFlow.qll @@ -177,6 +177,11 @@ private module Cached { operand.getDef() = instr } + /** + * Holds if data flows from `operand` to `instr`. + * + * This predicate ignores flow through `PhiInstruction`s to create a 'must flow' relation. + */ private predicate operandToInstructionStep(Operand operand, Instruction instr) { instr.(CopyInstruction).getSourceValueOperand() = operand or diff --git a/cpp/ql/test/query-tests/Likely Bugs/Memory Management/ReturnStackAllocatedMemory/ReturnStackAllocatedMemory.expected b/cpp/ql/test/query-tests/Likely Bugs/Memory Management/ReturnStackAllocatedMemory/ReturnStackAllocatedMemory.expected index 8f9d91fc1ad..6b8a59793a3 100644 --- a/cpp/ql/test/query-tests/Likely Bugs/Memory Management/ReturnStackAllocatedMemory/ReturnStackAllocatedMemory.expected +++ b/cpp/ql/test/query-tests/Likely Bugs/Memory Management/ReturnStackAllocatedMemory/ReturnStackAllocatedMemory.expected @@ -100,6 +100,12 @@ 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 | @@ -215,6 +221,13 @@ 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 | diff --git a/cpp/ql/test/query-tests/Likely Bugs/Memory Management/ReturnStackAllocatedMemory/test.cpp b/cpp/ql/test/query-tests/Likely Bugs/Memory Management/ReturnStackAllocatedMemory/test.cpp index a1726169654..2b53f9c0adf 100644 --- a/cpp/ql/test/query-tests/Likely Bugs/Memory Management/ReturnStackAllocatedMemory/test.cpp +++ b/cpp/ql/test/query-tests/Likely Bugs/Memory Management/ReturnStackAllocatedMemory/test.cpp @@ -220,4 +220,13 @@ auto make_read_port() void* get_sp() { int p; return (void*)&p; // GOOD: The function name makes it sound like the programmer intended to get the value of the stack pointer. +} + +int* id(int* px) { + return px; // GOOD +} + +void f() { + int x; + int* px = id(&x); // GOOD } \ No newline at end of file