On 16 August 2015 at 23:48, Scott Duplichan <sc...@notabs.org> wrote:
> ]Ard Biesheuvel [mailto:ard.biesheu...@linaro.org] wrote:
>
>  . . .
>
> ]> Thanks for this much needed tool chain definition consolidation. I ran
> ]> a build test with and without the patch. The build test uses GCC44-49
> ]> and Microsoft tool chains. Log files are here: http://notabs.org/uefi/tmp/.
> ]
> ]Thanks a lot for giving it a spin.
> ]
> ]> Here is what is see in the log files with respect to the patch:
> ]> 1) GCC49 X64 and IA32
> ]>    GenFw: ERROR 3000: Invalid Unsupported section alignment.
> ]
> ]I cannot reproduce this, unfortunately. Can you please check whether
> ]your BaseTools are up to date? There have been some changes recently
> ]to GenFw regarding section alignment which may cause this. My gcc is
> ]4.9.1 btw (Ubuntu)
> ]
> ]My Jenkins job has a 'git clean -dxf BaseTools/; make -C BaseTools' at
> ]the beginning so they are up to date, although I think that for GenFw,
> ]the git clean is not necessary. (Some other tools don't rebuild
> ]correctly if any of the common C code is modified)
>
> I used rebuilt BaseTools\Bin\Win32 with up to date source code. The SVN
> GenFw binary gives the same message. Adding some debug prints gives:
>
> Unsupported section alignment: sh_addr=84a0 addralign=64 mCoffOffset=84c0
>
> Objdump -h gives:
> d:\edk2build\edk2\Build\OvmfX64\RELEASE_GCC49\X64\MdeModulePkg\Core\
> Pei\PeiMain\DEBUG\PeiCore.dll: file format elf64-x86-64
>
> Sections:
> Idx Name  Size      VMA               LMA               File off  Algn
>   0 .text 00008250  0000000000000240  0000000000000240  000000c0  2**5
>           CONTENTS, ALLOC, LOAD, RELOC, READONLY, CODE
>   1 .data 000003b8  00000000000084a0  00000000000084a0  00008320  2**6
>           CONTENTS, ALLOC, LOAD, RELOC, DATA
>

Thanks for the analysis. As it turns out, this only happens in the debug build.

Anyway, as it turns out, GNU ld uses the same optimization that the
comments in ElfConvert[32|64].c allude to, i.e., if a section's
address is not aligned to its alignment, the misalignment should be
preserved. In this example, it means that mPcdInstance [which needs
64-byte alignment] is placed such that it will end up aligned
correctly only if .data is loaded 0x20 bytes into a 0x40 byte aligned
region.

The cause turns out to be the explicit base address for .data. The
following fixes it for me:

diff --git a/BaseTools/Scripts/GccBase.lds b/BaseTools/Scripts/GccBase.lds
index 4ee6d998532c..eef8325c96a5 100644
--- a/BaseTools/Scripts/GccBase.lds
+++ b/BaseTools/Scripts/GccBase.lds
@@ -44,7 +44,8 @@ SECTIONS {
    * between these sections is the same in the ELF and the PE/COFF versions of
    * this binary.
    */
-  .data ALIGN(ALIGNOF(.text)) : ALIGN(CONSTANT(COMMONPAGESIZE)) {
+  . = ALIGN(ALIGNOF(.text));
+  .data : ALIGN(CONSTANT(COMMONPAGESIZE)) {
     *(.data .data.* .gnu.linkonce.d.*)
     *(.bss .bss.* *COM*)
   }

so I will add this change to v2

> ]> 2) GCC46 ARM
> ]>    unrecognized command line option '-mno-unaligned-access'
> ]>
> ]
> ]Could you send me the output of gcc -v for this compiler? Mine is
> ]Linaro 4.6.3 which supports it fine, but the feature may be a Linaro
> ]contribution that only made it into 4.7 upstream. In any case, we
> ]cannot tolerate unaligned accesses so we may need to deprecate 4.6 or
> ]mandate that the Linaro version be used if older versions emit
> ]unaligned accesses.
>
> The GCC46 is the Windows hosted build from:
> http://sourceforge.net/projects/edk2developertoolsforwindows/
>
> It is built from the latest gcc 4.6 source code:
> http://ftpmirror.gnu.org/gcc/gcc-4.6.4/gcc-4.6.4.tar.bz2
>
> Using built-in specs.
> COLLECT_GCC=gcc
> COLLECT_LTO_WRAPPER=d:/edk2build/uefitools/gcc464-arm/bin/../lib/gcc/arm-linux-gnueabi/4.6.4/lto-wrapper.exe
> Target: arm-linux-gnueabi
> Configured with: ../gcc-4.6.4/configure --prefix=/gcc/xgcc 
> --libexecdir=/gcc/xgcc/lib --target=arm-linux-gnueabi --disable-werror 
> --disable-shared --disable-libssp --disable-bootstrap --disable-nls 
> --disable-libquadmath --without-headers --enable-languages=c 
> --with-gmp=/gcc/xgcc --with-mpfr=/gcc/xgcc --with-mpc=/gcc/xgcc 
> --with-libelf=/gcc/xgcc --with-pkgversion='EDK2 version 1.0' MAKEINFO=missing 
> --enable-twoprocess --disable-threads --disable-decimal-float 
> --disable-win32-registry --disable-libc --with-windres
> Thread model: single
> gcc version 4.6.4 (EDK2 version 1.0)
>
> Source code for gcc-linaro-4.6-2012.04 shows what you suspected:
> ChangeLog.linaro contains: (insv, extzv): Add unaligned-access support.
>

OK, this sucks.

Linaro dropped the ball on this one IMO. By backporting this issue to
4.6 and enabling it by default, we can no longer have a single 4.6
definition for both compilers, since for the Linaro on we must pass
-mno-unaligned-access and for the stock one we must not pass it.

The best way to fix this is remove 4.6, I think. We could disable the
optimization indirectly by generating code for ARMv5, but I don't
think that is desirable solution at all.

-- 
Ard.
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to