On 2015-12-05 22:26:53, Ard Biesheuvel wrote: > On 6 December 2015 at 01:44, Jordan Justen <[email protected]> wrote: > > On 2015-12-04 11:21:18, Ard Biesheuvel wrote: > >> On 4 December 2015 at 20:13, Jordan Justen <[email protected]> > >> wrote: > >> > On 2015-12-04 08:43:29, Ard Biesheuvel wrote: > >> >> On 4 December 2015 at 17:27, Laszlo Ersek <[email protected]> wrote: > >> >> > On 12/04/15 17:23, Paolo Bonzini wrote: > >> >> >> > >> >> >> > >> >> >> On 04/12/2015 11:39, Laszlo Ersek wrote: > >> >> >>> (4) Linking those two files into a complete program is a violation > >> >> >>> of > >> >> >>> "6.7 External definitions": > >> >> >>> > >> >> >>> [...] If an identifier declared with external linkage is used > >> >> >>> in an > >> >> >>> expression (other than as part of the operand of a *sizeof* > >> >> >>> operator), somewhere in the entire program there shall be > >> >> >>> exactly > >> >> >>> one external definition for the identifier [...] > >> >> >>> > >> >> >>> Again, how does the argument go? > >> >> >>> > >> >> >>> - In each file we have exactly one tentative definition, and no > >> >> >>> declaration that would be, in its own right, an external > >> >> >>> definition. > >> >> >>> Based on (2), the files must behave exactly as-if there was one > >> >> >>> external *definition* for "x" in each. > >> >> >>> > >> >> >>> - Argument (3) explains why "x" has external *linkage*. > >> >> >>> > >> >> >>> - Argument (4) applies to "x" because "x" has external linkage, and > >> >> >>> is > >> >> >>> used in a non-sizeof expression. And the requirement set forth in > >> >> >>> (4) > >> >> >>> is broken by the program because the program is required to behave > >> >> >>> exactly as if "x" had two external definitions. > >> >> >>> > >> >> >>> In practical terms: > >> >> >>> > >> >> >>> - If you compile the first version of the program (without any > >> >> >>> special > >> >> >>> options), it links together successfully: > >> >> >>> > >> >> >>> $ gcc -std=c89 -pedantic -Wall -Wextra -o f f1.c f2.c > >> >> >>> > >> >> >>> - If you compile the second (equivalent) version of the program, > >> >> >>> linkage fails: > >> >> >>> > >> >> >>> $ gcc -std=c89 -pedantic -Wall -Wextra -o f-b f1-b.c f2-b.c > >> >> >>> > >> >> >>> /tmp/cc8Isbn8.o:(.bss+0x0): multiple definition of `x' > >> >> >>> /tmp/cciQUlDz.o:(.bss+0x0): first defined here > >> >> >>> collect2: error: ld returned 1 exit status > >> >> >>> > >> >> >>> - The gcc bug is that linking the first version *too* should fail, > >> >> >>> without any particular options. > >> >> >> > >> >> >> That's true, but it would break existing code that declares > >> >> >> variables in > >> >> >> headers without "extern". > >> >> > > >> >> > I agree. After I posted the email I pondered what it would take to fix > >> >> > this in gcc. It would be impossible. :) > >> >> > > >> >> >> That's why Visual Studio does the same. > >> >> > > >> >> > Looks plausible. > >> >> > > >> >> > Thanks! > >> >> > Laszlo > >> >> > > >> >> > >> >> Indeed. Liming is looking into that. I spotted very few issues with > >> >> -fno-common, only two in CryptoPkg, so I think we should disallow > >> >> common allocations going forward if we can, especially since the > >> >> modularity of EDK2 makes it almost impossible to guarantee that it > >> >> never leads to problems. > >> >> > >> >> Qin has already acked a fix for one of those issues, I will ping him > >> >> for the other one. In the mean time, perhaps GCC users could double > >> >> check whether there is any breakage? Laszlo, could you check your OVMF > >> >> builds, please? > >> > > >> > I'm seeing a build error with MdeModulePkg and OvmfPkg. This is with > >> > gcc 5.2 using the GCC49 toolchain. > >> > > >> > Both cases seem to be unitialized global variables declared in a > >> > single .c file. I'm not sure what needs to be changed to fix this. > >> > > >> > MdeModulePkg: > >> > > >> > "ld" -o > >> > Build/MdeModule/DEBUG_GCC49/X64/MdeModulePkg/Application/MemoryProfileInfo/MemoryProfileInfo/DEBUG/MemoryProfileInfo.dll > >> > -nostdlib -n -q --gc-sections -z common-page-size=0x40 --entry > >> > _ModuleEntryPoint -u _ModuleEntryPoint -Map > >> > Build/MdeModule/DEBUG_GCC49/X64/MdeModulePkg/Application/MemoryProfileInfo/MemoryProfileInfo/DEBUG/MemoryProfileInfo.map > >> > -melf_x86_64 --oformat=elf64-x86-64 --start-group > >> > @Build/MdeModule/DEBUG_GCC49/X64/MdeModulePkg/Application/MemoryProfileInfo/MemoryProfileInfo/OUTPUT/static_library_files.lst > >> > --end-group --defsym=PECOFF_HEADER_SIZE=0x228 > >> > --script=BaseTools/Scripts/GccBase.lds > >> > `mNameString' referenced in section `.text.GetDriverNameString' of > >> > Build/MdeModule/DEBUG_GCC49/X64/MdeModulePkg/Application/MemoryProfileInfo/MemoryProfileInfo/OUTPUT/MemoryProfileInfo.lib(MemoryProfileInfo.obj): > >> > defined in discarded section `COMMON' of > >> > Build/MdeModule/DEBUG_GCC49/X64/MdeModulePkg/Application/MemoryProfileInfo/MemoryProfileInfo/OUTPUT/MemoryProfileInfo.lib(MemoryProfileInfo.obj) > >> > `mNameString' referenced in section `.text.GetDriverNameString' of > >> > Build/MdeModule/DEBUG_GCC49/X64/MdeModulePkg/Application/MemoryProfileInfo/MemoryProfileInfo/OUTPUT/MemoryProfileInfo.lib(MemoryProfileInfo.obj): > >> > defined in discarded section `COMMON' of > >> > Build/MdeModule/DEBUG_GCC49/X64/MdeModulePkg/Application/MemoryProfileInfo/MemoryProfileInfo/OUTPUT/MemoryProfileInfo.lib(MemoryProfileInfo.obj) > >> > `mNameString' referenced in section `.text.GetDriverNameString' of > >> > Build/MdeModule/DEBUG_GCC49/X64/MdeModulePkg/Application/MemoryProfileInfo/MemoryProfileInfo/OUTPUT/MemoryProfileInfo.lib(MemoryProfileInfo.obj): > >> > defined in discarded section `COMMON' of > >> > Build/MdeModule/DEBUG_GCC49/X64/MdeModulePkg/Application/MemoryProfileInfo/MemoryProfileInfo/OUTPUT/MemoryProfileInfo.lib(MemoryProfileInfo.obj) > >> > `mNameString' referenced in section `.text.GetDriverNameString' of > >> > Build/MdeModule/DEBUG_GCC49/X64/MdeModulePkg/Application/MemoryProfileInfo/MemoryProfileInfo/OUTPUT/MemoryProfileInfo.lib(MemoryProfileInfo.obj): > >> > defined in discarded section `COMMON' of > >> > Build/MdeModule/DEBUG_GCC49/X64/MdeModulePkg/Application/MemoryProfileInfo/MemoryProfileInfo/OUTPUT/MemoryProfileInfo.lib(MemoryProfileInfo.obj) > >> > `mNameString' referenced in section `.text.DumpMemoryProfileDriverInfo' > >> > of > >> > Build/MdeModule/DEBUG_GCC49/X64/MdeModulePkg/Application/MemoryProfileInfo/MemoryProfileInfo/OUTPUT/MemoryProfileInfo.lib(MemoryProfileInfo.obj): > >> > defined in discarded section `COMMON' of > >> > Build/MdeModule/DEBUG_GCC49/X64/MdeModulePkg/Application/MemoryProfileInfo/MemoryProfileInfo/OUTPUT/MemoryProfileInfo.lib(MemoryProfileInfo.obj) > >> > make: *** > >> > [Build/MdeModule/DEBUG_GCC49/X64/MdeModulePkg/Application/MemoryProfileInfo/MemoryProfileInfo/DEBUG/MemoryProfileInfo.dll] > >> > Error 1 > >> > > >> > OvmfPkg: > >> > > >> > "ld" -o > >> > Build/OvmfX64/DEBUG_GCC49/X64/IntelFrameworkModulePkg/Universal/StatusCode/Pei/StatusCodePei/DEBUG/StatusCodePei.dll > >> > -nostdlib -n -q --gc-sections -z common-page-size=0x40 --entry > >> > _ModuleEntryPoint -u _ModuleEntryPoint -Map > >> > Build/OvmfX64/DEBUG_GCC49/X64/IntelFrameworkModulePkg/Universal/StatusCode/Pei/StatusCodePei/DEBUG/StatusCodePei.map > >> > -melf_x86_64 --oformat=elf64-x86-64 --start-group > >> > @Build/OvmfX64/DEBUG_GCC49/X64/IntelFrameworkModulePkg/Universal/StatusCode/Pei/StatusCodePei/OUTPUT/static_library_files.lst > >> > --end-group --defsym=PECOFF_HEADER_SIZE=0x228 > >> > --script=BaseTools/Scripts/GccBase.lds > >> > `gNullVaList' referenced in section `.text.AsciiBSPrint' of > >> > Build/OvmfX64/DEBUG_GCC49/X64/MdePkg/Library/BasePrintLib/BasePrintLib/OUTPUT/BasePrintLib.lib(PrintLib.obj): > >> > defined in discarded section `COMMON' of > >> > Build/OvmfX64/DEBUG_GCC49/X64/MdePkg/Library/BasePrintLib/BasePrintLib/OUTPUT/BasePrintLib.lib(PrintLib.obj) > >> > GNUmakefile:406: recipe for target > >> > 'Build/OvmfX64/DEBUG_GCC49/X64/IntelFrameworkModulePkg/Universal/StatusCode/Pei/StatusCodePei/DEBUG/StatusCodePei.dll' > >> > failed > >> > make: *** > >> > [Build/OvmfX64/DEBUG_GCC49/X64/IntelFrameworkModulePkg/Universal/StatusCode/Pei/StatusCodePei/DEBUG/StatusCodePei.dll] > >> > Error 1 > >> > > >> > >> Is this a clean rebuild? > > > > Yes, it was. > > > > And did you regenerate your Conf/tools_def.txt as well?
Yes-ish. I symlink the templates from BaseTools/Conf to Conf. > I cannot reproduce the behavior where COMMON sections are created by > GCC in the presence of -fno-common. The issue appears to be that GCC44_ALL_CC_FLAGS needs to also be modified. OVMF seems reasonably happy with the flag on GCC49. I found this by searching for '-include AutoGen.h'. Maybe there are some other cases we should cover? -Jordan _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
