public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH] BaseTools/CommonLib: drop the use of MAX_ADDRESS
@ 2018-12-05  8:24 Ard Biesheuvel
  2018-12-05 12:08 ` Philippe Mathieu-Daudé
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Ard Biesheuvel @ 2018-12-05  8:24 UTC (permalink / raw)
  To: edk2-devel; +Cc: lersek, Ard Biesheuvel, Bob Feng, Liming Gao, Yonghong Zhu

The macro MAX_ADDRESS represents the largest virtual address that
is valid for a certain architecture. For the BaseTools, this quantity
is irrelevant, since the same tools can be used to build for different
targets.

Since we only refer to it in a single place, which is an ASSERT() that
doesn't seem particularly useful (it ensures that memcpy() will not
be called with arguments that will make it read beyond the end of the
address space and wrap around), let's drop the ASSERT and all references
to MAX_ADDRESS.

Cc: Bob Feng <bob.c.feng@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 BaseTools/Source/C/Include/AArch64/ProcessorBind.h | 5 -----
 BaseTools/Source/C/Include/Arm/ProcessorBind.h     | 5 -----
 BaseTools/Source/C/Include/Common/UefiBaseTypes.h  | 1 -
 BaseTools/Source/C/Include/Ia32/ProcessorBind.h    | 5 -----
 BaseTools/Source/C/Include/X64/ProcessorBind.h     | 5 -----
 BaseTools/Source/C/Common/CommonLib.c              | 1 -
 6 files changed, 22 deletions(-)

diff --git a/BaseTools/Source/C/Include/AArch64/ProcessorBind.h b/BaseTools/Source/C/Include/AArch64/ProcessorBind.h
index e7e9d83198a6..f956cab453f0 100644
--- a/BaseTools/Source/C/Include/AArch64/ProcessorBind.h
+++ b/BaseTools/Source/C/Include/AArch64/ProcessorBind.h
@@ -90,11 +90,6 @@ typedef INT64   INTN;
 ///
 #define MAX_2_BITS   0xC000000000000000
 
-///
-/// Maximum legal AARCH64  address
-///
-#define MAX_ADDRESS  0xFFFFFFFFFFFFFFFF
-
 ///
 /// The stack alignment required for AARCH64
 ///
diff --git a/BaseTools/Source/C/Include/Arm/ProcessorBind.h b/BaseTools/Source/C/Include/Arm/ProcessorBind.h
index be4aac97664d..856d2bd9eff7 100644
--- a/BaseTools/Source/C/Include/Arm/ProcessorBind.h
+++ b/BaseTools/Source/C/Include/Arm/ProcessorBind.h
@@ -88,11 +88,6 @@ typedef INT32   INTN;
 ///
 #define MAX_2_BITS   0xC0000000
 
-///
-/// Maximum legal ARM address
-///
-#define MAX_ADDRESS  0xFFFFFFFF
-
 ///
 /// The stack alignment required for ARM
 ///
diff --git a/BaseTools/Source/C/Include/Common/UefiBaseTypes.h b/BaseTools/Source/C/Include/Common/UefiBaseTypes.h
index aa1aef3ce638..696ac15e4cd5 100644
--- a/BaseTools/Source/C/Include/Common/UefiBaseTypes.h
+++ b/BaseTools/Source/C/Include/Common/UefiBaseTypes.h
@@ -170,6 +170,5 @@ typedef union {
 
 
 #define EFI_MAX_BIT               MAX_BIT
-#define EFI_MAX_ADDRESS           MAX_ADDRESS
 
 #endif
diff --git a/BaseTools/Source/C/Include/Ia32/ProcessorBind.h b/BaseTools/Source/C/Include/Ia32/ProcessorBind.h
index 4719b53d37fa..96ac691df042 100644
--- a/BaseTools/Source/C/Include/Ia32/ProcessorBind.h
+++ b/BaseTools/Source/C/Include/Ia32/ProcessorBind.h
@@ -131,11 +131,6 @@ typedef INT32   INTN;
 #define MAX_BIT     0x80000000
 #define MAX_2_BITS  0xC0000000
 
-//
-// Maximum legal IA-32 address
-//
-#define MAX_ADDRESS   0xFFFFFFFF
-
 //
 // Modifier to ensure that all protocol member functions and EFI intrinsics
 // use the correct C calling convention. All protocol member functions and
diff --git a/BaseTools/Source/C/Include/X64/ProcessorBind.h b/BaseTools/Source/C/Include/X64/ProcessorBind.h
index c625f8cef4a1..987738508333 100644
--- a/BaseTools/Source/C/Include/X64/ProcessorBind.h
+++ b/BaseTools/Source/C/Include/X64/ProcessorBind.h
@@ -150,11 +150,6 @@ typedef INT64   INTN;
 #define MAX_BIT     0x8000000000000000ULL
 #define MAX_2_BITS  0xC000000000000000ULL
 
-//
-// Maximum legal Itanium-based address
-//
-#define MAX_ADDRESS   0xFFFFFFFFFFFFFFFFULL
-
 //
 // Modifier to ensure that all protocol member functions and EFI intrinsics
 // use the correct C calling convention. All protocol member functions and
diff --git a/BaseTools/Source/C/Common/CommonLib.c b/BaseTools/Source/C/Common/CommonLib.c
index 42dfa821624d..5c40fdb5fd49 100644
--- a/BaseTools/Source/C/Common/CommonLib.c
+++ b/BaseTools/Source/C/Common/CommonLib.c
@@ -1236,7 +1236,6 @@ InternalAllocateCopyPool (
   VOID  *Memory;
 
   ASSERT (Buffer != NULL);
-  ASSERT (AllocationSize <= (MAX_ADDRESS - (UINTN) Buffer + 1));
 
   Memory = malloc (AllocationSize);
   if (Memory != NULL) {
-- 
2.19.2



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] BaseTools/CommonLib: drop the use of MAX_ADDRESS
  2018-12-05  8:24 [PATCH] BaseTools/CommonLib: drop the use of MAX_ADDRESS Ard Biesheuvel
@ 2018-12-05 12:08 ` Philippe Mathieu-Daudé
  2018-12-05 12:16 ` Laszlo Ersek
  2018-12-05 23:56 ` Gao, Liming
  2 siblings, 0 replies; 5+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-12-05 12:08 UTC (permalink / raw)
  To: Ard Biesheuvel, edk2-devel; +Cc: lersek, Liming Gao

On 5/12/18 9:24, Ard Biesheuvel wrote:
> The macro MAX_ADDRESS represents the largest virtual address that
> is valid for a certain architecture. For the BaseTools, this quantity
> is irrelevant, since the same tools can be used to build for different
> targets.
> 
> Since we only refer to it in a single place, which is an ASSERT() that
> doesn't seem particularly useful (it ensures that memcpy() will not
> be called with arguments that will make it read beyond the end of the
> address space and wrap around), let's drop the ASSERT and all references
> to MAX_ADDRESS.

This looks to me best this way, thanks for cleaning this.

> 
> Cc: Bob Feng <bob.c.feng@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Yonghong Zhu <yonghong.zhu@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
>  BaseTools/Source/C/Include/AArch64/ProcessorBind.h | 5 -----
>  BaseTools/Source/C/Include/Arm/ProcessorBind.h     | 5 -----
>  BaseTools/Source/C/Include/Common/UefiBaseTypes.h  | 1 -
>  BaseTools/Source/C/Include/Ia32/ProcessorBind.h    | 5 -----
>  BaseTools/Source/C/Include/X64/ProcessorBind.h     | 5 -----
>  BaseTools/Source/C/Common/CommonLib.c              | 1 -
>  6 files changed, 22 deletions(-)
> 
> diff --git a/BaseTools/Source/C/Include/AArch64/ProcessorBind.h b/BaseTools/Source/C/Include/AArch64/ProcessorBind.h
> index e7e9d83198a6..f956cab453f0 100644
> --- a/BaseTools/Source/C/Include/AArch64/ProcessorBind.h
> +++ b/BaseTools/Source/C/Include/AArch64/ProcessorBind.h
> @@ -90,11 +90,6 @@ typedef INT64   INTN;
>  ///
>  #define MAX_2_BITS   0xC000000000000000
>  
> -///
> -/// Maximum legal AARCH64  address
> -///
> -#define MAX_ADDRESS  0xFFFFFFFFFFFFFFFF
> -
>  ///
>  /// The stack alignment required for AARCH64
>  ///
> diff --git a/BaseTools/Source/C/Include/Arm/ProcessorBind.h b/BaseTools/Source/C/Include/Arm/ProcessorBind.h
> index be4aac97664d..856d2bd9eff7 100644
> --- a/BaseTools/Source/C/Include/Arm/ProcessorBind.h
> +++ b/BaseTools/Source/C/Include/Arm/ProcessorBind.h
> @@ -88,11 +88,6 @@ typedef INT32   INTN;
>  ///
>  #define MAX_2_BITS   0xC0000000
>  
> -///
> -/// Maximum legal ARM address
> -///
> -#define MAX_ADDRESS  0xFFFFFFFF
> -
>  ///
>  /// The stack alignment required for ARM
>  ///
> diff --git a/BaseTools/Source/C/Include/Common/UefiBaseTypes.h b/BaseTools/Source/C/Include/Common/UefiBaseTypes.h
> index aa1aef3ce638..696ac15e4cd5 100644
> --- a/BaseTools/Source/C/Include/Common/UefiBaseTypes.h
> +++ b/BaseTools/Source/C/Include/Common/UefiBaseTypes.h
> @@ -170,6 +170,5 @@ typedef union {
>  
>  
>  #define EFI_MAX_BIT               MAX_BIT
> -#define EFI_MAX_ADDRESS           MAX_ADDRESS
>  
>  #endif
> diff --git a/BaseTools/Source/C/Include/Ia32/ProcessorBind.h b/BaseTools/Source/C/Include/Ia32/ProcessorBind.h
> index 4719b53d37fa..96ac691df042 100644
> --- a/BaseTools/Source/C/Include/Ia32/ProcessorBind.h
> +++ b/BaseTools/Source/C/Include/Ia32/ProcessorBind.h
> @@ -131,11 +131,6 @@ typedef INT32   INTN;
>  #define MAX_BIT     0x80000000
>  #define MAX_2_BITS  0xC0000000
>  
> -//
> -// Maximum legal IA-32 address
> -//
> -#define MAX_ADDRESS   0xFFFFFFFF
> -
>  //
>  // Modifier to ensure that all protocol member functions and EFI intrinsics
>  // use the correct C calling convention. All protocol member functions and
> diff --git a/BaseTools/Source/C/Include/X64/ProcessorBind.h b/BaseTools/Source/C/Include/X64/ProcessorBind.h
> index c625f8cef4a1..987738508333 100644
> --- a/BaseTools/Source/C/Include/X64/ProcessorBind.h
> +++ b/BaseTools/Source/C/Include/X64/ProcessorBind.h
> @@ -150,11 +150,6 @@ typedef INT64   INTN;
>  #define MAX_BIT     0x8000000000000000ULL
>  #define MAX_2_BITS  0xC000000000000000ULL
>  
> -//
> -// Maximum legal Itanium-based address
> -//
> -#define MAX_ADDRESS   0xFFFFFFFFFFFFFFFFULL
> -
>  //
>  // Modifier to ensure that all protocol member functions and EFI intrinsics
>  // use the correct C calling convention. All protocol member functions and
> diff --git a/BaseTools/Source/C/Common/CommonLib.c b/BaseTools/Source/C/Common/CommonLib.c
> index 42dfa821624d..5c40fdb5fd49 100644
> --- a/BaseTools/Source/C/Common/CommonLib.c
> +++ b/BaseTools/Source/C/Common/CommonLib.c
> @@ -1236,7 +1236,6 @@ InternalAllocateCopyPool (
>    VOID  *Memory;
>  
>    ASSERT (Buffer != NULL);
> -  ASSERT (AllocationSize <= (MAX_ADDRESS - (UINTN) Buffer + 1));
>  
>    Memory = malloc (AllocationSize);
>    if (Memory != NULL) {
> 


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] BaseTools/CommonLib: drop the use of MAX_ADDRESS
  2018-12-05  8:24 [PATCH] BaseTools/CommonLib: drop the use of MAX_ADDRESS Ard Biesheuvel
  2018-12-05 12:08 ` Philippe Mathieu-Daudé
@ 2018-12-05 12:16 ` Laszlo Ersek
  2018-12-05 23:56 ` Gao, Liming
  2 siblings, 0 replies; 5+ messages in thread
From: Laszlo Ersek @ 2018-12-05 12:16 UTC (permalink / raw)
  To: Ard Biesheuvel, edk2-devel; +Cc: Bob Feng, Liming Gao, Yonghong Zhu

On 12/05/18 09:24, Ard Biesheuvel wrote:
> The macro MAX_ADDRESS represents the largest virtual address that
> is valid for a certain architecture. For the BaseTools, this quantity
> is irrelevant, since the same tools can be used to build for different
> targets.
> 
> Since we only refer to it in a single place, which is an ASSERT() that
> doesn't seem particularly useful (it ensures that memcpy() will not
> be called with arguments that will make it read beyond the end of the
> address space and wrap around), let's drop the ASSERT and all references
> to MAX_ADDRESS.
> 
> Cc: Bob Feng <bob.c.feng@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Yonghong Zhu <yonghong.zhu@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  BaseTools/Source/C/Include/AArch64/ProcessorBind.h | 5 -----
>  BaseTools/Source/C/Include/Arm/ProcessorBind.h     | 5 -----
>  BaseTools/Source/C/Include/Common/UefiBaseTypes.h  | 1 -
>  BaseTools/Source/C/Include/Ia32/ProcessorBind.h    | 5 -----
>  BaseTools/Source/C/Include/X64/ProcessorBind.h     | 5 -----
>  BaseTools/Source/C/Common/CommonLib.c              | 1 -
>  6 files changed, 22 deletions(-)
> 
> diff --git a/BaseTools/Source/C/Include/AArch64/ProcessorBind.h b/BaseTools/Source/C/Include/AArch64/ProcessorBind.h
> index e7e9d83198a6..f956cab453f0 100644
> --- a/BaseTools/Source/C/Include/AArch64/ProcessorBind.h
> +++ b/BaseTools/Source/C/Include/AArch64/ProcessorBind.h
> @@ -90,11 +90,6 @@ typedef INT64   INTN;
>  ///
>  #define MAX_2_BITS   0xC000000000000000
>  
> -///
> -/// Maximum legal AARCH64  address
> -///
> -#define MAX_ADDRESS  0xFFFFFFFFFFFFFFFF
> -
>  ///
>  /// The stack alignment required for AARCH64
>  ///
> diff --git a/BaseTools/Source/C/Include/Arm/ProcessorBind.h b/BaseTools/Source/C/Include/Arm/ProcessorBind.h
> index be4aac97664d..856d2bd9eff7 100644
> --- a/BaseTools/Source/C/Include/Arm/ProcessorBind.h
> +++ b/BaseTools/Source/C/Include/Arm/ProcessorBind.h
> @@ -88,11 +88,6 @@ typedef INT32   INTN;
>  ///
>  #define MAX_2_BITS   0xC0000000
>  
> -///
> -/// Maximum legal ARM address
> -///
> -#define MAX_ADDRESS  0xFFFFFFFF
> -
>  ///
>  /// The stack alignment required for ARM
>  ///
> diff --git a/BaseTools/Source/C/Include/Common/UefiBaseTypes.h b/BaseTools/Source/C/Include/Common/UefiBaseTypes.h
> index aa1aef3ce638..696ac15e4cd5 100644
> --- a/BaseTools/Source/C/Include/Common/UefiBaseTypes.h
> +++ b/BaseTools/Source/C/Include/Common/UefiBaseTypes.h
> @@ -170,6 +170,5 @@ typedef union {
>  
>  
>  #define EFI_MAX_BIT               MAX_BIT
> -#define EFI_MAX_ADDRESS           MAX_ADDRESS
>  
>  #endif
> diff --git a/BaseTools/Source/C/Include/Ia32/ProcessorBind.h b/BaseTools/Source/C/Include/Ia32/ProcessorBind.h
> index 4719b53d37fa..96ac691df042 100644
> --- a/BaseTools/Source/C/Include/Ia32/ProcessorBind.h
> +++ b/BaseTools/Source/C/Include/Ia32/ProcessorBind.h
> @@ -131,11 +131,6 @@ typedef INT32   INTN;
>  #define MAX_BIT     0x80000000
>  #define MAX_2_BITS  0xC0000000
>  
> -//
> -// Maximum legal IA-32 address
> -//
> -#define MAX_ADDRESS   0xFFFFFFFF
> -
>  //
>  // Modifier to ensure that all protocol member functions and EFI intrinsics
>  // use the correct C calling convention. All protocol member functions and
> diff --git a/BaseTools/Source/C/Include/X64/ProcessorBind.h b/BaseTools/Source/C/Include/X64/ProcessorBind.h
> index c625f8cef4a1..987738508333 100644
> --- a/BaseTools/Source/C/Include/X64/ProcessorBind.h
> +++ b/BaseTools/Source/C/Include/X64/ProcessorBind.h
> @@ -150,11 +150,6 @@ typedef INT64   INTN;
>  #define MAX_BIT     0x8000000000000000ULL
>  #define MAX_2_BITS  0xC000000000000000ULL
>  
> -//
> -// Maximum legal Itanium-based address
> -//
> -#define MAX_ADDRESS   0xFFFFFFFFFFFFFFFFULL
> -
>  //
>  // Modifier to ensure that all protocol member functions and EFI intrinsics
>  // use the correct C calling convention. All protocol member functions and
> diff --git a/BaseTools/Source/C/Common/CommonLib.c b/BaseTools/Source/C/Common/CommonLib.c
> index 42dfa821624d..5c40fdb5fd49 100644
> --- a/BaseTools/Source/C/Common/CommonLib.c
> +++ b/BaseTools/Source/C/Common/CommonLib.c
> @@ -1236,7 +1236,6 @@ InternalAllocateCopyPool (
>    VOID  *Memory;
>  
>    ASSERT (Buffer != NULL);
> -  ASSERT (AllocationSize <= (MAX_ADDRESS - (UINTN) Buffer + 1));
>  
>    Memory = malloc (AllocationSize);
>    if (Memory != NULL) {
> 

I agree; the malloc() on the next line will cover this.

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Thanks
Laszlo


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] BaseTools/CommonLib: drop the use of MAX_ADDRESS
  2018-12-05  8:24 [PATCH] BaseTools/CommonLib: drop the use of MAX_ADDRESS Ard Biesheuvel
  2018-12-05 12:08 ` Philippe Mathieu-Daudé
  2018-12-05 12:16 ` Laszlo Ersek
@ 2018-12-05 23:56 ` Gao, Liming
  2018-12-06  7:33   ` Ard Biesheuvel
  2 siblings, 1 reply; 5+ messages in thread
From: Gao, Liming @ 2018-12-05 23:56 UTC (permalink / raw)
  To: 'Ard Biesheuvel', edk2-devel@lists.01.org; +Cc: lersek@redhat.com

Reviewed-by: Liming Gao <liming.gao@intel.com>

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ard Biesheuvel
> Sent: Wednesday, December 5, 2018 4:24 PM
> To: edk2-devel@lists.01.org
> Cc: lersek@redhat.com; Gao, Liming <liming.gao@intel.com>
> Subject: [edk2] [PATCH] BaseTools/CommonLib: drop the use of MAX_ADDRESS
> 
> The macro MAX_ADDRESS represents the largest virtual address that
> is valid for a certain architecture. For the BaseTools, this quantity
> is irrelevant, since the same tools can be used to build for different
> targets.
> 
> Since we only refer to it in a single place, which is an ASSERT() that
> doesn't seem particularly useful (it ensures that memcpy() will not
> be called with arguments that will make it read beyond the end of the
> address space and wrap around), let's drop the ASSERT and all references
> to MAX_ADDRESS.
> 
> Cc: Bob Feng <bob.c.feng@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Yonghong Zhu <yonghong.zhu@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  BaseTools/Source/C/Include/AArch64/ProcessorBind.h | 5 -----
>  BaseTools/Source/C/Include/Arm/ProcessorBind.h     | 5 -----
>  BaseTools/Source/C/Include/Common/UefiBaseTypes.h  | 1 -
>  BaseTools/Source/C/Include/Ia32/ProcessorBind.h    | 5 -----
>  BaseTools/Source/C/Include/X64/ProcessorBind.h     | 5 -----
>  BaseTools/Source/C/Common/CommonLib.c              | 1 -
>  6 files changed, 22 deletions(-)
> 
> diff --git a/BaseTools/Source/C/Include/AArch64/ProcessorBind.h b/BaseTools/Source/C/Include/AArch64/ProcessorBind.h
> index e7e9d83198a6..f956cab453f0 100644
> --- a/BaseTools/Source/C/Include/AArch64/ProcessorBind.h
> +++ b/BaseTools/Source/C/Include/AArch64/ProcessorBind.h
> @@ -90,11 +90,6 @@ typedef INT64   INTN;
>  ///
>  #define MAX_2_BITS   0xC000000000000000
> 
> -///
> -/// Maximum legal AARCH64  address
> -///
> -#define MAX_ADDRESS  0xFFFFFFFFFFFFFFFF
> -
>  ///
>  /// The stack alignment required for AARCH64
>  ///
> diff --git a/BaseTools/Source/C/Include/Arm/ProcessorBind.h b/BaseTools/Source/C/Include/Arm/ProcessorBind.h
> index be4aac97664d..856d2bd9eff7 100644
> --- a/BaseTools/Source/C/Include/Arm/ProcessorBind.h
> +++ b/BaseTools/Source/C/Include/Arm/ProcessorBind.h
> @@ -88,11 +88,6 @@ typedef INT32   INTN;
>  ///
>  #define MAX_2_BITS   0xC0000000
> 
> -///
> -/// Maximum legal ARM address
> -///
> -#define MAX_ADDRESS  0xFFFFFFFF
> -
>  ///
>  /// The stack alignment required for ARM
>  ///
> diff --git a/BaseTools/Source/C/Include/Common/UefiBaseTypes.h b/BaseTools/Source/C/Include/Common/UefiBaseTypes.h
> index aa1aef3ce638..696ac15e4cd5 100644
> --- a/BaseTools/Source/C/Include/Common/UefiBaseTypes.h
> +++ b/BaseTools/Source/C/Include/Common/UefiBaseTypes.h
> @@ -170,6 +170,5 @@ typedef union {
> 
> 
>  #define EFI_MAX_BIT               MAX_BIT
> -#define EFI_MAX_ADDRESS           MAX_ADDRESS
> 
>  #endif
> diff --git a/BaseTools/Source/C/Include/Ia32/ProcessorBind.h b/BaseTools/Source/C/Include/Ia32/ProcessorBind.h
> index 4719b53d37fa..96ac691df042 100644
> --- a/BaseTools/Source/C/Include/Ia32/ProcessorBind.h
> +++ b/BaseTools/Source/C/Include/Ia32/ProcessorBind.h
> @@ -131,11 +131,6 @@ typedef INT32   INTN;
>  #define MAX_BIT     0x80000000
>  #define MAX_2_BITS  0xC0000000
> 
> -//
> -// Maximum legal IA-32 address
> -//
> -#define MAX_ADDRESS   0xFFFFFFFF
> -
>  //
>  // Modifier to ensure that all protocol member functions and EFI intrinsics
>  // use the correct C calling convention. All protocol member functions and
> diff --git a/BaseTools/Source/C/Include/X64/ProcessorBind.h b/BaseTools/Source/C/Include/X64/ProcessorBind.h
> index c625f8cef4a1..987738508333 100644
> --- a/BaseTools/Source/C/Include/X64/ProcessorBind.h
> +++ b/BaseTools/Source/C/Include/X64/ProcessorBind.h
> @@ -150,11 +150,6 @@ typedef INT64   INTN;
>  #define MAX_BIT     0x8000000000000000ULL
>  #define MAX_2_BITS  0xC000000000000000ULL
> 
> -//
> -// Maximum legal Itanium-based address
> -//
> -#define MAX_ADDRESS   0xFFFFFFFFFFFFFFFFULL
> -
>  //
>  // Modifier to ensure that all protocol member functions and EFI intrinsics
>  // use the correct C calling convention. All protocol member functions and
> diff --git a/BaseTools/Source/C/Common/CommonLib.c b/BaseTools/Source/C/Common/CommonLib.c
> index 42dfa821624d..5c40fdb5fd49 100644
> --- a/BaseTools/Source/C/Common/CommonLib.c
> +++ b/BaseTools/Source/C/Common/CommonLib.c
> @@ -1236,7 +1236,6 @@ InternalAllocateCopyPool (
>    VOID  *Memory;
> 
>    ASSERT (Buffer != NULL);
> -  ASSERT (AllocationSize <= (MAX_ADDRESS - (UINTN) Buffer + 1));
> 
>    Memory = malloc (AllocationSize);
>    if (Memory != NULL) {
> --
> 2.19.2
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] BaseTools/CommonLib: drop the use of MAX_ADDRESS
  2018-12-05 23:56 ` Gao, Liming
@ 2018-12-06  7:33   ` Ard Biesheuvel
  0 siblings, 0 replies; 5+ messages in thread
From: Ard Biesheuvel @ 2018-12-06  7:33 UTC (permalink / raw)
  To: Gao, Liming; +Cc: edk2-devel@lists.01.org, Laszlo Ersek

On Thu, 6 Dec 2018 at 00:56, Gao, Liming <liming.gao@intel.com> wrote:
>
> Reviewed-by: Liming Gao <liming.gao@intel.com>
>

Thanks all

Pushed as 6e8cad49a09d..67938bcc9d9e

> > -----Original Message-----
> > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ard Biesheuvel
> > Sent: Wednesday, December 5, 2018 4:24 PM
> > To: edk2-devel@lists.01.org
> > Cc: lersek@redhat.com; Gao, Liming <liming.gao@intel.com>
> > Subject: [edk2] [PATCH] BaseTools/CommonLib: drop the use of MAX_ADDRESS
> >
> > The macro MAX_ADDRESS represents the largest virtual address that
> > is valid for a certain architecture. For the BaseTools, this quantity
> > is irrelevant, since the same tools can be used to build for different
> > targets.
> >
> > Since we only refer to it in a single place, which is an ASSERT() that
> > doesn't seem particularly useful (it ensures that memcpy() will not
> > be called with arguments that will make it read beyond the end of the
> > address space and wrap around), let's drop the ASSERT and all references
> > to MAX_ADDRESS.
> >
> > Cc: Bob Feng <bob.c.feng@intel.com>
> > Cc: Liming Gao <liming.gao@intel.com>
> > Cc: Yonghong Zhu <yonghong.zhu@intel.com>
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > ---
> >  BaseTools/Source/C/Include/AArch64/ProcessorBind.h | 5 -----
> >  BaseTools/Source/C/Include/Arm/ProcessorBind.h     | 5 -----
> >  BaseTools/Source/C/Include/Common/UefiBaseTypes.h  | 1 -
> >  BaseTools/Source/C/Include/Ia32/ProcessorBind.h    | 5 -----
> >  BaseTools/Source/C/Include/X64/ProcessorBind.h     | 5 -----
> >  BaseTools/Source/C/Common/CommonLib.c              | 1 -
> >  6 files changed, 22 deletions(-)
> >
> > diff --git a/BaseTools/Source/C/Include/AArch64/ProcessorBind.h b/BaseTools/Source/C/Include/AArch64/ProcessorBind.h
> > index e7e9d83198a6..f956cab453f0 100644
> > --- a/BaseTools/Source/C/Include/AArch64/ProcessorBind.h
> > +++ b/BaseTools/Source/C/Include/AArch64/ProcessorBind.h
> > @@ -90,11 +90,6 @@ typedef INT64   INTN;
> >  ///
> >  #define MAX_2_BITS   0xC000000000000000
> >
> > -///
> > -/// Maximum legal AARCH64  address
> > -///
> > -#define MAX_ADDRESS  0xFFFFFFFFFFFFFFFF
> > -
> >  ///
> >  /// The stack alignment required for AARCH64
> >  ///
> > diff --git a/BaseTools/Source/C/Include/Arm/ProcessorBind.h b/BaseTools/Source/C/Include/Arm/ProcessorBind.h
> > index be4aac97664d..856d2bd9eff7 100644
> > --- a/BaseTools/Source/C/Include/Arm/ProcessorBind.h
> > +++ b/BaseTools/Source/C/Include/Arm/ProcessorBind.h
> > @@ -88,11 +88,6 @@ typedef INT32   INTN;
> >  ///
> >  #define MAX_2_BITS   0xC0000000
> >
> > -///
> > -/// Maximum legal ARM address
> > -///
> > -#define MAX_ADDRESS  0xFFFFFFFF
> > -
> >  ///
> >  /// The stack alignment required for ARM
> >  ///
> > diff --git a/BaseTools/Source/C/Include/Common/UefiBaseTypes.h b/BaseTools/Source/C/Include/Common/UefiBaseTypes.h
> > index aa1aef3ce638..696ac15e4cd5 100644
> > --- a/BaseTools/Source/C/Include/Common/UefiBaseTypes.h
> > +++ b/BaseTools/Source/C/Include/Common/UefiBaseTypes.h
> > @@ -170,6 +170,5 @@ typedef union {
> >
> >
> >  #define EFI_MAX_BIT               MAX_BIT
> > -#define EFI_MAX_ADDRESS           MAX_ADDRESS
> >
> >  #endif
> > diff --git a/BaseTools/Source/C/Include/Ia32/ProcessorBind.h b/BaseTools/Source/C/Include/Ia32/ProcessorBind.h
> > index 4719b53d37fa..96ac691df042 100644
> > --- a/BaseTools/Source/C/Include/Ia32/ProcessorBind.h
> > +++ b/BaseTools/Source/C/Include/Ia32/ProcessorBind.h
> > @@ -131,11 +131,6 @@ typedef INT32   INTN;
> >  #define MAX_BIT     0x80000000
> >  #define MAX_2_BITS  0xC0000000
> >
> > -//
> > -// Maximum legal IA-32 address
> > -//
> > -#define MAX_ADDRESS   0xFFFFFFFF
> > -
> >  //
> >  // Modifier to ensure that all protocol member functions and EFI intrinsics
> >  // use the correct C calling convention. All protocol member functions and
> > diff --git a/BaseTools/Source/C/Include/X64/ProcessorBind.h b/BaseTools/Source/C/Include/X64/ProcessorBind.h
> > index c625f8cef4a1..987738508333 100644
> > --- a/BaseTools/Source/C/Include/X64/ProcessorBind.h
> > +++ b/BaseTools/Source/C/Include/X64/ProcessorBind.h
> > @@ -150,11 +150,6 @@ typedef INT64   INTN;
> >  #define MAX_BIT     0x8000000000000000ULL
> >  #define MAX_2_BITS  0xC000000000000000ULL
> >
> > -//
> > -// Maximum legal Itanium-based address
> > -//
> > -#define MAX_ADDRESS   0xFFFFFFFFFFFFFFFFULL
> > -
> >  //
> >  // Modifier to ensure that all protocol member functions and EFI intrinsics
> >  // use the correct C calling convention. All protocol member functions and
> > diff --git a/BaseTools/Source/C/Common/CommonLib.c b/BaseTools/Source/C/Common/CommonLib.c
> > index 42dfa821624d..5c40fdb5fd49 100644
> > --- a/BaseTools/Source/C/Common/CommonLib.c
> > +++ b/BaseTools/Source/C/Common/CommonLib.c
> > @@ -1236,7 +1236,6 @@ InternalAllocateCopyPool (
> >    VOID  *Memory;
> >
> >    ASSERT (Buffer != NULL);
> > -  ASSERT (AllocationSize <= (MAX_ADDRESS - (UINTN) Buffer + 1));
> >
> >    Memory = malloc (AllocationSize);
> >    if (Memory != NULL) {
> > --
> > 2.19.2
> >
> > _______________________________________________
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2018-12-06  7:33 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-12-05  8:24 [PATCH] BaseTools/CommonLib: drop the use of MAX_ADDRESS Ard Biesheuvel
2018-12-05 12:08 ` Philippe Mathieu-Daudé
2018-12-05 12:16 ` Laszlo Ersek
2018-12-05 23:56 ` Gao, Liming
2018-12-06  7:33   ` Ard Biesheuvel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox