Re: [Mesa-dev] [PATCH] nir: fix dangling ssadef->name ptrs

2016-03-22 Thread Jason Ekstrand
Reviewed-by: Jason Ekstrand 

On Tue, Mar 22, 2016 at 12:35 PM, Rob Clark  wrote:

> From: Rob Clark 
>
> In many places, the convention is to pass an existing ssadef name ptr
> when construction/initializing a new nir_ssa_def.  But that goes badly
> (as noticed by garbage in nir_print output) when the original string
> gets freed.
>
> Just use ralloc_strdup() instead, and add ralloc_free() in the two
> places that would care (not that the strings wouldn't eventually get
> freed anyways).
>
> Also fixup the nir_search code which was directly setting ssadef->name
> to use the parent instruction as memctx.
>
> Signed-off-by: Rob Clark 
> ---
>  src/compiler/nir/nir.c| 4 +++-
>  src/compiler/nir/nir_search.c | 6 +++---
>  src/compiler/nir/nir_to_ssa.c | 2 ++
>  3 files changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/src/compiler/nir/nir.c b/src/compiler/nir/nir.c
> index b114981..20f1a18 100644
> --- a/src/compiler/nir/nir.c
> +++ b/src/compiler/nir/nir.c
> @@ -1317,12 +1317,13 @@ nir_instr_rewrite_dest(nir_instr *instr, nir_dest
> *dest, nir_dest new_dest)
>src_add_all_uses(dest->reg.indirect, instr, NULL);
>  }
>
> +/* note: does *not* take ownership of 'name' */
>  void
>  nir_ssa_def_init(nir_instr *instr, nir_ssa_def *def,
>   unsigned num_components,
>   unsigned bit_size, const char *name)
>  {
> -   def->name = name;
> +   def->name = ralloc_strdup(instr, name);
> def->parent_instr = instr;
> list_inithead(&def->uses);
> list_inithead(&def->if_uses);
> @@ -1339,6 +1340,7 @@ nir_ssa_def_init(nir_instr *instr, nir_ssa_def *def,
> }
>  }
>
> +/* note: does *not* take ownership of 'name' */
>  void
>  nir_ssa_dest_init(nir_instr *instr, nir_dest *dest,
>   unsigned num_components, unsigned bit_size,
> diff --git a/src/compiler/nir/nir_search.c b/src/compiler/nir/nir_search.c
> index 6df662a..842ff65 100644
> --- a/src/compiler/nir/nir_search.c
> +++ b/src/compiler/nir/nir_search.c
> @@ -464,7 +464,7 @@ construct_value(const nir_search_value *value,
>
>switch (c->type) {
>case nir_type_float:
> - load->def.name = ralloc_asprintf(mem_ctx, "%f", c->data.d);
> + load->def.name = ralloc_asprintf(load, "%f", c->data.d);
>   switch (bitsize->dest_size) {
>   case 32:
>  load->value.f32[0] = c->data.d;
> @@ -478,7 +478,7 @@ construct_value(const nir_search_value *value,
>   break;
>
>case nir_type_int:
> - load->def.name = ralloc_asprintf(mem_ctx, "%ld", c->data.i);
> + load->def.name = ralloc_asprintf(load, "%ld", c->data.i);
>   switch (bitsize->dest_size) {
>   case 32:
>  load->value.i32[0] = c->data.i;
> @@ -492,7 +492,7 @@ construct_value(const nir_search_value *value,
>   break;
>
>case nir_type_uint:
> - load->def.name = ralloc_asprintf(mem_ctx, "%lu", c->data.u);
> + load->def.name = ralloc_asprintf(load, "%lu", c->data.u);
>   switch (bitsize->dest_size) {
>   case 32:
>  load->value.u32[0] = c->data.u;
> diff --git a/src/compiler/nir/nir_to_ssa.c b/src/compiler/nir/nir_to_ssa.c
> index 0640607..d588d7d 100644
> --- a/src/compiler/nir/nir_to_ssa.c
> +++ b/src/compiler/nir/nir_to_ssa.c
> @@ -221,6 +221,7 @@ rewrite_def_forwards(nir_dest *dest, void *_state)
> list_del(&dest->reg.def_link);
> nir_ssa_dest_init(state->parent_instr, dest, reg->num_components,
>   reg->bit_size, name);
> +   ralloc_free(name);
>
> /* push our SSA destination on the stack */
> state->states[index].index++;
> @@ -274,6 +275,7 @@ rewrite_alu_instr_forward(nir_alu_instr *instr,
> rewrite_state *state)
>list_del(&instr->dest.dest.reg.def_link);
>nir_ssa_dest_init(&instr->instr, &instr->dest.dest, num_components,
>  reg->bit_size, name);
> +  ralloc_free(name);
>
>if (nir_op_infos[instr->op].output_size == 0) {
>   /*
> --
> 2.5.0
>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH] nir: fix dangling ssadef->name ptrs

2016-03-22 Thread Rob Clark
From: Rob Clark 

In many places, the convention is to pass an existing ssadef name ptr
when construction/initializing a new nir_ssa_def.  But that goes badly
(as noticed by garbage in nir_print output) when the original string
gets freed.

Just use ralloc_strdup() instead, and add ralloc_free() in the two
places that would care (not that the strings wouldn't eventually get
freed anyways).

Also fixup the nir_search code which was directly setting ssadef->name
to use the parent instruction as memctx.

Signed-off-by: Rob Clark 
---
 src/compiler/nir/nir.c| 4 +++-
 src/compiler/nir/nir_search.c | 6 +++---
 src/compiler/nir/nir_to_ssa.c | 2 ++
 3 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/src/compiler/nir/nir.c b/src/compiler/nir/nir.c
index b114981..20f1a18 100644
--- a/src/compiler/nir/nir.c
+++ b/src/compiler/nir/nir.c
@@ -1317,12 +1317,13 @@ nir_instr_rewrite_dest(nir_instr *instr, nir_dest 
*dest, nir_dest new_dest)
   src_add_all_uses(dest->reg.indirect, instr, NULL);
 }
 
+/* note: does *not* take ownership of 'name' */
 void
 nir_ssa_def_init(nir_instr *instr, nir_ssa_def *def,
  unsigned num_components,
  unsigned bit_size, const char *name)
 {
-   def->name = name;
+   def->name = ralloc_strdup(instr, name);
def->parent_instr = instr;
list_inithead(&def->uses);
list_inithead(&def->if_uses);
@@ -1339,6 +1340,7 @@ nir_ssa_def_init(nir_instr *instr, nir_ssa_def *def,
}
 }
 
+/* note: does *not* take ownership of 'name' */
 void
 nir_ssa_dest_init(nir_instr *instr, nir_dest *dest,
  unsigned num_components, unsigned bit_size,
diff --git a/src/compiler/nir/nir_search.c b/src/compiler/nir/nir_search.c
index 6df662a..842ff65 100644
--- a/src/compiler/nir/nir_search.c
+++ b/src/compiler/nir/nir_search.c
@@ -464,7 +464,7 @@ construct_value(const nir_search_value *value,
 
   switch (c->type) {
   case nir_type_float:
- load->def.name = ralloc_asprintf(mem_ctx, "%f", c->data.d);
+ load->def.name = ralloc_asprintf(load, "%f", c->data.d);
  switch (bitsize->dest_size) {
  case 32:
 load->value.f32[0] = c->data.d;
@@ -478,7 +478,7 @@ construct_value(const nir_search_value *value,
  break;
 
   case nir_type_int:
- load->def.name = ralloc_asprintf(mem_ctx, "%ld", c->data.i);
+ load->def.name = ralloc_asprintf(load, "%ld", c->data.i);
  switch (bitsize->dest_size) {
  case 32:
 load->value.i32[0] = c->data.i;
@@ -492,7 +492,7 @@ construct_value(const nir_search_value *value,
  break;
 
   case nir_type_uint:
- load->def.name = ralloc_asprintf(mem_ctx, "%lu", c->data.u);
+ load->def.name = ralloc_asprintf(load, "%lu", c->data.u);
  switch (bitsize->dest_size) {
  case 32:
 load->value.u32[0] = c->data.u;
diff --git a/src/compiler/nir/nir_to_ssa.c b/src/compiler/nir/nir_to_ssa.c
index 0640607..d588d7d 100644
--- a/src/compiler/nir/nir_to_ssa.c
+++ b/src/compiler/nir/nir_to_ssa.c
@@ -221,6 +221,7 @@ rewrite_def_forwards(nir_dest *dest, void *_state)
list_del(&dest->reg.def_link);
nir_ssa_dest_init(state->parent_instr, dest, reg->num_components,
  reg->bit_size, name);
+   ralloc_free(name);
 
/* push our SSA destination on the stack */
state->states[index].index++;
@@ -274,6 +275,7 @@ rewrite_alu_instr_forward(nir_alu_instr *instr, 
rewrite_state *state)
   list_del(&instr->dest.dest.reg.def_link);
   nir_ssa_dest_init(&instr->instr, &instr->dest.dest, num_components,
 reg->bit_size, name);
+  ralloc_free(name);
 
   if (nir_op_infos[instr->op].output_size == 0) {
  /*
-- 
2.5.0

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev