Skip to content

Commit 31508c5

Browse files
committed
fix: preserve BDE conversion bodies and assignment aliases
Address the two P2 findings from the independent review of PR #22581. Restrict scalar emplacement summaries to arithmetic or pointer inputs so user-defined conversions retain body analysis. Model copy/move assignment results as aliases of the receiver. Add positive and negative conversion regressions and write-through tests for both assignment forms and wrappers. Wrapper flow tests and model validation pass.
1 parent a4b9051 commit 31508c5

4 files changed

Lines changed: 71 additions & 1 deletion

File tree

cpp/ql/lib/ext/bdlb.values.model.yml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,15 +11,19 @@ extensions:
1111
- ["BloombergLP::bdlb", "NullableValue", False, "NullableValue", "(const NullableValue &)", "", "Argument[*0].Element[@]", "Argument[-1].Element[@]", "value", "manual"]
1212
- ["BloombergLP::bdlb", "NullableValue", False, "operator=", "(const NullableValue &)", "", "Argument[*0].Element[@]", "Argument[-1].Element[@]", "value", "manual"]
1313
- ["BloombergLP::bdlb", "NullableValue", False, "operator=", "(const NullableValue &)", "", "Argument[*0].Element[@]", "ReturnValue[*].Element[@]", "value", "manual"]
14+
- ["BloombergLP::bdlb", "NullableValue", False, "operator=", "(const NullableValue &)", "", "Argument[-1]", "ReturnValue[*]", "value", "manual"]
1415
- ["BloombergLP::bdlb", "NullableValue", False, "NullableValue", "(NullableValue &&)", "", "Argument[*0].Element[@]", "Argument[-1].Element[@]", "value", "manual"]
1516
- ["BloombergLP::bdlb", "NullableValue", False, "operator=", "(NullableValue &&)", "", "Argument[*0].Element[@]", "Argument[-1].Element[@]", "value", "manual"]
1617
- ["BloombergLP::bdlb", "NullableValue", False, "operator=", "(NullableValue &&)", "", "Argument[*0].Element[@]", "ReturnValue[*].Element[@]", "value", "manual"]
18+
- ["BloombergLP::bdlb", "NullableValue", False, "operator=", "(NullableValue &&)", "", "Argument[-1]", "ReturnValue[*]", "value", "manual"]
1719
- ["BloombergLP::bdlb", "VariantImp", False, "the", "", "", "Argument[-1].Element[@]", "ReturnValue[*@]", "value", "manual"]
1820
- ["BloombergLP::bdlb", "VariantImp", False, "the", "", "", "ReturnValue[*]", "Argument[-1].Element[]", "value", "manual"]
1921
- ["BloombergLP::bdlb", "VariantImp", False, "the", "", "", "ReturnValue[**]", "Argument[-1].Element[*]", "value", "manual"]
2022
- ["BloombergLP::bdlb", "VariantImp", False, "VariantImp", "(const VariantImp &,bslma::Allocator *)", "", "Argument[*0].Element[@]", "Argument[-1].Element[@]", "value", "manual"]
2123
- ["BloombergLP::bdlb", "VariantImp", False, "operator=", "(const VariantImp &)", "", "Argument[*0].Element[@]", "Argument[-1].Element[@]", "value", "manual"]
2224
- ["BloombergLP::bdlb", "VariantImp", False, "operator=", "(const VariantImp &)", "", "Argument[*0].Element[@]", "ReturnValue[*].Element[@]", "value", "manual"]
25+
- ["BloombergLP::bdlb", "VariantImp", False, "operator=", "(const VariantImp &)", "", "Argument[-1]", "ReturnValue[*]", "value", "manual"]
2326
- ["BloombergLP::bdlb", "VariantImp", False, "VariantImp", "(VariantImp &&)", "", "Argument[*0].Element[@]", "Argument[-1].Element[@]", "value", "manual"]
2427
- ["BloombergLP::bdlb", "VariantImp", False, "operator=", "(VariantImp &&)", "", "Argument[*0].Element[@]", "Argument[-1].Element[@]", "value", "manual"]
2528
- ["BloombergLP::bdlb", "VariantImp", False, "operator=", "(VariantImp &&)", "", "Argument[*0].Element[@]", "ReturnValue[*].Element[@]", "value", "manual"]
29+
- ["BloombergLP::bdlb", "VariantImp", False, "operator=", "(VariantImp &&)", "", "Argument[-1]", "ReturnValue[*]", "value", "manual"]

cpp/ql/lib/semmle/code/cpp/models/implementations/BdlbValues.qll

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,12 @@ private class ScalarEmplacement extends SummarizedCallable {
7676
this.getDeclaringType().hasQualifiedName("BloombergLP::bdlb", "VariantImp")
7777
) and
7878
this.getNumberOfParameters() = 1 and
79-
this.getParameter(0).getUnspecifiedType() instanceof ReferenceType and
79+
// Class inputs may invoke user-defined conversions even when the stored type is scalar.
80+
exists(Type inputType |
81+
inputType =
82+
this.getParameter(0).getUnspecifiedType().(ReferenceType).getBaseType().getUnspecifiedType() and
83+
(inputType instanceof ArithmeticType or inputType instanceof PointerType)
84+
) and
8085
exists(Type t |
8186
t = this.getType().getUnspecifiedType().(ReferenceType).getBaseType().getUnspecifiedType() and
8287
(t instanceof ArithmeticType or t instanceof PointerType)

cpp/ql/test/library-tests/dataflow/bdlb-values/emplacement.cpp

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,3 +44,36 @@ void ignoredArgumentVariant() {
4444
v.createInPlace<Constructed>(source(), 0);
4545
sink(v.the<Constructed>().value);
4646
}
47+
48+
// Scalar destinations can still require a user-defined input conversion.
49+
struct ConvertedNumber {
50+
int payload;
51+
operator int() const { return payload; }
52+
};
53+
struct IgnoredNumber {
54+
int payload;
55+
operator int() const { return 0; }
56+
};
57+
void nullableScalarConversion() {
58+
ConvertedNumber input = {source()};
59+
sink(static_cast<int>(input)); // $ ir
60+
BloombergLP::bdlb::NullableValue<int> n;
61+
n.makeValueInplace(input);
62+
sink(n.value()); // $ ir
63+
}
64+
void variantScalarConversion() {
65+
ConvertedNumber input = {source()};
66+
sink(static_cast<int>(input)); // $ ir
67+
BloombergLP::bdlb::Variant<int> v;
68+
v.createInPlace<int>(input);
69+
sink(v.the<int>()); // $ ir
70+
}
71+
void ignoredScalarConversions() {
72+
IgnoredNumber input = {source()};
73+
BloombergLP::bdlb::NullableValue<int> n;
74+
n.makeValueInplace(input);
75+
sink(n.value());
76+
BloombergLP::bdlb::Variant<int> v;
77+
v.createInPlace<int>(input);
78+
sink(v.the<int>());
79+
}

cpp/ql/test/library-tests/dataflow/bdlb-values/returned-references.cpp

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,3 +36,31 @@ void variantEmplaceReference() {
3636
v.createInPlace<int>(0) = source();
3737
sink(v.the<int>()); // $ ir
3838
}
39+
40+
void nullableCopyResultWrite() {
41+
NullableValue<int> a, b;
42+
b.makeValue(0);
43+
(a = static_cast<const NullableValue<int>&>(b)).value() = source();
44+
sink(a.value()); // $ ir
45+
}
46+
47+
void nullableMoveResultWrite() {
48+
NullableValue<int> a, b;
49+
b.makeValue(0);
50+
(a = static_cast<NullableValue<int>&&>(b)).value() = source();
51+
sink(a.value()); // $ ir
52+
}
53+
54+
void variantCopyResultWrite() {
55+
VariantImp<Types<int>> a, b;
56+
b.assign(0);
57+
(a = static_cast<const VariantImp<Types<int>>&>(b)).the<int>() = source();
58+
sink(a.the<int>()); // $ ir
59+
}
60+
61+
void variantMoveResultWrite() {
62+
VariantImp<Types<int>> a, b;
63+
b.assign(0);
64+
(a = static_cast<VariantImp<Types<int>>&&>(b)).the<int>() = source();
65+
sink(a.the<int>()); // $ ir
66+
}

0 commit comments

Comments
 (0)