| Message ID | 20250423120555.21318-1-philmd@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | [RFC] hw/i2c/aspeed_i2c: Make AspeedI2CClass::gap an plain unsigned type | expand |
On 4/23/25 14:05, Philippe Mathieu-Daudé wrote: > Convert AspeedI2CClass::gap to plain unsigned, using '0' > as "no gap" to avoid the followin UBSan warnings: > > SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ../../hw/i2c/aspeed_i2c.c:1559:16 > SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ../../hw/i2c/aspeed_i2c.c:1583:16 > SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ../../hw/i2c/aspeed_i2c.c:1608:16 > hw/i2c/aspeed_i2c.c:1608:16: runtime error: implicit conversion from type 'int' of value > -1 (32-bit, signed) to type 'uint8_t' (aka 'unsigned char') > changed the value to 255 (8-bit, unsigned) > > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> Looks fine. Reviewed-by: Cédric Le Goater <clg@redhat.com> Thanks, C. > --- > include/hw/i2c/aspeed_i2c.h | 2 +- > hw/i2c/aspeed_i2c.c | 5 +---- > 2 files changed, 2 insertions(+), 5 deletions(-) > > diff --git a/include/hw/i2c/aspeed_i2c.h b/include/hw/i2c/aspeed_i2c.h > index 2c4c81bd209..098356e5bac 100644 > --- a/include/hw/i2c/aspeed_i2c.h > +++ b/include/hw/i2c/aspeed_i2c.h > @@ -290,7 +290,7 @@ struct AspeedI2CClass { > uint8_t num_busses; > uint8_t reg_size; > uint32_t reg_gap_size; > - uint8_t gap; > + unsigned gap; > qemu_irq (*bus_get_irq)(AspeedI2CBus *); > > uint64_t pool_size; > diff --git a/hw/i2c/aspeed_i2c.c b/hw/i2c/aspeed_i2c.c > index a8fbb9f44a1..a45a4fd6cb7 100644 > --- a/hw/i2c/aspeed_i2c.c > +++ b/hw/i2c/aspeed_i2c.c > @@ -1215,7 +1215,7 @@ static void aspeed_i2c_realize(DeviceState *dev, Error **errp) > > for (i = 0; i < aic->num_busses; i++) { > Object *bus = OBJECT(&s->busses[i]); > - int offset = i < aic->gap ? 1 : 5; > + unsigned offset = i < aic->gap ? 1 : 5; > > if (!object_property_set_link(bus, "controller", OBJECT(s), errp)) { > return; > @@ -1556,7 +1556,6 @@ static void aspeed_2600_i2c_class_init(ObjectClass *klass, void *data) > > aic->num_busses = 16; > aic->reg_size = 0x80; > - aic->gap = -1; /* no gap */ > aic->bus_get_irq = aspeed_2600_i2c_bus_get_irq; > aic->pool_size = 0x20; > aic->pool_base = 0xC00; > @@ -1580,7 +1579,6 @@ static void aspeed_1030_i2c_class_init(ObjectClass *klass, void *data) > > aic->num_busses = 14; > aic->reg_size = 0x80; > - aic->gap = -1; /* no gap */ > aic->bus_get_irq = aspeed_2600_i2c_bus_get_irq; > aic->pool_size = 0x20; > aic->pool_base = 0xC00; > @@ -1605,7 +1603,6 @@ static void aspeed_2700_i2c_class_init(ObjectClass *klass, void *data) > aic->num_busses = 16; > aic->reg_size = 0x80; > aic->reg_gap_size = 0x80; > - aic->gap = -1; /* no gap */ > aic->bus_get_irq = aspeed_2600_i2c_bus_get_irq; > aic->pool_size = 0x20; > aic->pool_gap_size = 0xe0;
On 4/23/25 15:33, Cédric Le Goater wrote: > On 4/23/25 14:05, Philippe Mathieu-Daudé wrote: >> Convert AspeedI2CClass::gap to plain unsigned, using '0' >> as "no gap" to avoid the followin UBSan warnings: >> >> SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ../../hw/i2c/aspeed_i2c.c:1559:16 >> SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ../../hw/i2c/aspeed_i2c.c:1583:16 >> SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ../../hw/i2c/aspeed_i2c.c:1608:16 >> hw/i2c/aspeed_i2c.c:1608:16: runtime error: implicit conversion from type 'int' of value >> -1 (32-bit, signed) to type 'uint8_t' (aka 'unsigned char') >> changed the value to 255 (8-bit, unsigned) >> >> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> > > Looks fine. nope. It's breaking make check : ERROR:../tests/qtest/tpm-tis-i2c-test.c:104:tpm_tis_i2c_test_basic: assertion failed (access == TPM_TIS_ACCESS_TPM_REG_VALID_STS | TPM_TIS_ACCESS_TPM_ESTABLISHMENT): (255 == 129) Unexpected error in qio_channel_socket_writev() at ../io/channel-socket.c:622: /home/legoater/work/qemu/qemu-aspeed.git/build/tests/qtest/tpm-tis-i2c-test: Unable to write to socket: Bad file descriptorqemu-system-aarch64: tpm-emulator: Could not cleanly shutdown the TPM: Interrupted system call C.
diff --git a/include/hw/i2c/aspeed_i2c.h b/include/hw/i2c/aspeed_i2c.h index 2c4c81bd209..098356e5bac 100644 --- a/include/hw/i2c/aspeed_i2c.h +++ b/include/hw/i2c/aspeed_i2c.h @@ -290,7 +290,7 @@ struct AspeedI2CClass { uint8_t num_busses; uint8_t reg_size; uint32_t reg_gap_size; - uint8_t gap; + unsigned gap; qemu_irq (*bus_get_irq)(AspeedI2CBus *); uint64_t pool_size; diff --git a/hw/i2c/aspeed_i2c.c b/hw/i2c/aspeed_i2c.c index a8fbb9f44a1..a45a4fd6cb7 100644 --- a/hw/i2c/aspeed_i2c.c +++ b/hw/i2c/aspeed_i2c.c @@ -1215,7 +1215,7 @@ static void aspeed_i2c_realize(DeviceState *dev, Error **errp) for (i = 0; i < aic->num_busses; i++) { Object *bus = OBJECT(&s->busses[i]); - int offset = i < aic->gap ? 1 : 5; + unsigned offset = i < aic->gap ? 1 : 5; if (!object_property_set_link(bus, "controller", OBJECT(s), errp)) { return; @@ -1556,7 +1556,6 @@ static void aspeed_2600_i2c_class_init(ObjectClass *klass, void *data) aic->num_busses = 16; aic->reg_size = 0x80; - aic->gap = -1; /* no gap */ aic->bus_get_irq = aspeed_2600_i2c_bus_get_irq; aic->pool_size = 0x20; aic->pool_base = 0xC00; @@ -1580,7 +1579,6 @@ static void aspeed_1030_i2c_class_init(ObjectClass *klass, void *data) aic->num_busses = 14; aic->reg_size = 0x80; - aic->gap = -1; /* no gap */ aic->bus_get_irq = aspeed_2600_i2c_bus_get_irq; aic->pool_size = 0x20; aic->pool_base = 0xC00; @@ -1605,7 +1603,6 @@ static void aspeed_2700_i2c_class_init(ObjectClass *klass, void *data) aic->num_busses = 16; aic->reg_size = 0x80; aic->reg_gap_size = 0x80; - aic->gap = -1; /* no gap */ aic->bus_get_irq = aspeed_2600_i2c_bus_get_irq; aic->pool_size = 0x20; aic->pool_gap_size = 0xe0;
Convert AspeedI2CClass::gap to plain unsigned, using '0' as "no gap" to avoid the followin UBSan warnings: SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ../../hw/i2c/aspeed_i2c.c:1559:16 SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ../../hw/i2c/aspeed_i2c.c:1583:16 SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ../../hw/i2c/aspeed_i2c.c:1608:16 hw/i2c/aspeed_i2c.c:1608:16: runtime error: implicit conversion from type 'int' of value -1 (32-bit, signed) to type 'uint8_t' (aka 'unsigned char') changed the value to 255 (8-bit, unsigned) Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> --- include/hw/i2c/aspeed_i2c.h | 2 +- hw/i2c/aspeed_i2c.c | 5 +---- 2 files changed, 2 insertions(+), 5 deletions(-)