eal/rte_malloc: add alloc_size() attribute to allocation functions

Message ID 20200904221143.19744-1-stephen@networkplumber.org (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series eal/rte_malloc: add alloc_size() attribute to allocation functions |

Checks

Context Check Description
ci/Intel-compilation fail apply issues
ci/checkpatch warning coding style issues

Commit Message

Stephen Hemminger Sept. 4, 2020, 10:11 p.m. UTC
  By using the alloc_size() attribute the compiler can optimize
better and detect errors at compile time.

For example, Gcc will fail one of the invalid allocation examples
in app/test/test_malloc.c because the allocation is outside the
limits of memory.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 app/test/test_malloc.c              |  5 ++++-
 lib/librte_eal/include/rte_common.h |  7 ++++---
 lib/librte_eal/include/rte_malloc.h | 28 ++++++++++++++++------------
 3 files changed, 24 insertions(+), 16 deletions(-)
  

Comments

Burakov, Anatoly Sept. 14, 2020, 3:35 p.m. UTC | #1
On 04-Sep-20 11:11 PM, Stephen Hemminger wrote:
> By using the alloc_size() attribute the compiler can optimize
> better and detect errors at compile time.
> 
> For example, Gcc will fail one of the invalid allocation examples
> in app/test/test_malloc.c because the allocation is outside the
> limits of memory.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---

This is a very cool feature! LGTM

Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>

>   void *
> -rte_zmalloc_socket(const char *type, size_t size, unsigned align, int socket);
> +rte_zmalloc_socket(const char *type, size_t size, unsigned align, int socket)
> +	__rte_alloc_size(2);
>   
>   /**
>    * Replacement function for calloc(), using huge-page memory. Memory area is
> @@ -239,9 +243,9 @@ rte_zmalloc_socket(const char *type, size_t size, unsigned align, int socket);
>    *     align is not a power of two).
>    *   - Otherwise, the pointer to the allocated object.
>    */
> -__rte_malloc
>   void *

Where did this come from?

> -rte_calloc_socket(const char *type, size_t num, size_t size, unsigned align, int socket);
> +rte_calloc_socket(const char *type, size_t num, size_t size, unsigned align, int socket)
> +	__rte_alloc_size(2, 3);
>   
>   /**
>    * Frees the memory space pointed to by the provided pointer.
>
  
Stephen Hemminger Sept. 14, 2020, 4:40 p.m. UTC | #2
On Mon, 14 Sep 2020 16:35:58 +0100
"Burakov, Anatoly" <anatoly.burakov@intel.com> wrote:

> >    * Replacement function for calloc(), using huge-page memory. Memory area is
> > @@ -239,9 +243,9 @@ rte_zmalloc_socket(const char *type, size_t size, unsigned align, int socket);
> >    *     align is not a power of two).
> >    *   - Otherwise, the pointer to the allocated object.
> >    */
> > -__rte_malloc
> >   void *  
> 
> Where did this come from?

It was on an earlier patch, I based this on an abandoned tree, will rebase on main.
  
Thomas Monjalon Oct. 14, 2020, 11:02 p.m. UTC | #3
14/09/2020 18:40, Stephen Hemminger:
> On Mon, 14 Sep 2020 16:35:58 +0100
> "Burakov, Anatoly" <anatoly.burakov@intel.com> wrote:
> 
> > >    * Replacement function for calloc(), using huge-page memory. Memory area is
> > > @@ -239,9 +243,9 @@ rte_zmalloc_socket(const char *type, size_t size, unsigned align, int socket);
> > >    *     align is not a power of two).
> > >    *   - Otherwise, the pointer to the allocated object.
> > >    */
> > > -__rte_malloc
> > >   void *  
> > 
> > Where did this come from?
> 
> It was on an earlier patch, I based this on an abandoned tree, will rebase on main.

Will you send a new version?
  
Thomas Monjalon Oct. 19, 2020, 10:23 a.m. UTC | #4
14/09/2020 18:40, Stephen Hemminger:
> On Mon, 14 Sep 2020 16:35:58 +0100
> "Burakov, Anatoly" <anatoly.burakov@intel.com> wrote:
> 
> > >    * Replacement function for calloc(), using huge-page memory. Memory area is
> > > @@ -239,9 +243,9 @@ rte_zmalloc_socket(const char *type, size_t size, unsigned align, int socket);
> > >    *     align is not a power of two).
> > >    *   - Otherwise, the pointer to the allocated object.
> > >    */
> > > -__rte_malloc
> > >   void *  
> > 
> > Where did this come from?
> 
> It was on an earlier patch, I based this on an abandoned tree, will rebase on main.

Please could you rebase?
  

Patch

diff --git a/app/test/test_malloc.c b/app/test/test_malloc.c
index 71b3cfdde5cf..fdf77b4f6a14 100644
--- a/app/test/test_malloc.c
+++ b/app/test/test_malloc.c
@@ -846,6 +846,9 @@  test_malloc_bad_params(void)
 	if (bad_ptr != NULL)
 		goto err_return;
 
+#if defined(RTE_CC_GCC) || defined(RTE_CC_CLANG)
+	/* this test can not be built, will get trapped at compile time! */
+#else
 	/* rte_malloc expected to return null with size will cause overflow */
 	align = RTE_CACHE_LINE_SIZE;
 	size = (size_t)-8;
@@ -857,7 +860,7 @@  test_malloc_bad_params(void)
 	bad_ptr = rte_realloc(NULL, size, align);
 	if (bad_ptr != NULL)
 		goto err_return;
-
+#endif
 	return 0;
 
 err_return:
diff --git a/lib/librte_eal/include/rte_common.h b/lib/librte_eal/include/rte_common.h
index 6b85374c0fe5..0c2e03234d8d 100644
--- a/lib/librte_eal/include/rte_common.h
+++ b/lib/librte_eal/include/rte_common.h
@@ -117,10 +117,11 @@  typedef uint16_t unaligned_uint16_t;
  * cannot alias any other valid pointer and that the memory
  * contents are undefined. I.e behaves like malloc.
  */
-#if RTE_CC_IS_GNU
-#define __rte_malloc __attribute__((malloc))
+#if defined(RTE_CC_GCC) || defined(RTE_CC_CLANG)
+#define __rte_alloc_size(...) \
+	__attribute__((alloc_size(__VA_ARGS__)))
 #else
-#define __rte_malloc
+#define __rte_alloc_size(...)
 #endif
 
 /**
diff --git a/lib/librte_eal/include/rte_malloc.h b/lib/librte_eal/include/rte_malloc.h
index 721560122c70..3af64f87618e 100644
--- a/lib/librte_eal/include/rte_malloc.h
+++ b/lib/librte_eal/include/rte_malloc.h
@@ -54,7 +54,8 @@  struct rte_malloc_socket_stats {
  *   - Otherwise, the pointer to the allocated object.
  */
 void *
-rte_malloc(const char *type, size_t size, unsigned align);
+rte_malloc(const char *type, size_t size, unsigned align)
+	__rte_alloc_size(2);
 
 /**
  * Allocate zero'ed memory from the heap.
@@ -79,9 +80,9 @@  rte_malloc(const char *type, size_t size, unsigned align);
  *     align is not a power of two).
  *   - Otherwise, the pointer to the allocated object.
  */
-__rte_malloc
 void *
-rte_zmalloc(const char *type, size_t size, unsigned align);
+rte_zmalloc(const char *type, size_t size, unsigned align)
+	__rte_alloc_size(2);
 
 /**
  * Replacement function for calloc(), using huge-page memory. Memory area is
@@ -106,9 +107,9 @@  rte_zmalloc(const char *type, size_t size, unsigned align);
  *     align is not a power of two).
  *   - Otherwise, the pointer to the allocated object.
  */
-__rte_malloc
 void *
-rte_calloc(const char *type, size_t num, size_t size, unsigned align);
+rte_calloc(const char *type, size_t num, size_t size, unsigned align)
+	__rte_alloc_size(2, 3);
 
 /**
  * Replacement function for realloc(), using huge-page memory. Reserved area
@@ -131,7 +132,8 @@  rte_calloc(const char *type, size_t num, size_t size, unsigned align);
  *   - Otherwise, the pointer to the reallocated memory.
  */
 void *
-rte_realloc(void *ptr, size_t size, unsigned int align);
+rte_realloc(void *ptr, size_t size, unsigned int align)
+	__rte_alloc_size(2);
 
 /**
  * Replacement function for realloc(), using huge-page memory. Reserved area
@@ -157,7 +159,8 @@  rte_realloc(void *ptr, size_t size, unsigned int align);
  */
 __rte_experimental
 void *
-rte_realloc_socket(void *ptr, size_t size, unsigned int align, int socket);
+rte_realloc_socket(void *ptr, size_t size, unsigned int align, int socket)
+	__rte_alloc_size(2, 3);
 
 /**
  * This function allocates memory from the huge-page area of memory. The memory
@@ -182,9 +185,9 @@  rte_realloc_socket(void *ptr, size_t size, unsigned int align, int socket);
  *     align is not a power of two).
  *   - Otherwise, the pointer to the allocated object.
  */
-__rte_malloc
 void *
-rte_malloc_socket(const char *type, size_t size, unsigned align, int socket);
+rte_malloc_socket(const char *type, size_t size, unsigned align, int socket)
+	__rte_alloc_size(2);
 
 /**
  * Allocate zero'ed memory from the heap.
@@ -212,7 +215,8 @@  rte_malloc_socket(const char *type, size_t size, unsigned align, int socket);
  *   - Otherwise, the pointer to the allocated object.
  */
 void *
-rte_zmalloc_socket(const char *type, size_t size, unsigned align, int socket);
+rte_zmalloc_socket(const char *type, size_t size, unsigned align, int socket)
+	__rte_alloc_size(2);
 
 /**
  * Replacement function for calloc(), using huge-page memory. Memory area is
@@ -239,9 +243,9 @@  rte_zmalloc_socket(const char *type, size_t size, unsigned align, int socket);
  *     align is not a power of two).
  *   - Otherwise, the pointer to the allocated object.
  */
-__rte_malloc
 void *
-rte_calloc_socket(const char *type, size_t num, size_t size, unsigned align, int socket);
+rte_calloc_socket(const char *type, size_t num, size_t size, unsigned align, int socket)
+	__rte_alloc_size(2, 3);
 
 /**
  * Frees the memory space pointed to by the provided pointer.