The best thing would be if you could submit a verified PR at
https://github.com/apache/incubator-nuttx
Greg
On 3/19/2020 10:38 AM, Sebastian Perta wrote:
Hello,
I have no experience and I'm not directly involved with NuttX (I maintain one
of the compilers used to build NuttX),
so hopefully I did the right thing in reporting the following issue here.
Initially the was reported to me as a problem in the compiler however when I
investigated I found there no problem
In the compiler, the problem was in nuttx/sched/signal/sig_timedwait.c at line
359-360:
wd_start(rtcb->waitdog, waitticks,
(wdentry_t)nxsig_timeout, 1, wdparm.pvarg);
Looking at the declarations of nxsig_timeout and wdentry_t:
static void nxsig_timeout(int argc, wdparm_t itcb)
typedef CODE void (*wdentry_t)(int argc, uint32_t arg1, ...);
We can see those declarations are incompatible, if we remove the cast the most
compilers (GCC for example) will return
an warning (other compilers will go as far as to return an error).
... warning: passing argument ... from incompatible pointer type
[-Wincompatible-pointer-types]
... note: expected 'wdentry_t' {aka 'void (*)(int, int, ...)'} but argument is
of type 'void (*)(int, int)'
One might think this warning can be safely ignored, especially since it works
in many cases, however this is not
generally true it can fail on some targets depending on ABI specification. This
is made clear in the
ISO/IEC standard chapter 7.6 more exactly the following statement:
"The rightmost parameter plays a special role in the access mechanism, and will be
designated parmN in this description."
What this means is that "itcb" from nxsig_timeout can be at a different location from
where "arg1" from wdentry_t is expected it to be.
The target for which I've see this problem is Renesas RX. In case of RX itcb is
placed in register R2 while arg1 is placed on the stack.
The majority of functions in the code used in this way (casted to wdentry_t)
are variadic as well so this is not a problem in general,
just in a few cases (nxsig_timeout is the only one I found so far there might
be a few others)
In order to make the code ANSI/ISO compliant nxsig_timeout needs to be made
variadic as well:
static void nxsig_timeout(int argc, wdparm_t itcb, ...)
I hope you agree with my finding and fix it as this will make the code more
robust (incompatible pointer assignment is unsafe)
and more portable (other targets besides Renesas RX in which I have a vested
interest).
Thank you,
Sebastian
Renesas Electronics Europe GmbH, Geschaeftsfuehrer/President: Carsten Jauch,
Sitz der Gesellschaft/Registered office: Duesseldorf, Arcadiastrasse 10, 40472
Duesseldorf, Germany, Handelsregister/Commercial Register: Duesseldorf, HRB
3708 USt-IDNr./Tax identification no.: DE 119353406 WEEE-Reg.-Nr./WEEE reg.
no.: DE 14978647
