[avr] Fix PR 50739 - nameless error with -fmerge-all-constants

Submitted by Senthil Kumar Selvaraj on July 5, 2016, 4:50 a.m.

Details

Message ID 87furoy9gz.fsf@atmel.com
State New
Headers show

Commit Message

Senthil Kumar Selvaraj July 5, 2016, 4:50 a.m.
Hi,

  This patch fixes a problem with fmerge-all-constants and the progmem
  attribute - on trunk, the below testcase errors out with a section
  conflict error.

  When avr_asm_select_section renames .rodata.xyz section to
  .progmem.xyz and calls get_section, it passes in the same flags in
  sect. If the flags include SECTION_DECLARED, get_section barfs with a
  section conflict error - the section flag comparison logic strips off
  SECTION_DECLARED from existing section flags before comparing it with
  the new incoming flags.

  With -fmerge-all-constants, default_elf_select_section always returns
  .rodata.strx.x. varasm switches to that section when writing out the
  non progmem string literal, and that sets SECTION_DECLARED. The first
  call to get_section with the section name transformed to
  .progmem.data.strx.x then includes SECTION_DECLARED, but because this
  is a new section, the section flag conflict logic doesn't kick in. The
  second call to get_section, again including SECTION_DECLARED, triggers
  the section flag conflict logic and causes the error.

  Stripping off SECTION_DECLARED before calling get_section fixes the
  problem - the flag is supposed to be set by switch_section anyway.

  Reg testing showed no new regressions. Ok for trunk and backport to 6.x?

Regards
Senthil


gcc/testsuite/ChangeLog:

2016-07-05  Senthil Kumar Selvaraj  <senthil_kumar.selvaraj@atmel.com>

	PR target/50739	
	* gcc.target/avr/pr50739.c: New test.


gcc/ChangeLog:

2016-07-05  Senthil Kumar Selvaraj  <senthil_kumar.selvaraj@atmel.com>

	PR target/50739	
	* config/avr/avr.c (avr_asm_select_section):

Comments

Senthil Kumar Selvaraj July 5, 2016, 6:55 a.m.
Senthil Kumar Selvaraj writes:

> Hi,
>
>   This patch fixes a problem with fmerge-all-constants and the progmem
>   attribute - on trunk, the below testcase errors out with a section
>   conflict error.
>
>   When avr_asm_select_section renames .rodata.xyz section to
>   .progmem.xyz and calls get_section, it passes in the same flags in
>   sect. If the flags include SECTION_DECLARED, get_section barfs with a
>   section conflict error - the section flag comparison logic strips off
>   SECTION_DECLARED from existing section flags before comparing it with
>   the new incoming flags.
>
>   With -fmerge-all-constants, default_elf_select_section always returns
>   .rodata.strx.x. varasm switches to that section when writing out the
>   non progmem string literal, and that sets SECTION_DECLARED. The first
>   call to get_section with the section name transformed to
>   .progmem.data.strx.x then includes SECTION_DECLARED, but because this
>   is a new section, the section flag conflict logic doesn't kick in. The
>   second call to get_section, again including SECTION_DECLARED, triggers
>   the section flag conflict logic and causes the error.
>
>   Stripping off SECTION_DECLARED before calling get_section fixes the
>   problem - the flag is supposed to be set by switch_section anyway.
>
>   Reg testing showed no new regressions. Ok for trunk and backport to 6.x?
>
> Regards
> Senthil
>

Added missing description in Changelog entry.

gcc/testsuite/ChangeLog:

2016-07-05  Senthil Kumar Selvaraj  <senthil_kumar.selvaraj@atmel.com>

	PR target/50739	
	* gcc.target/avr/pr50739.c: New test.


gcc/ChangeLog:

2016-07-05  Senthil Kumar Selvaraj  <senthil_kumar.selvaraj@atmel.com>

	PR target/50739	
	* config/avr/avr.c (avr_asm_select_section): Strip off
	SECTION_DECLARED from flags when calling get_section.

Regards
Senthil

>
> gcc/testsuite/ChangeLog:
>
> 2016-07-05  Senthil Kumar Selvaraj  <senthil_kumar.selvaraj@atmel.com>
>
> 	PR target/50739	
> 	* gcc.target/avr/pr50739.c: New test.
>
>
> gcc/ChangeLog:
>
> 2016-07-05  Senthil Kumar Selvaraj  <senthil_kumar.selvaraj@atmel.com>
>
> 	PR target/50739	
> 	* config/avr/avr.c (avr_asm_select_section):
>
>
> diff --git gcc/config/avr/avr.c gcc/config/avr/avr.c
> index 18ed766..9b7b392 100644
> --- gcc/config/avr/avr.c
> +++ gcc/config/avr/avr.c
> @@ -9641,7 +9641,7 @@ avr_asm_select_section (tree decl, int reloc, unsigned HOST_WIDE_INT align)
>              {
>                const char *sname = ACONCAT ((new_prefix,
>                                              name + strlen (old_prefix), NULL));
> -              return get_section (sname, sect->common.flags, sect->named.decl);
> +              return get_section (sname, sect->common.flags & ~SECTION_DECLARED, sect->named.decl);
>              }
>          }
>  
> diff --git gcc/testsuite/gcc.target/avr/pr50739.c gcc/testsuite/gcc.target/avr/pr50739.c
> new file mode 100644
> index 0000000..a6850b7
> --- /dev/null
> +++ gcc/testsuite/gcc.target/avr/pr50739.c
> @@ -0,0 +1,7 @@
> +/* { dg-do compile } */
> +/* { dg-options "-fmerge-all-constants" } */
> +
> +char *ca = "123";
> +
> +const char a[] __attribute__((__progmem__))= "a";
> +const char b[] __attribute__((__progmem__))= "b";
Georg-Johann Lay July 5, 2016, 4:30 p.m.
Senthil Kumar Selvaraj schrieb:
> Senthil Kumar Selvaraj writes:
> 
>> Hi,
>>
>>   This patch fixes a problem with fmerge-all-constants and the progmem
>>   attribute - on trunk, the below testcase errors out with a section
>>   conflict error.
>>
>>   When avr_asm_select_section renames .rodata.xyz section to
>>   .progmem.xyz and calls get_section, it passes in the same flags in
>>   sect. If the flags include SECTION_DECLARED, get_section barfs with a
>>   section conflict error - the section flag comparison logic strips off
>>   SECTION_DECLARED from existing section flags before comparing it with
>>   the new incoming flags.
>>
>>   With -fmerge-all-constants, default_elf_select_section always returns
>>   .rodata.strx.x. varasm switches to that section when writing out the
>>   non progmem string literal, and that sets SECTION_DECLARED. The first
>>   call to get_section with the section name transformed to
>>   .progmem.data.strx.x then includes SECTION_DECLARED, but because this
>>   is a new section, the section flag conflict logic doesn't kick in. The
>>   second call to get_section, again including SECTION_DECLARED, triggers
>>   the section flag conflict logic and causes the error.
>>
>>   Stripping off SECTION_DECLARED before calling get_section fixes the
>>   problem - the flag is supposed to be set by switch_section anyway.
>>
>>   Reg testing showed no new regressions. Ok for trunk and backport to 6.x?
>>
>> Regards
>> Senthil
>>
> 
> Added missing description in Changelog entry.
> 
> gcc/testsuite/ChangeLog:
> 
> 2016-07-05  Senthil Kumar Selvaraj  <senthil_kumar.selvaraj@atmel.com>
> 
> 	PR target/50739	
> 	* gcc.target/avr/pr50739.c: New test.
> 
> 
> gcc/ChangeLog:
> 
> 2016-07-05  Senthil Kumar Selvaraj  <senthil_kumar.selvaraj@atmel.com>
> 
> 	PR target/50739	
> 	* config/avr/avr.c (avr_asm_select_section): Strip off
> 	SECTION_DECLARED from flags when calling get_section.
> 
> Regards
> Senthil
> 
>> gcc/testsuite/ChangeLog:
>>
>> 2016-07-05  Senthil Kumar Selvaraj  <senthil_kumar.selvaraj@atmel.com>
>>
>> 	PR target/50739	
>> 	* gcc.target/avr/pr50739.c: New test.
>>
>>
>> gcc/ChangeLog:
>>
>> 2016-07-05  Senthil Kumar Selvaraj  <senthil_kumar.selvaraj@atmel.com>
>>
>> 	PR target/50739	
>> 	* config/avr/avr.c (avr_asm_select_section):
>>
>>
>> diff --git gcc/config/avr/avr.c gcc/config/avr/avr.c
>> index 18ed766..9b7b392 100644
>> --- gcc/config/avr/avr.c
>> +++ gcc/config/avr/avr.c
>> @@ -9641,7 +9641,7 @@ avr_asm_select_section (tree decl, int reloc, unsigned HOST_WIDE_INT align)
>>              {
>>                const char *sname = ACONCAT ((new_prefix,
>>                                              name + strlen (old_prefix), NULL));
>> -              return get_section (sname, sect->common.flags, sect->named.decl);
>> +              return get_section (sname, sect->common.flags & ~SECTION_DECLARED, sect->named.decl);

Long line, should be wrapped.

Johann

>>              }
>>          }
>>  
>> diff --git gcc/testsuite/gcc.target/avr/pr50739.c gcc/testsuite/gcc.target/avr/pr50739.c
>> new file mode 100644
>> index 0000000..a6850b7
>> --- /dev/null
>> +++ gcc/testsuite/gcc.target/avr/pr50739.c
>> @@ -0,0 +1,7 @@
>> +/* { dg-do compile } */
>> +/* { dg-options "-fmerge-all-constants" } */
>> +
>> +char *ca = "123";
>> +
>> +const char a[] __attribute__((__progmem__))= "a";
>> +const char b[] __attribute__((__progmem__))= "b";
> 
>

Patch hide | download patch | download mbox

diff --git gcc/config/avr/avr.c gcc/config/avr/avr.c
index 18ed766..9b7b392 100644
--- gcc/config/avr/avr.c
+++ gcc/config/avr/avr.c
@@ -9641,7 +9641,7 @@  avr_asm_select_section (tree decl, int reloc, unsigned HOST_WIDE_INT align)
             {
               const char *sname = ACONCAT ((new_prefix,
                                             name + strlen (old_prefix), NULL));
-              return get_section (sname, sect->common.flags, sect->named.decl);
+              return get_section (sname, sect->common.flags & ~SECTION_DECLARED, sect->named.decl);
             }
         }
 
diff --git gcc/testsuite/gcc.target/avr/pr50739.c gcc/testsuite/gcc.target/avr/pr50739.c
new file mode 100644
index 0000000..a6850b7
--- /dev/null
+++ gcc/testsuite/gcc.target/avr/pr50739.c
@@ -0,0 +1,7 @@ 
+/* { dg-do compile } */
+/* { dg-options "-fmerge-all-constants" } */
+
+char *ca = "123";
+
+const char a[] __attribute__((__progmem__))= "a";
+const char b[] __attribute__((__progmem__))= "b";