RTCv1 + GCC7: rtc_decode() produces random output Topic is solved

Report here problems in any of ChibiOS components. This forum is NOT for support.
Thargon
Posts: 71
Joined: Wed Feb 04, 2015 5:03 pm
Location: CITEC, Bielefeld University, germany
Has thanked: 4 times
Been thanked: 6 times

RTCv1 + GCC7: rtc_decode() produces random output  Topic is solved

Postby Thargon » Mon Jul 09, 2018 11:02 am

Hi,

the RTCv1 driver produces random output when reading the current time for binaries compiled with GCC7.

I am running an STM32F103, so the RTCv1 LLD applies.
The compiler I use is GCC7: arm-none-eabi-gcc (GNU Tools for Arm Embedded Processors 7-2017-q4-major) 7.2.1 20170904 (release) [ARM/embedded-7-branch revision 255204]
I have enabled GCC option -std=c11, but the issue persists when compiling with -std=c99.

When calling rtcGetTime() it will again call rtc_lld_get_time(), which is specific to the LLD.
The latter then first calls rtcSTM32GetSecMsec() which reads the actual register values. I have checked those and this function seems to work fine.
rtc_lld_get_time() furthermore calls rtc_decode(), and here things start to get strange. rtc_decode() relies on localtime_r() to convert the tv_sec variable (a uint32_t holding the value of the RTC CNT registers) into a struct tm value tim. The result, however, seems to be quite random, so that the content of tim is unusable.
However, when I revert back to an older version of GCC (arm-none-eabi-gcc (GNU Tools for ARM Embedded Processors) 6.2.1 20161205 (release) [ARM/embedded-6-branch revision 243739]), everything works as expected.

I see two things that could be the cause of this issue: First, the pointer to tv_sec is casted to a time_t*, but since time_t is not well defined by the C standard, this cast might have become invalid for GCC7. Second, localtime_r() might fail internally, which could be caused by a GCC bug or configuration issues. Obviously, the former guess is the more probable one ;)
Anyway, I don't see the reason, why to call localtime_r() at all, especially since the time_t type involved is not well specified. As long as the origin date (e.g. 01-01-1980 like for the RTCDateTime type) is specified, the conversion could be easily done manually.

I would gladly commit a patch, if the issue can be confirmed by others!

Best regards,
Thomas

User avatar
Giovanni
Site Admin
Posts: 11025
Joined: Wed May 27, 2009 8:48 am
Location: Salerno, Italy
Has thanked: 354 times
Been thanked: 298 times
Contact:

Re: RTCv1 + GCC7: rtc_decode() produces random output

Postby Giovanni » Wed Jul 11, 2018 2:18 pm

Hi,

It could be caused by a C library change more than a compiler change, anyway, it would be great if you could analyse the issue, RTC needs also another change, alarm callback handling so it could be a good time rework it.

Giovanni

Thargon
Posts: 71
Joined: Wed Feb 04, 2015 5:03 pm
Location: CITEC, Bielefeld University, germany
Has thanked: 4 times
Been thanked: 6 times

Re: RTCv1 + GCC7: rtc_decode() produces random output

Postby Thargon » Fri Jul 13, 2018 3:11 pm

Hi,

I investigated a little and could fix the issue by only adding a few lines to hat_rtc_lld.c. I only had to slightly modify the rtc_decode() function (see below).

The issue was probably caused by the minor detail, that since C11 time_t is not defined to be an arithmetic type, but a real type. As a result, casts of pointer as it was done in RTCv1 are illegal with C11:

Code: Select all

// file: RTCv1/hal_rtc_lld.c

static void rtc_decode(uint32_t tv_sec,
                       uint32_t tv_msec,
                       RTCDateTime *timespec) {
  struct tm tim;
  struct tm *t;

  /* If the conversion is successful the function returns a pointer
     to the object the result was written into.*/
#if defined(__GNUC__) || defined(__CC_ARM)
  t = localtime_r((time_t *)&(tv_sec), &tim);    // FAIL: illegal cast from arithmetic to real
  osalDbgAssert(t != NULL, "conversion failed");
#else
  t = localtime(&tv_sec);                        // FAIL: illegal cast from arithmetic to real
  memcpy(&tim, t, sizeof(struct tm));
#endif

  rtcConvertStructTmToDateTime(&tim, tv_msec, timespec);
}

The issue can be easily fixed, though. Instead of propagating the pointer to the function argument (which is of arithmetic type uint32_t) to the localtime functions, I introduced an additional variable of type_t and let the compiler handle the type conversion:

Code: Select all

// file: RTCv1/hal_rtc_lld.c

static void rtc_decode(uint32_t tv_sec,
                       uint32_t tv_msec,
                       RTCDateTime *timespec) {
  struct tm tim;
  struct tm *t;
  const time_t time = tv_sec;                    // copy with implicit type conversion

  /* If the conversion is successful the function returns a pointer
     to the object the result was written into.*/
#if defined(__GNUC__) || defined(__CC_ARM)
  t = localtime_r(&time, &tim);
  osalDbgAssert(t != NULL, "conversion failed");
#else
  t = localtime(&time);
  memcpy(&tim, t, sizeof(struct tm));
#endif

  rtcConvertStructTmToDateTime(&tim, tv_msec, timespec);
}

Since my solution requires slightly more memory, I leave the decision to you whether you want to keep the old, more efficient code as well by using a switch, like

Code: Select all

#if (__STDC_VERSION__ >= 201112L)
/* C11 compatible code */
#else
/* more efficient code */
#endif


Best regards,
Thomas

User avatar
Giovanni
Site Admin
Posts: 11025
Joined: Wed May 27, 2009 8:48 am
Location: Salerno, Italy
Has thanked: 354 times
Been thanked: 298 times
Contact:

Re: RTCv1 + GCC7: rtc_decode() produces random output

Postby Giovanni » Fri Jul 13, 2018 3:14 pm

I would not go down to checking for compiler versions, thanks for the patch, I will integrate it.

Giovanni

User avatar
Giovanni
Site Admin
Posts: 11025
Joined: Wed May 27, 2009 8:48 am
Location: Salerno, Italy
Has thanked: 354 times
Been thanked: 298 times
Contact:

Re: RTCv1 + GCC7: rtc_decode() produces random output

Postby Giovanni » Sun Jul 15, 2018 9:23 am

Hi,

Fixed as bug #964.

Giovanni


Return to “Bug Reports”

Who is online

Users browsing this forum: No registered users and 1 guest