cp/decl.c: Set DECL_INITIAL before attribute processing

Attribute handlers may want to examine DECL_INITIAL for a decl, to
validate the attribute being applied. For C++, DECL_INITIAL is currently
not set until cp_finish_decl, by which time attribute validation has
already been performed.

For msp430-elf this causes the "persistent" attribute to always be
rejected for C++, since DECL_INITIAL must be non-null for the
attribute to be applied to a decl.

This patch ensures DECL_INITIAL is set for initialized decls early in
start_decl, before attribute handlers run. This allows the
initialization status of the decl to be examined by the handlers.
DECL_INITIAL must be restored to it's initial value after attribute
validation is performed, so as to not interfere with later decl
processing.

Successfully bootstrapped and regtested for x86_64-pc-linux-gnu, and
regtested for arm-eabi and msp430-elf.

Ok for trunk?
From 6fbb18ab081069fb2730360f9e09425b9b1f6a7d Mon Sep 17 00:00:00 2001
From: Jozef Lawrynowicz <jozef.l@mittosystems.com>
Date: Tue, 20 Oct 2020 14:03:42 +0100
Subject: [PATCH] cp/decl.c: Set DECL_INITIAL before attribute processing

Attribute handlers may want to examine DECL_INITIAL for a decl, to
validate the attribute being applied. For C++, DECL_INITIAL is currently
not set until cp_finish_decl, by which time attribute validation has
already been performed.

For msp430-elf this causes the "persistent" attribute to always be
rejected for C++, since DECL_INITIAL must be non-null for the
attribute to be applied to a decl.

This patch ensures DECL_INITIAL is set for initialized decls early in
start_decl, before attribute handlers run. This allows the
initialization status of the decl to be examined by the handlers.
DECL_INITIAL must be restored to it's initial value after attribute
validation is performed, so as to not interfere with later decl
processing.

gcc/cp/ChangeLog:

        * decl.c (start_decl): Set DECL_INITIAL for initialized decls
        before attribute processing.

gcc/testsuite/ChangeLog:

	* gcc.target/msp430/data-attributes-2.c: Adjust test.
	* g++.target/msp430/data-attributes.C: New test.
	* g++.target/msp430/msp430.exp: New test.
---
 gcc/cp/decl.c                                 | 13 +++++
 .../g++.target/msp430/data-attributes.C       | 52 +++++++++++++++++++
 gcc/testsuite/g++.target/msp430/msp430.exp    | 44 ++++++++++++++++
 .../gcc.target/msp430/data-attributes-2.c     |  1 +
 4 files changed, 110 insertions(+)
 create mode 100644 gcc/testsuite/g++.target/msp430/data-attributes.C
 create mode 100644 gcc/testsuite/g++.target/msp430/msp430.exp

On Mon, Oct 26, 2020 at 01:30:29PM +0000, Jozef Lawrynowicz wrote:
> Attribute handlers may want to examine DECL_INITIAL for a decl, to
> validate the attribute being applied. For C++, DECL_INITIAL is currently
> not set until cp_finish_decl, by which time attribute validation has
> already been performed.
> 
> For msp430-elf this causes the "persistent" attribute to always be
> rejected for C++, since DECL_INITIAL must be non-null for the
> attribute to be applied to a decl.
> 
> This patch ensures DECL_INITIAL is set for initialized decls early in
> start_decl, before attribute handlers run. This allows the
> initialization status of the decl to be examined by the handlers.
> DECL_INITIAL must be restored to it's initial value after attribute
> validation is performed, so as to not interfere with later decl
> processing.
> 
> Successfully bootstrapped and regtested for x86_64-pc-linux-gnu, and
> regtested for arm-eabi and msp430-elf.
> 
> Ok for trunk?

Amended slightly misleading comment, shown below, in the attached patch.

diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index 0f32dd88bad..4c959377077 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -5235,8 +5235,8 @@ start_decl (const cp_declarator *declarator,
       return error_mark_node;
     }

-  /* Save the DECL_INITIAL value since we clobber it with error_mark_node if
-     INITIALIZED is true.  */
+  /* Save the DECL_INITIAL value in case it gets clobbered to assist
+     with attribute validation.  */
   initial = DECL_INITIAL (decl);

   if (initialized)
From fad6cc4df13e00c55d381e82772438161282a008 Mon Sep 17 00:00:00 2001
From: Jozef Lawrynowicz <jozef.l@mittosystems.com>
Date: Tue, 20 Oct 2020 14:03:42 +0100
Subject: [PATCH] cp/decl.c: Set DECL_INITIAL before attribute processing

Attribute handlers may want to examine DECL_INITIAL for a decl, to
validate the attribute being applied. For C++, DECL_INITIAL is currently
not set until cp_finish_decl, by which time attribute validation has
already been performed.

For msp430-elf this causes the "persistent" attribute to always be
rejected for C++, since DECL_INITIAL must be non-null for the
attribute to be applied to a decl.

This patch ensures DECL_INITIAL is set for initialized decls early in
start_decl, before attribute handlers run. This allows the
initialization status of the decl to be examined by the handlers.
DECL_INITIAL must be restored to it's initial value after attribute
validation is performed, so as to not interfere with later decl
processing.

gcc/cp/ChangeLog:

        * decl.c (start_decl): Set DECL_INITIAL for initialized decls
        before attribute processing.

gcc/testsuite/ChangeLog:

	* gcc.target/msp430/data-attributes-2.c: Adjust test.
	* g++.target/msp430/data-attributes.C: New test.
	* g++.target/msp430/msp430.exp: New test.
---
 gcc/cp/decl.c                                 | 13 +++++
 .../g++.target/msp430/data-attributes.C       | 52 +++++++++++++++++++
 gcc/testsuite/g++.target/msp430/msp430.exp    | 44 ++++++++++++++++
 .../gcc.target/msp430/data-attributes-2.c     |  1 +
 4 files changed, 110 insertions(+)
 create mode 100644 gcc/testsuite/g++.target/msp430/data-attributes.C
 create mode 100644 gcc/testsuite/g++.target/msp430/msp430.exp

diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index 5f370e60b4e..4c959377077 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -5210,6 +5210,7 @@ start_decl (const cp_declarator *declarator,
   bool was_public;
   int flags;
   bool alias;
+  tree initial;
 
   *pushed_scope_p = NULL_TREE;
 
@@ -5234,6 +5235,10 @@ start_decl (const cp_declarator *declarator,
       return error_mark_node;
     }
 
+  /* Save the DECL_INITIAL value in case it gets clobbered to assist
+     with attribute validation.  */
+  initial = DECL_INITIAL (decl);
+
   if (initialized)
     {
       if (! toplevel_bindings_p ()
@@ -5243,6 +5248,10 @@ start_decl (const cp_declarator *declarator,
       DECL_EXTERNAL (decl) = 0;
       if (toplevel_bindings_p ())
 	TREE_STATIC (decl) = 1;
+      /* Tell 'cplus_decl_attributes' this is an initialized decl,
+	 even though we might not yet have the initializer expression.  */
+      if (!DECL_INITIAL (decl))
+	DECL_INITIAL (decl) = error_mark_node;
     }
   alias = lookup_attribute ("alias", DECL_ATTRIBUTES (decl)) != 0;
   
@@ -5261,6 +5270,10 @@ start_decl (const cp_declarator *declarator,
   /* Set attributes here so if duplicate decl, will have proper attributes.  */
   cplus_decl_attributes (&decl, attributes, flags);
 
+  /* Restore the original DECL_INITIAL that we may have clobbered earlier to
+     assist with attribute validation.  */
+  DECL_INITIAL (decl) = initial;
+
   /* Dllimported symbols cannot be defined.  Static data members (which
      can be initialized in-class and dllimported) go through grokfield,
      not here, so we don't need to exclude those decls when checking for
diff --git a/gcc/testsuite/g++.target/msp430/data-attributes.C b/gcc/testsuite/g++.target/msp430/data-attributes.C
new file mode 100644
index 00000000000..4e2139e93f7
--- /dev/null
+++ b/gcc/testsuite/g++.target/msp430/data-attributes.C
@@ -0,0 +1,52 @@
+/* { dg-do compile } */
+/* { dg-skip-if "" { *-*-* } { "-mcpu=msp430" } } */
+/* { dg-options "-mlarge" } */
+
+/* The msp430-specific variable attributes "lower", "upper", either", "noinit"
+   and "persistent", all conflict with one another.
+   These attributes also conflict with the "section" attribute, since they
+   specify sections to put the variables into.  */
+int __attribute__((persistent)) p = 10;
+int __attribute__((persistent,lower)) pl = 20; /* { dg-warning "ignoring attribute 'lower' because it conflicts with attribute 'persistent'" } */
+int __attribute__((persistent,upper)) pu = 20; /* { dg-warning "ignoring attribute 'upper' because it conflicts with attribute 'persistent'" } */
+int __attribute__((persistent,either)) pe = 20; /* { dg-warning "ignoring attribute 'either' because it conflicts with attribute 'persistent'" } */
+/* This one results in an error because the handler for persistent sets the
+   section to .persistent there and then.  */
+int __attribute__((persistent,section(".data.foo"))) ps = 20; /* { dg-error "section of 'ps' conflicts with previous declaration" } */
+int __attribute__((persistent,noinit)) pn = 2; /* { dg-warning "'noinit' attribute cannot be applied to variables with specific sections" } */
+int __attribute__((persistent)) zz; /* { dg-warning "variables marked with 'persistent' attribute must be initialized" } */
+
+int __attribute__((noinit)) n;
+int __attribute__((noinit,lower)) nl; /* { dg-warning "ignoring attribute 'lower' because it conflicts with attribute 'noinit'" } */
+int __attribute__((noinit,upper)) nu; /* { dg-warning "ignoring attribute 'upper' because it conflicts with attribute 'noinit'" } */
+int __attribute__((noinit,either)) ne; /* { dg-warning "ignoring attribute 'either' because it conflicts with attribute 'noinit'" } */
+int __attribute__((noinit,persistent)) np; /* { dg-warning "ignoring attribute 'persistent' because it conflicts with attribute 'noinit'" } */
+int __attribute__((noinit,section(".data.foo"))) ns; /* { dg-warning "ignoring attribute 'section' because it conflicts with attribute 'noinit'" } */
+
+int __attribute__((lower)) l = 20;
+int __attribute__((lower,upper)) lu = 20; /* { dg-warning "ignoring attribute 'upper' because it conflicts with attribute 'lower'" } */
+int __attribute__((lower,either)) le = 20; /* { dg-warning "ignoring attribute 'either' because it conflicts with attribute 'lower'" } */
+int __attribute__((lower,persistent)) lp = 20; /* { dg-warning "ignoring attribute 'persistent' because it conflicts with attribute 'lower'" } */
+int __attribute__((lower,noinit)) ln; /* { dg-warning "ignoring attribute 'noinit' because it conflicts with attribute 'lower'" } */
+int __attribute__((lower,section(".data.foo"))) ls = 30;
+
+int __attribute__((upper)) u = 20;
+int __attribute__((upper,lower)) ul = 20; /* { dg-warning "ignoring attribute 'lower' because it conflicts with attribute 'upper'" } */
+int __attribute__((upper,either)) ue = 20; /* { dg-warning "ignoring attribute 'either' because it conflicts with attribute 'upper'" } */
+int __attribute__((upper,persistent)) up = 20; /* { dg-warning "ignoring attribute 'persistent' because it conflicts with attribute 'upper'" } */
+int __attribute__((upper,noinit)) un; /* { dg-warning "ignoring attribute 'noinit' because it conflicts with attribute 'upper'" } */
+int __attribute__((upper,section(".data.foo"))) us = 30; /* { dg-warning "ignoring attribute 'section' because it conflicts with attribute 'upper'" } */
+
+int __attribute__((either)) e = 20;
+int __attribute__((either,lower)) el = 20; /* { dg-warning "ignoring attribute 'lower' because it conflicts with attribute 'either'" } */
+int __attribute__((either,upper)) ee = 20; /* { dg-warning "ignoring attribute 'upper' because it conflicts with attribute 'either'" } */
+int __attribute__((either,persistent)) ep = 20; /* { dg-warning "ignoring attribute 'persistent' because it conflicts with attribute 'either'" } */
+int __attribute__((either,noinit)) en; /* { dg-warning "ignoring attribute 'noinit' because it conflicts with attribute 'either'" } */
+int __attribute__((either,section(".data.foo"))) es = 30; /* { dg-warning "ignoring attribute 'section' because it conflicts with attribute 'either'" } */
+
+int __attribute__((section(".data.foo"))) s = 20;
+int __attribute__((section(".data.foo"),noinit)) sn; /* { dg-warning "ignoring attribute 'noinit' because it conflicts with attribute 'section'" } */
+int __attribute__((section(".data.foo"),persistent)) sp = 20; /* { dg-warning "ignoring attribute 'persistent' because it conflicts with attribute 'section'" } */
+int __attribute__((section(".data.foo"),lower)) sl = 2;
+int __attribute__((section(".data.foo"),upper)) su = 20; /* { dg-warning "ignoring attribute 'upper' because it conflicts with attribute 'section'" } */
+int __attribute__((section(".data.foo"),either)) se = 2; /* { dg-warning "ignoring attribute 'either' because it conflicts with attribute 'section'" } */
diff --git a/gcc/testsuite/g++.target/msp430/msp430.exp b/gcc/testsuite/g++.target/msp430/msp430.exp
new file mode 100644
index 00000000000..3574c0fe71f
--- /dev/null
+++ b/gcc/testsuite/g++.target/msp430/msp430.exp
@@ -0,0 +1,44 @@
+#  Specific regression driver for MSP430.
+#  Copyright (C) 2020 Free Software Foundation, Inc.
+#
+#  This file is part of GCC.
+#
+#  GCC is free software; you can redistribute it and/or modify it
+#  under the terms of the GNU General Public License as published by
+#  the Free Software Foundation; either version 3, or (at your option)
+#  any later version.
+#
+#  GCC is distributed in the hope that it will be useful, but
+#  WITHOUT ANY WARRANTY; without even the implied warranty of
+#  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+#  General Public License for more details.
+#
+#  You should have received a copy of the GNU General Public License
+#  along with GCC; see the file COPYING3.  If not see
+#  <http://www.gnu.org/licenses/>.  */
+
+# GCC testsuite that uses the `dg.exp' driver.
+
+# Exit immediately if this isn't an MSP430 target.
+if {![istarget msp430*-*-*] } then {
+  return
+}
+
+# Load support procs.
+load_lib g++-dg.exp
+
+global DEFAULT_CXXFLAGS
+if ![info exists DEFAULT_CXXFLAGS] then {
+    set DEFAULT_CXXFLAGS " -pedantic-errors"
+}
+
+# Initialize `dg'.
+dg-init
+
+# Main loop.
+dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/*.C]] \
+        "" $DEFAULT_CXXFLAGS
+
+# All done.
+dg-finish
+
diff --git a/gcc/testsuite/gcc.target/msp430/data-attributes-2.c b/gcc/testsuite/gcc.target/msp430/data-attributes-2.c
index 6113e99f052..4e2139e93f7 100644
--- a/gcc/testsuite/gcc.target/msp430/data-attributes-2.c
+++ b/gcc/testsuite/gcc.target/msp430/data-attributes-2.c
@@ -14,6 +14,7 @@ int __attribute__((persistent,either)) pe = 20; /* { dg-warning "ignoring attrib
    section to .persistent there and then.  */
 int __attribute__((persistent,section(".data.foo"))) ps = 20; /* { dg-error "section of 'ps' conflicts with previous declaration" } */
 int __attribute__((persistent,noinit)) pn = 2; /* { dg-warning "'noinit' attribute cannot be applied to variables with specific sections" } */
+int __attribute__((persistent)) zz; /* { dg-warning "variables marked with 'persistent' attribute must be initialized" } */
 
 int __attribute__((noinit)) n;
 int __attribute__((noinit,lower)) nl; /* { dg-warning "ignoring attribute 'lower' because it conflicts with attribute 'noinit'" } */