[C patch] PR49551

[C patch] PR49551

[Prathamesh Kulkarni]

[-- Attachment #1: Type: text/plain, Size: 2409 bytes --]

Hi,
The attached patch tries to fix PR49551.

For following test-case:
int x = 1;
int x;

Passing -O -fdata-sections causes ICE for target arm-linux-gnueabihf.
The bug is latent on trunk, after r221297. Reverting r221297 reproduces ICE.

Before r221297:
ICE was caused in get_variable_section because it hit the following
assert in varasm.c:get_variable_section():

gcc_assert (DECL_SECTION_NAME (decl) == NULL
            && ADDR_SPACE_GENERIC_P (as));

test.c:2:1: internal compiler error: in get_variable_section, at varasm.c:1158
 int x;
 ^
0xd4133b get_variable_section(tree_node*, bool)
/home/prathamesh.kulkarni/gnu-toolchain/src/gcc.git~bug-49551-reverted/gcc/varasm.c:1157
0xd4211e get_block_for_decl
/home/prathamesh.kulkarni/gnu-toolchain/src/gcc.git~bug-49551-reverted/gcc/varasm.c:1223
0xd42c61 make_decl_rtl(tree_node*)
/home/prathamesh.kulkarni/gnu-toolchain/src/gcc.git~bug-49551-reverted/gcc/varasm.c:1353
0x53785f merge_decls
/home/prathamesh.kulkarni/gnu-toolchain/src/gcc.git~bug-49551-reverted/gcc/c/c-decl.c:2709

This happens because merge_decls() sets DECL_COMMON (olddecl) to 1 here:
 memcpy ((char *) olddecl + sizeof (struct tree_decl_common),
                  (char *) newdecl + sizeof (struct tree_decl_common),
                  tree_code_size (TREE_CODE (olddecl)) - sizeof
(struct tree_decl_common));

In merge_decls, the following condition becomes true and
make_decl_rtl() is called.

if (DECL_RTL_SET_P (olddecl)
    && (TREE_CODE (olddecl) == FUNCTION_DECL
        || (TREE_CODE (olddecl) == VAR_DECL
            && TREE_STATIC (olddecl))))
  make_decl_rtl (olddecl);

DECL_RTL_SET_P (olddecl) is set to true because notice_global_symbol()
called make_decl_rtl() for olddecl via DECL_RTL(). make_decl_rtl()
eventually calls get_variable_section() and hits the assert.

After r221297:
r221297 removes call to DECL_RTL() in notice_global_symbol().
So in merge_decls(), the above if condition becomes false and
make_decl_rtl() doesn't get called thereby not producing the ICE.

However DECL_COMMON (olddecl) is still incorrectly set to 1 after the
call to memcpy(), while DECL_COMMON (olddecl) should be 0 for the
above test-case. This patch tries to correctly adjust DECL_COMMON
(newdecl) before it is copied into olddecl. Does it look reasonable ?

Bootstrapped and tested on x86_64-unknown-linux-gnu, cross tested on
arm-linux-gnueabihf using qemu.

Thank you,
Prathamesh

[-- Attachment #2: 49551.diff --]
[-- Type: text/plain, Size: 1074 bytes --]

diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c
index 4f6761d..84e56d7 100644
--- a/gcc/c/c-decl.c
+++ b/gcc/c/c-decl.c
@@ -2631,6 +2631,12 @@ merge_decls (tree newdecl, tree olddecl, tree newtype, tree oldtype)
   else if (DECL_PRESERVE_P (newdecl))
     DECL_PRESERVE_P (olddecl) = 1;
 
+  /* Merge DECL_COMMON */
+  if (VAR_P (olddecl) && VAR_P (newdecl)
+      && !lookup_attribute ("common", DECL_ATTRIBUTES (newdecl))
+      && !lookup_attribute ("nocommon", DECL_ATTRIBUTES (newdecl)))
+    DECL_COMMON (newdecl) = DECL_COMMON (newdecl) && DECL_COMMON (olddecl);
+
   /* Copy most of the decl-specific fields of NEWDECL into OLDDECL.
      But preserve OLDDECL's DECL_UID, DECL_CONTEXT and
      DECL_ARGUMENTS (if appropriate).  */
diff --git a/gcc/testsuite/gcc.dg/pr49551.c b/gcc/testsuite/gcc.dg/pr49551.c
new file mode 100644
index 0000000..204f06d
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr49551.c
@@ -0,0 +1,7 @@
+/* { dg-do compile } */
+/* { dg-options "-O -fdata-sections" } */
+
+int x = 1;
+int x;
+
+/* { dg-final { scan-assembler-not {comm[\t ]+x} } } */

[-- Attachment #3: ChangeLog.txt --]
[-- Type: text/plain, Size: 162 bytes --]

2015-05-31  Prathamesh Kulkarni  <prathamesh.kulkarni@linaro.org>

	PR c/49551
	* c-decl.c (merge_decls): Merge DECL_COMMON. 
	* gcc.dg/pr49551.c: New test-case.

Re: [C patch] PR49551

[Joseph Myers]

On Sun, 31 May 2015, Prathamesh Kulkarni wrote:

> However DECL_COMMON (olddecl) is still incorrectly set to 1 after the
> call to memcpy(), while DECL_COMMON (olddecl) should be 0 for the
> above test-case. This patch tries to correctly adjust DECL_COMMON
> (newdecl) before it is copied into olddecl. Does it look reasonable ?
> 
> Bootstrapped and tested on x86_64-unknown-linux-gnu, cross tested on
> arm-linux-gnueabihf using qemu.

This patch is OK.

-- 
Joseph S. Myers
joseph@codesourcery.com