From 1aa8adb472e019c464f9ef85880946be7774f465 Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Sat, 2 Aug 2025 13:00:26 +0100 Subject: [PATCH 1/6] C++: Add test. --- .../GlobalValueNumbering/ir_gvn.expected | 40 +++++++++++++++++++ .../GlobalValueNumbering/test.cpp | 11 +++++ 2 files changed, 51 insertions(+) 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..c95bea12f5aa 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 +# 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 +# 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 +# 169| r169_2(glval) = Convert : r169_1 +# 169| valnum = unique +# 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 From 0d9e29825039a02ecfa879b53868b6565ae5fe19 Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Sun, 3 Aug 2025 12:17:19 +0100 Subject: [PATCH 2/6] C++: Specifier-only converting instructions preserve GVNs. --- .../gvn/internal/ValueNumberingInternal.qll | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) 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()) ) ) } From c726285cac6f0d7340de508e30ef72fb66a040d3 Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Sun, 3 Aug 2025 12:17:31 +0100 Subject: [PATCH 3/6] C++: Sync identical files. --- .../gvn/internal/ValueNumberingInternal.qll | 22 +++++++++++++++++++ .../gvn/internal/ValueNumberingInternal.qll | 22 +++++++++++++++++++ 2 files changed, 44 insertions(+) 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()) ) ) } From b807ee471802c267793dc8e8c911b95544003ef3 Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Sat, 2 Aug 2025 13:02:47 +0100 Subject: [PATCH 4/6] C++: Accept test changes. --- .../valuenumbering/GlobalValueNumbering/ir_gvn.expected | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) 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 c95bea12f5aa..4688c5b75285 100644 --- a/cpp/ql/test/library-tests/valuenumbering/GlobalValueNumbering/ir_gvn.expected +++ b/cpp/ql/test/library-tests/valuenumbering/GlobalValueNumbering/ir_gvn.expected @@ -1156,7 +1156,7 @@ test.cpp: # 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 +# 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 @@ -1164,15 +1164,15 @@ test.cpp: # 168| r168_1(glval) = VariableAddress[s2] : # 168| valnum = unique # 168| r168_2(glval) = VariableAddress[s] : -# 168| valnum = r167_1, r168_2, r169_1 +# 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 +# 169| valnum = r167_1, r168_2, r169_1, r169_2 # 169| r169_2(glval) = Convert : r169_1 -# 169| valnum = unique +# 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 From 851c498b37bcaf6b18b53f77a484b88b3556c450 Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Sat, 2 Aug 2025 16:23:53 +0100 Subject: [PATCH 5/6] C++: Accept test changes. This is a FP that's been present since we put the IR into production in #2851. --- .../StrncpyFlippedArgs/StrncpyFlippedArgs.expected | 1 - 1 file changed, 1 deletion(-) 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. | From 65b1b7f63e85101646cb8bbae710430a23a92b1e Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Sat, 2 Aug 2025 16:28:53 +0100 Subject: [PATCH 6/6] C++: Add change note. --- cpp/ql/lib/change-notes/2025-08-02-gvn.md | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 cpp/ql/lib/change-notes/2025-08-02-gvn.md 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