New Defects reported by Coverity Scan for RTEMS

Sebastian Huber sebastian.huber at embedded-brains.de
Tue Aug 30 18:56:13 UTC 2022



On 30/08/2022 19:48, Joel Sherrill wrote:
> 
> 
> On Tue, Aug 30, 2022 at 9:53 AM Sebastian Huber 
> <sebastian.huber at embedded-brains.de 
> <mailto:sebastian.huber at embedded-brains.de>> wrote:
> 
> 
> 
>     On 30/08/2022 16:37, Joel Sherrill wrote:
>      >
>      > On Tue, Aug 30, 2022 at 8:44 AM Sebastian Huber
>      > <sebastian.huber at embedded-brains.de
>     <mailto:sebastian.huber at embedded-brains.de>
>      > <mailto:sebastian.huber at embedded-brains.de
>     <mailto:sebastian.huber at embedded-brains.de>>> wrote:
>      >
>      >     On 30/08/2022 15:40, Joel Sherrill wrote:
>      >      >
>      >      >
>      >      > On Tue, Aug 30, 2022 at 12:05 AM Sebastian Huber
>      >      > <sebastian.huber at embedded-brains.de
>     <mailto:sebastian.huber at embedded-brains.de>
>      >     <mailto:sebastian.huber at embedded-brains.de
>     <mailto:sebastian.huber at embedded-brains.de>>
>      >      > <mailto:sebastian.huber at embedded-brains.de
>     <mailto:sebastian.huber at embedded-brains.de>
>      >     <mailto:sebastian.huber at embedded-brains.de
>     <mailto:sebastian.huber at embedded-brains.de>>>> wrote:
>      >      >
>      >      >     On 30/08/2022 00:56, scan-admin at coverity.com
>     <mailto:scan-admin at coverity.com>
>      >     <mailto:scan-admin at coverity.com <mailto:scan-admin at coverity.com>>
>      >      >     <mailto:scan-admin at coverity.com
>     <mailto:scan-admin at coverity.com>
>      >     <mailto:scan-admin at coverity.com
>     <mailto:scan-admin at coverity.com>>> wrote:
>      >      >      > ** CID 1512552:  High impact quality  (Y2K38_SAFETY)
>      >      >      > /cpukit/score/src/kern_tc.c: 1804 in
>     _Timecounter_Windup()
>      >      >      >
>      >      >      >
>      >      >      >
>      >      >
>      >     
>       ________________________________________________________________________________________________________
>      >      >      > *** CID 1512552:  High impact quality  (Y2K38_SAFETY)
>      >      >      > /cpukit/score/src/kern_tc.c: 1804 in
>     _Timecounter_Windup()
>      >      >      > 1798          /* Go live with the new struct
>     timehands. */
>      >      >      > 1799     #ifdef FFCLOCK
>      >      >      > 1800          switch (sysclock_active) {
>      >      >      > 1801          case SYSCLOCK_FBCK:
>      >      >      > 1802     #endif
>      >      >      > 1803                  time_second =
>     th->th_microtime.tv_sec;
>      >      >      >>>>      CID 1512552:  High impact quality 
>     (Y2K38_SAFETY)
>      >      >      >>>>      A "time_t" value is stored in an integer
>     with too few
>      >      >     bits to accommodate it.  The expression
>     "th->th_offset.sec"
>      >     is cast
>      >      >     to "int32_t".
>      >      >      > 1804                  time_uptime = th->th_offset.sec;
>      >      >      > 1805     #ifdef FFCLOCK
>      >      >      > 1806                  break;
>      >      >      > 1807          case SYSCLOCK_FFWD:
>      >      >      > 1808                  time_second =
>      >     fftimehands->tick_time_lerp.sec;
>      >      >      > 1809                  time_uptime =
>      >      >     fftimehands->tick_time_lerp.sec - ffclock_boottime.sec;
>      >      >      >
>      >      >      > ** CID 1512551:    (Y2K38_SAFETY)
>      >      >
>      >      >     This seems to be a new Coverity feature. The Newlib time_t
>      >      >     definition is:
>      >      >
>      >      >     #if defined(_USE_LONG_TIME_T) || __LONG_MAX__ >
>     0x7fffffffL
>      >      >     #define _TIME_T_ long
>      >      >     #else
>      >      >     #define _TIME_T_ __int_least64_t
>      >      >     #endif
>      >      >     typedef _TIME_T_        __time_t;
>      >      >
>      >      >     Does Coverity use the Newlib header files? The
>      >     _USE_LONG_TIME_T should
>      >      >     be undefined for RTEMS.
>      >      >
>      >      >
>      >      > Yes it should. It works by doing something like this:
>      >      >
>      >      > cov-build waf|make
>      >      >
>      >      > And looking at the newlib headers, I agree everything
>     looks like
>      >     it should
>      >      > be defined to _int_least64_t. And preprocessing a simple file
>      >     that includes
>      >      > <sys/time.h> shows that it is typed to that.
>      >      >
>      >      > But.... something else is going on. time_uptime is defined to
>      >      > _Timecounter_Time_uptime
>      >      > which is an int32_t.
>      >
>      >     Oh, sorry, I didn't notice this. Then this is a false positive.
>      >     Using an
>      >     int32_t for uptime seconds is enough.
>      >
>      >
>      > The variable  time_uptime is still defined as two separate types. The
>      > prototype and the declaration do not match.
> 
>     I think this is all right:
> 
>     #ifndef __rtems__
>     volatile time_t time_second = 1;
>     volatile time_t time_uptime = 1;
>     #else /* __rtems__ */
>     volatile time_t time_second = TOD_SECONDS_1970_THROUGH_1988;
>     volatile int32_t time_uptime = 1;
>     #endif /* __rtems__ */
> 
>      >
>      > Even if int32_t is wide enough for seconds of uptime, it is an
>     assignment
>      > that narrows types. Casting to make this narrowing intentional
>     would at
>      > least hint this was intentional.
> 
>       From looking at the other Y2K38_SAFETY reports, a cast probably
>     doesn't
>     help to get rid of it.
> 
>      >
>      > But better would be to use the proper type and just make it
>     time_t like
>      > FreeBSD.
> 
>     The time_uptime is used in the libbsd. This is a performance
>     optimization.
> 
> 
> And the declaration you show is mismatched because of these:
> 
> cpukit/include/machine/_kernel_time.h:#define   time_uptime 
> _Timecounter_Time_uptime
> cpukit/include/rtems/score/timecounter.h:extern volatile int32_t 
> _Timecounter_Time_uptime;
> 
> This is inconsistent with the declaration of time_uptime above.

Sorry, I still don't see the issue with:

#ifndef __rtems__
volatile time_t time_second = 1;
volatile time_t time_uptime = 1;
#else /* __rtems__ */
volatile time_t time_second = TOD_SECONDS_1970_THROUGH_1988;
volatile int32_t time_uptime = 1;
#endif /* __rtems__ */

> 
> 
>     There is also an unsolved issue with the time_second. On 32-bit targets
>     you can't reliably read this value.
> 
> 
> I assume you mean because it will take two accesses?

Yes, there are two loads in arbitrary order and this is an issue if bit 
32 increments.

-- 
embedded brains GmbH
Herr Sebastian HUBER
Dornierstr. 4
82178 Puchheim
Germany
email: sebastian.huber at embedded-brains.de
phone: +49-89-18 94 741 - 16
fax:   +49-89-18 94 741 - 08

Registergericht: Amtsgericht München
Registernummer: HRB 157899
Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler
Unsere Datenschutzerklärung finden Sie hier:
https://embedded-brains.de/datenschutzerklaerung/


More information about the devel mailing list