1. Patchew
  2. EDK2
  3. View series

[edk2-devel] [PATCH] MdePkg Base.h: Delete prototype for __builtin_return_address

Jessica Clarke posted 1 patch 3 years, 9 months ago
Failed in applying to current master (apply log)
There is a newer version of this series
MdePkg/Include/Base.h | 1 -
1 file changed, 1 deletion(-)
[edk2-devel] [PATCH] MdePkg Base.h: Delete prototype for __builtin_return_address
Posted by Jessica Clarke 3 years, 9 months ago 
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1004

Being a compiler builtin, the type of __builtin_return_address is
already known to the compiler so no prototype is needed. Clang also
errors out when redeclaring certain builtins like this[1], though
currently only for ones with custom type checking. At the moment,
__builtin_return_address does not use custom type checking and so does
not trigger this error, however, the CHERI fork of LLVM, which will form
the basis of the toolchain for Arm's experimental Morello platform, does
use custom type checking for it, and so gives an error. Thus, simply
delete the unnecessary line.

[1] https://github.com/llvm/llvm-project/commit/41af97137572ad6d4dafc872e7ecf6bbb08d4984

Cc: Leif Lindholm <leif@nuviainc.com>
Signed-off-by: Jessica Clarke <jrtc27@jrtc27.com>
---
 MdePkg/Include/Base.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/MdePkg/Include/Base.h b/MdePkg/Include/Base.h
index 85a091b9d5..8e4271f6ea 100644
--- a/MdePkg/Include/Base.h
+++ b/MdePkg/Include/Base.h
@@ -1273,7 +1273,6 @@ typedef UINTN RETURN_STATUS;
   **/
   #define RETURN_ADDRESS(L)     ((L == 0) ? _ReturnAddress() : (VOID *) 0)
 #elif defined (__GNUC__) || defined (__clang__)
-  void * __builtin_return_address (unsigned int level);
   /**
     Get the return address of the calling function.
 
-- 
2.20.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#62840): https://edk2.groups.io/g/devel/message/62840
Mute This Topic: https://groups.io/mt/75682100/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] MdePkg Base.h: Delete prototype for __builtin_return_address
Posted by Leif Lindholm 3 years, 9 months ago 
+Mike, Liming

On Mon, Jul 20, 2020 at 14:49:46 +0100, Jessica Clarke wrote:
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1004
> 
> Being a compiler builtin, the type of __builtin_return_address is
> already known to the compiler so no prototype is needed. Clang also
> errors out when redeclaring certain builtins like this[1], though
> currently only for ones with custom type checking. At the moment,
> __builtin_return_address does not use custom type checking and so does
> not trigger this error, however, the CHERI fork of LLVM, which will form
> the basis of the toolchain for Arm's experimental Morello platform, does
> use custom type checking for it, and so gives an error. Thus, simply
> delete the unnecessary line.
> 
> [1] https://github.com/llvm/llvm-project/commit/41af97137572ad6d4dafc872e7ecf6bbb08d4984
> 
> Cc: Leif Lindholm <leif@nuviainc.com>
> Signed-off-by: Jessica Clarke <jrtc27@jrtc27.com>
> ---
>  MdePkg/Include/Base.h | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/MdePkg/Include/Base.h b/MdePkg/Include/Base.h
> index 85a091b9d5..8e4271f6ea 100644
> --- a/MdePkg/Include/Base.h
> +++ b/MdePkg/Include/Base.h
> @@ -1273,7 +1273,6 @@ typedef UINTN RETURN_STATUS;
>    **/
>    #define RETURN_ADDRESS(L)     ((L == 0) ? _ReturnAddress() : (VOID *) 0)
>  #elif defined (__GNUC__) || defined (__clang__)
> -  void * __builtin_return_address (unsigned int level);

Agreed this looks somewhat bonkers.
And I can't see any ill effects from dropping it, so:
Reviewed-by: Leif Lindholm <leif@nuviainc.com>

>    /**
>      Get the return address of the calling function.
>  
> -- 
> 2.20.1
> 

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#62839): https://edk2.groups.io/g/devel/message/62839
Mute This Topic: https://groups.io/mt/75682100/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] MdePkg Base.h: Delete prototype for __builtin_return_address
Posted by Liming Gao 3 years, 9 months ago 
Clarke:
  Do you mean CLANG compiler may have the different prototype for __builtin_return_address()? If so, dose __builtin_return_address (L) always work? 

Thanks
Liming
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Leif Lindholm
> Sent: Monday, July 20, 2020 10:05 PM
> To: Jessica Clarke <jrtc27@jrtc27.com>
> Cc: devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>
> Subject: Re: [edk2-devel] [PATCH] MdePkg Base.h: Delete prototype for __builtin_return_address
> 
> +Mike, Liming
> 
> On Mon, Jul 20, 2020 at 14:49:46 +0100, Jessica Clarke wrote:
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1004
> >
> > Being a compiler builtin, the type of __builtin_return_address is
> > already known to the compiler so no prototype is needed. Clang also
> > errors out when redeclaring certain builtins like this[1], though
> > currently only for ones with custom type checking. At the moment,
> > __builtin_return_address does not use custom type checking and so does
> > not trigger this error, however, the CHERI fork of LLVM, which will form
> > the basis of the toolchain for Arm's experimental Morello platform, does
> > use custom type checking for it, and so gives an error. Thus, simply
> > delete the unnecessary line.
> >
> > [1] https://github.com/llvm/llvm-project/commit/41af97137572ad6d4dafc872e7ecf6bbb08d4984
> >
> > Cc: Leif Lindholm <leif@nuviainc.com>
> > Signed-off-by: Jessica Clarke <jrtc27@jrtc27.com>
> > ---
> >  MdePkg/Include/Base.h | 1 -
> >  1 file changed, 1 deletion(-)
> >
> > diff --git a/MdePkg/Include/Base.h b/MdePkg/Include/Base.h
> > index 85a091b9d5..8e4271f6ea 100644
> > --- a/MdePkg/Include/Base.h
> > +++ b/MdePkg/Include/Base.h
> > @@ -1273,7 +1273,6 @@ typedef UINTN RETURN_STATUS;
> >    **/
> >    #define RETURN_ADDRESS(L)     ((L == 0) ? _ReturnAddress() : (VOID *) 0)
> >  #elif defined (__GNUC__) || defined (__clang__)
> > -  void * __builtin_return_address (unsigned int level);
> 
> Agreed this looks somewhat bonkers.
> And I can't see any ill effects from dropping it, so:
> Reviewed-by: Leif Lindholm <leif@nuviainc.com>
> 
> >    /**
> >      Get the return address of the calling function.
> >
> > --
> > 2.20.1
> >
> 
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#62845): https://edk2.groups.io/g/devel/message/62845
Mute This Topic: https://groups.io/mt/75682100/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] MdePkg Base.h: Delete prototype for __builtin_return_address
Posted by Jessica Clarke 3 years, 9 months ago 
It will always look like void *__builtin_return_address(unsigned) and
work like that, just that under the hood it's being implemented
slightly differently, which has the unfortunate side-effect of
triggering this bug. We've been compiling the embedded EDK2 in our
CheriBSD with that line deleted for years now with no issue.

Jess

> On 20 Jul 2020, at 15:58, Gao, Liming <liming.gao@intel.com> wrote:
> 
> Clarke:
>  Do you mean CLANG compiler may have the different prototype for __builtin_return_address()? If so, dose __builtin_return_address (L) always work? 
> 
> Thanks
> Liming
>> -----Original Message-----
>> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Leif Lindholm
>> Sent: Monday, July 20, 2020 10:05 PM
>> To: Jessica Clarke <jrtc27@jrtc27.com>
>> Cc: devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>
>> Subject: Re: [edk2-devel] [PATCH] MdePkg Base.h: Delete prototype for __builtin_return_address
>> 
>> +Mike, Liming
>> 
>> On Mon, Jul 20, 2020 at 14:49:46 +0100, Jessica Clarke wrote:
>>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1004
>>> 
>>> Being a compiler builtin, the type of __builtin_return_address is
>>> already known to the compiler so no prototype is needed. Clang also
>>> errors out when redeclaring certain builtins like this[1], though
>>> currently only for ones with custom type checking. At the moment,
>>> __builtin_return_address does not use custom type checking and so does
>>> not trigger this error, however, the CHERI fork of LLVM, which will form
>>> the basis of the toolchain for Arm's experimental Morello platform, does
>>> use custom type checking for it, and so gives an error. Thus, simply
>>> delete the unnecessary line.
>>> 
>>> [1] https://github.com/llvm/llvm-project/commit/41af97137572ad6d4dafc872e7ecf6bbb08d4984
>>> 
>>> Cc: Leif Lindholm <leif@nuviainc.com>
>>> Signed-off-by: Jessica Clarke <jrtc27@jrtc27.com>
>>> ---
>>> MdePkg/Include/Base.h | 1 -
>>> 1 file changed, 1 deletion(-)
>>> 
>>> diff --git a/MdePkg/Include/Base.h b/MdePkg/Include/Base.h
>>> index 85a091b9d5..8e4271f6ea 100644
>>> --- a/MdePkg/Include/Base.h
>>> +++ b/MdePkg/Include/Base.h
>>> @@ -1273,7 +1273,6 @@ typedef UINTN RETURN_STATUS;
>>>   **/
>>>   #define RETURN_ADDRESS(L)     ((L == 0) ? _ReturnAddress() : (VOID *) 0)
>>> #elif defined (__GNUC__) || defined (__clang__)
>>> -  void * __builtin_return_address (unsigned int level);
>> 
>> Agreed this looks somewhat bonkers.
>> And I can't see any ill effects from dropping it, so:
>> Reviewed-by: Leif Lindholm <leif@nuviainc.com>
>> 
>>>   /**
>>>     Get the return address of the calling function.
>>> 
>>> --
>>> 2.20.1
>>> 
>> 
>> 
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#62846): https://edk2.groups.io/g/devel/message/62846
Mute This Topic: https://groups.io/mt/75682100/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] MdePkg Base.h: Delete prototype for __builtin_return_address
Posted by Liming Gao 3 years, 9 months ago 
Jess:
  OK. Seemly, there is no impact with this change. Reviewed-by: Liming Gao <liming.gao@intel.com>

Thanks
Liming
-----Original Message-----
From: Jessica Clarke <jrtc27@jrtc27.com> 
Sent: 2020年7月20日 23:06
To: Gao, Liming <liming.gao@intel.com>
Cc: devel@edk2.groups.io; leif@nuviainc.com; Kinney, Michael D <michael.d.kinney@intel.com>
Subject: Re: [edk2-devel] [PATCH] MdePkg Base.h: Delete prototype for __builtin_return_address

It will always look like void *__builtin_return_address(unsigned) and work like that, just that under the hood it's being implemented slightly differently, which has the unfortunate side-effect of triggering this bug. We've been compiling the embedded EDK2 in our CheriBSD with that line deleted for years now with no issue.

Jess

> On 20 Jul 2020, at 15:58, Gao, Liming <liming.gao@intel.com> wrote:
> 
> Clarke:
>  Do you mean CLANG compiler may have the different prototype for __builtin_return_address()? If so, dose __builtin_return_address (L) always work? 
> 
> Thanks
> Liming
>> -----Original Message-----
>> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Leif 
>> Lindholm
>> Sent: Monday, July 20, 2020 10:05 PM
>> To: Jessica Clarke <jrtc27@jrtc27.com>
>> Cc: devel@edk2.groups.io; Kinney, Michael D 
>> <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>
>> Subject: Re: [edk2-devel] [PATCH] MdePkg Base.h: Delete prototype for 
>> __builtin_return_address
>> 
>> +Mike, Liming
>> 
>> On Mon, Jul 20, 2020 at 14:49:46 +0100, Jessica Clarke wrote:
>>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1004
>>> 
>>> Being a compiler builtin, the type of __builtin_return_address is 
>>> already known to the compiler so no prototype is needed. Clang also 
>>> errors out when redeclaring certain builtins like this[1], though 
>>> currently only for ones with custom type checking. At the moment, 
>>> __builtin_return_address does not use custom type checking and so 
>>> does not trigger this error, however, the CHERI fork of LLVM, which 
>>> will form the basis of the toolchain for Arm's experimental Morello 
>>> platform, does use custom type checking for it, and so gives an 
>>> error. Thus, simply delete the unnecessary line.
>>> 
>>> [1] 
>>> https://github.com/llvm/llvm-project/commit/41af97137572ad6d4dafc872
>>> e7ecf6bbb08d4984
>>> 
>>> Cc: Leif Lindholm <leif@nuviainc.com>
>>> Signed-off-by: Jessica Clarke <jrtc27@jrtc27.com>
>>> ---
>>> MdePkg/Include/Base.h | 1 -
>>> 1 file changed, 1 deletion(-)
>>> 
>>> diff --git a/MdePkg/Include/Base.h b/MdePkg/Include/Base.h index 
>>> 85a091b9d5..8e4271f6ea 100644
>>> --- a/MdePkg/Include/Base.h
>>> +++ b/MdePkg/Include/Base.h
>>> @@ -1273,7 +1273,6 @@ typedef UINTN RETURN_STATUS;
>>>   **/
>>>   #define RETURN_ADDRESS(L)     ((L == 0) ? _ReturnAddress() : (VOID *) 0)
>>> #elif defined (__GNUC__) || defined (__clang__)
>>> -  void * __builtin_return_address (unsigned int level);
>> 
>> Agreed this looks somewhat bonkers.
>> And I can't see any ill effects from dropping it, so:
>> Reviewed-by: Leif Lindholm <leif@nuviainc.com>
>> 
>>>   /**
>>>     Get the return address of the calling function.
>>> 
>>> --
>>> 2.20.1
>>> 
>> 
>> 
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#62939): https://edk2.groups.io/g/devel/message/62939
Mute This Topic: https://groups.io/mt/75682100/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] MdePkg Base.h: Delete prototype for __builtin_return_address
Posted by Zhiguang Liu 3 years, 9 months ago 
Reviewed-by: Zhiguang Liu <zhiguang.liu@intel.com>

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Leif
> Lindholm
> Sent: Monday, July 20, 2020 10:05 PM
> To: Jessica Clarke <jrtc27@jrtc27.com>
> Cc: devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@intel.com>;
> Gao, Liming <liming.gao@intel.com>
> Subject: Re: [edk2-devel] [PATCH] MdePkg Base.h: Delete prototype for
> __builtin_return_address
> 
> +Mike, Liming
> 
> On Mon, Jul 20, 2020 at 14:49:46 +0100, Jessica Clarke wrote:
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1004
> >
> > Being a compiler builtin, the type of __builtin_return_address is
> > already known to the compiler so no prototype is needed. Clang also
> > errors out when redeclaring certain builtins like this[1], though
> > currently only for ones with custom type checking. At the moment,
> > __builtin_return_address does not use custom type checking and so does
> > not trigger this error, however, the CHERI fork of LLVM, which will
> > form the basis of the toolchain for Arm's experimental Morello
> > platform, does use custom type checking for it, and so gives an error.
> > Thus, simply delete the unnecessary line.
> >
> > [1]
> > https://github.com/llvm/llvm-
> project/commit/41af97137572ad6d4dafc872e7
> > ecf6bbb08d4984
> >
> > Cc: Leif Lindholm <leif@nuviainc.com>
> > Signed-off-by: Jessica Clarke <jrtc27@jrtc27.com>
> > ---
> >  MdePkg/Include/Base.h | 1 -
> >  1 file changed, 1 deletion(-)
> >
> > diff --git a/MdePkg/Include/Base.h b/MdePkg/Include/Base.h index
> > 85a091b9d5..8e4271f6ea 100644
> > --- a/MdePkg/Include/Base.h
> > +++ b/MdePkg/Include/Base.h
> > @@ -1273,7 +1273,6 @@ typedef UINTN RETURN_STATUS;
> >    **/
> >    #define RETURN_ADDRESS(L)     ((L == 0) ? _ReturnAddress() : (VOID *) 0)
> >  #elif defined (__GNUC__) || defined (__clang__)
> > -  void * __builtin_return_address (unsigned int level);
> 
> Agreed this looks somewhat bonkers.
> And I can't see any ill effects from dropping it, so:
> Reviewed-by: Leif Lindholm <leif@nuviainc.com>
> 
> >    /**
> >      Get the return address of the calling function.
> >
> > --
> > 2.20.1
> >
> 
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#62936): https://edk2.groups.io/g/devel/message/62936
Mute This Topic: https://groups.io/mt/75682100/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.