Fix crash when using an int model in a repeater with a negative value (#2063)

Make sure that we return an unsigned for row_count() in C++ and Rust by ensuring an unsigned int model at creation time.

For the interpreter this "worked" by chance as casting a negative floating
number to usize automatically caps at zero, and all values are stored as f64. For safety this patch
applies the same fix though, to be on the safe side.
This commit is contained in:
Simon Hausmann 2023-01-14 21:20:14 +01:00 committed by GitHub
parent 1701352177
commit 96c80a2dd1
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 70 additions and 7 deletions

View file

@ -768,12 +768,12 @@ public:
}; };
/// Model to be used when we just want to repeat without data. /// Model to be used when we just want to repeat without data.
struct IntModel : Model<int> struct UIntModel : Model<int>
{ {
/// Constructs a new IntModel with \a d rows. /// Constructs a new IntModel with \a d rows.
IntModel(int d) : data(d) { } UIntModel(uint32_t d) : data(d) { }
/// \private /// \private
int data; uint32_t data;
/// \copydoc Model::row_count /// \copydoc Model::row_count
int row_count() const override { return data; } int row_count() const override { return data; }
std::optional<int> row_data(int value) const override std::optional<int> row_data(int value) const override

View file

@ -1466,7 +1466,7 @@ fn generate_sub_component(
if repeated.model.ty(&ctx) == Type::Bool { if repeated.model.ty(&ctx) == Type::Bool {
// bool converts to int // bool converts to int
// FIXME: don't do a heap allocation here // FIXME: don't do a heap allocation here
model = format!("std::make_shared<slint::private_api::IntModel>({})", model) model = format!("std::make_shared<slint::private_api::UIntModel>({})", model)
} }
// FIXME: optimize if repeated.model.is_constant() // FIXME: optimize if repeated.model.is_constant()
@ -2247,7 +2247,7 @@ fn compile_expression(expr: &llr::Expression, ctx: &EvaluationContext) -> String
format!("slint::SharedString::from_number({})", f) format!("slint::SharedString::from_number({})", f)
} }
(Type::Float32, Type::Model) | (Type::Int32, Type::Model) => { (Type::Float32, Type::Model) | (Type::Int32, Type::Model) => {
format!("std::make_shared<slint::private_api::IntModel>({})", f) format!("std::make_shared<slint::private_api::UIntModel>(std::max(0, {}))", f)
} }
(Type::Array(_), Type::Model) => f, (Type::Array(_), Type::Model) => f,
(Type::Float32, Type::Color) => { (Type::Float32, Type::Color) => {

View file

@ -1785,7 +1785,7 @@ fn compile_expression(expr: &Expression, ctx: &EvaluationContext) -> TokenStream
)) ))
} }
(Type::Float32, Type::Model) | (Type::Int32, Type::Model) => { (Type::Float32, Type::Model) | (Type::Int32, Type::Model) => {
quote!(slint::private_unstable_api::re_exports::ModelRc::new(#f as usize)) quote!(slint::private_unstable_api::re_exports::ModelRc::new(#f.max(Default::default()) as usize))
} }
(Type::Float32, Type::Color) => { (Type::Float32, Type::Color) => {
quote!(slint::private_unstable_api::re_exports::Color::from_argb_encoded(#f as u32)) quote!(slint::private_unstable_api::re_exports::Color::from_argb_encoded(#f as u32))

View file

@ -47,7 +47,7 @@ impl Model for ValueModel {
0 0
} }
} }
Value::Number(x) => *x as usize, Value::Number(x) => x.max(Default::default()) as usize,
Value::Void => 0, Value::Void => 0,
Value::Model(model_ptr) => model_ptr.row_count(), Value::Model(model_ptr) => model_ptr.row_count(),
x => panic!("Invalid model {:?}", x), x => panic!("Invalid model {:?}", x),

View file

@ -0,0 +1,63 @@
// Copyright © SixtyFPS GmbH <info@slint-ui.com>
// SPDX-License-Identifier: GPL-3.0-only OR LicenseRef-Slint-commercial
TestCase := Rectangle {
width: 100phx;
height: 100phx;
property <int> creation-count: 0;
property <length> test-height: preferred-height;
property <int> repeater-count: -10;
VerticalLayout {
for _ in repeater-count: Rectangle {
preferred-height: 10px;
init => {
creation-count += 1;
}
}
}
}
/*
```cpp
auto handle = TestCase::create();
const TestCase &instance = *handle;
assert_eq(instance.get_creation_count(), 0);
assert_eq(instance.get_test_height(), 0.);
assert_eq(instance.get_creation_count(), 0);
instance.set_repeater_count(2);
assert_eq(instance.get_test_height(), 20.);
assert_eq(instance.get_creation_count(), 2);
```
```rust
let instance = TestCase::new();
assert_eq!(instance.get_creation_count(), 0);
assert_eq!(instance.get_test_height(), 0.);
assert_eq!(instance.get_creation_count(), 0);
instance.set_repeater_count(2);
assert_eq!(instance.get_test_height(), 20.);
assert_eq!(instance.get_creation_count(), 2);
```
```js
var instance = new slint.TestCase();
assert.equal(instance.creation_count, 0);
assert.equal(instance.test_height, 0.);
assert.equal(instance.creation_count, 0);
instance.repeater_count = 2;
assert.equal(instance.test_height, 20.);
assert.equal(instance.creation_count, 2);
```
*/