Code not compatible with up-to-date build environment (gcc 4.6.2, avr-libc 1.8.0)

[GoogleCodeExporter]

What steps will reproduce the problem?
0. use up-to-date build environment (avr-gcc 4.6.2, avr-libc 1.8.0)
1. make

What is the expected output? 
compiled code

What do you see instead?
errors like:

error: variable 'foo' must be const in order to be put into read-only section 
by means of '__attribute__((progmem))' -- this is for old avr-libc

open9x.h: error: 'prog_uint8_t' does not name a type -- with updated avr-libc

What version of the product are you using? On what operating system?
Fedora 16 (linux kernel 3.1.5), avr-gcc 4.6.2, avr-libc 1.8.0

Please provide any additional information below.
gcc 4.6.1 is known to be broken, but 4.6.2 is ok
( http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49764 )

with new avr-libc, problem is bigger:

Now you get:
error: 'prog_char' does not name a type
and other alike. There is a change in pgmspace.h:
==============
   This typedef is now deprecated because the usage of the __progmem__ 
   attribute on a type is not supported in GCC. However, the use of the 
   __progmem__ attribute on a variable declaration is supported, and this is 
   now the recommended usage.

   The typedef is only visible if the macro __PROG_TYPES_COMPAT__
   has been defined before including <avr/pgmspace.h> (either by a
   #define directive, or by a -D compiler option.)
==============
Because g++ does not handle PROGMEM correctly (it's attribute in typedef,
right?) avr-libc doped all prog_uchar and other prog_* variables (it won't work
anyway). In new version you need to add #define __PROG_TYPES_COMPAT__ (before
pgmspace include) if you want to compile your old code, other way you'll get
lot of "does not name a type" errors. Using that #define changes errors to 
warnings "prog_char is deprecated".

Anyway, fast fix to get it compiled is that #define. The Right Way (not using 
undocumented gcc's features that got removed) is to change
(const or not) prog_char *
to something like
const char * PROGMEM

avr-libc added const everywhere already as it's the only way expected to work.

http://lists.nongnu.org/archive/html/avr-libc-commit/2012-01/msg00006.html

That all still does not help, because (old) PSTR definition is not compatible 
with new gcc (4.6.2, containing The Fix). It was fixed in avr-libc, but your 
copy of that definition in open9x.h needs that fix too:

-#define PSTR(s) (__extension__({static prog_char APM __c[] = (s);&__c[0];}))
+#define PSTR(s) (__extension__({static const prog_char APM __c[] = 
(s);&__c[0];}))

Also I had to remove --combine option. From gcc upstream (4.6 changes):
unrecognized option '-combine':
    The -combine option has been removed, it has been very buggy and has
    been superceeded by -flto.  Either drop it, or build with -flto.

That's short summary how to compile the code with in new build environment. 

The result does not work with all combination of avr-gcc 4.3,4.4,4.5,4.6 and 
avr-libc 1.6.7..1.8.0 (btw, avr-libc is 2.5 years old, the oldest maintained 
version of gcc is 4.4.6).

I know it's not a good solution for everyone to update his build environment, 
so I've searched for solution that would work with any avr-libc & avr-gcc 
combination (starting avr-gcc 4.3.3 & avr-libc 1.6.7 that can be found in most 
wide spread build environments on windows and mac). It was quite difficult 
because different old version are... "different" or just not compatible). It's 
quite a mess... I got everything from "only initialized variables...", 
"conflict of sections",... to data not placed in program memory at all. 

In the end, I've found following solution (PROGMEM redefinition is not needed 
with avr-gcc 4.6.2): 

============ example: ======================
#include <avr/pgmspace.h>
#undef PROGMEM
#define PROGMEM __attribute__(( section(".progmem.data") ))

typedef char pm_char;
const pm_char hello[] PROGMEM = "Hello world";

int foo(const pm_char *s)
{
  const pm_char *str = s; 
============================================

This uses new type pm_* instead of prog_* because new avr-libc does not provide 
such types unless you use special define in which case it spawns tons of 
"prog_* is deprecated" warnings. Anyway, prog_* or pm_* does not affect 
variable storage at all, it's just we can see this is some variable stored in 
program section, so we can't use it directly.

Every variable using data stored in progmem section needs to be const or 
avr-gcc won't like you.

On variable initialization and *only* on variable initialization, there must be 
PROGMEM used - only where you specify the real data which should be stored in 
program section.

This builds fine with avr-libc 1.6.7 & avr-gcc 4.3.3 AND avr-libc 1.8.0 & 
avr-gcc 4.6.2 without single warning. (I had to use -flto instead of -combine, 
but old version does not know -flto, new version dropped -combine, so this 
can't be fixed without automake or cmake scripts. Anyway, it's much easier to 
change one line in Makefile than change whole code.)

I've attached a patch with changes that applies above changes. I've build 
open9x without single warning and used it in my tx - added new aircraft 
configuration and used it with simulator for one hour.

Original issue reported on code.google.com by mih...@mihlit.cz on 1 Feb 2012 at 10:34

Attachments:

• newdifff.patch

[GoogleCodeExporter]

Ok I will review it tomorrow. Thanks a lot for the patch!

Original comment by bson...@gmail.com on 1 Feb 2012 at 10:41

• Changed state: Started

[GoogleCodeExporter]

Here I don't have the file pgmtypes.h
It brings problems with pm_char type. Do you know if there is a #ifdef that we 
could add somewhere?

Original comment by bson...@gmail.com on 2 Feb 2012 at 6:45

[GoogleCodeExporter]

My mistake, pgmtypes.h is new file. I've forgot to call svn add before 
generating patch with svn diff. As I wrote before, these are not special types, 
they have just pm_ prefix to show they point to progmem data, so they can't be 
used directly. Alto, this is not special file I took somewhere, it's new file 
where I put type definitions similar to those from avr/pgmspace.h with new 
prefix pm_ so they do not conflict. If you want you can put it in open9x.h or 
somewhere else.

New patch is attached.
Changes:
- removed forgotten comment from open9x.h
- added pgmtypes.h
- added change to util/xbm2lbm.rb
- svn diff called from top directory, not from src/

Original comment by mih...@mihlit.cz on 2 Feb 2012 at 7:28

Attachments:

• newdiff2.patch

[GoogleCodeExporter]

Would it be possible you update with latest revision? I have a lot of conflicts 
that I think you will not have! I will not commit anything meanwhile!

Original comment by bson...@gmail.com on 2 Feb 2012 at 7:34

[GoogleCodeExporter]

ok, refreshed patch - against revision 185

Original comment by mih...@mihlit.cz on 2 Feb 2012 at 8:30

Attachments:

• newdiff2.patch

[GoogleCodeExporter]

refreshed patch - revision 185. This time for real (in comment 5, it was the 
old one again)

Original comment by mih...@mihlit.cz on 2 Feb 2012 at 8:33

Attachments:

• newdiff3.patch

[GoogleCodeExporter]

That's a really good work. It seems to compile perfectly here.

One more check:
Could you compile with this command: make clean ; make PCB=STD EXT=FRSKY and 
send me the .lss file, I would like to check the stack for the 
__getSwitch(int8_t swtch) function which is recursive.

Also what is the size of the binary produced on the new compiler (with options 
abobe)?
Here I have:

Program:   58914 bytes (89.9% Full)
(.text + .data + .bootloader)

Data:       3254 bytes (79.4% Full)
(.data + .bss + .noinit)

If the last compiler version does better, I will change before producing next 
official version.

Thanks again for this work. Provided these last checks are ok, I am ok for you 
to commit them.

Bertrand.

Original comment by bson...@gmail.com on 2 Feb 2012 at 9:57

[GoogleCodeExporter]

It took a while, I had to update avr-binutils to 2.22 because avr-objdump 2.20 
did not understand dwarf data produced by new avr-gcc (BFD: Dwarf Error: 
mangled line number section.) with updated avr-binutils, this error is gone.

attached is open9x.lss file obtained with avr-gcc 4.6.2, make PCB=STD EXT=FRSKY 
and following Makefile patch:
Index: Makefile
===================================================================
--- Makefile    (revision 185)
+++ Makefile    (working copy)
@@ -364,7 +364,7 @@
 CFLAGS += $(patsubst %,-I%,$(EXTRAINCDIRS))
 CFLAGS += $(CSTANDARD)

-CFLAGS+= --combine -fwhole-program
+CFLAGS+= -flto -fwhole-program

 #---------------- Compiler Options C++ ----------------
@@ -394,6 +394,7 @@
 #CPPFLAGS += -Wa,-adhlns=$(<:%.cpp=$(OBJDIR)/%.lst)
 CPPFLAGS += $(patsubst %,-I%,$(EXTRAINCDIRS))
 #CPPFLAGS += $(CSTANDARD)
+CPPFLAGS += -flto

 AVRGCCFLAGS = -fno-inline-small-functions

$ avr-size open9x.elf 
   text    data     bss     dec     hex filename
  57796      26    3228   61050    ee7a open9x.elf

man page with -flto information:
http://gcc.gnu.org/onlinedocs/gcc-4.6.2/gcc/Optimize-Options.html

Original comment by mih...@mihlit.cz on 2 Feb 2012 at 10:52

Attachments:

• open9x.lss

[GoogleCodeExporter]

About the __getSwitch function, it is ok (no change), then it may be used in 
recursive switches without any limitation (the limit is 12 = the number of 
virtual switches)

The size is really good, 1000bytes saved. Do you know where it has been saved?

BTW you convinced me, I have to update my compiler now. Please commit your 
changes, you are commiter.

Thanks again,
Bertrand.

Original comment by bson...@gmail.com on 2 Feb 2012 at 11:08

[GoogleCodeExporter]

Where it has been saved? In CPPFLAGS.
-combine: ... Currently the only language for which this is supported is C. ... 
so in Makefile you have -combine only in CFLAGS, not in CPPFLAGS.

When building, I put -flto to both CFLAGS and CPPFLAGS:
CPPFLAGS & CFLAGS: text size=57796
CFLAGS only: text size=58422

Btw, when I try to add -combine to CPPFLAGS too (using avr-gcc 4.3.3), the 
result is much smaller: 52k, but it's not supported and I did not test the 
result, so I don't know what output it produces.

Original comment by mih...@mihlit.cz on 2 Feb 2012 at 11:38

[GoogleCodeExporter]

Issue fixed in revision 186.

To whom it may concern:
To make the code compatible in the future, use following style when working 
with progmem variables:
- usual declaration:
const pm_char *str;
int foo(const pm_char *src,...
-> use pm_ instead of prog_ prefix, always use 'const' and never use PROGMEM

- variable definition with pointer initialization:
const pm_char *str = s; 
-> the same as above, do not use PROGMEM

- variable definition with data initialization:
const pm_char *str PROGMEM = "hello world"; 
-> use pm_ instead of prog_ prefix, always use 'const' and use PROGMEM here 
(only here)

And once again, what's behind this:
prog_char or pm_char... they are just usual variables, nothing special. They 
are just numbers for both MCU and compiler, it just depends how you use them.

PROGMEM just says where to put initialization data, it does not affect type nor 
variable anyway. That's the reason why it belongs (and in fact makes sense) to 
put it only where you initialize data.

Original comment by mih...@mihlit.cz on 2 Feb 2012 at 11:53

• Changed state: Fixed