Backend: Fix double free of the cloned_module

Submitted by Pan Xiuli on June 15, 2017, 8:45 a.m.

Details

Message ID 1497516342-20162-1-git-send-email-xiuli.pan@intel.com
State New
Headers show
Series "Backend: Fix double free of the cloned_module" ( rev: 1 ) in Beignet

Not browsing as part of any series.

Commit Message

Pan Xiuli June 15, 2017, 8:45 a.m.
From: Pan Xiuli <xiuli.pan@intel.com>

In the llvmToGen function the module will be deleted, we only need to
delete the cloned_module when the first llvmToGen success.

Signed-off-by: Pan Xiuli <xiuli.pan@intel.com>
---
 backend/src/backend/program.cpp | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

Patch hide | download patch | download mbox

diff --git a/backend/src/backend/program.cpp b/backend/src/backend/program.cpp
index 724058c..8fb33c4 100644
--- a/backend/src/backend/program.cpp
+++ b/backend/src/backend/program.cpp
@@ -154,7 +154,12 @@  namespace gbe {
         //suppose file exists and llvmToGen will not return false.
         llvmToGen(*unit, fileName, module, 0, strictMath, OCL_PROFILING_LOG, error);
       }
+    } else {
+      if(cloned_module){
+        delete (llvm::Module*) cloned_module;
+      }
     }
+
     if(unit->getValid()){
       std::string error2;
       if (this->buildFromUnit(*unit, error2)){
@@ -163,9 +168,6 @@  namespace gbe {
       error = error + error2;
     }
     delete unit;
-    if(cloned_module){
-      delete (llvm::Module*) cloned_module;
-    }
     return ret;
   }
 

Comments

Because llvmToGen accept the filename argument, so it need to create and delete module.
I think the module should not be deleted in llvmToGen, the caller decide to delete or not.
I will send another patch to refine it.

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

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

> Xiuli Pan

> Sent: Thursday, June 15, 2017 16:46

> To: beignet@lists.freedesktop.org

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

> Subject: [Beignet] [PATCH] Backend: Fix double free of the cloned_module

> 

> From: Pan Xiuli <xiuli.pan@intel.com>

> 

> In the llvmToGen function the module will be deleted, we only need to

> delete the cloned_module when the first llvmToGen success.

> 

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

> ---

>  backend/src/backend/program.cpp | 8 +++++---

>  1 file changed, 5 insertions(+), 3 deletions(-)

> 

> diff --git a/backend/src/backend/program.cpp

> b/backend/src/backend/program.cpp index 724058c..8fb33c4 100644

> --- a/backend/src/backend/program.cpp

> +++ b/backend/src/backend/program.cpp

> @@ -154,7 +154,12 @@ namespace gbe {

>          //suppose file exists and llvmToGen will not return false.

>          llvmToGen(*unit, fileName, module, 0, strictMath,

> OCL_PROFILING_LOG, error);

>        }

> +    } else {

> +      if(cloned_module){

> +        delete (llvm::Module*) cloned_module;

> +      }

>      }

> +

>      if(unit->getValid()){

>        std::string error2;

>        if (this->buildFromUnit(*unit, error2)){ @@ -163,9 +168,6 @@

> namespace gbe {

>        error = error + error2;

>      }

>      delete unit;

> -    if(cloned_module){

> -      delete (llvm::Module*) cloned_module;

> -    }

>      return ret;

>    }

> 

> --

> 2.7.4

> 

> _______________________________________________

> Beignet mailing list

> Beignet@lists.freedesktop.org

> https://lists.freedesktop.org/mailman/listinfo/beignet
OK, the chaos in llvmToGen is the root cause of this bug, refine that function may solve the  problem.

-----Original Message-----
From: Yang, Rong R 

Sent: Thursday, June 22, 2017 09:38
To: Pan, Xiuli <xiuli.pan@intel.com>; beignet@lists.freedesktop.org
Cc: Pan, Xiuli <xiuli.pan@intel.com>
Subject: RE: [Beignet] [PATCH] Backend: Fix double free of the cloned_module

Because llvmToGen accept the filename argument, so it need to create and delete module.
I think the module should not be deleted in llvmToGen, the caller decide to delete or not.
I will send another patch to refine it.

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

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

> Of Xiuli Pan

> Sent: Thursday, June 15, 2017 16:46

> To: beignet@lists.freedesktop.org

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

> Subject: [Beignet] [PATCH] Backend: Fix double free of the 

> cloned_module

> 

> From: Pan Xiuli <xiuli.pan@intel.com>

> 

> In the llvmToGen function the module will be deleted, we only need to 

> delete the cloned_module when the first llvmToGen success.

> 

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

> ---

>  backend/src/backend/program.cpp | 8 +++++---

>  1 file changed, 5 insertions(+), 3 deletions(-)

> 

> diff --git a/backend/src/backend/program.cpp 

> b/backend/src/backend/program.cpp index 724058c..8fb33c4 100644

> --- a/backend/src/backend/program.cpp

> +++ b/backend/src/backend/program.cpp

> @@ -154,7 +154,12 @@ namespace gbe {

>          //suppose file exists and llvmToGen will not return false.

>          llvmToGen(*unit, fileName, module, 0, strictMath, 

> OCL_PROFILING_LOG, error);

>        }

> +    } else {

> +      if(cloned_module){

> +        delete (llvm::Module*) cloned_module;

> +      }

>      }

> +

>      if(unit->getValid()){

>        std::string error2;

>        if (this->buildFromUnit(*unit, error2)){ @@ -163,9 +168,6 @@ 

> namespace gbe {

>        error = error + error2;

>      }

>      delete unit;

> -    if(cloned_module){

> -      delete (llvm::Module*) cloned_module;

> -    }

>      return ret;

>    }

> 

> --

> 2.7.4

> 

> _______________________________________________

> Beignet mailing list

> Beignet@lists.freedesktop.org

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