[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[avr-gcc-list] Fixing PR44643 vs. PSTR macro
From: |
Georg-Johann Lay |
Subject: |
[avr-gcc-list] Fixing PR44643 vs. PSTR macro |
Date: |
Mon, 28 Mar 2011 13:59:49 +0200 |
User-agent: |
Thunderbird 2.0.0.24 (X11/20100302) |
In order to fix gcc's PR44643, see http://gcc.gnu.org/PR44643 , I
attached a proposed patch.
At current, the problem is that avr.c:avr_insert_attributes() which
implements targetm.insert_attributes sets TREE_READONLY to 1. This
causes an assertion failure in c-typeck.c.
The patch makes avr-gcc to error if __attribute__((progmem)) is
attached to a variable that is not qualified const.
This means that at least PSTR in avr-libc from include/avr/pgmspace.h
must be changed.
The current avr-libc implementation of PSTR, however, reads
#define PSTR(s) (__extension__({static char __c[] PROGMEM = (s);
&__c[0];}))
Which should read instead
#define PSTR(s) (__extension__({static char __c[] PROGMEM = (s);
(const char*) &__c[0];}))
#define PSTR(s) (__extension__({static char __c[] PROGMEM = (s);
&__c[0];}))
There is no clue what is better; depending on surrounding source and
warning options (like -Wcast-qual) there will be warnings like
"assignment discards 'const' qualifier from pointer target type" or
"cast discards '__attribute__((const))' qualifier from pointer target
type".
This means that the patch will most probably trigger bunch of
warnings/errors on much code in user land.
Note that the hook cannot change the tree node, i.e. the type cannot
be made 'const' by means of __attribute__((progmem)). (No one would
like the side effects caused by that, anyway.)
An other approach would be to ignore the progmem attribute for
non-const objects. But as such code will break at runtime anyway, that
is no good idea.
Moreover, I am not happy with the text of the error message.
Please comment on patch.
Thanks, Johann
Index: config/avr/avr.c
===================================================================
--- config/avr/avr.c (Revision 171599)
+++ config/avr/avr.c (Arbeitskopie)
@@ -5149,14 +5149,21 @@ avr_insert_attributes (tree node, tree *
&& (TREE_STATIC (node) || DECL_EXTERNAL (node))
&& avr_progmem_p (node, *attributes))
{
- static const char dsec[] = ".progmem.data";
- *attributes = tree_cons (get_identifier ("section"),
- build_tree_list (NULL, build_string (strlen (dsec), dsec)),
- *attributes);
+ if (TREE_READONLY (node))
+ {
+ static const char dsec[] = ".progmem.data";
- /* ??? This seems sketchy. Why can't the user declare the
- thing const in the first place? */
- TREE_READONLY (node) = 1;
+ *attributes = tree_cons (get_identifier ("section"),
+ build_tree_list (NULL, build_string (strlen
(dsec), dsec)),
+ *attributes);
+
+ }
+ else
+ {
+ error_at (DECL_SOURCE_LOCATION (node),
+ "variable %qE is not const but has
__attribute__((progmem))",
+ node);
+ }
}
}
- [avr-gcc-list] Fixing PR44643 vs. PSTR macro,
Georg-Johann Lay <=