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