New Defects reported by Coverity Scan for RTEMS

[Sebastian Huber]

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>> 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>>> 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>> 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.

There is also an unsolved issue with the time_second. On 32-bit targets 
you can't reliably read this value.

-- 
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/