Skip to content

Commit c560c75

Browse files
committed
C++: Add QLDoc for BufferMayWriteSideEffectFieldStoreQualifierNode
1 parent 49dd576 commit c560c75

File tree

2 files changed

+19
-11
lines changed

2 files changed

+19
-11
lines changed

cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -325,6 +325,19 @@ private predicate fieldReadStep(Node node1, FieldContent f, Node node2) {
325325
)
326326
}
327327

328+
/**
329+
* When a store step happens in a function that looks like an array write such as:
330+
* ```cpp
331+
* void f(int* pa) {
332+
* pa = source();
333+
* }
334+
* ```
335+
* it can be a write to an array, but it can also happen that `f` is called as `f(&a.x)`. If that is
336+
* the case, the `ArrayContent` that was written by the call to `f` should be popped off the access
337+
* path, and a `FieldContent` containing `x` should be pushed instead.
338+
* So this case pops `ArrayContent` off the access path, and the `fieldStoreStepAfterArraySuppression`
339+
* predicate in `storeStep` ensures that we push the right `FieldContent` onto the access path.
340+
*/
328341
predicate suppressArrayRead(Node node1, ArrayContent a, Node node2) {
329342
a = TArrayContent() and
330343
exists(BufferMayWriteSideEffectInstruction write, ChiInstruction chi |
@@ -383,17 +396,6 @@ private predicate arrayReadStep(Node node1, ArrayContent a, Node node2) {
383396
predicate readStep(Node node1, Content f, Node node2) {
384397
fieldReadStep(node1, f, node2) or
385398
arrayReadStep(node1, f, node2) or
386-
// When a store step happens in a function that looks like an array write such as:
387-
// ```cpp
388-
// void f(int* pa) {
389-
// *pa = source();
390-
// }
391-
// ```
392-
// it can be a write to an array, but it can also happen that `f` is called as `f(&a.x)`. If that is
393-
// the case, the `ArrayContent` that was written by the call to `f` should be popped off the access
394-
// path, and a `FieldContent` containing `x` should be pushed instead.
395-
// So this case pops `ArrayContent` off the access path, and the `fieldStoreStepAfterArraySuppression`
396-
// predicate in `storeStep` ensures that we push the right `FieldContent` onto the access path.
397399
suppressArrayRead(node1, f, node2)
398400
}
399401

cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -394,6 +394,12 @@ private FieldAddressInstruction getFieldInstruction(Instruction instr) {
394394
result = instr.(CopyValueInstruction).getUnary()
395395
}
396396

397+
/**
398+
* The target of a `fieldStoreStepAfterArraySuppression` store step, which is used to convert
399+
* an `ArrayContent` to a `FieldContent` when the `BufferMayWriteSideEffect` instruction stores
400+
* into a field. See the QLDoc for `suppressArrayRead` for an example of where such a conversion
401+
* is inserted.
402+
*/
397403
private class BufferMayWriteSideEffectFieldStoreQualifierNode extends PartialDefinitionNode {
398404
override ChiInstruction instr;
399405
BufferMayWriteSideEffectInstruction write;

0 commit comments

Comments
 (0)