Linker sometimes moves non-PROGMEM data into PROGMEM

[MHotchin]

I have been chasing a problem with time calculations that has now boiled down to the linker is moving non-PROGMEM data into PROGMEM, because it's the same as other data that IS supposed to be in PROGMEM.

I'm running IDE 1.8.10, and the board manager and library manager report everything up to date. OS is Windows7 SP1 fully patched. Reproduces on both an UNO and a Mega 2560.

Before I go further, here's the sketch I've boiled things down to:

#include <Arduino.h>

#include <TimeLib.h>
#include <Timezone.h>

namespace
{
	const uint8_t daysArray[] PROGMEM = {31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31};
}

//  'TimeChangeRule' parameters to calculate local time.
//  N.A. Pacific Time Zone (L.A., Vancouver)
#define DaylightSavingTime "PDT", Second, Sun, Mar, 2, -7 * 60
#define StandardTime "PST", First, Sun, Nov, 2, -8 * 60

TimeChangeRule PDT = {DaylightSavingTime};
TimeChangeRule PST = {StandardTime};
Timezone tz(PDT, PST);

void setup()
{
	Serial.begin(115200);

	while (!Serial);

	tmElements_t tm;

	// Wed, 04 Mar 2020 06:52:24 GMT
	long tNow = 1583304745L;

	Serial.print(F("Current Unix time: "));
	Serial.println(tNow);

	auto tLocal = tz.toLocal(tNow);

	Serial.print(F("Current Local time: "));
	Serial.println(tLocal);

	//  Make sure arrays are linked in by using them
	Serial.println((uint32_t)daysArray, HEX);

	tm.Year = CalendarYrToTm(2020);
	tm.Month = 3; //  March
	tm.Day = 1;
	tm.Hour = 2;
	tm.Minute = 0;
	tm.Second = 0;

	auto t = makeTime(tm);

	Serial.println(F("makeTime()"));
	Serial.println(t);
}

void loop()
{

}

As is, the sketch botches the time calculation in makeTime(). I traced through the code, and found that in this case, the array of month lengths in the 'Time' library was complete nonsense.

Comment out the following line, and everything works!

Serial.println((uint32_t)daysArray, HEX);

Looking at the map files from this web-site:
https://sparks.gogo.co.nz/avr-ram-use.html

The WORKING version has the following:

RAM: Global Variables/Objects
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     157 Bytes Serial
      40 Bytes tz
      18 Bytes vtable for HardwareSerial
      12 Bytes monthDays
      12 Bytes PST
      12 Bytes PDT
       7 Bytes tm
       4 Bytes timer0_overflow_count
       4 Bytes timer0_millis
       4 Bytes cacheTime
       1 Bytes timer0_fract



RAM: Initialisation (eg, Strings not put in Flash)
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 800102:	1f 1e       	adc	r1, r31
  800104:	1f 1e       	adc	r1, r31
  800106:	1f 1f       	adc	r17, r31
  800108:	1e 1f       	adc	r17, r30
  80010a:	1e 1f       	adc	r17, r30

0080010c <vtable for HardwareSerial>:
  80010c:	00 00 00 00 8f 03 ef 02 1c 03 f7 03 4d 03 2b 03     ............M.+.
  80011c:	3f 03 0d 0a 00 50 44 54 00 50 53 54 00 00           ?....PDT.PST..

FLASH: Size Of Functions
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

 [deleted for brevity]

FLASH: Variable Initial Values and PROGMEM/F() etc...
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      52 Bytes setup()

The BROKEN version is:

RAM: Global Variables/Objects
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     157 Bytes Serial
      40 Bytes tz
      18 Bytes vtable for HardwareSerial
      12 Bytes PST
      12 Bytes PDT
       7 Bytes tm
       4 Bytes timer0_overflow_count
       4 Bytes timer0_millis
       4 Bytes cacheTime
       1 Bytes timer0_fract



RAM: Initialisation (eg, Strings not put in Flash)
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 800102:	00 00       	nop
  800104:	95 03       	fmuls	r17, r21
  800106:	f5 02       	muls	r31, r21
  800108:	22 03       	mulsu	r18, r18
  80010a:	fd 03       	fmulsu	r23, r21
  80010c:	53 03       	mulsu	r21, r19
  80010e:	31 03       	mulsu	r19, r17
  800110:	45 03       	mulsu	r20, r21
  800112:	0d 0a       	sbc	r0, r29
  800114:	00 50       	subi	r16, 0x00	; 0
  800116:	44 54       	subi	r20, 0x44	; 68
  800118:	00 50       	subi	r16, 0x00	; 0
  80011a:	53 54       	subi	r21, 0x43	; 67
	...

FLASH: Size Of Functions
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

 [NOTE - removed for brevity]

FLASH: Variable Initial Values and PROGMEM/F() etc...
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      52 Bytes setup()
      12 Bytes monthDays
      12 Bytes (anonymous namespace)::daysArray

Note that the 'monthDays' array has moved to flash! It's a global variable in the 'Time' library, and is NOT marked PROGMEM. If I change any of the values in 'daysArray', then everything works again:

RAM: Global Variables/Objects
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     157 Bytes Serial
      40 Bytes tz
      18 Bytes vtable for HardwareSerial
      12 Bytes monthDays
      12 Bytes PST
      12 Bytes PDT
       7 Bytes tm
       4 Bytes timer0_overflow_count
       4 Bytes timer0_millis
       4 Bytes cacheTime
       1 Bytes timer0_fract



RAM: Initialisation (eg, Strings not put in Flash)
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 800102:	1f 1e       	adc	r1, r31
  800104:	1f 1e       	adc	r1, r31
  800106:	1f 1f       	adc	r17, r31
  800108:	1e 1f       	adc	r17, r30
  80010a:	1e 1f       	adc	r17, r30

0080010c <vtable for HardwareSerial>:
  80010c:	00 00 00 00 95 03 f5 02 22 03 fd 03 53 03 31 03     ........"...S.1.
  80011c:	45 03 0d 0a 00 50 44 54 00 50 53 54 00 00           E....PDT.PST..

FLASH: Size Of Functions
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

 [NOTE - removed for brevity]

FLASH: Variable Initial Values and PROGMEM/F() etc...
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      52 Bytes setup()
      12 Bytes (anonymous namespace)::daysArray

[Floessie]

Isn't that arduino/toolchain-avr#59?

[MHotchin]

Certainly looks the same to me. I guess that makes this report a duplicate, and can be closed as such.

[matthijskooijman]

Hm, but that bug suggests that this is solved by upgrading to gcc 7.3, which I think is used in the latest AVR core, which @MHotchin implies to run (given the board manager "reports everything up to date", though he does have IDE 1.8.10 which is not the latest).

@MHotchin could you tell use what version of the AVR core and gcc you are seeing this with exactly? Easiest would be to just paste the output of a verbose compilation, which should tell us enough.

[MHotchin]

IDE reports using tools from "Arduino15\packages\arduino\tools\avr-gcc\7.3.0-atmel3.6.1-arduino5".

verbose.txt

[Floessie]

There seems to be another bug lurking:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92606

Johann suggests -fno-ipa-icf-variables as a workaround. Don't know if that's already available in 7.3, though.

[MHotchin]

Just tested -
adding -fno-ipa-icf-variables to 'compiler.cpp.flags' in platform.txt fixes this particular example.

[MHotchin]

So, going back to my original sketch that I was debugging:
WITHOUT -fno-ipa-icf-variables (original, broken version)

Program GateController size: 62,156 bytes (used 24% of a 253,952 byte maximum) (23.27 secs)
Minimum Memory Usage: 1966 bytes (24% of a 8192 byte maximum)

WITH the flag (functionality untested as yet):
Program size: 62,192 bytes (used 24% of a 253,952 byte maximum) (25.83 secs)
Minimum Memory Usage: 2002 bytes (24% of a 8192 byte maximum)

So, both RAM and FLASH usage went up,and by the same amount, which is a bit unexpected. I'll have to dig into the map files to see what the differences are.

[MHotchin]

RAM usage went up 36 bytes. 12 are from the 'monthDays' array, as expected. The other 24 bytes are unaccounted for in the MAP files.

[matthijskooijman]

@MHotchin for comparing symbols and sizes in an .elf file, https://github.com/matthijskooijman/scripts/blob/master/Arduino/diff-elf-symbols might be helpful. It compares avr-objdump -t output. Note sure if that would produce different results than the map files, though, but maybe it seems some more hidden (?) symbols.

[MHotchin]

Here's the output from avr-objdump -t --special_syms for both configurations.
map_old.txt
map_new.txt
The only relevant change I could find was line 43, where '_ZL2monthDays' moves from .text to .data.
@matthijskooijman, I'm running windows so I don't have easy access to bash. If you could run your script over these two files I'd appreciate it.

[matthijskooijman]

The only relevant change I could find was line 43, where '_ZL2monthDays' moves from .text to .data.

My script agreess:

matthijs@grubby:~$ diff-elf-symbols map_old.txt map_new.txt 
368c368
 0000000c .text                                 _ZL9monthDays
---
 0000000c .data                                 _ZL9monthDays

The flag you used completely disables the "merge duplicate symbols" optimization pass (https://github.com/gcc-mirror/gcc/blob/master/gcc/ipa-icf.c). What I suspect, is that this pass might cause multiple symbols to become aliases and though they remain distinct symbols, they end up at the same address, so they take up space only once. Looking through the map file (using the regex `` to find subsequent lines with identical addresses, after sorting the file), I found for example:

00800335 l     O .data  0000000c _ZTV12PasswordFile
00800335 l     O .data  0000000c _ZTV6SdFile
00800335 l     O .data  0000000c _ZTVN12_GLOBAL__N_110GateSdFileE 

vs

00800359 l     O .data  0000000c _ZTV6SdFile
008002bd l     O .data  0000000c _ZTVN12_GLOBAL__N_110GateSdFileE
00800247 l     O .data  0000000c _ZTV12PasswordFile

So thats 2 times 0xc bytes, so 24 extra bytes in Flash and RAM, explaining the difference (I did not find any others, there were more duplicate addresses in .text, but they were duplicate in both, so probably produced by another optimization pass, or explicitly specified in the code maybe).

I'm running windows so I don't have easy access to bash.

A very easy way to get bash is to install git, the default version comes with "git bash" (bash compiled by/for the git project) which just works.

[MHotchin]

I've just verified that this fix does the trick in my original project. For now I'm going to keep the flag in my platform.local.txt, so I have a work around. The small increase in RAM usage doesn't bother me, seeing as I saved three times as much by fixing the INADDR_NONE bug in my local copy (#1007).

I guess it's up to the AVR platform maintainers now to decide if this flag should be added to platform.txt until g++ is fixed (if ever).