The C++ IR currently has a very clunky way of specifying the type of an IR entity (`Instruction`, `Operand`, `IRVariable`, etc.). There are three separate predicates: `getType()`, `isGLValue()`, and `getSize()`. All three are necessary, rather than just having a `getType()` predicate, because some IR entities have types that are not represented via an existing `Type` object in the AST. Examples include the type for an lvalue returned from a `VariableAddress` instruction, the type for an array slice being zero-initialized in a variable initializer, and several others. It is very easy for QL code to just check the `getType()` predicate, while forgetting to use `isGLValue()` to determine if that type is the actual type of the entity (the prvalue case) or the type referred to by a glvalue entity. Furthermore, the C++ type system creates potentially many different `Type` objects for the same underlying type (e.g. typedefs, using declarations, `const`/`volatile` qualifiers, etc.), making it more difficult to tell when two entities have semantically equivalent types.
In addition, other languages for which we want to enable the IR have somewhat different type systems. The various language type systems differ in their structure, although they tend to share the basic building blocks necessary for the IR.
To address all of the above problems, I've introduced a new class hierarchy, rooted at the class `IRType`, that represents a bare-bones type system that is independent of source language (at least across C/C++/C#/Java). A type's identity is based on its kind (signed integer, unsigned integer, floating-point, Boolean, blob, etc.), size and in the case of blob types, a "tag" to differentiate between different classes and structs. No distinction is made between, say `signed int` and plain `int`, or between different language integer types that have the same signedness and size (e.g. `unsigned int` vs. `wchar_t` on Linux). `IRType` is intended for use by language-agnostic IR-based analyses, including range analysis, dataflow, SSA construction, and alias analysis. The set of available `IRType`s is determined by predicate provided by the language library implementation (e.g. `hasSignedIntegerType(int byteSize)`.
In addition to `IRType`, each language now defines a type alias named `LanguageType`, representing the type of an IR entity in more language-specific terms. The only predicate requried on `LanguageType` is `getIRType()`, which returns the single `IRType` object for the language-neutral representation of that `LanguageType`. All other predicates on and subclasses of `LanguageType` are language-specific. There may be many instances of `LanguageType` that map to a given `IRType`, to allow for typedefs, etc.
Most of the changes are mechanical changes in the IR construction code, to return the correct type for each IR entity. SSA construction has also been updated to avoid dependencies on language-specific types.
I have not yet removed the original `getType()` predicates that just return `Type`. These can be removed once we move the remaining existing libraries to use `IRType`.
Test results are, by design, pretty much unchanged. Once case changed for inline asm, because the previously IR generation for it played a little fast and loose with the input/output expressions. The test case now includes both input and output variables. The generated IR for `Conditional_LValue` is now more correct, because we now have a way to represent an lvalue of an lvalue. `syntax-zoo` is still a hot mess. Most of the changed outputs are due to wobble from having multiple functions with the same name, but with a slightly different order of evaluation due to the type changes. Others are wobble from already-invalid IR. A couple non-wobbly places have improved slightly, though.
The C# part of this change is waiting for #2005 to be merged, since that has some of the necessary C# implementation.
Lack of support for the GCC vector extensions was causing a bunch of sanity failures in the syntax zoo. This PR adds minimal IR generation support for these types.
Added `VectorAggregateLiteral`, and factored most of `ArrayAggregateLiteral` out into the common base class `ArrayOrVectorAggregateLiteral`. I'd be happy to merge these all into `ArrayAggregateLiteral` if we don't care about the distinction.
Made a few tweaks to `TranslatedArrayExpr` to compute the element type by looking at the result type of the `ArrayExpr`, not the type of the base operand. Note that this means that for `T a[10]; a[i] = foo;`, the result of the `PointerAdd` for `a[i]` will now be `glvalue<T>`, not `T*`. This is actually more faithful to the source language, and has no semantic difference on the IR.
Added some missing `getInstructionElementSize()` overrides.
Added the new `BuiltIn` opcode, renamed the existing `BuiltInInstruction` to `BuiltInOperationInstruction`, and made any `BuiltInOperation` that we don't specifically handle translate to `BuiltIn`. `BuiltInOperationInstruction` now has a way to get the specific `BuiltInOperation`.
Added `getCanonicalQLClass()` overrides for `GNUVectorType` and `BuiltInOperation`.
Added a simple IR test for vector types.
There were two problems here.
1. The inline predicates `isInitialized` and `isValueInitialized` on
`ArrayAggregateLiteral` caused their callers to materialize every
`int` that was a valid index into the array. This was slow on huge
value-initialized arrays.
2. The `isInitialized` predicate was used in the `TInstructionTag` IPA
type, creating a numbered tuple for each integer in it. This seemed
to be entirely unnecessary since the `TranslatedElement`s using those
tags were already indexed appropriately.
My original fix in https://github.com/Semmle/ql/pull/1661 fixed my minimal test case, but did not fix the original failure in a Linux snapshot. The real fix is to simply not create a `TranslatedDeclarationEntry` for an extern declaration, and have `TranslatedDeclStmt` skip any such declarations. I've added a regression test for that case (multiple extern declarations with same location in a macro expansion, with control flow between them). I did verify that it generates correct IR, and that it fixes all of the "use not dominated by definition" failures in Linux.
The underlying extractor bug, that caused the above issue also caused PrintAST to print garbage. I've worked around the bug in PrintAST.qll.
I've also fixed a bug in the control flow for `try`/`catch`, where there was missing flow from the `CatchByType` of the last handler of a `try` to the enclosing handler (or `Unwind`). Hat tip to @AndreiDiaconu1 for spotting this bug.
Previously, where we had a function-scoped `DeclarationEntry` for an extern variable or function, we would generate a `NoOp` instruction for it. There's nothing wrong with this by itself, although it was unnecessary. However, I've hit an extractor issue (Jira ticket already opened) that commonly causes multiple `DeclStmt`s to share a single `DeclarationEntry` child on extern declarations, so removing the `NoOp` instructions is an easy way to work around the extractor issue.
The previous version of the test used `0 = 1;` to test an lvalue-typed
`ErrorExpr`, but the extractor replaced the whole assignment expression
with `ErrorExpr` instead of just the LHS. This variation of the test
only leads to an `ErrorExpr` for the part of the syntax that's supposed
to be an lvalue-typed expression, so that's an improvement.
Unfortunately it still doesn't demonstrate that we can `Store` into an
address computed by an `ErrorExpr`.
Before this change, `delete` and `delete[]` expressions had no control
flow after them, which caused the reachability analysis to remove all
code after a delete expression. This commit adds placeholder support for
delete expression by translating them to `NoOp` instructions so their
presence doesn't cause large chunks of the program to be removed.
IR construction was missing support for C++ 11 range-based `for` loops. The extractor generates ASTs for the compiler-generated implementation already, so I had enough information to generate IR. I've expanded on some of the predicates in `RangeBasedForStmt` to access the desugared information.
One complication was that the `DeclStmt`s for the compiler-generated variables seem to have results for `getDeclaration()` but not for `getDeclarationEntry()`. This required handling these slightly differently than we do for other `DeclStmt`s.
The flow for range-based `for` is actually easier than for a regular `for`, because all three components (init, condition, and update) are always present.
