lgtm: Add query to detect problematic uses of chunk_from_chars()

GCC 9+ and clang 4+ (partially) optimize out usages of
chunk_from_chars() if the value is read outside of the block where the
macro is used.  For instance:

```
chunk_t chunk = chunk_empty;
if (...)
{
	chunk = chunk_from_chars(0x01, 0x06);
}
/* do something with chunk */
```

The chunk_from_chars() macro expands to a chunk_t declaration, which is
technically only defined inside that block.

Still, with older GCC versions the fourth line was compiled to something
like this:

```
mov     WORD PTR [rsp+14], 1537 # 0x0106 in little-endian
lea     rdx, [rsp+14]
mov     ecx, 2
```

However, with GCC 9.1 and -O2 the first instruction might be omitted
(strangely the others usually were not, so the chunk pointed to whatever
was stored on the stack).  It's not easily reproducible, so there are
situations where the seemingly identical code is not optimized in this
way.

This query should detect such problematic uses of the macro (definition
and usage in different blocks).

References #3249.
This commit is contained in:
Tobias Brunner 2020-01-27 15:16:51 +01:00
parent 9c6ab71782
commit 8ea13bbc5c
1 changed files with 51 additions and 0 deletions

View File

@ -0,0 +1,51 @@
/**
* @name Invalid use of chunk_from_chars() macro
* @description The chunk_from_chars() macro creates a temporary chunk_t, which
* is not defined outside of the block in which it has been used,
* therefore, compilers might optimize out the assignment.
* @kind path-problem
* @problem.severity error
* @id strongswan/invalid-chunk-from-chars
* @tags correctness
* @precision very-high
*/
import cpp
import DataFlow::PathGraph
import semmle.code.cpp.dataflow.DataFlow
class ChunkFromChars extends Expr {
ChunkFromChars() {
this = any(MacroInvocation mi |
mi.getOutermostMacroAccess().getMacroName() = "chunk_from_chars"
/* ignore global static uses of the macro */
and exists (Block b | mi.getExpr().getEnclosingBlock() = b)
).getExpr()
}
}
class ChunkFromCharsUsage extends DataFlow::Configuration {
ChunkFromCharsUsage() { this = "ChunkFromCharsUsage" }
override predicate isSource(DataFlow::Node source) {
source.asExpr() instanceof ChunkFromChars
}
override predicate isSink(DataFlow::Node sink) {
exists(sink.asExpr())
}
override predicate isBarrierOut(DataFlow::Node node) {
/* don't track beyond function calls */
exists(FunctionCall fc | node.asExpr().getParent*() = fc)
}
}
Block enclosingBlock(Block b) {
result = b.getEnclosingBlock()
}
from ChunkFromCharsUsage usage, DataFlow::PathNode source, DataFlow::PathNode sink
where
usage.hasFlowPath(source, sink)
and not source.getNode().asExpr().getEnclosingBlock() = enclosingBlock*(sink.getNode().asExpr().getEnclosingBlock())
select source, source, sink, "Invalid use of chunk_from_chars() result in sibling/parent block."