utests: fix image_from_buffer bugs

Submitted by Pan Xiuli on Nov. 6, 2015, 1:42 a.m.

Details

Message ID 53C176F57F7E09459ED90EC4B7F4D8DC6E8D50@shsmsx102.ccr.corp.intel.com
State New
Headers show
Series "utests: fix image_from_buffer bugs" ( rev: 3 ) in Beignet

Not browsing as part of any series.

Commit Message

Pan Xiuli Nov. 6, 2015, 1:42 a.m.
Ping for pushed.

-----Original Message-----
From: Luo, Xionghu 

Sent: Wednesday, October 28, 2015 9:42 AM
To: Pan, Xiuli <xiuli.pan@intel.com>; beignet@lists.freedesktop.org
Cc: Pan, Xiuli <xiuli.pan@intel.com>
Subject: RE: [Beignet] [PATCH] utests: fix image_from_buffer bugs

This patch LGTM.
Thanks.

Luo Xionghu
Best Regards

-----Original Message-----
From: Beignet [mailto:beignet-bounces@lists.freedesktop.org] On Behalf Of Pan Xiuli

Sent: Tuesday, October 27, 2015 2:16 PM
To: beignet@lists.freedesktop.org
Cc: Pan, Xiuli
Subject: [Beignet] [PATCH] utests: fix image_from_buffer bugs

Fixed 2 bugs:
1.This test case uses usrptr, so we should never free the orginal buffer space, otherwise undefined behavior would happen: adding or losing one header file causing data in front broken, NDRangeKernel fail etc.
2.The utest need to test when to free image from buffer and the buffer, but the utest helper function will released it again and causes libc made some warnings. We just make the global variable to NULL to avoid these questions.
These will fix the utests image_from_buffer broken.

Signed-off-by: Pan Xiuli <xiuli.pan@intel.com>

---
 utests/image_from_buffer.cpp | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

   desc.image_row_pitch = w * sizeof(uint32_t);
 
   desc.buffer = 0;
-  OCL_CREATE_IMAGE(buf[0], CL_MEM_COPY_HOST_PTR, &format, &desc, buf_data[0]);
+  OCL_CREATE_IMAGE(buf[0], CL_MEM_COPY_HOST_PTR, &format, &desc, 
+ src_data);
 
   desc.buffer = buff;
   OCL_CREATE_IMAGE(buf[1], 0, &format, &desc, NULL); @@ -58,9 +58,6 @@ static void image_from_buffer(void)
   desc.image_row_pitch = 0;
   OCL_CREATE_IMAGE(buf[2], CL_MEM_WRITE_ONLY, &format, &desc, NULL);
 
-  free(buf_data[0]);
-  buf_data[0] = NULL;
-
   OCL_SET_ARG(0, sizeof(cl_mem), &buf[1]);
   OCL_SET_ARG(1, sizeof(cl_mem), &buf[2]);
 
@@ -87,6 +84,8 @@ static void image_from_buffer(void)
   OCL_UNMAP_BUFFER_GTT(1);
   OCL_UNMAP_BUFFER_GTT(2);
 
+  free(src_data);
+
   //spec didn't tell the sequence of release buffer of image. so release either buffer or image first is ok here.
   //we follow the rule of destroy the bo at the last release, then the access of buffer after release image is legal
   //and vice verse.
@@ -98,6 +97,8 @@ static void image_from_buffer(void)
   clReleaseMemObject(buf[1]);
 #endif
   clReleaseMemObject(buf[2]);
+  buf[1] = NULL;
+  buf[2] = NULL;
 }
 
 MAKE_UTEST_FROM_FUNCTION(image_from_buffer);
--
2.1.4

_______________________________________________
Beignet mailing list
Beignet@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/beignet

Patch hide | download patch | download mbox

diff --git a/utests/image_from_buffer.cpp b/utests/image_from_buffer.cpp index 78d6797..b1171d1 100644
--- a/utests/image_from_buffer.cpp
+++ b/utests/image_from_buffer.cpp
@@ -32,13 +32,13 @@  static void image_from_buffer(void)
 
   // Setup kernel and images
   size_t buffer_sz = sizeof(uint32_t) * w * h;
-  //buf_data[0] = (uint32_t*) malloc(buffer_sz);
-  buf_data[0] = (uint32_t*)memalign(base_address_alignment, buffer_sz);
+  uint32_t* src_data;
+  src_data = (uint32_t*)memalign(base_address_alignment, buffer_sz);
   for (uint32_t j = 0; j < h; ++j)
     for (uint32_t i = 0; i < w; i++)
-      ((uint32_t*)buf_data[0])[j * w + i] = j * w + i;
+      src_data[j * w + i] = j * w + i;
 
-  cl_mem buff = clCreateBuffer(ctx, CL_MEM_READ_ONLY | CL_MEM_USE_HOST_PTR, buffer_sz, buf_data[0], &error);
+  cl_mem buff = clCreateBuffer(ctx, CL_MEM_READ_ONLY | 
+ CL_MEM_USE_HOST_PTR, buffer_sz, src_data, &error);
 
   OCL_ASSERT(error == CL_SUCCESS);
   format.image_channel_order = CL_RGBA; @@ -49,7 +49,7 @@ static void image_from_buffer(void)

Comments

Pushed.

> -----Original Message-----

> From: Pan, Xiuli

> Sent: Friday, November 6, 2015 9:43

> To: Luo, Xionghu; beignet@lists.freedesktop.org

> Cc: Yang, Rong R

> Subject: RE: [Beignet] [PATCH] utests: fix image_from_buffer bugs

> 

> Ping for pushed.

> 

> -----Original Message-----

> From: Luo, Xionghu

> Sent: Wednesday, October 28, 2015 9:42 AM

> To: Pan, Xiuli <xiuli.pan@intel.com>; beignet@lists.freedesktop.org

> Cc: Pan, Xiuli <xiuli.pan@intel.com>

> Subject: RE: [Beignet] [PATCH] utests: fix image_from_buffer bugs

> 

> This patch LGTM.

> Thanks.

> 

> Luo Xionghu

> Best Regards

> 

> -----Original Message-----

> From: Beignet [mailto:beignet-bounces@lists.freedesktop.org] On Behalf Of

> Pan Xiuli

> Sent: Tuesday, October 27, 2015 2:16 PM

> To: beignet@lists.freedesktop.org

> Cc: Pan, Xiuli

> Subject: [Beignet] [PATCH] utests: fix image_from_buffer bugs

> 

> Fixed 2 bugs:

> 1.This test case uses usrptr, so we should never free the orginal buffer space,

> otherwise undefined behavior would happen: adding or losing one header

> file causing data in front broken, NDRangeKernel fail etc.

> 2.The utest need to test when to free image from buffer and the buffer, but

> the utest helper function will released it again and causes libc made some

> warnings. We just make the global variable to NULL to avoid these questions.

> These will fix the utests image_from_buffer broken.

> 

> Signed-off-by: Pan Xiuli <xiuli.pan@intel.com>

> ---

>  utests/image_from_buffer.cpp | 17 +++++++++--------

>  1 file changed, 9 insertions(+), 8 deletions(-)

> 

> diff --git a/utests/image_from_buffer.cpp b/utests/image_from_buffer.cpp

> index 78d6797..b1171d1 100644

> --- a/utests/image_from_buffer.cpp

> +++ b/utests/image_from_buffer.cpp

> @@ -32,13 +32,13 @@ static void image_from_buffer(void)

> 

>    // Setup kernel and images

>    size_t buffer_sz = sizeof(uint32_t) * w * h;

> -  //buf_data[0] = (uint32_t*) malloc(buffer_sz);

> -  buf_data[0] = (uint32_t*)memalign(base_address_alignment, buffer_sz);

> +  uint32_t* src_data;

> +  src_data = (uint32_t*)memalign(base_address_alignment, buffer_sz);

>    for (uint32_t j = 0; j < h; ++j)

>      for (uint32_t i = 0; i < w; i++)

> -      ((uint32_t*)buf_data[0])[j * w + i] = j * w + i;

> +      src_data[j * w + i] = j * w + i;

> 

> -  cl_mem buff = clCreateBuffer(ctx, CL_MEM_READ_ONLY |

> CL_MEM_USE_HOST_PTR, buffer_sz, buf_data[0], &error);

> +  cl_mem buff = clCreateBuffer(ctx, CL_MEM_READ_ONLY |

> + CL_MEM_USE_HOST_PTR, buffer_sz, src_data, &error);

> 

>    OCL_ASSERT(error == CL_SUCCESS);

>    format.image_channel_order = CL_RGBA; @@ -49,7 +49,7 @@ static void

> image_from_buffer(void)

>    desc.image_row_pitch = w * sizeof(uint32_t);

> 

>    desc.buffer = 0;

> -  OCL_CREATE_IMAGE(buf[0], CL_MEM_COPY_HOST_PTR, &format, &desc,

> buf_data[0]);

> +  OCL_CREATE_IMAGE(buf[0], CL_MEM_COPY_HOST_PTR, &format, &desc,

> + src_data);

> 

>    desc.buffer = buff;

>    OCL_CREATE_IMAGE(buf[1], 0, &format, &desc, NULL); @@ -58,9 +58,6

> @@ static void image_from_buffer(void)

>    desc.image_row_pitch = 0;

>    OCL_CREATE_IMAGE(buf[2], CL_MEM_WRITE_ONLY, &format, &desc,

> NULL);

> 

> -  free(buf_data[0]);

> -  buf_data[0] = NULL;

> -

>    OCL_SET_ARG(0, sizeof(cl_mem), &buf[1]);

>    OCL_SET_ARG(1, sizeof(cl_mem), &buf[2]);

> 

> @@ -87,6 +84,8 @@ static void image_from_buffer(void)

>    OCL_UNMAP_BUFFER_GTT(1);

>    OCL_UNMAP_BUFFER_GTT(2);

> 

> +  free(src_data);

> +

>    //spec didn't tell the sequence of release buffer of image. so release either

> buffer or image first is ok here.

>    //we follow the rule of destroy the bo at the last release, then the access of

> buffer after release image is legal

>    //and vice verse.

> @@ -98,6 +97,8 @@ static void image_from_buffer(void)

>    clReleaseMemObject(buf[1]);

>  #endif

>    clReleaseMemObject(buf[2]);

> +  buf[1] = NULL;

> +  buf[2] = NULL;

>  }

> 

>  MAKE_UTEST_FROM_FUNCTION(image_from_buffer);

> --

> 2.1.4

> 

> _______________________________________________

> Beignet mailing list

> Beignet@lists.freedesktop.org

> http://lists.freedesktop.org/mailman/listinfo/beignet