diff --git a/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/gvn/ValueNumbering.qll b/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/gvn/ValueNumbering.qll index 6791f77b7b6..ec6d786a684 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/gvn/ValueNumbering.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/gvn/ValueNumbering.qll @@ -52,6 +52,11 @@ newtype TValueNumber = ) { inheritanceConversionValueNumber(_, irFunc, opcode, baseClass, derivedClass, operand) } or + TLoadTotalOverlapValueNumber( + IRFunction irFunc, IRType type, ValueNumber memOperand, ValueNumber operand + ) { + loadTotalOverlapValueNumber(_, irFunc, type, memOperand, operand) + } or TUniqueValueNumber(IRFunction irFunc, Instruction instr) { uniqueValueNumber(instr, irFunc) } /** @@ -101,12 +106,18 @@ class ValueNumber extends TValueNumber { * The use of `p.x` on line 3 is linked to the definition of `p` on line 1 as well, but is not * congruent to that definition because `p.x` accesses only a subset of the memory defined by `p`. */ -private class CongruentCopyInstruction extends CopyInstruction { +class CongruentCopyInstruction extends CopyInstruction { CongruentCopyInstruction() { this.getSourceValueOperand().getDefinitionOverlap() instanceof MustExactlyOverlap } } +class LoadTotalOverlapInstruction extends LoadInstruction { + LoadTotalOverlapInstruction() { + this.getSourceValueOperand().getDefinitionOverlap() instanceof MustTotallyOverlap + } +} + /** * Holds if this library knows how to assign a value number to the specified instruction, other than * a `unique` value number that is never shared by multiple instructions. @@ -131,6 +142,8 @@ private predicate numberableInstruction(Instruction instr) { instr instanceof PointerArithmeticInstruction or instr instanceof CongruentCopyInstruction + or + instr instanceof LoadTotalOverlapInstruction } private predicate variableAddressValueNumber( @@ -205,6 +218,7 @@ private predicate unaryValueNumber( instr.getEnclosingIRFunction() = irFunc and not instr instanceof InheritanceConversionInstruction and not instr instanceof CopyInstruction and + not instr instanceof FieldAddressInstruction and instr.getOpcode() = opcode and instr.getResultIRType() = type and valueNumber(instr.getUnary()) = operand @@ -221,6 +235,16 @@ private predicate inheritanceConversionValueNumber( valueNumber(instr.getUnary()) = operand } +private predicate loadTotalOverlapValueNumber( + LoadTotalOverlapInstruction instr, IRFunction irFunc, IRType type, ValueNumber memOperand, + ValueNumber operand +) { + instr.getEnclosingIRFunction() = irFunc and + instr.getResultIRType() = type and + valueNumber(instr.getAnOperand().(MemoryOperand).getAnyDef()) = memOperand and + valueNumberOfOperand(instr.getAnOperand().(AddressOperand)) = operand +} + /** * Holds if `instr` should be assigned a unique value number because this library does not know how * to determine if two instances of that instruction are equivalent. @@ -313,6 +337,11 @@ private ValueNumber nonUniqueValueNumber(Instruction instr) { TPointerArithmeticValueNumber(irFunc, opcode, type, elementSize, leftOperand, rightOperand) ) or + exists(IRType type, ValueNumber memOperand, ValueNumber operand | + loadTotalOverlapValueNumber(instr, irFunc, type, memOperand, operand) and + result = TLoadTotalOverlapValueNumber(irFunc, type, memOperand, operand) + ) + or // The value number of a copy is just the value number of its source value. result = valueNumber(instr.(CongruentCopyInstruction).getSourceValue()) ) diff --git a/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/gvn/ValueNumbering.qll b/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/gvn/ValueNumbering.qll index 6791f77b7b6..ec6d786a684 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/gvn/ValueNumbering.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/gvn/ValueNumbering.qll @@ -52,6 +52,11 @@ newtype TValueNumber = ) { inheritanceConversionValueNumber(_, irFunc, opcode, baseClass, derivedClass, operand) } or + TLoadTotalOverlapValueNumber( + IRFunction irFunc, IRType type, ValueNumber memOperand, ValueNumber operand + ) { + loadTotalOverlapValueNumber(_, irFunc, type, memOperand, operand) + } or TUniqueValueNumber(IRFunction irFunc, Instruction instr) { uniqueValueNumber(instr, irFunc) } /** @@ -101,12 +106,18 @@ class ValueNumber extends TValueNumber { * The use of `p.x` on line 3 is linked to the definition of `p` on line 1 as well, but is not * congruent to that definition because `p.x` accesses only a subset of the memory defined by `p`. */ -private class CongruentCopyInstruction extends CopyInstruction { +class CongruentCopyInstruction extends CopyInstruction { CongruentCopyInstruction() { this.getSourceValueOperand().getDefinitionOverlap() instanceof MustExactlyOverlap } } +class LoadTotalOverlapInstruction extends LoadInstruction { + LoadTotalOverlapInstruction() { + this.getSourceValueOperand().getDefinitionOverlap() instanceof MustTotallyOverlap + } +} + /** * Holds if this library knows how to assign a value number to the specified instruction, other than * a `unique` value number that is never shared by multiple instructions. @@ -131,6 +142,8 @@ private predicate numberableInstruction(Instruction instr) { instr instanceof PointerArithmeticInstruction or instr instanceof CongruentCopyInstruction + or + instr instanceof LoadTotalOverlapInstruction } private predicate variableAddressValueNumber( @@ -205,6 +218,7 @@ private predicate unaryValueNumber( instr.getEnclosingIRFunction() = irFunc and not instr instanceof InheritanceConversionInstruction and not instr instanceof CopyInstruction and + not instr instanceof FieldAddressInstruction and instr.getOpcode() = opcode and instr.getResultIRType() = type and valueNumber(instr.getUnary()) = operand @@ -221,6 +235,16 @@ private predicate inheritanceConversionValueNumber( valueNumber(instr.getUnary()) = operand } +private predicate loadTotalOverlapValueNumber( + LoadTotalOverlapInstruction instr, IRFunction irFunc, IRType type, ValueNumber memOperand, + ValueNumber operand +) { + instr.getEnclosingIRFunction() = irFunc and + instr.getResultIRType() = type and + valueNumber(instr.getAnOperand().(MemoryOperand).getAnyDef()) = memOperand and + valueNumberOfOperand(instr.getAnOperand().(AddressOperand)) = operand +} + /** * Holds if `instr` should be assigned a unique value number because this library does not know how * to determine if two instances of that instruction are equivalent. @@ -313,6 +337,11 @@ private ValueNumber nonUniqueValueNumber(Instruction instr) { TPointerArithmeticValueNumber(irFunc, opcode, type, elementSize, leftOperand, rightOperand) ) or + exists(IRType type, ValueNumber memOperand, ValueNumber operand | + loadTotalOverlapValueNumber(instr, irFunc, type, memOperand, operand) and + result = TLoadTotalOverlapValueNumber(irFunc, type, memOperand, operand) + ) + or // The value number of a copy is just the value number of its source value. result = valueNumber(instr.(CongruentCopyInstruction).getSourceValue()) ) diff --git a/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/gvn/ValueNumbering.qll b/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/gvn/ValueNumbering.qll index 6791f77b7b6..ec6d786a684 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/gvn/ValueNumbering.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/gvn/ValueNumbering.qll @@ -52,6 +52,11 @@ newtype TValueNumber = ) { inheritanceConversionValueNumber(_, irFunc, opcode, baseClass, derivedClass, operand) } or + TLoadTotalOverlapValueNumber( + IRFunction irFunc, IRType type, ValueNumber memOperand, ValueNumber operand + ) { + loadTotalOverlapValueNumber(_, irFunc, type, memOperand, operand) + } or TUniqueValueNumber(IRFunction irFunc, Instruction instr) { uniqueValueNumber(instr, irFunc) } /** @@ -101,12 +106,18 @@ class ValueNumber extends TValueNumber { * The use of `p.x` on line 3 is linked to the definition of `p` on line 1 as well, but is not * congruent to that definition because `p.x` accesses only a subset of the memory defined by `p`. */ -private class CongruentCopyInstruction extends CopyInstruction { +class CongruentCopyInstruction extends CopyInstruction { CongruentCopyInstruction() { this.getSourceValueOperand().getDefinitionOverlap() instanceof MustExactlyOverlap } } +class LoadTotalOverlapInstruction extends LoadInstruction { + LoadTotalOverlapInstruction() { + this.getSourceValueOperand().getDefinitionOverlap() instanceof MustTotallyOverlap + } +} + /** * Holds if this library knows how to assign a value number to the specified instruction, other than * a `unique` value number that is never shared by multiple instructions. @@ -131,6 +142,8 @@ private predicate numberableInstruction(Instruction instr) { instr instanceof PointerArithmeticInstruction or instr instanceof CongruentCopyInstruction + or + instr instanceof LoadTotalOverlapInstruction } private predicate variableAddressValueNumber( @@ -205,6 +218,7 @@ private predicate unaryValueNumber( instr.getEnclosingIRFunction() = irFunc and not instr instanceof InheritanceConversionInstruction and not instr instanceof CopyInstruction and + not instr instanceof FieldAddressInstruction and instr.getOpcode() = opcode and instr.getResultIRType() = type and valueNumber(instr.getUnary()) = operand @@ -221,6 +235,16 @@ private predicate inheritanceConversionValueNumber( valueNumber(instr.getUnary()) = operand } +private predicate loadTotalOverlapValueNumber( + LoadTotalOverlapInstruction instr, IRFunction irFunc, IRType type, ValueNumber memOperand, + ValueNumber operand +) { + instr.getEnclosingIRFunction() = irFunc and + instr.getResultIRType() = type and + valueNumber(instr.getAnOperand().(MemoryOperand).getAnyDef()) = memOperand and + valueNumberOfOperand(instr.getAnOperand().(AddressOperand)) = operand +} + /** * Holds if `instr` should be assigned a unique value number because this library does not know how * to determine if two instances of that instruction are equivalent. @@ -313,6 +337,11 @@ private ValueNumber nonUniqueValueNumber(Instruction instr) { TPointerArithmeticValueNumber(irFunc, opcode, type, elementSize, leftOperand, rightOperand) ) or + exists(IRType type, ValueNumber memOperand, ValueNumber operand | + loadTotalOverlapValueNumber(instr, irFunc, type, memOperand, operand) and + result = TLoadTotalOverlapValueNumber(irFunc, type, memOperand, operand) + ) + or // The value number of a copy is just the value number of its source value. result = valueNumber(instr.(CongruentCopyInstruction).getSourceValue()) ) diff --git a/cpp/ql/test/library-tests/valuenumbering/GlobalValueNumbering/GlobalValueNumbering.expected b/cpp/ql/test/library-tests/valuenumbering/GlobalValueNumbering/GlobalValueNumbering.expected index d7c3740aed3..d5d46ee0b72 100644 --- a/cpp/ql/test/library-tests/valuenumbering/GlobalValueNumbering/GlobalValueNumbering.expected +++ b/cpp/ql/test/library-tests/valuenumbering/GlobalValueNumbering/GlobalValueNumbering.expected @@ -30,3 +30,11 @@ | test.cpp:105:11:105:12 | (Base *)... | 105:c11-c12 106:c14-c35 107:c11-c12 | | test.cpp:105:11:105:12 | pd | 105:c11-c12 106:c33-c34 | | test.cpp:105:15:105:15 | b | 105:c15-c15 107:c15-c15 109:c10-c10 | +| test.cpp:125:11:125:12 | pa | 125:c11-c12 126:c11-c12 128:c3-c4 129:c11-c12 | +| test.cpp:125:15:125:15 | x | 125:c15-c15 126:c15-c15 128:c7-c7 | +| test.cpp:136:11:136:18 | global_a | 136:c11-c18 137:c11-c18 139:c3-c10 | +| test.cpp:136:21:136:21 | x | 136:c21-c21 137:c21-c21 139:c13-c13 | +| test.cpp:144:11:144:12 | pa | 144:c11-c12 145:c11-c12 147:c3-c4 149:c11-c12 | +| test.cpp:145:15:145:15 | y | 145:c15-c15 147:c7-c7 | +| test.cpp:153:11:153:18 | global_a | 153:c11-c18 154:c11-c18 156:c3-c10 | +| test.cpp:153:21:153:21 | x | 153:c21-c21 154:c21-c21 | diff --git a/cpp/ql/test/library-tests/valuenumbering/GlobalValueNumbering/ir_gvn.expected b/cpp/ql/test/library-tests/valuenumbering/GlobalValueNumbering/ir_gvn.expected index a2d2f5b6f5c..ad702554776 100644 --- a/cpp/ql/test/library-tests/valuenumbering/GlobalValueNumbering/ir_gvn.expected +++ b/cpp/ql/test/library-tests/valuenumbering/GlobalValueNumbering/ir_gvn.expected @@ -118,7 +118,7 @@ test.cpp: # 16| r16_6(glval) = VariableAddress[global01] : # 16| valnum = r16_6 # 16| r16_7(int) = Load : &:r16_6, ~m12_3 -# 16| valnum = unique +# 16| valnum = r16_7 # 16| r16_8(int) = Add : r16_5, r16_7 # 16| valnum = r16_8 # 16| r16_9(glval) = VariableAddress[x] : @@ -138,21 +138,21 @@ test.cpp: # 17| r17_6(glval) = VariableAddress[global01] : # 17| valnum = r16_6 # 17| r17_7(int) = Load : &:r17_6, ~m12_3 -# 17| valnum = unique +# 17| valnum = r16_7 # 17| r17_8(int) = Add : r17_5, r17_7 -# 17| valnum = r17_8 +# 17| valnum = r16_8 # 17| r17_9(glval) = VariableAddress[x] : # 17| valnum = r13_1 # 17| m17_10(int) = Store : &:r17_9, r17_8 -# 17| valnum = r17_8 +# 17| valnum = r16_8 # 18| r18_1(glval) = VariableAddress[x] : # 18| valnum = r13_1 # 18| r18_2(int) = Load : &:r18_1, m17_10 -# 18| valnum = r17_8 +# 18| valnum = r16_8 # 18| r18_3(glval) = VariableAddress[y] : # 18| valnum = r13_3 # 18| m18_4(int) = Store : &:r18_3, r18_2 -# 18| valnum = r17_8 +# 18| valnum = r16_8 # 19| v19_1(void) = NoOp : # 12| r12_10(glval) = VariableAddress[#return] : # 12| valnum = unique @@ -307,7 +307,7 @@ test.cpp: # 43| r43_6(glval) = VariableAddress[global03] : # 43| valnum = r43_6 # 43| r43_7(int) = Load : &:r43_6, ~m39_3 -# 43| valnum = unique +# 43| valnum = r43_7 # 43| r43_8(int) = Add : r43_5, r43_7 # 43| valnum = r43_8 # 43| r43_9(glval) = VariableAddress[x] : @@ -339,21 +339,21 @@ test.cpp: # 45| r45_6(glval) = VariableAddress[global03] : # 45| valnum = r43_6 # 45| r45_7(int) = Load : &:r45_6, ~m39_3 -# 45| valnum = unique +# 45| valnum = r43_7 # 45| r45_8(int) = Add : r45_5, r45_7 -# 45| valnum = r45_8 +# 45| valnum = r43_8 # 45| r45_9(glval) = VariableAddress[x] : # 45| valnum = r40_1 # 45| m45_10(int) = Store : &:r45_9, r45_8 -# 45| valnum = r45_8 +# 45| valnum = r43_8 # 46| r46_1(glval) = VariableAddress[x] : # 46| valnum = r40_1 # 46| r46_2(int) = Load : &:r46_1, m45_10 -# 46| valnum = r45_8 +# 46| valnum = r43_8 # 46| r46_3(glval) = VariableAddress[y] : # 46| valnum = r40_3 # 46| m46_4(int) = Store : &:r46_3, r46_2 -# 46| valnum = r45_8 +# 46| valnum = r43_8 # 47| v47_1(void) = NoOp : # 39| v39_14(void) = ReturnIndirection : &:r39_12, ~m44_6 # 39| r39_15(glval) = VariableAddress[#return] : @@ -410,9 +410,9 @@ test.cpp: # 53| r53_3(char *) = Load : &:r53_2, m49_7 # 53| valnum = m49_7 # 53| r53_4(char) = Load : &:r53_3, ~m49_9 -# 53| valnum = unique +# 53| valnum = r53_4 # 53| r53_5(int) = Convert : r53_4 -# 53| valnum = unique +# 53| valnum = r53_5 # 53| r53_6(int) = Constant[0] : # 53| valnum = r53_6 # 53| r53_7(bool) = CompareNE : r53_5, r53_6 @@ -448,9 +448,9 @@ test.cpp: # 56| r56_7(char *) = Load : &:r56_6, m49_7 # 56| valnum = m49_7 # 56| r56_8(char) = Load : &:r56_7, ~m49_9 -# 56| valnum = unique +# 56| valnum = r53_4 # 56| r56_9(int) = Convert : r56_8 -# 56| valnum = unique +# 56| valnum = r53_5 # 56| r56_10(bool) = CompareNE : r56_5, r56_9 # 56| valnum = unique # 56| v56_11(void) = ConditionalBranch : r56_10 @@ -807,17 +807,17 @@ test.cpp: # 107| r107_4(glval) = FieldAddress[b] : r107_3 # 107| valnum = r105_5 # 107| r107_5(int) = Load : &:r107_4, ~m104_9 -# 107| valnum = r107_5 +# 107| valnum = r105_6 # 107| m107_6(int) = Store : &:r107_1, r107_5 -# 107| valnum = r107_5 +# 107| valnum = r105_6 # 109| r109_1(glval) = VariableAddress[#return] : # 109| valnum = r109_1 # 109| r109_2(glval) = VariableAddress[y] : # 109| valnum = r107_1 # 109| r109_3(int) = Load : &:r109_2, m107_6 -# 109| valnum = r107_5 +# 109| valnum = r105_6 # 109| m109_4(int) = Store : &:r109_1, r109_3 -# 109| valnum = r107_5 +# 109| valnum = r105_6 # 104| v104_10(void) = ReturnIndirection : &:r104_8, m104_9 # 104| r104_11(glval) = VariableAddress[#return] : # 104| valnum = r109_1 @@ -850,3 +850,297 @@ test.cpp: # 112| v112_7(void) = UnmodeledUse : mu* # 112| v112_8(void) = AliasedUse : m112_3 # 112| v112_9(void) = ExitFunction : + +# 124| void test_read_arg_same(A*, int) +# 124| Block 0 +# 124| v124_1(void) = EnterFunction : +# 124| m124_2(unknown) = AliasedDefinition : +# 124| valnum = unique +# 124| m124_3(unknown) = InitializeNonLocal : +# 124| valnum = unique +# 124| m124_4(unknown) = Chi : total:m124_2, partial:m124_3 +# 124| valnum = unique +# 124| mu124_5(unknown) = UnmodeledDefinition : +# 124| valnum = unique +# 124| r124_6(glval) = VariableAddress[pa] : +# 124| valnum = r124_6 +# 124| m124_7(A *) = InitializeParameter[pa] : &:r124_6 +# 124| valnum = m124_7 +# 124| r124_8(A *) = Load : &:r124_6, m124_7 +# 124| valnum = m124_7 +# 124| m124_9(unknown) = InitializeIndirection[pa] : &:r124_8 +# 124| valnum = unique +# 124| r124_10(glval) = VariableAddress[n] : +# 124| valnum = r124_10 +# 124| m124_11(int) = InitializeParameter[n] : &:r124_10 +# 124| valnum = m124_11 +# 125| r125_1(glval) = VariableAddress[b] : +# 125| valnum = unique +# 125| r125_2(glval) = VariableAddress[pa] : +# 125| valnum = r124_6 +# 125| r125_3(A *) = Load : &:r125_2, m124_7 +# 125| valnum = m124_7 +# 125| r125_4(glval) = FieldAddress[x] : r125_3 +# 125| valnum = r125_4 +# 125| r125_5(int) = Load : &:r125_4, ~m124_9 +# 125| valnum = r125_5 +# 125| m125_6(int) = Store : &:r125_1, r125_5 +# 125| valnum = r125_5 +# 126| r126_1(glval) = VariableAddress[c] : +# 126| valnum = unique +# 126| r126_2(glval) = VariableAddress[pa] : +# 126| valnum = r124_6 +# 126| r126_3(A *) = Load : &:r126_2, m124_7 +# 126| valnum = m124_7 +# 126| r126_4(glval) = FieldAddress[x] : r126_3 +# 126| valnum = r125_4 +# 126| r126_5(int) = Load : &:r126_4, ~m124_9 +# 126| valnum = r125_5 +# 126| m126_6(int) = Store : &:r126_1, r126_5 +# 126| valnum = r125_5 +# 128| r128_1(glval) = VariableAddress[n] : +# 128| valnum = r124_10 +# 128| r128_2(int) = Load : &:r128_1, m124_11 +# 128| valnum = m124_11 +# 128| r128_3(glval) = VariableAddress[pa] : +# 128| valnum = r124_6 +# 128| r128_4(A *) = Load : &:r128_3, m124_7 +# 128| valnum = m124_7 +# 128| r128_5(glval) = FieldAddress[x] : r128_4 +# 128| valnum = r125_4 +# 128| m128_6(int) = Store : &:r128_5, r128_2 +# 128| valnum = m124_11 +# 128| m128_7(unknown) = Chi : total:m124_9, partial:m128_6 +# 128| valnum = unique +# 129| r129_1(glval) = VariableAddress[d] : +# 129| valnum = unique +# 129| r129_2(glval) = VariableAddress[pa] : +# 129| valnum = r124_6 +# 129| r129_3(A *) = Load : &:r129_2, m124_7 +# 129| valnum = m124_7 +# 129| r129_4(glval) = FieldAddress[x] : r129_3 +# 129| valnum = r125_4 +# 129| r129_5(int) = Load : &:r129_4, m128_6 +# 129| valnum = m124_11 +# 129| m129_6(int) = Store : &:r129_1, r129_5 +# 129| valnum = m124_11 +# 130| v130_1(void) = NoOp : +# 124| v124_12(void) = ReturnIndirection : &:r124_8, ~m128_7 +# 124| v124_13(void) = ReturnVoid : +# 124| v124_14(void) = UnmodeledUse : mu* +# 124| v124_15(void) = AliasedUse : m124_3 +# 124| v124_16(void) = ExitFunction : + +# 135| void test_read_global_same() +# 135| Block 0 +# 135| v135_1(void) = EnterFunction : +# 135| m135_2(unknown) = AliasedDefinition : +# 135| valnum = unique +# 135| m135_3(unknown) = InitializeNonLocal : +# 135| valnum = unique +# 135| m135_4(unknown) = Chi : total:m135_2, partial:m135_3 +# 135| valnum = unique +# 135| mu135_5(unknown) = UnmodeledDefinition : +# 135| valnum = unique +# 136| r136_1(glval) = VariableAddress[b] : +# 136| valnum = unique +# 136| r136_2(glval) = VariableAddress[global_a] : +# 136| valnum = r136_2 +# 136| r136_3(A *) = Load : &:r136_2, ~m135_3 +# 136| valnum = r136_3 +# 136| r136_4(glval) = FieldAddress[x] : r136_3 +# 136| valnum = r136_4 +# 136| r136_5(int) = Load : &:r136_4, ~m135_4 +# 136| valnum = r136_5 +# 136| m136_6(int) = Store : &:r136_1, r136_5 +# 136| valnum = r136_5 +# 137| r137_1(glval) = VariableAddress[c] : +# 137| valnum = unique +# 137| r137_2(glval) = VariableAddress[global_a] : +# 137| valnum = r136_2 +# 137| r137_3(A *) = Load : &:r137_2, ~m135_3 +# 137| valnum = r136_3 +# 137| r137_4(glval) = FieldAddress[x] : r137_3 +# 137| valnum = r136_4 +# 137| r137_5(int) = Load : &:r137_4, ~m135_4 +# 137| valnum = r137_5 +# 137| m137_6(int) = Store : &:r137_1, r137_5 +# 137| valnum = r137_5 +# 139| r139_1(glval) = VariableAddress[global_n] : +# 139| valnum = unique +# 139| r139_2(int) = Load : &:r139_1, ~m135_3 +# 139| valnum = r139_2 +# 139| r139_3(glval) = VariableAddress[global_a] : +# 139| valnum = r136_2 +# 139| r139_4(A *) = Load : &:r139_3, ~m135_3 +# 139| valnum = r136_3 +# 139| r139_5(glval) = FieldAddress[x] : r139_4 +# 139| valnum = r136_4 +# 139| m139_6(int) = Store : &:r139_5, r139_2 +# 139| valnum = r139_2 +# 139| m139_7(unknown) = Chi : total:m135_4, partial:m139_6 +# 139| valnum = unique +# 140| r140_1(glval) = VariableAddress[d] : +# 140| valnum = unique +# 140| r140_2(glval) = VariableAddress[global_a] : +# 140| valnum = r136_2 +# 140| r140_3(A *) = Load : &:r140_2, ~m139_7 +# 140| valnum = unique +# 140| r140_4(glval) = FieldAddress[x] : r140_3 +# 140| valnum = unique +# 140| r140_5(int) = Load : &:r140_4, ~m139_7 +# 140| valnum = r140_5 +# 140| m140_6(int) = Store : &:r140_1, r140_5 +# 140| valnum = r140_5 +# 141| v141_1(void) = NoOp : +# 135| v135_6(void) = ReturnVoid : +# 135| v135_7(void) = UnmodeledUse : mu* +# 135| v135_8(void) = AliasedUse : ~m139_7 +# 135| v135_9(void) = ExitFunction : + +# 143| void test_read_arg_different(A*) +# 143| Block 0 +# 143| v143_1(void) = EnterFunction : +# 143| m143_2(unknown) = AliasedDefinition : +# 143| valnum = unique +# 143| m143_3(unknown) = InitializeNonLocal : +# 143| valnum = unique +# 143| m143_4(unknown) = Chi : total:m143_2, partial:m143_3 +# 143| valnum = unique +# 143| mu143_5(unknown) = UnmodeledDefinition : +# 143| valnum = unique +# 143| r143_6(glval) = VariableAddress[pa] : +# 143| valnum = r143_6 +# 143| m143_7(A *) = InitializeParameter[pa] : &:r143_6 +# 143| valnum = m143_7 +# 143| r143_8(A *) = Load : &:r143_6, m143_7 +# 143| valnum = m143_7 +# 143| m143_9(unknown) = InitializeIndirection[pa] : &:r143_8 +# 143| valnum = unique +# 144| r144_1(glval) = VariableAddress[b] : +# 144| valnum = unique +# 144| r144_2(glval) = VariableAddress[pa] : +# 144| valnum = r143_6 +# 144| r144_3(A *) = Load : &:r144_2, m143_7 +# 144| valnum = m143_7 +# 144| r144_4(glval) = FieldAddress[x] : r144_3 +# 144| valnum = r144_4 +# 144| r144_5(int) = Load : &:r144_4, ~m143_9 +# 144| valnum = r144_5 +# 144| m144_6(int) = Store : &:r144_1, r144_5 +# 144| valnum = r144_5 +# 145| r145_1(glval) = VariableAddress[c] : +# 145| valnum = unique +# 145| r145_2(glval) = VariableAddress[pa] : +# 145| valnum = r143_6 +# 145| r145_3(A *) = Load : &:r145_2, m143_7 +# 145| valnum = m143_7 +# 145| r145_4(glval) = FieldAddress[y] : r145_3 +# 145| valnum = r145_4 +# 145| r145_5(int) = Load : &:r145_4, ~m143_9 +# 145| valnum = r145_5 +# 145| m145_6(int) = Store : &:r145_1, r145_5 +# 145| valnum = r145_5 +# 147| r147_1(glval) = VariableAddress[global_n] : +# 147| valnum = unique +# 147| r147_2(int) = Load : &:r147_1, ~m143_3 +# 147| valnum = r147_2 +# 147| r147_3(glval) = VariableAddress[pa] : +# 147| valnum = r143_6 +# 147| r147_4(A *) = Load : &:r147_3, m143_7 +# 147| valnum = m143_7 +# 147| r147_5(glval) = FieldAddress[y] : r147_4 +# 147| valnum = r145_4 +# 147| m147_6(int) = Store : &:r147_5, r147_2 +# 147| valnum = r147_2 +# 147| m147_7(unknown) = Chi : total:m143_9, partial:m147_6 +# 147| valnum = unique +# 149| r149_1(glval) = VariableAddress[d] : +# 149| valnum = unique +# 149| r149_2(glval) = VariableAddress[pa] : +# 149| valnum = r143_6 +# 149| r149_3(A *) = Load : &:r149_2, m143_7 +# 149| valnum = m143_7 +# 149| r149_4(glval) = FieldAddress[x] : r149_3 +# 149| valnum = r144_4 +# 149| r149_5(int) = Load : &:r149_4, ~m143_9 +# 149| valnum = r144_5 +# 149| m149_6(int) = Store : &:r149_1, r149_5 +# 149| valnum = r144_5 +# 150| v150_1(void) = NoOp : +# 143| v143_10(void) = ReturnIndirection : &:r143_8, ~m147_7 +# 143| v143_11(void) = ReturnVoid : +# 143| v143_12(void) = UnmodeledUse : mu* +# 143| v143_13(void) = AliasedUse : m143_3 +# 143| v143_14(void) = ExitFunction : + +# 152| void test_read_global_different(int) +# 152| Block 0 +# 152| v152_1(void) = EnterFunction : +# 152| m152_2(unknown) = AliasedDefinition : +# 152| valnum = unique +# 152| m152_3(unknown) = InitializeNonLocal : +# 152| valnum = unique +# 152| m152_4(unknown) = Chi : total:m152_2, partial:m152_3 +# 152| valnum = unique +# 152| mu152_5(unknown) = UnmodeledDefinition : +# 152| valnum = unique +# 152| r152_6(glval) = VariableAddress[n] : +# 152| valnum = r152_6 +# 152| m152_7(int) = InitializeParameter[n] : &:r152_6 +# 152| valnum = m152_7 +# 153| r153_1(glval) = VariableAddress[b] : +# 153| valnum = unique +# 153| r153_2(glval) = VariableAddress[global_a] : +# 153| valnum = r153_2 +# 153| r153_3(A *) = Load : &:r153_2, ~m152_3 +# 153| valnum = r153_3 +# 153| r153_4(glval) = FieldAddress[x] : r153_3 +# 153| valnum = r153_4 +# 153| r153_5(int) = Load : &:r153_4, ~m152_4 +# 153| valnum = r153_5 +# 153| m153_6(int) = Store : &:r153_1, r153_5 +# 153| valnum = r153_5 +# 154| r154_1(glval) = VariableAddress[c] : +# 154| valnum = unique +# 154| r154_2(glval) = VariableAddress[global_a] : +# 154| valnum = r153_2 +# 154| r154_3(A *) = Load : &:r154_2, ~m152_3 +# 154| valnum = r153_3 +# 154| r154_4(glval) = FieldAddress[x] : r154_3 +# 154| valnum = r153_4 +# 154| r154_5(int) = Load : &:r154_4, ~m152_4 +# 154| valnum = r154_5 +# 154| m154_6(int) = Store : &:r154_1, r154_5 +# 154| valnum = r154_5 +# 156| r156_1(glval) = VariableAddress[n] : +# 156| valnum = r152_6 +# 156| r156_2(int) = Load : &:r156_1, m152_7 +# 156| valnum = m152_7 +# 156| r156_3(glval) = VariableAddress[global_a] : +# 156| valnum = r153_2 +# 156| r156_4(A *) = Load : &:r156_3, ~m152_3 +# 156| valnum = r153_3 +# 156| r156_5(glval) = FieldAddress[y] : r156_4 +# 156| valnum = unique +# 156| m156_6(int) = Store : &:r156_5, r156_2 +# 156| valnum = m152_7 +# 156| m156_7(unknown) = Chi : total:m152_4, partial:m156_6 +# 156| valnum = unique +# 158| r158_1(glval) = VariableAddress[d] : +# 158| valnum = unique +# 158| r158_2(glval) = VariableAddress[global_a] : +# 158| valnum = r153_2 +# 158| r158_3(A *) = Load : &:r158_2, ~m156_7 +# 158| valnum = unique +# 158| r158_4(glval) = FieldAddress[x] : r158_3 +# 158| valnum = unique +# 158| r158_5(int) = Load : &:r158_4, ~m156_7 +# 158| valnum = r158_5 +# 158| m158_6(int) = Store : &:r158_1, r158_5 +# 158| valnum = r158_5 +# 159| v159_1(void) = NoOp : +# 152| v152_8(void) = ReturnVoid : +# 152| v152_9(void) = UnmodeledUse : mu* +# 152| v152_10(void) = AliasedUse : ~m156_7 +# 152| v152_11(void) = ExitFunction : diff --git a/cpp/ql/test/library-tests/valuenumbering/GlobalValueNumbering/test.cpp b/cpp/ql/test/library-tests/valuenumbering/GlobalValueNumbering/test.cpp index b618ec97f0a..3e878ac2356 100644 --- a/cpp/ql/test/library-tests/valuenumbering/GlobalValueNumbering/test.cpp +++ b/cpp/ql/test/library-tests/valuenumbering/GlobalValueNumbering/test.cpp @@ -42,7 +42,7 @@ int test03(int p0, int p1, int* p2) { x = p0 + p1 + global03; *p2 = 0; // Might change global03 - x = p0 + p1 + global03; // Not the same value + x = p0 + p1 + global03; // BUG: Not the same value, but given the same value number (this is likely due to #2696) y = x; } @@ -115,3 +115,45 @@ void test06() { "a"; "c"; } + +struct A { + int x; + int y; +}; + +void test_read_arg_same(A *pa, int n) { + int b = pa->x; + int c = pa->x; + + pa->x = n; + int d = pa->x; +} + +A* global_a; +int global_n; + +void test_read_global_same() { + int b = global_a->x; + int c = global_a->x; + + global_a->x = global_n; + int d = global_a->x; +} + +void test_read_arg_different(A *pa) { + int b = pa->x; + int c = pa->y; + + pa->y = global_n; + + int d = pa->x; +} + +void test_read_global_different(int n) { + int b = global_a->x; + int c = global_a->x; + + global_a->y = n; + + int d = global_a->x; +} \ No newline at end of file diff --git a/cpp/ql/test/query-tests/Likely Bugs/RedundantNullCheckSimple/RedundantNullCheckSimple.cpp b/cpp/ql/test/query-tests/Likely Bugs/RedundantNullCheckSimple/RedundantNullCheckSimple.cpp index 2b16aa51f4f..2760dcb349c 100644 --- a/cpp/ql/test/query-tests/Likely Bugs/RedundantNullCheckSimple/RedundantNullCheckSimple.cpp +++ b/cpp/ql/test/query-tests/Likely Bugs/RedundantNullCheckSimple/RedundantNullCheckSimple.cpp @@ -17,7 +17,7 @@ void test_not_same_basic_block(int *p) { void test_indirect(int **p) { int x; x = **p; - if (*p == nullptr) { // BAD [NOT DETECTED] + if (*p == nullptr) { // BAD return; } } diff --git a/cpp/ql/test/query-tests/Likely Bugs/RedundantNullCheckSimple/RedundantNullCheckSimple.expected b/cpp/ql/test/query-tests/Likely Bugs/RedundantNullCheckSimple/RedundantNullCheckSimple.expected index 17328288965..34b7957b817 100644 --- a/cpp/ql/test/query-tests/Likely Bugs/RedundantNullCheckSimple/RedundantNullCheckSimple.expected +++ b/cpp/ql/test/query-tests/Likely Bugs/RedundantNullCheckSimple/RedundantNullCheckSimple.expected @@ -1,5 +1,6 @@ | RedundantNullCheckSimple.cpp:4:7:4:7 | Load: p | This null check is redundant because the value is $@ in any case | RedundantNullCheckSimple.cpp:3:7:3:8 | Load: * ... | dereferenced here | | RedundantNullCheckSimple.cpp:13:8:13:8 | Load: p | This null check is redundant because the value is $@ in any case | RedundantNullCheckSimple.cpp:10:11:10:12 | Load: * ... | dereferenced here | +| RedundantNullCheckSimple.cpp:20:7:20:8 | Load: * ... | This null check is redundant because the value is $@ in any case | RedundantNullCheckSimple.cpp:19:7:19:9 | Load: * ... | dereferenced here | | RedundantNullCheckSimple.cpp:48:12:48:12 | Load: p | This null check is redundant because the value is $@ in any case | RedundantNullCheckSimple.cpp:51:10:51:11 | Load: * ... | dereferenced here | | RedundantNullCheckSimple.cpp:79:7:79:9 | Load: * ... | This null check is redundant because the value is $@ in any case | RedundantNullCheckSimple.cpp:78:7:78:10 | Load: * ... | dereferenced here | | RedundantNullCheckSimple.cpp:93:13:93:13 | Load: p | This null check is redundant because the value is $@ in any case | RedundantNullCheckSimple.cpp:92:13:92:18 | Load: * ... | dereferenced here | diff --git a/csharp/ql/src/semmle/code/csharp/ir/implementation/raw/gvn/ValueNumbering.qll b/csharp/ql/src/semmle/code/csharp/ir/implementation/raw/gvn/ValueNumbering.qll index 6791f77b7b6..ec6d786a684 100644 --- a/csharp/ql/src/semmle/code/csharp/ir/implementation/raw/gvn/ValueNumbering.qll +++ b/csharp/ql/src/semmle/code/csharp/ir/implementation/raw/gvn/ValueNumbering.qll @@ -52,6 +52,11 @@ newtype TValueNumber = ) { inheritanceConversionValueNumber(_, irFunc, opcode, baseClass, derivedClass, operand) } or + TLoadTotalOverlapValueNumber( + IRFunction irFunc, IRType type, ValueNumber memOperand, ValueNumber operand + ) { + loadTotalOverlapValueNumber(_, irFunc, type, memOperand, operand) + } or TUniqueValueNumber(IRFunction irFunc, Instruction instr) { uniqueValueNumber(instr, irFunc) } /** @@ -101,12 +106,18 @@ class ValueNumber extends TValueNumber { * The use of `p.x` on line 3 is linked to the definition of `p` on line 1 as well, but is not * congruent to that definition because `p.x` accesses only a subset of the memory defined by `p`. */ -private class CongruentCopyInstruction extends CopyInstruction { +class CongruentCopyInstruction extends CopyInstruction { CongruentCopyInstruction() { this.getSourceValueOperand().getDefinitionOverlap() instanceof MustExactlyOverlap } } +class LoadTotalOverlapInstruction extends LoadInstruction { + LoadTotalOverlapInstruction() { + this.getSourceValueOperand().getDefinitionOverlap() instanceof MustTotallyOverlap + } +} + /** * Holds if this library knows how to assign a value number to the specified instruction, other than * a `unique` value number that is never shared by multiple instructions. @@ -131,6 +142,8 @@ private predicate numberableInstruction(Instruction instr) { instr instanceof PointerArithmeticInstruction or instr instanceof CongruentCopyInstruction + or + instr instanceof LoadTotalOverlapInstruction } private predicate variableAddressValueNumber( @@ -205,6 +218,7 @@ private predicate unaryValueNumber( instr.getEnclosingIRFunction() = irFunc and not instr instanceof InheritanceConversionInstruction and not instr instanceof CopyInstruction and + not instr instanceof FieldAddressInstruction and instr.getOpcode() = opcode and instr.getResultIRType() = type and valueNumber(instr.getUnary()) = operand @@ -221,6 +235,16 @@ private predicate inheritanceConversionValueNumber( valueNumber(instr.getUnary()) = operand } +private predicate loadTotalOverlapValueNumber( + LoadTotalOverlapInstruction instr, IRFunction irFunc, IRType type, ValueNumber memOperand, + ValueNumber operand +) { + instr.getEnclosingIRFunction() = irFunc and + instr.getResultIRType() = type and + valueNumber(instr.getAnOperand().(MemoryOperand).getAnyDef()) = memOperand and + valueNumberOfOperand(instr.getAnOperand().(AddressOperand)) = operand +} + /** * Holds if `instr` should be assigned a unique value number because this library does not know how * to determine if two instances of that instruction are equivalent. @@ -313,6 +337,11 @@ private ValueNumber nonUniqueValueNumber(Instruction instr) { TPointerArithmeticValueNumber(irFunc, opcode, type, elementSize, leftOperand, rightOperand) ) or + exists(IRType type, ValueNumber memOperand, ValueNumber operand | + loadTotalOverlapValueNumber(instr, irFunc, type, memOperand, operand) and + result = TLoadTotalOverlapValueNumber(irFunc, type, memOperand, operand) + ) + or // The value number of a copy is just the value number of its source value. result = valueNumber(instr.(CongruentCopyInstruction).getSourceValue()) ) diff --git a/csharp/ql/src/semmle/code/csharp/ir/implementation/unaliased_ssa/gvn/ValueNumbering.qll b/csharp/ql/src/semmle/code/csharp/ir/implementation/unaliased_ssa/gvn/ValueNumbering.qll index 6791f77b7b6..ec6d786a684 100644 --- a/csharp/ql/src/semmle/code/csharp/ir/implementation/unaliased_ssa/gvn/ValueNumbering.qll +++ b/csharp/ql/src/semmle/code/csharp/ir/implementation/unaliased_ssa/gvn/ValueNumbering.qll @@ -52,6 +52,11 @@ newtype TValueNumber = ) { inheritanceConversionValueNumber(_, irFunc, opcode, baseClass, derivedClass, operand) } or + TLoadTotalOverlapValueNumber( + IRFunction irFunc, IRType type, ValueNumber memOperand, ValueNumber operand + ) { + loadTotalOverlapValueNumber(_, irFunc, type, memOperand, operand) + } or TUniqueValueNumber(IRFunction irFunc, Instruction instr) { uniqueValueNumber(instr, irFunc) } /** @@ -101,12 +106,18 @@ class ValueNumber extends TValueNumber { * The use of `p.x` on line 3 is linked to the definition of `p` on line 1 as well, but is not * congruent to that definition because `p.x` accesses only a subset of the memory defined by `p`. */ -private class CongruentCopyInstruction extends CopyInstruction { +class CongruentCopyInstruction extends CopyInstruction { CongruentCopyInstruction() { this.getSourceValueOperand().getDefinitionOverlap() instanceof MustExactlyOverlap } } +class LoadTotalOverlapInstruction extends LoadInstruction { + LoadTotalOverlapInstruction() { + this.getSourceValueOperand().getDefinitionOverlap() instanceof MustTotallyOverlap + } +} + /** * Holds if this library knows how to assign a value number to the specified instruction, other than * a `unique` value number that is never shared by multiple instructions. @@ -131,6 +142,8 @@ private predicate numberableInstruction(Instruction instr) { instr instanceof PointerArithmeticInstruction or instr instanceof CongruentCopyInstruction + or + instr instanceof LoadTotalOverlapInstruction } private predicate variableAddressValueNumber( @@ -205,6 +218,7 @@ private predicate unaryValueNumber( instr.getEnclosingIRFunction() = irFunc and not instr instanceof InheritanceConversionInstruction and not instr instanceof CopyInstruction and + not instr instanceof FieldAddressInstruction and instr.getOpcode() = opcode and instr.getResultIRType() = type and valueNumber(instr.getUnary()) = operand @@ -221,6 +235,16 @@ private predicate inheritanceConversionValueNumber( valueNumber(instr.getUnary()) = operand } +private predicate loadTotalOverlapValueNumber( + LoadTotalOverlapInstruction instr, IRFunction irFunc, IRType type, ValueNumber memOperand, + ValueNumber operand +) { + instr.getEnclosingIRFunction() = irFunc and + instr.getResultIRType() = type and + valueNumber(instr.getAnOperand().(MemoryOperand).getAnyDef()) = memOperand and + valueNumberOfOperand(instr.getAnOperand().(AddressOperand)) = operand +} + /** * Holds if `instr` should be assigned a unique value number because this library does not know how * to determine if two instances of that instruction are equivalent. @@ -313,6 +337,11 @@ private ValueNumber nonUniqueValueNumber(Instruction instr) { TPointerArithmeticValueNumber(irFunc, opcode, type, elementSize, leftOperand, rightOperand) ) or + exists(IRType type, ValueNumber memOperand, ValueNumber operand | + loadTotalOverlapValueNumber(instr, irFunc, type, memOperand, operand) and + result = TLoadTotalOverlapValueNumber(irFunc, type, memOperand, operand) + ) + or // The value number of a copy is just the value number of its source value. result = valueNumber(instr.(CongruentCopyInstruction).getSourceValue()) )