[Beignet] [Patch v2] GBE/PRINTF: store variable instead of pointer in "slots".

[Junyan He]

LGTM, very thanks for fixing this bug

> -----Original Message-----
> From: xionghu.luo at intel.com
> Sent: Mon, 10 Aug 2015 15:40:13 +0800
> To: beignet at lists.freedesktop.org
> Subject: [Beignet] [Patch v2] GBE/PRINTF: store variable instead of
> pointer in "slots".
> 
> From: Luo Xionghu <xionghu.luo at intel.com>
> 
> this could fix the bug:
> https://bugs.freedesktop.org/show_bug.cgi?id=90472
> v2: the vector "slots" stores the pointer of PrintfSlot from vector
> "fmts",
> but the push_back operation of "fmts" will cause resize if capacity is
> not
> enough and call the copy constructor and destructor of that PrintfSlot,
> leading to a illegal pointer in "slots", so this patch change to store
> the
> variable instead of pointer.
>   update the destructor of PrintfSlot according to the SLOT_TYPE.
> 
> Signed-off-by: Luo Xionghu <xionghu.luo at intel.com>
> ---
>  backend/src/ir/printf.cpp               | 21 +++++++++++++++++++--
>  backend/src/ir/printf.hpp               | 17 +++++++----------
>  backend/src/llvm/llvm_printf_parser.cpp |  2 +-
>  3 files changed, 27 insertions(+), 13 deletions(-)
> 
> diff --git a/backend/src/ir/printf.cpp b/backend/src/ir/printf.cpp
> index 3d9b2fd..eb1c199 100644
> --- a/backend/src/ir/printf.cpp
> +++ b/backend/src/ir/printf.cpp
> @@ -31,6 +31,23 @@ namespace gbe
> 
>      pthread_mutex_t PrintfSet::lock = PTHREAD_MUTEX_INITIALIZER;
> 
> +    PrintfSlot::~PrintfSlot(void)
> +    {
> +        if (ptr)
> +        {
> +          if (type == PRINTF_SLOT_TYPE_STRING) {
> +            free(ptr);
> +            ptr = NULL;
> +          } else if (type == PRINTF_SLOT_TYPE_STATE) {
> +            delete state;
> +            state = NULL;
> +          } else {
> +            type = PRINTF_SLOT_TYPE_NONE;
> +            ptr = NULL;
> +          }
> +        }
> +    }
> +
>      uint32_t PrintfSet::append(PrintfFmt* fmt, Unit& unit)
>      {
>        fmts.push_back(*fmt);
> @@ -40,12 +57,12 @@ namespace gbe
>          if (f->type == PRINTF_SLOT_TYPE_STRING)
>            continue;
> 
> -        slots.push_back(&(*f));
> +        slots.push_back(*f);
>        }
> 
>        /* Update the total size of size. */
>        if (slots.size() > 0)
> -        sizeOfSize = slots.back()->state->out_buf_sizeof_offset
> +        sizeOfSize = slots.back().state->out_buf_sizeof_offset
>                       + getPrintfBufferElementSize(slots.size() - 1);
> 
>        return (uint32_t)fmts.size();
> diff --git a/backend/src/ir/printf.hpp b/backend/src/ir/printf.hpp
> index cbab759..df58437 100644
> --- a/backend/src/ir/printf.hpp
> +++ b/backend/src/ir/printf.hpp
> @@ -159,10 +159,7 @@ namespace gbe
>          ptr = p;
>        }
> 
> -      ~PrintfSlot(void) {
> -        if (ptr)
> -          free(ptr);
> -      }
> +      ~PrintfSlot(void);
>      };
> 
>      class Context;
> @@ -177,7 +174,7 @@ namespace gbe
>          }
> 
>          for (size_t i = 0; i < other.slots.size(); ++i) {
> -          PrintfSlot* s = other.slots[i];
> +          PrintfSlot s = other.slots[i];
>            slots.push_back(s);
>          }
> 
> @@ -215,15 +212,15 @@ namespace gbe
>        uint8_t getIndexBufBTI() const { return btiIndexBuf; }
> 
>        uint32_t getPrintfBufferElementSize(uint32_t i) {
> -        PrintfSlot* slot = slots[i];
> +        PrintfSlot& slot = slots[i];
>          int vec_num = 1;
> -        if (slot->state->vector_n > 0) {
> -          vec_num = slot->state->vector_n;
> +        if (slot.state->vector_n > 0) {
> +          vec_num = slot.state->vector_n;
>          }
> 
>          assert(vec_num > 0 && vec_num <= 16);
> 
> -        switch (slot->state->conversion_specifier) {
> +        switch (slot.state->conversion_specifier) {
>            case PRINTF_CONVERSION_I:
>            case PRINTF_CONVERSION_D:
>            case PRINTF_CONVERSION_O:
> @@ -257,7 +254,7 @@ namespace gbe
> 
>      private:
>        vector<PrintfFmt> fmts;
> -      vector<PrintfSlot*> slots;
> +      vector<PrintfSlot> slots;
>        uint32_t sizeOfSize; // Total sizeof size.
>        friend struct LockOutput;
>        uint8_t btiBuf;
> diff --git a/backend/src/llvm/llvm_printf_parser.cpp
> b/backend/src/llvm/llvm_printf_parser.cpp
> index 2f85443..3d84457 100644
> --- a/backend/src/llvm/llvm_printf_parser.cpp
> +++ b/backend/src/llvm/llvm_printf_parser.cpp
> @@ -291,7 +291,7 @@ again:
>  #if 0
>      {
>        int j = 0;
> -      for (auto &s : *printf_fmt) {
> +      for (auto &s : printf_fmt->first) {
>          j++;
>          if (s.type == PRINTF_SLOT_TYPE_STATE) {
>            fprintf(stderr, "---- %d ---: state : \n", j);
> --
> 1.9.1
> 
> _______________________________________________
> Beignet mailing list
> Beignet at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/beignet