Re: [edk2-devel] [PATCH] OvmfPkg/Sec: Use correct prototype of ProcessLibraryConstructorList()

["Laszlo Ersek" <lersek@redhat.com>]

On 2/5/24 01:44, Laszlo Ersek wrote:
> Hello Ard,
> 
> On 2/2/24 19:24, Ard Biesheuvel wrote:
>> From: Ard Biesheuvel <ardb@kernel.org>
>>
>> The prototype of ProcessLibraryConstructorList(), which is generated by
>> the build tools, depends on the boot phase, and SEC and PEI differ in
>> this regard. However, OVMF's SEC implemention includes PeimEntryPoint.h
>> and calls the PEI version of the protoype (passing NULL arguments),
>> whereas the actual implementation observed by the compiler follows the
>> SEC version, which takes VOID.
>>
>> The compiler usually doesn't spot the difference, but the LTO linker
>> does, and throws an error:
>>
>> MdePkg/Include/Library/PeimEntryPoint.h:74:1: error: type of 'ProcessLibraryConstructorList' does not match original declaration [-Werror=lto-type-mismatch]
>>    74 | ProcessLibraryConstructorList (
>>       | ^
>> Build/OvmfX64/RELEASE_GCC5/X64/OvmfPkg/Sec/SecMain/DEBUG/AutoGen.c:310:1: note: type mismatch in parameter 1
>>   310 | ProcessLibraryConstructorList (
>>       | ^
>> Build/OvmfX64/RELEASE_GCC5/X64/OvmfPkg/Sec/SecMain/DEBUG/AutoGen.c:310:1: note: type 'void' should match type 'void *'
>> Build/OvmfX64/RELEASE_GCC5/X64/OvmfPkg/Sec/SecMain/DEBUG/AutoGen.c:310:1: note: 'ProcessLibraryConstructorList' was previously declared here
>>
>> Fix this by dropping the #include, and providing a declaration of
>> ProcessLibraryConstructorList() in SecMain.c
>>
>> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=4643
>> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
>> ---
>>  OvmfPkg/Sec/SecMain.c | 9 +++++++--
>>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> This issue was reported 5+ years ago:
> 
>   https://bugzilla.tianocore.org/show_bug.cgi?id=990
> 
> I asked for a general solution in BaseTools:
> 
>   https://bugzilla.tianocore.org/show_bug.cgi?id=991
> 
> Liming recommended that BaseTools should generate a proper declaration
> in "AutoGen.h":
> 
>   https://bugzilla.tianocore.org/show_bug.cgi?id=991#c2
> 
> But nothing happened.
> 
>>
>> diff --git a/OvmfPkg/Sec/SecMain.c b/OvmfPkg/Sec/SecMain.c
>> index 31da5d0ace51..7b7d4793984c 100644
>> --- a/OvmfPkg/Sec/SecMain.c
>> +++ b/OvmfPkg/Sec/SecMain.c
>> @@ -11,7 +11,6 @@
>>  
>>  #include <PiPei.h>
>>  
>> -#include <Library/PeimEntryPoint.h>
>>  #include <Library/BaseLib.h>
>>  #include <Library/DebugLib.h>
>>  #include <Library/BaseMemoryLib.h>
>> @@ -39,6 +38,12 @@ typedef struct _SEC_IDT_TABLE {
>>    IA32_IDT_GATE_DESCRIPTOR    IdtTable[SEC_IDT_ENTRY_COUNT];
>>  } SEC_IDT_TABLE;
>>  
>> +VOID
>> +EFIAPI
>> +ProcessLibraryConstructorList (
>> +  VOID
>> +  );
>> +
> 
> This is my problem right here.
> 
> Why do 10+ files have to do this individually? See the above BZs for
> details.

I'm working on patches that will solve this problem centrally; I'm
including this patch at the front of that series.

Thanks
Laszlo

> 
> Well, whatever.
> 
> What a terrible project.
> 
> *shrug*
> 
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> 
>>  VOID
>>  EFIAPI
>>  SecStartupPhase2 (
>> @@ -844,7 +849,7 @@ SecCoreStartupWithStack (
>>      InitializeCpuExceptionHandlers (NULL);
>>    }
>>  
>> -  ProcessLibraryConstructorList (NULL, NULL);
>> +  ProcessLibraryConstructorList ();
>>  
>>    if (!SevEsIsEnabled ()) {
>>      //
> 
> 
> 
> 
> 
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115191): https://edk2.groups.io/g/devel/message/115191
Mute This Topic: https://groups.io/mt/104167522/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-