Dne 2025-04-02 06:43, Takashi Yamamoto napsal:
After some debugging I found out that up_alarm_tick_cancel in
sched/sched/sched_timerexpiration.c reads an incorrect ticks value
from
clock_time2ticks macro. It received 0 seconds, 0x77358ns as a
parameter,
which - rounded up - should yield 1 tick with USEC_PER_TICK set to
1000.
Instead, it was returning 29 ticks. I tried to make a simple
standalone
reimplementation of clock_time2ticks and NSEC2TICK from
include/nuttx/clock.h to see what exactly is going on and it worked
well, returned correct result of 1. I then tried to compare the
generated assembly and found out the difference: while the
reimplementation was calculating (nsec + 999999) / 1000000, NuttX was
counting (nsec + 16959) / 16960. The former is 0xf423f and 0xf4240 in
hex, the latter is 0x423f and 0x4240 which led me to the idea that the
compiler truncates something to 16bit - that something turned to be
NSEC_PER_TICK.
do you mean a compiler bug?
otherwise, if your implementation of clock_time2ticks and NSEC2TICK
works better,
why don't you fix the nuttx version of them?
Hello,
thanks for the response (and also thanks to raiden00pl for opening the
PR.)
I have to admit that I don't know much about compilers to make any firm
judgement on this, but as far as I understood how things are supposed to
work, this didn't seem like a compiler bug to me. The reasoning for this
is that I remember getting a warning like this in the past:
main.c:936:19: warning: integer overflow in expression [-Woverflow]
constant = 1000 * 1000;
I remember being confused by this because "constant" was declared
uint32_t, therefore 1000 * 1000 should not overflow it. It's some time
ago so I don't remember exactly how much research I put into it and what
came of it but my working knowledge (currently) is that both of those
literals are considered to be 16 bit integers (for AVR that is.) Then,
when the compiler does the compile-time calculation, it stays in 16 bit
range and the result overflows. Marking one of the operands as a larger
type (1000L) fixes the warning - presumably because the compiler now
treats one operand as a 32 bit int and the result of the computation
does fit into 32 bit range. Feel free to correct me if I am wrong on
this.
This warning can be reproduced on x86_64 too, just with greater numbers:
unsigned long i;
i = 4000000000 * 4000000000;
sizeof.c:8:17: warning: integer overflow in expression of type ‘long
int’ results in ‘-2446744073709551616’ [-Woverflow]
So in short - this boils down to if the compiler is permitted to treat
the result of 16bit * 16bit compile-time multiplication as a 16bit
number. If that is the case, then its behaviour is IMO "correct" -
except that it (I think) should issue a warning since it compiles with
-Wall.
As for the other question - I think there may have been a
misunderstanding as I don't have any "my" implementation of
clock_time2ticks and NSEC2TICK. The patch only marks some constants to
be of type long, implementation of both clock_time2ticks and NSEC2TICK
remains unchanged.
