On 30/08/2022 16:37, Joel Sherrill wrote:
On Tue, Aug 30, 2022 at 8:44 AM Sebastian Huber
<sebastian.hu...@embedded-brains.de
<mailto:sebastian.hu...@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.hu...@embedded-brains.de
<mailto:sebastian.hu...@embedded-brains.de>
> <mailto:sebastian.hu...@embedded-brains.de
<mailto:sebastian.hu...@embedded-brains.de>>> wrote:
>
> On 30/08/2022 00:56, scan-ad...@coverity.com
<mailto:scan-ad...@coverity.com>
> <mailto:scan-ad...@coverity.com
<mailto:scan-ad...@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.hu...@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/
_______________________________________________
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel