avr-gcc-list
[Top][All Lists]
Advanced

[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);
+        }
     }
 }
 

reply via email to

[Prev in Thread] Current Thread [Next in Thread]