Re: [Paparazzi-devel] Source code analysis of paparazzi

[Matthieu Lemerre]

Hi Felix,

Felix Ruess <address@hidden> writes:

> Hi Mathieu,
>
> very nice, I'm sure we can put together a representative set of
> configurations (fixedwing, rotorcraft, lpc21, stm32f1/f4).

That would be great, thanks.

> Any particular reason why you choose v5.0.5? For us it would make a
> lot more sense if you could use the latest stable version v5.2

I think it was the stable version when I first started working on
this. I'll see if I can upgrade.

>
> Regarding the LED macro, could you please tell us what compiler/toolchain
> you use?
> I can't see where the (unsigned long) cast would come from, to me it looks
> like LED_INIT(4) should evaluate to something like
>
> LED_DIR(4) |= _BV(LED_PIN(4))
> _LED_DIR(LED_4_BANK) |= _BV(LED_4_PIN)
> IO_1_DIR |= _BV(31)
>
> and with _BV from sw/include/std.h in (not resolving the io dir...)
>
> IO_1_DIR |= (1 << (31))
>
> which might still be wrong, but doesn't have the explicit cast and seemed
> to work (at least I could use LED_4 on my board...

I don't use any specific compiler, I use Frama-C to parse and analyse
the source code. The (unsigned long) cast was added by Frama-C (which
explicits implicit casts); (1 << (31)) is signed, and I guess IO_1_DIR
translates to an unsigned lvalue, so there is an implicit cast there.

Thus it seems that what's wrong is the _BV macro, which should translate
to ((1UL << (31)).

What this bug really means is that the behaviour of the (1 << 31)
expression is undefined behaviour as for the C standard, which means
that the compiler is free to compile code that uses this expression
arbitrarily. Probably your compiler compiles this code as you would
expect (and thus you do not see the bug), but a compiler that would be
"too smart" could do something wrong...

Regards
Matthieu