diff --git a/cpp/ql/lib/change-notes/2025-08-02-gvn.md b/cpp/ql/lib/change-notes/2025-08-02-gvn.md new file mode 100644 index 000000000000..70c9f7dbb158 --- /dev/null +++ b/cpp/ql/lib/change-notes/2025-08-02-gvn.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* The global value numbering library (`semmle.code.cpp.valuenumbering.GlobalValueNumbering` and `semmle.code.cpp.ir.ValueNumbering`) has been improved so more expressions are assigned the same value number. \ No newline at end of file diff --git a/cpp/ql/lib/semmle/code/cpp/ir/implementation/aliased_ssa/gvn/internal/ValueNumberingInternal.qll b/cpp/ql/lib/semmle/code/cpp/ir/implementation/aliased_ssa/gvn/internal/ValueNumberingInternal.qll index ec0038917743..66d499112b81 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/implementation/aliased_ssa/gvn/internal/ValueNumberingInternal.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/implementation/aliased_ssa/gvn/internal/ValueNumberingInternal.qll @@ -43,6 +43,23 @@ newtype TValueNumber = } or TUniqueValueNumber(IRFunction irFunc, Instruction instr) { uniqueValueNumber(instr, irFunc) } +/** + * A `ConvertInstruction` which converts data of type `T` to data of type `U` + * where `T` and `U` only differ in specifiers. For example, if `T` is `int` + * and `U` is `const T` this is a conversion from a non-const integer to a + * const integer. + * + * Generally, the value number of a converted value is different from the value + * number of an unconverted value, but conversions which only modify specifiers + * leave the resulting value bitwise identical to the old value. + */ +class TypePreservingConvertInstruction extends ConvertInstruction { + TypePreservingConvertInstruction() { + pragma[only_bind_out](this.getResultType().getUnspecifiedType()) = + pragma[only_bind_out](this.getUnary().getResultType().getUnspecifiedType()) + } +} + /** * A `CopyInstruction` whose source operand's value is congruent to the definition of that source * operand. @@ -216,6 +233,7 @@ private predicate unaryValueNumber( not instr instanceof InheritanceConversionInstruction and not instr instanceof CopyInstruction and not instr instanceof FieldAddressInstruction and + not instr instanceof TypePreservingConvertInstruction and instr.getOpcode() = opcode and tvalueNumber(instr.getUnary()) = operand } @@ -351,6 +369,10 @@ private TValueNumber nonUniqueValueNumber(Instruction instr) { or // The value number of a copy is just the value number of its source value. result = tvalueNumber(instr.(CongruentCopyInstruction).getSourceValue()) + or + // The value number of a type-preserving conversion is just the value + // number of the unconverted value. + result = tvalueNumber(instr.(TypePreservingConvertInstruction).getUnary()) ) ) } diff --git a/cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/gvn/internal/ValueNumberingInternal.qll b/cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/gvn/internal/ValueNumberingInternal.qll index ec0038917743..66d499112b81 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/gvn/internal/ValueNumberingInternal.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/gvn/internal/ValueNumberingInternal.qll @@ -43,6 +43,23 @@ newtype TValueNumber = } or TUniqueValueNumber(IRFunction irFunc, Instruction instr) { uniqueValueNumber(instr, irFunc) } +/** + * A `ConvertInstruction` which converts data of type `T` to data of type `U` + * where `T` and `U` only differ in specifiers. For example, if `T` is `int` + * and `U` is `const T` this is a conversion from a non-const integer to a + * const integer. + * + * Generally, the value number of a converted value is different from the value + * number of an unconverted value, but conversions which only modify specifiers + * leave the resulting value bitwise identical to the old value. + */ +class TypePreservingConvertInstruction extends ConvertInstruction { + TypePreservingConvertInstruction() { + pragma[only_bind_out](this.getResultType().getUnspecifiedType()) = + pragma[only_bind_out](this.getUnary().getResultType().getUnspecifiedType()) + } +} + /** * A `CopyInstruction` whose source operand's value is congruent to the definition of that source * operand. @@ -216,6 +233,7 @@ private predicate unaryValueNumber( not instr instanceof InheritanceConversionInstruction and not instr instanceof CopyInstruction and not instr instanceof FieldAddressInstruction and + not instr instanceof TypePreservingConvertInstruction and instr.getOpcode() = opcode and tvalueNumber(instr.getUnary()) = operand } @@ -351,6 +369,10 @@ private TValueNumber nonUniqueValueNumber(Instruction instr) { or // The value number of a copy is just the value number of its source value. result = tvalueNumber(instr.(CongruentCopyInstruction).getSourceValue()) + or + // The value number of a type-preserving conversion is just the value + // number of the unconverted value. + result = tvalueNumber(instr.(TypePreservingConvertInstruction).getUnary()) ) ) } diff --git a/cpp/ql/lib/semmle/code/cpp/ir/implementation/unaliased_ssa/gvn/internal/ValueNumberingInternal.qll b/cpp/ql/lib/semmle/code/cpp/ir/implementation/unaliased_ssa/gvn/internal/ValueNumberingInternal.qll index ec0038917743..66d499112b81 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/implementation/unaliased_ssa/gvn/internal/ValueNumberingInternal.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/implementation/unaliased_ssa/gvn/internal/ValueNumberingInternal.qll @@ -43,6 +43,23 @@ newtype TValueNumber = } or TUniqueValueNumber(IRFunction irFunc, Instruction instr) { uniqueValueNumber(instr, irFunc) } +/** + * A `ConvertInstruction` which converts data of type `T` to data of type `U` + * where `T` and `U` only differ in specifiers. For example, if `T` is `int` + * and `U` is `const T` this is a conversion from a non-const integer to a + * const integer. + * + * Generally, the value number of a converted value is different from the value + * number of an unconverted value, but conversions which only modify specifiers + * leave the resulting value bitwise identical to the old value. + */ +class TypePreservingConvertInstruction extends ConvertInstruction { + TypePreservingConvertInstruction() { + pragma[only_bind_out](this.getResultType().getUnspecifiedType()) = + pragma[only_bind_out](this.getUnary().getResultType().getUnspecifiedType()) + } +} + /** * A `CopyInstruction` whose source operand's value is congruent to the definition of that source * operand. @@ -216,6 +233,7 @@ private predicate unaryValueNumber( not instr instanceof InheritanceConversionInstruction and not instr instanceof CopyInstruction and not instr instanceof FieldAddressInstruction and + not instr instanceof TypePreservingConvertInstruction and instr.getOpcode() = opcode and tvalueNumber(instr.getUnary()) = operand } @@ -351,6 +369,10 @@ private TValueNumber nonUniqueValueNumber(Instruction instr) { or // The value number of a copy is just the value number of its source value. result = tvalueNumber(instr.(CongruentCopyInstruction).getSourceValue()) + or + // The value number of a type-preserving conversion is just the value + // number of the unconverted value. + result = tvalueNumber(instr.(TypePreservingConvertInstruction).getUnary()) ) ) } 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 ecf31ac87019..4688c5b75285 100644 --- a/cpp/ql/test/library-tests/valuenumbering/GlobalValueNumbering/ir_gvn.expected +++ b/cpp/ql/test/library-tests/valuenumbering/GlobalValueNumbering/ir_gvn.expected @@ -1145,3 +1145,43 @@ test.cpp: # 152| v152_7(void) = ReturnVoid : # 152| v152_8(void) = AliasedUse : ~m156_7 # 152| v152_9(void) = ExitFunction : + +# 166| void test_constMemberFunction() +# 166| Block 0 +# 166| v166_1(void) = EnterFunction : +# 166| m166_2(unknown) = AliasedDefinition : +# 166| valnum = unique +# 166| m166_3(unknown) = InitializeNonLocal : +# 166| valnum = unique +# 166| m166_4(unknown) = Chi : total:m166_2, partial:m166_3 +# 166| valnum = unique +# 167| r167_1(glval) = VariableAddress[s] : +# 167| valnum = r167_1, r168_2, r169_1, r169_2 +# 167| m167_2(StructWithConstMemberFunction) = Uninitialized[s] : &:r167_1 +# 167| valnum = m167_2, m168_4, r168_3 +# 167| m167_3(unknown) = Chi : total:m166_4, partial:m167_2 +# 167| valnum = unique +# 168| r168_1(glval) = VariableAddress[s2] : +# 168| valnum = unique +# 168| r168_2(glval) = VariableAddress[s] : +# 168| valnum = r167_1, r168_2, r169_1, r169_2 +# 168| r168_3(StructWithConstMemberFunction) = Load[s] : &:r168_2, m167_2 +# 168| valnum = m167_2, m168_4, r168_3 +# 168| m168_4(StructWithConstMemberFunction) = Store[s2] : &:r168_1, r168_3 +# 168| valnum = m167_2, m168_4, r168_3 +# 169| r169_1(glval) = VariableAddress[s] : +# 169| valnum = r167_1, r168_2, r169_1, r169_2 +# 169| r169_2(glval) = Convert : r169_1 +# 169| valnum = r167_1, r168_2, r169_1, r169_2 +# 169| r169_3(glval) = FunctionAddress[constMemberFunction] : +# 169| valnum = unique +# 169| v169_4(void) = Call[constMemberFunction] : func:r169_3, this:r169_2 +# 169| m169_5(unknown) = ^CallSideEffect : ~m167_3 +# 169| valnum = unique +# 169| m169_6(unknown) = Chi : total:m167_3, partial:m169_5 +# 169| valnum = unique +# 169| v169_7(void) = ^IndirectReadSideEffect[-1] : &:r169_2, ~m169_6 +# 170| v170_1(void) = NoOp : +# 166| v166_5(void) = ReturnVoid : +# 166| v166_6(void) = AliasedUse : ~m169_6 +# 166| v166_7(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 dfef91006e38..ed98bad30578 100644 --- a/cpp/ql/test/library-tests/valuenumbering/GlobalValueNumbering/test.cpp +++ b/cpp/ql/test/library-tests/valuenumbering/GlobalValueNumbering/test.cpp @@ -156,4 +156,15 @@ void test_read_global_different(int n) { global_a->y = n; int d = global_a->x; +} + +struct StructWithConstMemberFunction { + int x; + void constMemberFunction() const; +}; + +void test_constMemberFunction() { + StructWithConstMemberFunction s; + StructWithConstMemberFunction s2 = s; + s.constMemberFunction(); } \ No newline at end of file diff --git a/cpp/ql/test/query-tests/Likely Bugs/Memory Management/StrncpyFlippedArgs/StrncpyFlippedArgs.expected b/cpp/ql/test/query-tests/Likely Bugs/Memory Management/StrncpyFlippedArgs/StrncpyFlippedArgs.expected index 1fe46acbdde6..19fe89a78c16 100644 --- a/cpp/ql/test/query-tests/Likely Bugs/Memory Management/StrncpyFlippedArgs/StrncpyFlippedArgs.expected +++ b/cpp/ql/test/query-tests/Likely Bugs/Memory Management/StrncpyFlippedArgs/StrncpyFlippedArgs.expected @@ -13,7 +13,6 @@ | test.cpp:49:2:49:9 | call to strcpy_s | Potentially unsafe call to strcpy_s; second argument should be size of destination. | | test.cpp:62:3:62:9 | call to strncpy | Potentially unsafe call to strncpy; third argument should be size of destination. | | test.cpp:65:3:65:9 | call to strncpy | Potentially unsafe call to strncpy; third argument should be size of destination. | -| test.cpp:70:2:70:8 | call to strncpy | Potentially unsafe call to strncpy; third argument should be size of destination. | | test.cpp:81:3:81:9 | call to strncpy | Potentially unsafe call to strncpy; third argument should be size of destination. | | test.cpp:84:3:84:9 | call to strncpy | Potentially unsafe call to strncpy; third argument should be size of destination. | | test.cpp:105:2:105:10 | call to wcsxfrm_l | Potentially unsafe call to wcsxfrm_l; third argument should be size of destination. |