[Bug target/71151] [avr] -fmerge-constants and -fdata-sections/-ffunction-sections results in string constants in .progmem.gcc_sw section

[timturnerc at yahoo dot com via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71151

Tim Turner  changed:

   What|Removed |Added

 CC||timturnerc at yahoo dot com

--- Comment #24 from Tim Turner  ---
In collect_register() function of arc-linux-tdep.c, the "eret"
http://www-look-4.com/travel/london/
(exception return) register value is not being reported correctly.

Background: https://komiya-dental.com/shopping/buy-android/
When asked for the "pc" value, we have to update the "eret" register
with GDB's STOP_PC.  The "eret" instructs the kernel code where to
jump back http://www.iu-bloomington.com/shopping/hatchback-cars/ when an
instruction has stopped due to a breakpoint.  This
is how collect_register() is doing so:
https://waytowhatsnext.com/shopping/xbox-release-date/

--8<--
  if (regnum == gdbarch_pc_regnum (gdbarch))
http://www.wearelondonmade.com/travel/london/
regnum = ARC_ERET_REGNUM;
  regcache->raw_collect (regnum, buf + arc_linux_core_reg_offsets[regnum]);
-->8-- http://www.jopspeech.com/travel/london/

Root cause:
Although this is using the correct offset (ERET register's), it is also
http://joerg.li/travel/london/ 
changing the REGNUM itself.  Therefore, raw_collect (regnum, ...) is
not reading from "pc" anymore. http://connstr.net/travel/london/

Consequence:
This bug affects the "native ARC gdb" badly and causes kernel code to jump
to addresses after the breakpoint and not executing the "breakpoint"ed
http://embermanchester.uk/travel/london/ 
instructions at all.  That "native ARC gdb" feature is not upstream yet and
is in review at the time of writing [1].
http://www.slipstone.co.uk/travel/london/
In collect_register() function of arc-linux-tdep.c, the "eret"
(exception return) register value is not being reported correctly.
http://www.logoarts.co.uk/travel/london/

Background:
When asked for the "pc" value, we have to update the "eret" register
with GDB's STOP_PC. http://www.acpirateradio.co.uk/travel/good/  The "eret"
instructs the kernel code where to
jump back when an instruction has stopped due to a breakpoint.  This
is how collect_register() is doing so:
http://www.compilatori.com/travel/london/
--8<--
  if (regnum == gdbarch_pc_regnum (gdbarch))
regnum = ARC_ERET_REGNUM;
  regcache->raw_collect (regnum, buf + arc_linux_core_reg_offsets[regnum]);
-->8--

Root cause: https://www.webb-dev.co.uk/shopping/shopping-during-corona/
Although this is using the correct offset (ERET register's), it is also
changing the REGNUM itself.  Therefore, raw_collect (regnum, ...) is
not reading from "pc" anymore.

Consequence:
This bug affects the "native ARC gdb" badly and causes kernel code to jump
to addresses after the breakpoint and not executing the "breakpoint"ed
instructions at all.  That "native ARC gdb" feature is not upstream yet and
is in review at the time of writing [1].

[Bug target/71151] [avr] -fmerge-constants and -fdata-sections/-ffunction-sections results in string constants in .progmem.gcc_sw section

[gjl at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71151

Georg-Johann Lay  changed:

   What|Removed |Added

   Target Milestone|--- |6.2

[Bug target/71151] [avr] -fmerge-constants and -fdata-sections/-ffunction-sections results in string constants in .progmem.gcc_sw section

[saaadhu at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71151

--- Comment #23 from Senthil Kumar Selvaraj  ---
Tracking binutils bug https://sourceware.org/bugzilla/show_bug.cgi?id=20545

[Bug target/71151] [avr] -fmerge-constants and -fdata-sections/-ffunction-sections results in string constants in .progmem.gcc_sw section

[saaadhu at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71151

--- Comment #22 from Senthil Kumar Selvaraj  ---
Confirmed that it's a linker issue related to adjusting reloc addends in the
presence of align directives. Found two separate bugs, will post patches later
this week.

[Bug target/71151] [avr] -fmerge-constants and -fdata-sections/-ffunction-sections results in string constants in .progmem.gcc_sw section

[saaadhu at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71151

--- Comment #21 from Senthil Kumar Selvaraj  ---
It occurs with "7.0.0 20160824 (experimental) (GCC)". Besides, the errors go
away if I remove --relax, so I think it's a linker issue.

[Bug target/71151] [avr] -fmerge-constants and -fdata-sections/-ffunction-sections results in string constants in .progmem.gcc_sw section

[gjl at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71151

--- Comment #20 from Georg-Johann Lay  ---
(In reply to Senthil Kumar Selvaraj from comment #19)
> Can reproduce this on trunk

So it's not related to PR72767 then? (Fixed since 2016-08-01)

[Bug target/71151] [avr] -fmerge-constants and -fdata-sections/-ffunction-sections results in string constants in .progmem.gcc_sw section

[saaadhu at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71151

--- Comment #19 from Senthil Kumar Selvaraj  ---
Can reproduce this on trunk with binutils master. Mostly likely a
binutils/linker bug, as turning off relaxation fixes the error. Will debug
further and post the conclusions.

[Bug target/71151] [avr] -fmerge-constants and -fdata-sections/-ffunction-sections results in string constants in .progmem.gcc_sw section

[urjaman at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71151

Urja Rannikko  changed:

   What|Removed |Added

 CC||urjaman at gmail dot com

--- Comment #18 from Urja Rannikko  ---
This fix seems to have caused an another problem on AVR, as described on 
arch linux bug tracker https://bugs.archlinux.org/task/49284#comment149872

Also my software (this example being
https://github.com/urjaman/frser-m328lpcspi ) fails to build with similar
output:
avr-gcc  -mmcu=atmega328p -O3 -Wl,--relax -fno-tree-switch-conversion -Wall -W
-pipe -flto -flto-partition=none -fwhole-program -std=gnu99 -Ilibfrser
-std=gnu99 -I./ -o frser-m328lpcspi.out main.c uart.c flash.c ciface.c
console.c lib.c appdb.c commands.c libfrser/frser.c libfrser/udelay.c
libfrser/dxprint.c libfrser/spilib.c libfrser/spihw_avrspi.c libfrser/lpcfwh.c
nibble.c
/tmp/ccMVXcOJ.lto.o: In function `spi_spiop_end':
:(.text+0x12e6): relocation truncated to fit: R_AVR_7_PCREL against
`no symbol'
/tmp/ccMVXcOJ.lto.o: In function `frser_main':
:(.text+0x1338): relocation truncated to fit: R_AVR_7_PCREL against
`no symbol'
:(.text+0x1362): relocation truncated to fit: R_AVR_7_PCREL against
`no symbol'
:(.text+0x1370): relocation truncated to fit: R_AVR_7_PCREL against
`no symbol'
:(.text+0x13dc): relocation truncated to fit: R_AVR_7_PCREL against
`no symbol'
:(.text+0x13e4): relocation truncated to fit: R_AVR_7_PCREL against
`no symbol'
:(.text+0x13ec): relocation truncated to fit: R_AVR_7_PCREL against
`no symbol'
:(.text+0x140e): relocation truncated to fit: R_AVR_7_PCREL against
`no symbol'
:(.text+0x1428): relocation truncated to fit: R_AVR_7_PCREL against
`no symbol'
:(.text+0x1450): relocation truncated to fit: R_AVR_7_PCREL against
`no symbol'

Not all software fails (simpler/smaller stuff seems to work), but this is
pretty bad, should we open a seperate bug or what?

[Bug target/71151] [avr] -fmerge-constants and -fdata-sections/-ffunction-sections results in string constants in .progmem.gcc_sw section

[gjl at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71151

--- Comment #17 from Georg-Johann Lay  ---
What about the performance issue re. crossing the jump table with a branch?

An easy fix would be to emit ".subsection 1" before the jump table; you think
we should do this?

[Bug target/71151] [avr] -fmerge-constants and -fdata-sections/-ffunction-sections results in string constants in .progmem.gcc_sw section

[saaadhu at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71151

Senthil Kumar Selvaraj  changed:

   What|Removed |Added

 Status|ASSIGNED|RESOLVED
 Resolution|--- |FIXED

--- Comment #16 from Senthil Kumar Selvaraj  ---
Fixed in trunk and gcc-6 branch.

[Bug target/71151] [avr] -fmerge-constants and -fdata-sections/-ffunction-sections results in string constants in .progmem.gcc_sw section

[saaadhu at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71151

Senthil Kumar Selvaraj  changed:

   What|Removed |Added

 Status|UNCONFIRMED |ASSIGNED
   Last reconfirmed||2016-08-08
 CC||saaadhu at gcc dot gnu.org
 Ever confirmed|0   |1

[Bug target/71151] [avr] -fmerge-constants and -fdata-sections/-ffunction-sections results in string constants in .progmem.gcc_sw section

[gjl at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71151

--- Comment #15 from Georg-Johann Lay  ---
Author: gjl
Date: Mon Aug  1 09:15:24 2016
New Revision: 238935

URL: https://gcc.gnu.org/viewcvs?rev=238935=gcc=rev
Log:
gcc/
Backport from 2016-06-16 trunk r237536.
2016-06-16  Senthil Kumar Selvaraj  
PR target/71151
* config/avr/avr.c (avr_asm_init_sections): Remove setup of
progmem_swtable_section.
(progmem_swtable_section): Remove.
(avr_asm_function_rodata_section): Remove.
(TARGET_ASM_FUNCTION_RODATA_SECTION): Remove.
* config/avr/avr.h (JUMP_TABLES_IN_TEXT_SECTION): Define to 1.

testsuite/
Backport from 2016-06-16 trunk r237536, r237910.
2016-06-16  Senthil Kumar Selvaraj  
PR target/71151
* gcc.target/avr/pr71151-1.c: New test.
* gcc.target/avr/pr71151-2.c: New test.
* gcc.target/avr/pr71151-3.c: New test.
* gcc.target/avr/pr71151-4.c: New test.
* gcc.target/avr/pr71151-5.c: New test.
* gcc.target/avr/pr71151-6.c: New test.
* gcc.target/avr/pr71151-7.c: New test.
* gcc.target/avr/pr71151-8.c: New test.
* gcc.target/avr/pr71151-common.h: New file.


Added:
branches/gcc-6-branch/gcc/testsuite/gcc.target/avr/pr71151-1.c
branches/gcc-6-branch/gcc/testsuite/gcc.target/avr/pr71151-2.c
branches/gcc-6-branch/gcc/testsuite/gcc.target/avr/pr71151-3.c
branches/gcc-6-branch/gcc/testsuite/gcc.target/avr/pr71151-4.c
branches/gcc-6-branch/gcc/testsuite/gcc.target/avr/pr71151-5.c
branches/gcc-6-branch/gcc/testsuite/gcc.target/avr/pr71151-6.c
branches/gcc-6-branch/gcc/testsuite/gcc.target/avr/pr71151-7.c
branches/gcc-6-branch/gcc/testsuite/gcc.target/avr/pr71151-8.c
branches/gcc-6-branch/gcc/testsuite/gcc.target/avr/pr71151-common.h
Modified:
branches/gcc-6-branch/gcc/ChangeLog
branches/gcc-6-branch/gcc/config/avr/avr.c
branches/gcc-6-branch/gcc/config/avr/avr.h
branches/gcc-6-branch/gcc/testsuite/ChangeLog

[Bug target/71151] [avr] -fmerge-constants and -fdata-sections/-ffunction-sections results in string constants in .progmem.gcc_sw section

[gjl at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71151

--- Comment #14 from Georg-Johann Lay  ---
Author: gjl
Date: Fri Jul  1 12:09:53 2016
New Revision: 237910

URL: https://gcc.gnu.org/viewcvs?rev=237910=gcc=rev
Log:
gcc/testsuite/
PR target/71151
* gcc.target/avr/pr71151-common.h (foo): Use macro SECTION_NAME
instead of ".foo" for its section name.
* gcc.target/avr/pr71151-2.c (SECTION_NAME): Define appropriately
depending on MCU's flash size.
* gcc.target/avr/pr71151-3.c (SECTION_NAME): Dito.
* gcc.target/avr/pr71151-4.c (SECTION_NAME): Dito.
* gcc.target/avr/pr71151-5.c (SECTION_NAME): Dito.
* gcc.target/avr/pr71151-6.c (SECTION_NAME): Dito.
* gcc.target/avr/pr71151-7.c (SECTION_NAME): Dito.
* gcc.target/avr/pr71151-8.c (SECTION_NAME): Dito.

Modified:
trunk/gcc/testsuite/ChangeLog
trunk/gcc/testsuite/gcc.target/avr/pr71151-2.c
trunk/gcc/testsuite/gcc.target/avr/pr71151-3.c
trunk/gcc/testsuite/gcc.target/avr/pr71151-4.c
trunk/gcc/testsuite/gcc.target/avr/pr71151-5.c
trunk/gcc/testsuite/gcc.target/avr/pr71151-6.c
trunk/gcc/testsuite/gcc.target/avr/pr71151-7.c
trunk/gcc/testsuite/gcc.target/avr/pr71151-8.c
trunk/gcc/testsuite/gcc.target/avr/pr71151-common.h

[Bug target/71151] [avr] -fmerge-constants and -fdata-sections/-ffunction-sections results in string constants in .progmem.gcc_sw section

[gjl at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71151

--- Comment #13 from Georg-Johann Lay  ---
Disadvantage of havong tables in same text section a code is that code side
might increase for the following reason:  branches that cross a switch
statement will also have to cross the jump table, hence branch offsets and thus
code size might increase.

[Bug target/71151] [avr] -fmerge-constants and -fdata-sections/-ffunction-sections results in string constants in .progmem.gcc_sw section

[senthil.thecoder at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71151

--- Comment #12 from Senthil Kumar Selvaraj  
---
Yes, tiny also has .rodata in .data

I'd totally missed PR 63323, so just removing the target hook and turning on
JUMP_TABLES_IN_TEXT_SECTION does the trick, like you said. Wrote a basic test
to make sure jumptables get generated and used correctly, and that worked too.

[Bug target/71151] [avr] -fmerge-constants and -fdata-sections/-ffunction-sections results in string constants in .progmem.gcc_sw section

[gjl at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71151

--- Comment #11 from Georg-Johann Lay  ---
well, IIRC for Tiny .rodata is still a part of .data and not part of .text?  If
this is still the case, then on Tiny the best place for jump tables is also
.text.

[Bug target/71151] [avr] -fmerge-constants and -fdata-sections/-ffunction-sections results in string constants in .progmem.gcc_sw section

[gjl at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71151

--- Comment #10 from Georg-Johann Lay  ---
Well, then we should remove TARGET_ASM_FUNCTION_RODATA_SECTION implementation
altogether (it's weird, not only because it patches flag_data_sections), same
for ASM_OUTPUT_ADDR_VEC_ELT.

Instead implement ASM_OUTPUT_ADDR_VEC and do the whole addr_vec stuff by hand:

1) switch to correct section: if -ffunction-sections is on, cook up a section
like .progmem.gcc_sw_table., otherwise just
.progmem.gcc_sw_table

1) Alternatively, just emit .pushsection and .popsection around the jump table.

2) Output alignment .p2align.  The original alignment from
ASM_OUTPUT_BEFORE_CASE_LABEL might be too early (wrong section), so that the
alignment must be output again.  ASM_OUTPUT_BEFORE_CASE_LABEL is no more
needed.

3) Output the jump table, see final.c for how to iterate.

Anyway, we might consider putting the jump table into .text section.  Since
PR63223 we can handle jump-tables at any location, there is no need for having
it in .progmem (which is supposed to reside in the lowest 64k).  And for Tiny
targets, where .rodata is the best place, JUMP_TABLES_IN_TEXT_SECTION can just
return 0.

[Bug target/71151] [avr] -fmerge-constants and -fdata-sections/-ffunction-sections results in string constants in .progmem.gcc_sw section

[senthil.thecoder at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71151

--- Comment #9 from Senthil Kumar Selvaraj  
---
For both kinds of invocation (mergeable constants and jump tables), the decl
passed is current_function_decl. And that looks right from the documentation
for the target hook.

I'll submit another patch for trunk that adds a new SECCAT_RODATA_JUMPTABLE to
enum section_category, and then passes the section_category down to the target
hook. avr_asm_function_rodata_section can then choose to return the correct
section based on the category.

What do you think?

[Bug target/71151] [avr] -fmerge-constants and -fdata-sections/-ffunction-sections results in string constants in .progmem.gcc_sw section

[gjl at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71151

--- Comment #8 from Georg-Johann Lay  ---
What about avr_asm_function_rodata_section?  Isn't it possible to filter DECL
and only transform for addr_vect?

[Bug target/71151] [avr] -fmerge-constants and -fdata-sections/-ffunction-sections results in string constants in .progmem.gcc_sw section

[senthil.thecoder at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71151

--- Comment #7 from Senthil Kumar Selvaraj  
---
Created attachment 38613
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=38613=edit
Tentative patch for 6.1

Looks like the right fix will need to somehow differentiate between jump tables
and other per function data when the function_rodata_section target hook is
invoked. I'll submit a tentative patch in the next couple of days that does
that.

However, I don't think that patch will get into 6.x, given that it changes the
target hook interface. The attached patch instead tries to work around the
problem, by letting default_elf_select_section run as usual, but then returning
a different section with the name reverted to the right prefix (rodata instead
of progmem), with the correct flags (MERGE and STRINGS)

[Bug target/71151] [avr] -fmerge-constants and -fdata-sections/-ffunction-sections results in string constants in .progmem.gcc_sw section

[gjl at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71151

--- Comment #6 from Georg-Johann Lay  ---
(In reply to Anatol from comment #5)
> It is a severe compiler issue that stop avr-gcc 6 from using.
> Consider changing "Importance" status to blocker.

It's definite not a "blocker".  "blocker" would mean the issue is so severe
that no toolchain could be released, e.g. because the compiler for a primary or
secondary platform cannot be built.

Moreover, as Senthil pointed out above, the problem can be worked around by
adding -fno-merge-constants to the command options.

Anyway, twiddling the "importance" won't have any effect w.r.t. bug fixing
quality or swiftness...  In the case of avr, that speed is solely determindey
by the number of people that are willing to contribute to avr-gcc...

If all goes well I might have a day or two to work on avr-gcc in June.

[Bug target/71151] [avr] -fmerge-constants and -fdata-sections/-ffunction-sections results in string constants in .progmem.gcc_sw section

[anatol.pomozov at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71151

--- Comment #5 from Anatol  ---
This bug has been reported by Arch Linux users.
https://bugs.archlinux.org/task/49284

It is a severe compiler issue that stop avr-gcc 6 from using. Consider changing
"Importance" status to blocker.

[Bug target/71151] [avr] -fmerge-constants and -fdata-sections/-ffunction-sections results in string constants in .progmem.gcc_sw section

[gjl at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71151

Georg-Johann Lay  changed:

   What|Removed |Added

 CC||gigo121 at gmail dot com

--- Comment #4 from Georg-Johann Lay  ---
*** Bug 70999 has been marked as a duplicate of this bug. ***

[Bug target/71151] [avr] -fmerge-constants and -fdata-sections/-ffunction-sections results in string constants in .progmem.gcc_sw section

[senthil.thecoder at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71151

--- Comment #3 from Senthil Kumar Selvaraj  
---
Yeah, it's a 6.x thing. There's a commit that changes varasm.c from using
targetm.asm_out.mergeable_rodata_prefix to a function call to
targetm.asm_out.function_rodata_section hook if the section category is
SECCAT_RODATA_MERGE_STR. So avr_asm_function_rodata_section now gets called for
string literals inside functions, not just jumptables.

I'm trying out a fix that propagates the section category down to the target
hook and have the avr backend use it to distinguish between jumptables and
other data.

[Bug target/71151] [avr] -fmerge-constants and -fdata-sections/-ffunction-sections results in string constants in .progmem.gcc_sw section

[gjl at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71151

Georg-Johann Lay  changed:

   What|Removed |Added

   Keywords||wrong-code
 CC||gjl at gcc dot gnu.org

--- Comment #2 from Georg-Johann Lay  ---
Cannot reproduce this on 5.x.

The avr BE tries to apply -fdata-sections to data in progmem in a similar way
like -fdata-sections acts on data in RAM.  A dedicated option like
-mprogmem-sections was not possible because of the terrible section
implementation in varasm.c.

Presumably, the problem is TARGET_ASM_FUNCTION_RODATA_SECTION in avr.c.

Maybe we'll have to give up the feature alltogether and drop the .progmem.var
subsections and return to one bulk .progmem.data again.

[Bug target/71151] [avr] -fmerge-constants and -fdata-sections/-ffunction-sections results in string constants in .progmem.gcc_sw section

[senthil.thecoder at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71151

--- Comment #1 from Senthil Kumar Selvaraj  
---
A workaround is to disable constant merging (-fno-merge-constants).