Order of updating members of struct

Go To Last Post
19 posts / 0 new
Author
Message
#1
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I am a recent assembly to c convert, so I am still learning c. Forgive me if this is a noob question.

I am having a problem writing members of a struct. If I update them in the order they were declared, everything works fine. If I update them in reverse order, two of them take on incorrect values.

header:

typedef struct {
	uint16_t	year;
	uint8_t		month;
	uint8_t		mday;
	uint8_t		hour;
	uint8_t		min;
	uint8_t		sec;
} RTC_time_t;

working access functions:

/* current RTC time */
volatile RTC_time_t t;

void rtc_gettime (RTC_time_t * rtc)
{
	ATOMIC_BLOCK(ATOMIC_FORCEON)
	{
		rtc->year = t.year;
		rtc->month = t.month;
		rtc->mday = t.mday;
		rtc->hour = t.hour;
		rtc->min = t.min;
		rtc->sec = t.sec;
	}
}

void rtc_settime (RTC_time_t * rtc)
{
	ATOMIC_BLOCK(ATOMIC_FORCEON)
	{
		t.year = rtc->year;
		t.month = rtc->month;
		t.mday = rtc->mday;
		t.hour = rtc->hour;
		t.min = rtc->min;
		t.sec = rtc->sec;
	}
}

void rtc_init (void)
{
	/* Reset time to Jan 1, '10 */
	ATOMIC_BLOCK(ATOMIC_FORCEON)
	{
		t.year = 2010;
		t.month = 1;
		t.mday = 1;
		t.hour = 0;
		t.min = 0;
		t.sec = 0;
	}
}

The RTC updates the global t in an ISR

Now in the main code:

RTC_time_t rtc;

rtc_gettime(&rtc);
xprintf(PSTR("?u/?u/?u ?02u:?02u:?02u\n"), rtc.year, rtc.month, rtc.mday, rtc.hour, rtc.min, rtc.sec);

Assume that the ? are percent signs (can't post percent).

When I run the above, I get something like:

2010/1/1 00:00:08

If, however, I change the order in which I update the structure t (in this case, the rtc_init function):

void rtc_init (void)
{
	/* Reset time to Jan 1, '10 */
	ATOMIC_BLOCK(ATOMIC_FORCEON)
	{
		t.sec = 0;
		t.min = 0;
		t.hour = 0;
		t.mday = 1;
		t.month = 1;
		t.year = 2010;
	}
}

then I get the following output:

55808/7/1 00:00:08

The year happens to be 256 * 2010 with the overflow (year is uint16_t) going into the month.

The disassembly of the working rtc_init:

		t.year = 2010;
    7a1c:	8a ed       	ldi	r24, 0xDA	; 218
    7a1e:	97 e0       	ldi	r25, 0x07	; 7
    7a20:	80 93 1b 2b 	sts	0x2B1B, r24
    7a24:	90 93 1c 2b 	sts	0x2B1C, r25
		t.month = 1;
    7a28:	81 e0       	ldi	r24, 0x01	; 1
    7a2a:	80 93 1d 2b 	sts	0x2B1D, r24
		t.mday = 1;
    7a2e:	80 93 1e 2b 	sts	0x2B1E, r24
		t.hour = 0;
    7a32:	10 92 1f 2b 	sts	0x2B1F, r1
		t.min = 0;
    7a36:	10 92 20 2b 	sts	0x2B20, r1
		t.sec = 0;
    7a3a:	10 92 21 2b 	sts	0x2B21, r1

which is pretty straightforward.

The disassembly of the nonworking version:

                t.sec = 0;
    7a1c:	10 92 21 2b 	sts	0x2B21, r1
		t.min = 0;
    7a20:	e0 e2       	ldi	r30, 0x20	; 32
    7a22:	fb e2       	ldi	r31, 0x2B	; 43
    7a24:	10 82       	st	Z, r1
		t.hour = 0;
    7a26:	12 92       	st	-Z, r1
		t.mday = 1;
    7a28:	81 e0       	ldi	r24, 0x01	; 1
    7a2a:	82 93       	st	-Z, r24
		t.month = 1;
    7a2c:	82 93       	st	-Z, r24
		t.year = 2010;
    7a2e:	8a ed       	ldi	r24, 0xDA	; 218
    7a30:	97 e0       	ldi	r25, 0x07	; 7
    7a32:	31 97       	sbiw	r30, 0x01	; 1
    7a34:	81 93       	st	Z+, r24
    7a36:	90 83       	st	Z, r25
    7a38:	32 97       	sbiw	r30, 0x02	; 2

As you can see, the high byte of t.year is being updated with the low byte of 2010. Then t.month is overwritten with the high byte of 2010. The compiler should have subtracted 2 from Z (the first sbiw instruction), but it only subtracts 1. It does manage to subtract 2 in the last instruction, which seems rather pointless.

Clearly, the compiler (gcc 4.3.3) is not doing what I intend. But maybe it is just following orders (stupid in, stupid out). Am I doing something stupid?

I can easily just go with the working code, but I need to understand why this happens.

https://www.mattairtech.com/
ARM Cortex M and XMEGA development boards / Gentoo Linux

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I agree that because of the pre-decrement after the t.month=1 in the failing case Z=0x2B1D so that after the SBIW it is 0x2B1C when the following code is behaving as if it were 0x2B1B

This does look like a compiler bug to me - wonder what others think?

Just one thing - when you say "4.3.3" exactly how did you come by this:

1) WinAVR20100110
2) built yourself with Bingo600's script
3) Used a prebuilt Bingo600 .deb from wrightflyer.co.uk
4) pulled a repository build of avr-gcc from some Linux distro

If (4) I'd maybe try again with any of (1), (2) or (3)

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

This seems to me as a genuine bug in avr-gcc (trying to optimise overly smart), however unlikely that may sound.

JW

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Your code looks very much like the glue function in the ElmChan FATfs code.

The C looks absolutely fine to me. It should not make an difference what order you assign your time_t members. Personally I just assign the whole structure.

  t = *rtc;    // assign structure

  *rtc = t;    // assign structure

I have no intention of struggling with the ASM. Try an updated compiler. Do you still have a problem?

David.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

clawson wrote:
Just one thing - when you say "4.3.3" exactly how did you come by this:

1) WinAVR20100110
2) built yourself with Bingo600's script
3) Used a prebuilt Bingo600 .deb from wrightflyer.co.uk
4) pulled a repository build of avr-gcc from some Linux distro

If (4) I'd maybe try again with any of (1), (2) or (3)

I bet on (4). ;-)

I have a different result with 4.3.3 (WinAVR):

  58:	10 92 06 01 	sts	0x0106, r1
      t.min = 0;
  5c:	e5 e0       	ldi	r30, 0x05	; 5
  5e:	f1 e0       	ldi	r31, 0x01	; 1
  60:	10 82       	st	Z, r1
      t.hour = 0;
  62:	12 92       	st	-Z, r1
      t.mday = 1;
  64:	81 e0       	ldi	r24, 0x01	; 1
  66:	82 93       	st	-Z, r24
      t.month = 1;
  68:	82 93       	st	-Z, r24
      t.year = 2010;
  6a:	8a ed       	ldi	r24, 0xDA	; 218
  6c:	97 e0       	ldi	r25, 0x07	; 7
  6e:	92 93       	st	-Z, r25
  70:	82 93       	st	-Z, r24

Stefan Ernst

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:

Try an updated compiler

On the whole 4.3.3 is the one that most of us are using right now. While there are 4.4 and now 4.5 versions Eric/Joerg have suggested that they may have AVR specific problems preventing a move to their genral usage.

But it could well be that this 4.3.3 is missing some of its vital patches.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I used Bingo's script to build on Gentoo Linux (source based distro). It was a version prior to the current one that is more in line with WinAVR20100110. However, I thought I saw the same problem with a version compiled with the actual WinAVR20100110 on Windows, but I will re-check. I will try upgrading again and report back.

https://www.mattairtech.com/
ARM Cortex M and XMEGA development boards / Gentoo Linux

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Can you break the problem code out into the smallest buildable program that still shows the error so that we can all build and test against various gcc versions? Tfrancuz, for example, just reported that he has a working build of 4.5.0 so it'd be very interesting to see how this works in that.

Cliff

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I compiled a minimal version and the problem does not exist. There is no attempt to use the Z register pair, just sts instructions. I will build it back up until the problem re-appears.

https://www.mattairtech.com/
ARM Cortex M and XMEGA development boards / Gentoo Linux

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

The minimal code that replicates the problem is:

#include 

typedef struct {
	uint16_t	year;
	uint8_t		month;
	uint8_t		mday;
	uint8_t		hour;
	uint8_t		min;
	uint8_t		sec;
} RTC_time_t;

volatile RTC_time_t t;

void rtc_init (void)
{
	t.sec = 0;
	t.min = 0;
	t.hour = 0;
	t.mday = 1;
	t.month = 1;
	t.year = 2010;
	//t.month = 1;
	//t.mday = 1;
	//t.hour = 0;
	//t.min = 0;
	//t.sec = 0;
}

void main(void)
{
	rtc_init();
	
	while (1);
}

Removing the volatile modifier eliminates the problem (no attempt to use st along with Z, just sts).

https://www.mattairtech.com/
ARM Cortex M and XMEGA development boards / Gentoo Linux

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Disabling optimizations also removes the problem (I was using OPT = s)

https://www.mattairtech.com/
ARM Cortex M and XMEGA development boards / Gentoo Linux

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

WinAVR20100110 (also 4.3.3 - but possibly patched differently) produces:

void rtc_init (void) 
{ 
   t.sec = 0; 
  7c:	10 92 66 00 	sts	0x0066, r1
   t.min = 0; 
  80:	e5 e6       	ldi	r30, 0x65	; 101
  82:	f0 e0       	ldi	r31, 0x00	; 0
  84:	10 82       	st	Z, r1
   t.hour = 0; 
  86:	12 92       	st	-Z, r1
   t.mday = 1; 
  88:	81 e0       	ldi	r24, 0x01	; 1
  8a:	82 93       	st	-Z, r24
   t.month = 1; 
  8c:	82 93       	st	-Z, r24
   t.year = 2010; 
  8e:	8a ed       	ldi	r24, 0xDA	; 218
  90:	97 e0       	ldi	r25, 0x07	; 7
  92:	92 93       	st	-Z, r25
  94:	82 93       	st	-Z, r24
   //t.month = 1; 
   //t.mday = 1; 
   //t.hour = 0; 
   //t.min = 0; 
   //t.sec = 0; 
} 
  96:	08 95       	ret

which looks like it will work and in the simulator it is proven to do so.

It does look like your 4.3.3 may be missing a patch or two. These are the ones applied to WinAVR:

 Directory of C:\WinAVR-20100110

19/01/2010  19:08               576 40-binutils-2.19-wrong-arch.patch
19/01/2010  19:08               493 41-binutils-2.19-avr25-wraparound-reloc.patc
h
19/01/2010  19:08             1,657 42-binutils-2.19-bug-9841.patch
19/01/2010  19:08            22,161 50-binutils-2.19-xmega.patch
19/01/2010  19:08            11,578 51-binutils-2.19-new-devices.patch
               5 File(s)         36,465 bytes

 Directory of C:\WinAVR-20100110\source\avr\binutils\2.19

19/01/2010  19:08            16,260 30-binutils-2.19-avr-size.patch
19/01/2010  19:08           168,711 31-binutils-2.19-avr-coff.patch
19/01/2010  19:08             1,301 32-binutils-2.19-new-sections.patch
19/01/2010  19:08             4,881 33-binutils-2.19-data-origin.patch
19/01/2010  19:08             1,113 34-binutils-2.19-as-dwarf.patch
19/01/2010  19:08               982 35-binutils-2.19-dwarf2-AVRStudio-workaround
.patch
19/01/2010  19:08               576 40-binutils-2.19-wrong-arch.patch
19/01/2010  19:08               493 41-binutils-2.19-avr25-wraparound-reloc.patc
h
19/01/2010  19:08             1,657 42-binutils-2.19-bug-9841.patch
19/01/2010  19:08            22,161 50-binutils-2.19-xmega.patch
19/01/2010  19:08            11,578 51-binutils-2.19-new-devices.patch
              11 File(s)        229,713 bytes
 Directory of C:\WinAVR-20100110\source\avr\gcc\4.3.3

19/01/2010  19:08               392 20-gcc-4.3.3-libiberty-Makefile.in.patch
19/01/2010  19:08               924 21-gcc-4.3.3-disable-libssp.patch
19/01/2010  19:08               339 23-gcc-4.3.3-ada-Makefile.patch
19/01/2010  19:08               629 30-gcc-4.3.3-param-inline-call-cost.patch
19/01/2010  19:08             1,656 40-gcc-4.3.3-bug-10768-by-adacore.patch
19/01/2010  19:08            11,119 41-gcc-4.3.3-bug-11259_v3.patch
19/01/2010  19:08             1,606 42-gcc-4.3.3-bug-spill-v4.patch
19/01/2010  19:08             2,209 43-gcc-4.3.3-bug-35013.patch
19/01/2010  19:08             1,348 44-gcc-4.3.3-libgcc16.patch
19/01/2010  19:08             2,914 45-gcc-4.3.3-bug-33009.patch
19/01/2010  19:08               445 46-gcc-4.3.3-andsi3-wrong-attribute.patch
19/01/2010  19:08             7,348 47-gcc-4.3.3-bug-18145-v4.patch
19/01/2010  19:08            10,560 50-gcc-4.3.3-mega256-v2.patch
19/01/2010  19:08            33,036 51-gcc-4.3.3-xmega-v13.patch
19/01/2010  19:08            24,718 52-gcc-4.3.3-new-devices.patch
19/01/2010  19:08             7,276 60-gcc-4.3.3-osmain.patch
19/01/2010  19:08            16,365 61-gcc-4.3.3-builtins_v6.patch
19/01/2010  19:08             6,084 62-gcc-4.3.3-ada-mlib.patch
19/01/2010  19:08             3,458 63-gcc-4.3.3-ada-freestanding.patch
19/01/2010  19:08             2,645 64-gcc-4.3.3-ada-timebase.patch
19/01/2010  19:08             1,312 65-gcc-4.3.3-ada-gnat1_print_path.patch
19/01/2010  19:08             3,026 66-gcc-4.3.3-ada-optim_static_addr.patch
              22 File(s)        139,409 bytes

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

and I was using the build from the
build-avr-gcc-4.3.3-binutils-2.20-libc-1.6.8-insight6.8-dude-5.10-insight-patch
script

https://www.mattairtech.com/
ARM Cortex M and XMEGA development boards / Gentoo Linux

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

If I compile for the atmega32u2, the problem goes away. Compiling for atxmega128a1 causes the problem.

https://www.mattairtech.com/
ARM Cortex M and XMEGA development boards / Gentoo Linux

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

physicist wrote:
If I compile for the atmega32u2, the problem goes away. Compiling for atxmega128a1 causes the problem.

Wow.

Care to post the whole chain of commands you used to build this, so maybe others (Cliff?) could try to reproduce it?

JW

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I am using the WinAVR Makefile Template written by Eric B. Weddington, Jörg Wunsch, et al.
The definition that I changed in the makefile was MCU

Can anyone confirm this problem when using MCU = atxmega128a1 versus MCU = atmega32u2 (or something else) in the makefile? I have attached the makefile I used (I had to add the .txt extension to attach).

Attachment(s): 

https://www.mattairtech.com/
ARM Cortex M and XMEGA development boards / Gentoo Linux

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I also see the problem when compiling using WinAVR20100110 on Windows, again with volatile and MCU = atxmega128a1. I compiled from within AVR Studio so am using its makefile.

https://www.mattairtech.com/
ARM Cortex M and XMEGA development boards / Gentoo Linux

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Ah, I built for mega16 as I had no way to know what chip was involved. So that presumably explains it. I guess that because of different opcode timings in Xmega the code generator must take a different path (and be relatively untested). I guess you are in a position to now report this as a bug at Savannha as you have the test case complete with Makefile.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

It looks like possibly a copy-paste error was made in the patch-xmega file. The following starting on line 573 of patch-xmega (from the build-avr-gcc-4.3.4-binutils-2.20-libc-1.6.8-insight6.8-dude-5.10-insight-patch script):

+        return *l = 4, (AS2 (sbiw,?r0,1)    CR_TAB 
+                        AS2 (st,?p0+,%A1)   CR_TAB
+                        AS2 (st,?p0,%B1)    CR_TAB
+                        AS2 (sbiw,?r0,2));

I think should be:

+        return *l = 4, (AS2 (sbiw,?r0,2)    CR_TAB 
+                        AS2 (st,?p0+,%A1)   CR_TAB
+                        AS2 (st,?p0,%B1)    CR_TAB
+                        AS2 (sbiw,?r0,1));

(question marks are actually percent signs)

This may have been caused by copying code from one of the post-increment sections utilizing adiw, then converting it to pre-decrement utilizing sbiw, then forgetting to swap the 2 and 1.

I assume that the second sbiw instruction is needed to put the pointer back at the location it was at after the first sbiw.

Someone more knowledgeable about GCC's innards should check this. Attached is a patch for the patch-xmega, um, patch. It works for me.

This bug is being tracked at http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43876

Attachment(s): 

https://www.mattairtech.com/
ARM Cortex M and XMEGA development boards / Gentoo Linux