Skip to content

Commit 2264ec7

Browse files
author
Dave Bartolomeo
committed
C++: Better type preservation in getVariableType()
`getVariableType()` is used to compute the actual semantic type of a variable from its declared type. That's where we handle pointer and function decay for parameters, and it's also where we handle arrays of unknown bound initialized with an initializer of known bound. Previously, even if neither of the above situations applied, the type that we returned was the `getUnspecifiedType()` of the variable. This meant that, for example, `const char* p` would be treated as `char *`. This is inconsistent with how we handle types elsewhere in IR construction, where we preserve typedefs and cv-qualifiers when creating the `CppType` of an `IRVariable`, `Instruction`, or `Operand`. The only visible effect this fix has is to fix the inferred result type for `Phi` instructions for variables affect by this change in `getVariableType()` behavior. Previously, we would see the variable accessed as both `const char*` and as `char*`, so we'd fall back to the canonical pointer type, which is `decltype(nullptr)`. Now, we see the same type for all accesses to the variable, so we use that type as the type of the SSA memory location and as the result type of the `Phi` instruction.
1 parent 90dc14c commit 2264ec7

File tree

2 files changed

+18
-15
lines changed

2 files changed

+18
-15
lines changed

cpp/ql/src/semmle/code/cpp/ir/internal/IRUtilities.qll

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,17 +23,20 @@ Type getVariableType(Variable v) {
2323
then
2424
result = getDecayedType(declaredType)
2525
or
26-
not exists(getDecayedType(declaredType)) and result = declaredType
26+
not exists(getDecayedType(declaredType)) and result = v.getType()
2727
else
2828
if declaredType instanceof ArrayType and not declaredType.(ArrayType).hasArraySize()
2929
then
30-
result = v.getInitializer().getExpr().getUnspecifiedType()
30+
result = v.getInitializer().getExpr().getType()
3131
or
32-
not exists(v.getInitializer()) and result = declaredType
33-
else result = declaredType
32+
not exists(v.getInitializer()) and result = v.getType()
33+
else result = v.getType()
3434
)
3535
}
3636

37+
/**
38+
* Holds if the database contains a `case` label with the specified minimum and maximum value.
39+
*/
3740
predicate hasCaseEdge(SwitchCase switchCase, string minValue, string maxValue) {
3841
minValue = switchCase.getExpr().getFullyConverted().getValue() and
3942
if exists(switchCase.getEndExpr())

cpp/ql/test/library-tests/valuenumbering/GlobalValueNumbering/ir_gvn.expected

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -433,27 +433,27 @@ test.cpp:
433433
#-----| Goto -> Block 3
434434

435435
# 56| Block 3
436-
# 56| m56_1(decltype(nullptr)) = Phi : from 2:m55_4, from 5:m56_23
436+
# 56| m56_1(char *) = Phi : from 2:m55_4, from 5:m56_23
437437
# 56| valnum = m56_1, r56_13, r56_20, r56_3, r59_2
438-
# 56| r56_2(glval<char *>) = VariableAddress[ptr] :
438+
# 56| r56_2(glval<char *>) = VariableAddress[ptr] :
439439
# 56| valnum = r50_1, r55_3, r56_12, r56_19, r56_2, r59_1
440-
# 56| r56_3(char *) = Load : &:r56_2, m56_1
440+
# 56| r56_3(char *) = Load : &:r56_2, m56_1
441441
# 56| valnum = m56_1, r56_13, r56_20, r56_3, r59_2
442-
# 56| r56_4(char) = Load : &:r56_3, ~m49_4
442+
# 56| r56_4(char) = Load : &:r56_3, ~m49_4
443443
# 56| valnum = r56_14, r56_4, r59_3
444-
# 56| r56_5(int) = Convert : r56_4
444+
# 56| r56_5(int) = Convert : r56_4
445445
# 56| valnum = r56_15, r56_5, r59_4
446-
# 56| r56_6(glval<char *>) = VariableAddress[str] :
446+
# 56| r56_6(glval<char *>) = VariableAddress[str] :
447447
# 56| valnum = r49_6, r53_2, r56_6
448-
# 56| r56_7(char *) = Load : &:r56_6, m49_7
448+
# 56| r56_7(char *) = Load : &:r56_6, m49_7
449449
# 56| valnum = m49_7, r49_8, r53_3, r56_7
450-
# 56| r56_8(char) = Load : &:r56_7, ~m49_9
450+
# 56| r56_8(char) = Load : &:r56_7, ~m49_9
451451
# 56| valnum = r53_4, r56_8
452-
# 56| r56_9(int) = Convert : r56_8
452+
# 56| r56_9(int) = Convert : r56_8
453453
# 56| valnum = r53_5, r56_9
454-
# 56| r56_10(bool) = CompareNE : r56_5, r56_9
454+
# 56| r56_10(bool) = CompareNE : r56_5, r56_9
455455
# 56| valnum = unique
456-
# 56| v56_11(void) = ConditionalBranch : r56_10
456+
# 56| v56_11(void) = ConditionalBranch : r56_10
457457
#-----| False -> Block 6
458458
#-----| True -> Block 4
459459

0 commit comments

Comments
 (0)