Using -fdata-sections causes string literals in functions to go into .progmem.sw_gcc, instead of .rodata.str. This is incorrect, since code using them expects data memory addresses, not flash.
$ cat printf.c
extern void bar(const char*);
$ avr-gcc -mmcu=atmega32u2 printf.c -S -Os -fdata-sections -ffunction-sections
$ cat printf.s
jaguar:~/scratch$ cat printf.s
__SP_H__ = 0x3e
__SP_L__ = 0x3d
__SREG__ = 0x3f
__tmp_reg__ = 0
__zero_reg__ = 1
.type foo, @function
/* prologue: function */
/* frame size = 0 */
/* stack size = 0 */
.L__stack_usage = 0
.size foo, .-foo
.ident "GCC: (GNU) 6.1.0"
As the testcase shows, the string goes into progmem.gcc_sw_table.<fn_name>.*, and this gets placed in flash by the linker script. Reading from the address in r25:24 will obviously not give the correct results - it will read from whatever happens to be mapped at the same address in the data address space.
A workaround is to disable constant merging (-fno-merge-constants).
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.
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 70999 has been marked as a duplicate of this bug. ***
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.
(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.
Created attachment 38613 [details]
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)
What about avr_asm_function_rodata_section? Isn't it possible to filter DECL and only transform for addr_vect?
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?
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.<function-asm-name>, 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.
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.
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.
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.
Date: Fri Jul 1 12:09:53 2016
New Revision: 237910
* 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.
Date: Mon Aug 1 09:15:24 2016
New Revision: 238935
Backport from 2016-06-16 trunk r237536.
2016-06-16 Senthil Kumar Selvaraj <email@example.com>
* config/avr/avr.c (avr_asm_init_sections): Remove setup of
* config/avr/avr.h (JUMP_TABLES_IN_TEXT_SECTION): Define to 1.
Backport from 2016-06-16 trunk r237536, r237910.
2016-06-16 Senthil Kumar Selvaraj <firstname.lastname@example.org>
* 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.
Fixed in trunk and gcc-6 branch.
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?
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':
<artificial>:(.text+0x12e6): relocation truncated to fit: R_AVR_7_PCREL against `no symbol'
/tmp/ccMVXcOJ.lto.o: In function `frser_main':
<artificial>:(.text+0x1338): relocation truncated to fit: R_AVR_7_PCREL against `no symbol'
<artificial>:(.text+0x1362): relocation truncated to fit: R_AVR_7_PCREL against `no symbol'
<artificial>:(.text+0x1370): relocation truncated to fit: R_AVR_7_PCREL against `no symbol'
<artificial>:(.text+0x13dc): relocation truncated to fit: R_AVR_7_PCREL against `no symbol'
<artificial>:(.text+0x13e4): relocation truncated to fit: R_AVR_7_PCREL against `no symbol'
<artificial>:(.text+0x13ec): relocation truncated to fit: R_AVR_7_PCREL against `no symbol'
<artificial>:(.text+0x140e): relocation truncated to fit: R_AVR_7_PCREL against `no symbol'
<artificial>:(.text+0x1428): relocation truncated to fit: R_AVR_7_PCREL against `no symbol'
<artificial>:(.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?
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.
(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)
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.
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.
Tracking binutils bug https://sourceware.org/bugzilla/show_bug.cgi?id=20545