Moved IR flavors into "implementation", with internal files under "implementation/internal". Made `IRBlockConstruction` just a nested module of `IRConstruction`/`SSAConstruction`, so it gets picked up from the `Construction` parameter of the `IR` module, rather than being picked up just from being in the same directory as `IRBlock`.
This change makes the public IR.qll module resolve to the flavor of the IR that we want queries to use. Today, this is the aliased SSA flavor of the IR. Should we add additional IR iterations in the future, we'll update IR.qll to resolve to whichever one we consider the default.
I moved the PrintIR.ql and IRSanity.ql queries into the internal directories of the corresponding flavors. There's still a PrintIR.ql and an IRSanity.ql in the public IR directory, which use the same IR flavor as the public IR.qll.
There are no real code changes here, other than to fix up `import`s. All tests still hae the same output, as expected.
A future commit will hide the IR flavors other than the one we want queries to use directly.
This commit was reverted on `master` but should remain on `next`, so I'm
reverting the revert before merging `master` into `next`.
This reverts commit adda4c91cf.
This commit to update test changes got merged to Semmle/ql master but
doesn't belong there because it's not compatible with how the 1.18
extractor works. The corresponding extractor change got merged to the
internal-repo master right after the internal branch for 1.18 was taken.
This reverts commit d4f9b5eb52.
Globals can still have declarations in declaration statements.
We already rule out local variables etc via the isTopLevel check,
so we don't need to consider DeclStmt.
Many classes have been declared with `extends @cfgnode` because they
should be implemented internally as a control-flow node but should not
expose the member predicates of `ControlFlowNode` to their users. After
the transition in a1e44041e it became mandatory to convert explicitly
between the `Element`-derived `ControlFlowNode` and the raw dbtype
`@cfgnode`, and that commit inserted numerous such conversions as a
result of having all those classes that did not derive from `Element` in
the standard library.
It was also confusing and error-prone that the libraries implementing
`ControlFlowNode` referred to `ControlFlowNode`. This seemingly cyclic
reference worked out because the libraries did not call the predicates
on `ControlFlowNode` whose implementation they were part of.
Both these problems are now solved by adding a new class
`ControlFlowNodeBase extends Element` that should be used in preference
to `@cfgnode` everywhere. This class is for exactly those use cases
where `@cfgnode` should be seen as an `Element` without having too many
member predicates on it.
The classes that move from extending `@cfgnode` to extending
`ControlFlowNodeBase` are: `BasicBlock`, `AdditionalControlFlowEdge`,
`DefOrUse`, `SsaDefinition`, `SubBasicBlock` and `RangeSsaDefinition`.
These previously had to define their own `toString` rootdef, which
typically had some dummy string as result (like `"BasicBlock"`), but now
their `toString` is part of the `Element` rootdef and should not be
overridden otherwise `Element.toString` will sometimes have multiple
results. Removing these dummy `toString` predicates had some effects on
the tests that are included in this commit.
The `getLocation` family of predicates is affected like `toString`, but
the situation is slightly different. Some of these classes had genuinely
useful alternative definitions of locations. Fortunately, they all used
`hasLocationInfo`, which is preferred over `getLocation` by the QL
engine. Because `Element` does not define `getLocationInfo`, each class
can create its own rootdef of this predicate like before.
Now that opcodes are in their own module that isn't imported into the global namespace, `Opcode::Call` no longer conflicts with `Call` from the ASTs. I've renamed `Opcode::Invoke` to `Opcode::Call`.
PrintAST.ql orders the functions by location, then in lexicographical order of the function signature. This is supposed to ensure a stable ordering, but functions without a location were not getting assigned an order at all.
I introduced some unnecessary base classes in the `TranslatedExpr` hierarchy with a previous commit. This commit refactors the hierarchy a bit to align with the following high-level description:
`TranslatedExpr` represents a translated piece of an `Expr`. Each `Expr` has exactly one `TranslatedCoreExpr`, which produces the result of that `Expr` ignoring any lvalue-to-rvalue conversion on its result. If an lvalue-to-rvalue converison is present, there is an additional `TranslatedLoad` for that `Expr` to do the conversion. For higher-level `Expr`s like `NewExpr`, there can also be additional `TranslatedExpr`s to represent the sub-operations within the overall `Expr`, such as the allocator call.
These expressions are a little trickier than most because they include an implicit call to an allocator function. The database tells us which function to call, but we have to synthesize the allocation size and alignment arguments ourselves. The alignment argument, if it exists, is always a constant, but the size argument requires multiplication by the element count for most `NewArrayExpr`s. I introduced the new `TranslatedAllocationSize` class to handle this.
The IR avoids having non-trivially-copyable and non-trivially-assignable types in register results, because objects of those types need to exist at a particular memory location. The `InitializeParameter` and `Uninitialized` instructions were violating this restriction because they returned register results, which were then stored into the destination location via a `Store`.
This change makes those two instructions take the destination address as an operand, and return a memory result representing the (un-)initialized memory, removing the need for a separate `Store` instruction.
Casts to `void` did not have a semantic conversion type in the AST, so they also weren't getting generated correctly in the IR. I've added a `VoidConversion` class to the AST, along with tests. I've also added IR translation for such conversions, using a new `ConvertToVoid` opcode. I'm not sure if it's really necessary to generate an instruction to represent this, but it may be useful for detecting values that are explicitly unused (e.g. return value from a call).
I added two new sanity queries for the IR to detect the following:
- IR blocks with no successors, which usually indicates bad IR translation
- Phi instruction without an operand for one of the predecessor blocks.
These sanity queries found another subtle IR translation bug. If an expression that is normally translated as a condition (e.g. `&&`, `||`, or parens in certain contexts) has a constant value, we were not creating a `TranslatedExpr` for the expression at all. I changed it to always treat a constant condition as a non-condition expression.
