Hi all,
I have found a same problem which Sebastian pointed out in another processor.
Not only Renesus RX, for example, AVR has similar ABI spec.
To fix the problem, I have investigated the callers of wd_start().
Please let me discuss about the problem in github issue #740.
Thanks,
Yuuichi Nakamura
> From: Sebastian Perta <sebastian.pe...@renesas.com>
> Sent: Friday, March 20, 2020 1:38 AM
> To: dev@nuttx.apache.org
> Subject: problem with nxsig_timeout (sig_timedwait.c)
>
> 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
