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

[GoogleCodeExporter]

What Version of the firmware are you trying to compile?
trunk-r667

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

gruvin9x.h:275:14: 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 
( http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49764 )

Main issue was caused by the compiler 4.6.1, but version 4.6.2 should be 
working and there are still problems with the code. Together with new avr-libc, 
problem is even bigger.

Now you get:
lcd.h:73:63: error: 'prog_char' does not name a type
gruvin9x.h:275:14: error: 'prog_uint8_t' 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 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 gruvin9x.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.

Anyway, to compile this, I had to add const on several places. This is with gcc 
4.6.2, so with original issue fixed. It's not going to be better without 
changes in the code.

I had to make changes in attached diff to get gruvin9x compiled.

Original issue reported on code.google.com by mih...@mihlit.cz on 17 Jan 2012 at 11:00

Attachments:

• changes.patch

[GoogleCodeExporter]

Hmm. OK, thanks. I didn't even know there was an update avr-libc. Where does 
one get it and find out how to install it? (I'm on a Mac. I rely on CrossPack 
to install a compatible version of AVR-GCC.) And what if it breaks a million 
other things / projects by upgrading?

I certainly *can* see the sense of the changes they've made and the reasons to 
comply. But I'm nervous about upgrading, for fear of it creating a massive 
amount of work, for a lot of people. That said, it clearly needs to happen some 
time.

Most importantly then -- if we apply all the code changes required for v4.6.2 
compliance, will it still compile under older versions? (I'm using v4.3.3 -- 
from CrossPack.) If not, then making the changes would force all g9x compilers 
to be upgraded as the risk of breaking other projects on those people's 
machines.

So what to do? Hmmm.

Original comment by gru...@gmail.com on 18 Jan 2012 at 3:47

• Changed state: Accepted

[GoogleCodeExporter]

I've just discovered that the latest version of CrossPack-AVR still only 
contains GCC v4.3.3. On that basis, I do not readily see how I (or anyone else 
running on a Mac) can upgrade. Therefore, it looks as though we might have to 
declare the gruvin9x project simply not compatible with AVR-GCC v4.6.x.

Comments, suggestions welcomed.

Original comment by gru...@gmail.com on 18 Jan 2012 at 3:58

[GoogleCodeExporter]

For reference, I'm  currently working with the following tool-chain component 
versions:

avr-libc-1.6.7cvs: 1.6.7
binutils: 2.19
gcc-4: 4.3.3
make: 3.81

... as recorded in CrossPack-AVR/versions.txt

Original comment by gru...@gmail.com on 18 Jan 2012 at 4:03

[GoogleCodeExporter]

I don't know where you can get avr-libc update for Mac. Source code can be 
downloaded from project pages: http://www.nongnu.org/avr-libc/ 1.6.7 is 2.5 
years old. Based on gcc home page gcc 4.4.6 is the oldest maintained version.

Looking at individual changes:
- adding const to prog_* types. In old (and new) version 'const' is turned on 
by __const__ attribute. You can combine it with 'const' keyword without any 
problem and it'll work in both old and new build environment.

- the same is valid to adding const to PROGMEM variables (it needs to be added 
to variable declaration, not typedef. if you use typedef char PROGMEM *pchar; 
pchar foo; you need to change the pchar foo to const pchar foo. Adding const to 
typedef won't have any effect in new version).

- the same is valid for your PSTR definition change

- for #define __PROG_TYPES_COMPAT__ - it won't have any effect in old avr-libc, 
because it does not know it.

- for the long term solution (drop prog_* types completely, use ordinary types 
and add PROGMEM keyword to variable declarations everywhere). it's already 
supported by old avr-libc. The resulting type will be the same, so it will work 
too. avr-libc does not force anyone to use prog_* types.

- -combine -> -flto change is the only problem, because old gcc version does 
not know "-flto" option. This should go to autotools configure script or cmake, 
but gruvin9x uses only static Makefile.

So, it should be safe to do all changes except the -flto one.

Anyway, you can try to apply the patch I needed for building, just omit the 
Makefile section with -flto, and you'll see if it works in old build 
environment or not :)

Original comment by mih...@mihlit.cz on 18 Jan 2012 at 10:14

[GoogleCodeExporter]

Wow

Wow again. This really surprises me. Seems CrossPack is not being
maintained as I thought it was.

OK. Thanks.

Good to know.

I can't see automake being of much use for a cross-compiler, not to
mention the extra work it would take for just one small issue. Right
now, we're not using -flto link optimisation and I'd question if it
would help, given all the manual optimisation work that's been done.
So I guess we can pretty safely just leave it out.

(CMAKE would certainly be of no practical use what-so-ever. So not going there.)

True enough. I'll try to find time. If it does work -- and it looks as
though it should, thanks -- then we best set about making it a
permanent change.

Thanks again.

Original comment by gru...@gmail.com on 19 Jan 2012 at 12:15

[GoogleCodeExporter]

> we're not using -flto link optimisation and I'd question if it
> would help, given all the manual optimisation work that's been done.

without -flto:
   text    data     bss     dec     hex filename
  91668     214    3925   95807   1763f gruvin9x.elf

394439  gruvin9x.elf
258466  gruvin9x.hex

with -flto
   text    data     bss     dec     hex filename
  79104     104    3924   83132   144bc gruvin9x.elf

355932  gruvin9x.elf
222809  gruvin9x.hex

So with -flto it's about 13 % smaller

NOTE: it's with default settings (Makefile). It's without -combine because this 
option was dropped by gcc upstream.

Original comment by mih...@mihlit.cz on 20 Jan 2012 at 10:51

[GoogleCodeExporter]

I've since learned that current/latest available version of both CrossPack for 
Mac OS X and WinAVR share the older GCC v4.3.3 versions. I guess this still 
need to be addressed at some point. But I'm making it low priority, for now.

Original comment by gru...@gmail.com on 21 Jan 2012 at 2:23

• Added labels: Priority-Low
• Removed labels: Priority-Medium

[GoogleCodeExporter]

re the, 'with versus without -ftlo' comment #6, above, which I missed earlier 
...

Wow. OK. That's actually quite impressive. Apparently, we haven't been as 
clever with manually specified static functions as I thought.

This new knowledge does tend to swing things the other way for me (in favour up 
upgrading to the later compiler sooner than later) with the caveat that we'd 
need to check interrupt latency timings and so forth, to ensure that automatic 
optimisation performed by -ftlo hasn't done damage -- perhaps by forcing 
'calls', where there was once inline code. (And all that assuming that I've 
even understood the role and function -ftlo correctly.) 

Thanks again. :-D

Original comment by gru...@gmail.com on 21 Jan 2012 at 9:02

[GoogleCodeExporter]

I was able to get old avr-gcc 4.3.3 and avr-libc 1.6.7. I searched for solution 
that would work with any avr-libc & avr-gcc combination (starting avr-gcc 4.3.3 
& avr-libc 1.6.7). 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 (any version) or avr-g++ 4.6.2 and newer): 

============ 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 any combination of avr-libc 1.6.7, 1.7.1, 1.8.0 & avr-gcc 
4.3.3, 4.6.2, 4.7.0 without single warning.

I've added also automatic check for -flto/--combine based on avr-gcc version.

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

Patch changes:
- prog_* changed to pm_* because new avr-libc does not provide such types
- const added everywhere
- PROGMEM added only where initialization data are specified, removed from 
other places
- splash start/end markers are now part of the splash data block (splash points 
to the same data + start offset), because some gcc versions can optimize 
position of variables, this alse required lbm format changes
- *.lbm now contains only raw data, variable definition is in original file, 
only initialization data are included
- avr-gcc check added to Makefile for -combine/-flto
- fixed -DLOGS -> -DTELEMLOGS in Makefile (forgotten in commit r1471

Original comment by mih...@mihlit.cz on 10 Feb 2012 at 9:40

Attachments:

• pgmfix.patch

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

Patch applied and tested 2012-02-11 05:07 UTC by Gruvin

Committed as r1474

Nice to have this one sorted out for us! Thank you again!.

Original comment by gru...@gmail.com on 11 Feb 2012 at 4:11