fix invalid memory references in gdb/jit.c

Message ID 8ff4437eb869559c2f917dcb49d3cacd@thelig.ht
State New, archived
Headers

Commit Message

Rian Hunter Aug. 7, 2018, 1:05 a.m. UTC
  These patches fix the case where calling block_open() non-1 amount of 
times causes seg faults. This is when using the custom jit reader API.

    /* First run over all the gdb_block objects, creating a real block
@@ -780,7 +783,8 @@ finalize_symtab (struct gdb_symtab *stab, struct 
objfile *objfile)

    for (gdb_block_iter = stab->blocks, gdb_block_iter_tmp = 
gdb_block_iter->next;
         gdb_block_iter;
-       gdb_block_iter = gdb_block_iter_tmp)
+       gdb_block_iter = gdb_block_iter_tmp,
+        gdb_block_iter_tmp = (gdb_block_iter ? gdb_block_iter->next  : 
NULL))
      {
        xfree ((void *) gdb_block_iter->name);
        xfree (gdb_block_iter);
  

Comments

Simon Marchi Aug. 7, 2018, 10:04 p.m. UTC | #1
On 2018-08-06 21:05, Rian Hunter wrote:
> These patches fix the case where calling block_open() non-1 amount of
> times causes seg faults. This is when using the custom jit reader API.

Hi Rian,

Thanks for the patch.  Would it be possible to create a test case or 
enhance an existing one to illustrate the problem (and prevent it coming 
back)?  I haven't looked in details, but 
gdb/testsuite/gdb.base/jit-reader.exp looks promising.

Also, could you please try sending your patch using git-send-email next 
time?  It helps a lot when applying it.

> diff --git a/gdb/jit.c b/gdb/jit.c
> index e6b3cc25ca..78295f0dc2 100644
> --- a/gdb/jit.c
> +++ b/gdb/jit.c
> @@ -684,8 +684,11 @@ finalize_symtab (struct gdb_symtab *stab, struct
> objfile *objfile)
>    /* (begin, end) will contain the PC range this entire blockvector
>       spans.  */
>    BLOCKVECTOR_MAP (bv) = NULL;
> -  begin = stab->blocks->begin;
> -  end = stab->blocks->end;
> +  if (stab->blocks)

if (stab->blocks != nullptr)

> +    {
> +      begin = stab->blocks->begin;
> +      end = stab->blocks->end;
> +    }
>    BLOCKVECTOR_NBLOCKS (bv) = actual_nblocks;
> 
>    /* First run over all the gdb_block objects, creating a real block
> @@ -780,7 +783,8 @@ finalize_symtab (struct gdb_symtab *stab, struct
> objfile *objfile)
> 
>    for (gdb_block_iter = stab->blocks, gdb_block_iter_tmp =
> gdb_block_iter->next;
>         gdb_block_iter;
> -       gdb_block_iter = gdb_block_iter_tmp)
> +       gdb_block_iter = gdb_block_iter_tmp,
> +        gdb_block_iter_tmp = (gdb_block_iter ? gdb_block_iter->next  : 
> NULL))
>      {
>        xfree ((void *) gdb_block_iter->name);
>        xfree (gdb_block_iter);

I'm not sure I see how this helps when stab->blocks is NULL.  This is 
how the look initialization is done:

   gdb_block_iter = stab->blocks, gdb_block_iter_tmp = 
gdb_block_iter->next

If stab->blocks is null, gdb_block_iter will be too, and 
"gdb_block_iter->next" will segfault, won't it?

Simon
  

Patch

diff --git a/gdb/jit.c b/gdb/jit.c
index e6b3cc25ca..78295f0dc2 100644
--- a/gdb/jit.c
+++ b/gdb/jit.c
@@ -684,8 +684,11 @@  finalize_symtab (struct gdb_symtab *stab, struct 
objfile *objfile)
    /* (begin, end) will contain the PC range this entire blockvector
       spans.  */
    BLOCKVECTOR_MAP (bv) = NULL;
-  begin = stab->blocks->begin;
-  end = stab->blocks->end;
+  if (stab->blocks)
+    {
+      begin = stab->blocks->begin;
+      end = stab->blocks->end;
+    }
    BLOCKVECTOR_NBLOCKS (bv) = actual_nblocks;