Bug 23571

Summary: arm: COMMONPAGESIZE issue
Product: binutils Reporter: Jernej Škrabec <jernej.skrabec>
Component: ldAssignee: Alan Modra <amodra>
Status: RESOLVED FIXED    
Severity: normal CC: glibc
Priority: P2    
Version: 2.32   
Target Milestone: ---   
Host: Target:
Build: Last reconfirmed: 2018-08-26 00:00:00
Attachments: U-Boot sections after
U-Boot sections before

Description Jernej Škrabec 2018-08-25 11:58:49 UTC
Created attachment 11210 [details]
U-Boot sections after

Hello,

since the commit 702d167134149f420ea3bcbc194d63a2653a0c27 (powerpc common-page-size), U-Boot for some armhf targets is not linked properly anymore. Issue lies in following linker script: http://git.denx.de/?p=u-boot.git;a=blob;f=arch/arm/cpu/u-boot.lds

If "CONSTANT(COMMONPAGESIZE)" is replaced with "0x1000", U-Boot works properly. Alternatively, if aforementioned commit is reverted, it works too.

It looks like COMMONPAGESIZE is 0. You can find attached sections info before and after that commit.
Comment 1 Jernej Škrabec 2018-08-25 11:59:41 UTC
Created attachment 11211 [details]
U-Boot sections before
Comment 2 Jernej Škrabec 2018-08-25 12:01:35 UTC
Sorry, I put same description to both files.

Attachment which has unaligned __secure_start is the result of linking after mentioned commit.
Comment 3 Alan Modra 2018-08-26 04:10:26 UTC
Description fixed.  The problem was caused by this part of the commit
+       * ldmain.c (main): Move config.maxpagesize and
+       config.commonpagesize initialization to..
+       * ldemul.c (after_parse_default): ..here.
in conjunction with the fact that the align argument in an output section statement is evaluated only once.  ie. ld really only expects an integer there, not an expression that might change.
Comment 4 Sourceware Commits 2018-08-26 13:18:21 UTC
The master branch has been updated by Alan Modra <amodra@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=3d9c8f6b3f033a6092425b7344647fb51dbed5c6

commit 3d9c8f6b3f033a6092425b7344647fb51dbed5c6
Author: Alan Modra <amodra@gmail.com>
Date:   Sun Aug 26 14:23:38 2018 +0930

    Delay evaluation of alignment expressions in output sections
    
    git commit 702d16713 broke expressions using CONSTANT(COMMONPAGESIZE)
    in ALIGN or SUBALIGN of output section statements, because these
    optional fields were evaluated at script parse time and the patch in
    question delayed setting of config.commonpagesize.  The right thing to
    do is keep the tree representation of those fields for later
    evaluation.
    
    	PR 23571
    	* ldlang.h (section_alignment): Make it an expression tree.
    	(subsection_alignment): Likewise.
    	* ldlang.c (topower): Delete.
    	(output_section_statement_newfunc): Adjust initialization.
    	(init_os): Evaluate section_alignment.
    	(lang_size_sections_1): Likewise.
    	(size_input_section): Evaluate subsection_alignment.
    	(lang_enter_output_section_statement): Don't evaluate here.
    	(lang_new_phdr): Use exp_get_vma rather than exp_get_value_int.
    	* ldexp.h (exp_get_value_int): Delete.
    	(exp_get_power): Declare.
    	* ldexp.c (exp_get_value_int): Delete.
    	(exp_get_power): New function.
    	* emultempl/pe.em (place_orphan): Build expression for section
    	alignment.
    	* emultempl/pep.em (place_orphan): Likewise.
    	* testsuite/ld-scripts/pr23571.d,
    	* testsuite/ld-scripts/pr23571.t: New test.
    	* testsuite/ld-scripts/align.exp: Run it.
Comment 5 Alan Modra 2018-08-26 13:25:54 UTC
Patch applied.  I'll note that the U-Boot script would have worked if the align expression had been put on the left of the colon.  Like this:

        .__secure_start
#ifndef CONFIG_ARMV7_SECURE_BASE
                ALIGN(CONSTANT(COMMONPAGESIZE))
#endif
        : {
                KEEP(*(.__secure_start))
        }
Comment 6 Jernej Škrabec 2018-08-26 15:41:49 UTC
It works, thanks!