public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Andrew Fish <afish@apple.com>
To: Sergei Temerkhanov <s.temerkhanov@gmail.com>
Cc: "Gao, Liming" <liming.gao@intel.com>,
	"edk2-devel@lists.01.org" <edk2-devel@lists.01.org>
Subject: Re: [PATCH] MdePkg: Fix undefined behavior on variadic parameters
Date: Wed, 17 May 2017 20:06:48 -0700	[thread overview]
Message-ID: <D2558173-7A3C-4DF4-9990-58785DB04078@apple.com> (raw)
In-Reply-To: <CAPEA6dbcukh6yeoskgE5OXba2vCCO7h2f2a=uwwSM5kV+b3xWA@mail.gmail.com>

Sergei,

We are hitting this too. 

Our temporary work around was to suppress the warning for the libraries via our platforms DSC file so we did not need to override things in edk2. 

You can override the library build options if you add them to the [Components] section. 

MdePkg/Library/UefiLib/UefiLib.inf {
  <BuildOptions>
    XCODE:*_*_*_CC_FLAGS  = -Wno-varargs
}

Thanks,

Andrew Fish

> On May 17, 2017, at 5:26 PM, Sergei Temerkhanov <s.temerkhanov@gmail.com> wrote:
> 
> On Wed, May 17, 2017 at 6:30 PM, Gao, Liming <liming.gao@intel.com> wrote:
>> Sergey:
>>  Now, VS and GCC don't report such issue. How do you find them?
> 
> Clang build for ARM64 revealed it.
> 
>> And, this change modifies the function API, does it impact the consumer code?
> 
> No changes to the consumer code are needed - integer promotions are
> handled by the compilers. In fact, even the ABI remains the same b/c
> full words are passed as function parameters.
> 
> Regards,
> Sergey
> 
>> 
>> Thanks
>> Liming
>>> -----Original Message-----
>>> From: Sergei Temerkhanov [mailto:s.temerkhanov@gmail.com]
>>> Sent: Tuesday, May 16, 2017 8:11 PM
>>> To: Gao, Liming <liming.gao@intel.com>
>>> Cc: edk2-devel@lists.01.org
>>> Subject: Re: [edk2] [PATCH] MdePkg: Fix undefined behavior on variadic parameters
>>> 
>>> On Tue, May 16, 2017 at 8:10 AM, Gao, Liming <liming.gao@intel.com> wrote:
>>>> Sergey:
>>>>  Could you give more detail on the undefined behavior on variadic parameters?
>>>> 
>>>>  I see https://bugzilla.tianocore.org/show_bug.cgi?id=410 describe this issues found in the latest CLANG tool chain. Do you find
>>> other tool chain reports it?
>>> 
>>> Yes, this is exactly the bug this patch fixes.
>>> 
>>> As per the C99 standard:
>>> "The parameter parmN is the identifier of the rightmost parameter in
>>> the variable parameter list in the function definition (the one just
>>> before the , ...). If the parameter parmN is declared with the
>>> register storage class, with a function or array type, or with a type
>>> that is not compatible with the type that results after application of
>>> the default argument promotions, the behavior is undefined."
>>> 
>>> That's exactly the case here since BOOLEAN is a typedef for unsigned
>>> char. It undergoes a promotion to an unsigned int which is not a
>>> compatible type for unsigned char. Correct me if I'm wrong.
>>> 
>>> Regards,
>>> Sergey
>>> 
>>>> 
>>>> Thanks
>>>> Liming
>>>>> -----Original Message-----
>>>>> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Sergey Temerkhanov
>>>>> Sent: Tuesday, May 16, 2017 10:57 AM
>>>>> To: edk2-devel@lists.01.org
>>>>> Subject: [edk2] [PATCH] MdePkg: Fix undefined behavior on variadic parameters
>>>>> 
>>>>> Fix undefined behavior by avoiding parameter type promotion
>>>>> 
>>>>> Signed-off-by: Sergey Temerkhanov <s.temerkhanov@gmail.com>
>>>>> ---
>>>>> MdePkg/Include/Library/UefiLib.h | 2 +-
>>>>> MdePkg/Library/UefiLib/UefiLib.c | 2 +-
>>>>> 2 files changed, 2 insertions(+), 2 deletions(-)
>>>>> 
>>>>> diff --git a/MdePkg/Include/Library/UefiLib.h b/MdePkg/Include/Library/UefiLib.h
>>>>> index 0b14792..4e4697c 100644
>>>>> --- a/MdePkg/Include/Library/UefiLib.h
>>>>> +++ b/MdePkg/Include/Library/UefiLib.h
>>>>> @@ -818,7 +818,7 @@ CHAR8 *
>>>>> EFIAPI
>>>>> GetBestLanguage (
>>>>>   IN CONST CHAR8  *SupportedLanguages,
>>>>> -  IN BOOLEAN      Iso639Language,
>>>>> +  IN UINTN      Iso639Language,
>>>>>   ...
>>>>>   );
>>>>> 
>>>>> diff --git a/MdePkg/Library/UefiLib/UefiLib.c b/MdePkg/Library/UefiLib/UefiLib.c
>>>>> index a7eee01..74528ec 100644
>>>>> --- a/MdePkg/Library/UefiLib/UefiLib.c
>>>>> +++ b/MdePkg/Library/UefiLib/UefiLib.c
>>>>> @@ -1514,7 +1514,7 @@ CHAR8 *
>>>>> EFIAPI
>>>>> GetBestLanguage (
>>>>>   IN CONST CHAR8  *SupportedLanguages,
>>>>> -  IN BOOLEAN      Iso639Language,
>>>>> +  IN UINTN      Iso639Language,
>>>>>   ...
>>>>>   )
>>>>> {
>>>>> --
>>>>> 2.7.4
>>>>> 
>>>>> _______________________________________________
>>>>> edk2-devel mailing list
>>>>> edk2-devel@lists.01.org
>>>>> https://lists.01.org/mailman/listinfo/edk2-devel
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel



  reply	other threads:[~2017-05-18  3:06 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-05-16  2:56 [PATCH] Arm: GICv3: Don't access GIC_ICDIPR for interrupts 0..31 Sergey Temerkhanov
2017-05-16  2:56 ` [PATCH] MdePkg: Fix undefined behavior on variadic parameters Sergey Temerkhanov
2017-05-16  5:10   ` Gao, Liming
2017-05-16 12:10     ` Sergei Temerkhanov
2017-05-17 15:30       ` Gao, Liming
2017-05-18  0:26         ` Sergei Temerkhanov
2017-05-18  3:06           ` Andrew Fish [this message]
2017-05-19  3:01             ` Sergei Temerkhanov
2017-05-18 10:19       ` Laszlo Ersek
2017-05-19  1:35         ` Gao, Liming
2017-05-19  8:12           ` Laszlo Ersek
2017-05-19  2:45         ` Sergei Temerkhanov
2017-05-19  8:16           ` Laszlo Ersek
2017-05-24  2:14             ` Gao, Liming
2017-05-24  2:46               ` Kinney, Michael D
2017-05-24 11:15                 ` Sergei Temerkhanov
2017-05-16  2:56 ` [PATCH] MdeModulePkg: " Sergey Temerkhanov
2017-05-18 16:08 ` [PATCH] Arm: GICv3: Don't access GIC_ICDIPR for interrupts 0..31 Ard Biesheuvel
2017-05-19  2:37   ` Sergei Temerkhanov
2017-05-19 10:26     ` Leif Lindholm
2017-05-19 14:31       ` Sergei Temerkhanov
2017-05-22 17:45         ` Leif Lindholm
2017-05-23 10:23           ` Ard Biesheuvel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=D2558173-7A3C-4DF4-9990-58785DB04078@apple.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox