Re: [PATCH 1/3] MdePkg: add RETURNS_TWICE attribute

["Gao, Liming" <liming.gao@intel.com>]

Michael:
  I try VS compiler. When I add NORETURN as the first attribute of the function declaration. It can pass build. On VS compiler, NORETURN is required to be placed ahead of EFIAPI.

VOID
EFIAPI
NORETURN
LongJump (
  IN      BASE_LIBRARY_JUMP_BUFFER  *JumpBuffer,
  IN      UINTN                     Value
  );

==>

NORETURN
VOID
EFIAPI
LongJump (
  IN      BASE_LIBRARY_JUMP_BUFFER  *JumpBuffer,
  IN      UINTN                     Value
  );

Thanks
Liming
From: Michael Zimmermann [mailto:sigmaepsilon92@gmail.com]
Sent: Wednesday, December 27, 2017 4:01 AM
To: Gao, Liming <liming.gao@intel.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; edk2-devel@lists.01.org; Kinney, Michael D <michael.d.kinney@intel.com>
Subject: Re: [edk2] [PATCH 1/3] MdePkg: add RETURNS_TWICE attribute

Liming:
I've never used VS to compile edk2 but to me it looks like it doesn't like what NORETURN is expanding to ('__declspec(noreturn)').

If that is the case it's a generic bug because this patch set did not add the NORETURN macro.


On Tue, Dec 26, 2017 at 5:51 PM, Gao, Liming <liming.gao@intel.com<mailto:liming.gao@intel.com>> wrote:
Michael:
  I suggest to add comments for each definition although there is redundant.

  Besides, after I apply these three patches, and build MdePkg.dsc with VS2015x86. It will report below error. Could you help look it?

        "C:\Program Files (x86)\Microsoft Visual Studio 14.0\Vc\bin\cl.exe" /Foc:\r9tips\allpkg\edk2\Build\Mde\DEBUG_VS2015x86\IA32\MdePkg\Library\BaseCacheMaintenanceLib\BaseCacheMaintenanceLib\OUTPUT\.\X86Cache.obj /nologo /arch:IA32 /c /WX /GS- /W4 /Gs32768 /D UNICODE /O1b2 /GL /FIAutoGen.h /EHs-c- /GR- /GF /Gy /Zi /Gm /Gw -D DISABLE_NEW_DEPRECATED_INTERFACES /Ic:\r9tips\allpkg\edk2\MdePkg\Library\BaseCacheMaintenanceLib  /Ic:\r9tips\allpkg\edk2\Build\Mde\DEBUG_VS2015x86\IA32\MdePkg\Library\BaseCacheMaintenanceLib\BaseCacheMaintenanceLib\DEBUG  /Ic:\r9tips\allpkg\edk2\MdePkg  /Ic:\r9tips\allpkg\edk2\MdePkg\Include  /Ic:\r9tips\allpkg\edk2\MdePkg\Include\Ia32 c:\r9tips\allpkg\edk2\MdePkg\Library\BaseCacheMaintenanceLib\X86Cache.c
X86Cache.c
c:\r9tips\allpkg\edk2\MdePkg\Include\Library/BaseLib.h(4933): error C2059: syntax error: 'type'
NMAKE : fatal error U1077: '"C:\Program Files (x86)\Microsoft Visual Studio 14.0\Vc\bin\cl.exe"' : return code '0x2'
Stop.

Thanks
Liming
From: Michael Zimmermann [mailto:sigmaepsilon92@gmail.com<mailto:sigmaepsilon92@gmail.com>]
Sent: Monday, December 25, 2017 11:50 PM
To: Gao, Liming <liming.gao@intel.com<mailto:liming.gao@intel.com>>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org<mailto:ard.biesheuvel@linaro.org>>; edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>

Subject: Re: [edk2] [PATCH 1/3] MdePkg: add RETURNS_TWICE attribute

Liming:
The other macros have comments both before the compiler directives and before each define for each compiler.
To me it looks like these are slightly differently formulated only and kinda redundant too.
Is there a rule or do you have suggestions for writing comments for this kind of macro?

On Mon, Dec 25, 2017 at 4:11 AM, Gao, Liming <liming.gao@intel.com<mailto:liming.gao@intel.com>> wrote:
Micha:
   Could you add comments for new macro RETURNS_TWICE like others, such as ANALYZER_NORETURN?

>-----Original Message-----
>From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org<mailto:ard.biesheuvel@linaro.org>]
>Sent: Friday, December 22, 2017 11:24 PM
>To: M1cha <sigmaepsilon92@gmail.com<mailto:sigmaepsilon92@gmail.com>>
>Cc: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>; Kinney, Michael D
><michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>; Gao, Liming <liming.gao@intel.com<mailto:liming.gao@intel.com>>
>Subject: Re: [edk2] [PATCH 1/3] MdePkg: add RETURNS_TWICE attribute
>
>On 22 December 2017 at 07:23, M1cha <sigmaepsilon92@gmail.com<mailto:sigmaepsilon92@gmail.com>> wrote:
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Michael Zimmermann <sigmaepsilon92@gmail.com<mailto:sigmaepsilon92@gmail.com>>
>> ---
>>  MdePkg/Include/Base.h | 10 ++++++++++
>>  1 file changed, 10 insertions(+)
>>
>> diff --git a/MdePkg/Include/Base.h b/MdePkg/Include/Base.h
>> index 22ab5d3715fb..c863de407418 100644
>> --- a/MdePkg/Include/Base.h
>> +++ b/MdePkg/Include/Base.h
>> @@ -218,6 +218,16 @@ VERIFY_SIZE_OF (__VERIFY_UINT32_ENUM_SIZE,
>4);
>>    #endif
>>  #endif
>>
>> +#ifndef RETURNS_TWICE
>> +  #if defined (__GNUC__) || defined (__clang__)
>> +    #define RETURNS_TWICE  __attribute__((returns_twice))
>> +  #elif defined(_MSC_EXTENSIONS) && !defined(MDE_CPU_EBC)
>> +    #define RETURNS_TWICE
>> +  #else
>> +    #define RETURNS_TWICE
>
>What is the point of having two versions that are #defined to nothing?
>
>> +  #endif
>> +#endif
>> +
>>  //
>>  // For symbol name in assembly code, an extra "_" is sometimes necessary
>>  //
>> --
>> 2.15.1
>>