Fix double-free when creating more than one block in JIT debug info reader

A double-free happens when using a JIT debug info reader that creates
more than one block.  In the loop that frees blocks in finalize_symtab,
at the very end, the gdb_block_iter_tmp variable is set initially, but
not changed as the loop advances.  If we have two blocks, the first
iteration frees the first block, the second iteration frees the second
block, but the third iteration tries to free the second block again, as
gdb_block_iter_tmp keeps pointing on the second block.

Fix it by assigning the gdb_block_iter_tmp variable in the loop.

I have improved the jit-reader.exp test to cover this case, by adding a
second "JIT-ed" function and creating a block for it.  I have renamed
the existing function to something I find a bit more descriptive.  There
are no significant changes to jit-reader.exp itself, only updates
following the renaming.  The important changes are in jithost.c
(generate a new function) and in jitreader.c (create a gdb_block for
that function).

This was found because of an ASan report:

$ ./gdb testsuite/outputs/gdb.base/jit-reader/jit-reader -ex "jit-reader-load /home/simark/build/binutils-gdb/gdb/testsuite/outputs/gdb.base/jit-reader/jitreader.so" -ex r
Reading symbols from testsuite/outputs/gdb.base/jit-reader/jit-reader...
Starting program: /home/simark/build/binutils-gdb/gdb/testsuite/outputs/gdb.base/jit-reader/jit-reader
=================================================================
==1751048==ERROR: AddressSanitizer: heap-use-after-free on address 0x604000042eb8 at pc 0x5650ef8eec88 bp 0x7ffe52767290 sp 0x7ffe52767280
READ of size 8 at 0x604000042eb8 thread T0
    #0 0x5650ef8eec87 in finalize_symtab /home/simark/src/binutils-gdb/gdb/jit.c:768
    #1 0x5650ef8eef88 in jit_object_close_impl /home/simark/src/binutils-gdb/gdb/jit.c:797
    #2 0x7fbbda986278 in read_debug_info /home/simark/src/binutils-gdb/gdb/testsuite/gdb.base/jitreader.c:71
    #3 0x5650ef8ef56b in jit_reader_try_read_symtab /home/simark/src/binutils-gdb/gdb/jit.c:850
    #4 0x5650ef8effe3 in jit_register_code /home/simark/src/binutils-gdb/gdb/jit.c:948
    #5 0x5650ef8f2c92 in jit_event_handler(gdbarch*) /home/simark/src/binutils-gdb/gdb/jit.c:1396
    #6 0x5650ef0d137e in handle_jit_event /home/simark/src/binutils-gdb/gdb/breakpoint.c:5470
    [snip]

0x604000042eb8 is located 40 bytes inside of 48-byte region [0x604000042e90,0x604000042ec0)
freed by thread T0 here:
    #0 0x7fbbe57376b0 in __interceptor_free /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cc:122
    #1 0x5650ef8f350b in xfree<gdb_block> /home/simark/src/binutils-gdb/gdb/gdbsupport/common-utils.h:62
    #2 0x5650ef8eeca9 in finalize_symtab /home/simark/src/binutils-gdb/gdb/jit.c:769
    #3 0x5650ef8eef88 in jit_object_close_impl /home/simark/src/binutils-gdb/gdb/jit.c:797
    #4 0x7fbbda986278 in read_debug_info /home/simark/src/binutils-gdb/gdb/testsuite/gdb.base/jitreader.c:71
    #5 0x5650ef8ef56b in jit_reader_try_read_symtab /home/simark/src/binutils-gdb/gdb/jit.c:850
    #6 0x5650ef8effe3 in jit_register_code /home/simark/src/binutils-gdb/gdb/jit.c:948
    #7 0x5650ef8f2c92 in jit_event_handler(gdbarch*) /home/simark/src/binutils-gdb/gdb/jit.c:1396
    #8 0x5650ef0d137e in handle_jit_event /home/simark/src/binutils-gdb/gdb/breakpoint.c:5470
    [snip]

previously allocated by thread T0 here:
    #0 0x7fbbe5737cd8 in __interceptor_calloc /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cc:153
    #1 0x5650eef662f3 in xcalloc /home/simark/src/binutils-gdb/gdb/alloc.c:100
    #2 0x5650ef8f34ea in xcnew<gdb_block> /home/simark/src/binutils-gdb/gdb/gdbsupport/poison.h:122
    #3 0x5650ef8ed467 in jit_block_open_impl /home/simark/src/binutils-gdb/gdb/jit.c:557
    #4 0x7fbbda98620a in read_debug_info /home/simark/src/binutils-gdb/gdb/testsuite/gdb.base/jitreader.c:60
    #5 0x5650ef8ef56b in jit_reader_try_read_symtab /home/simark/src/binutils-gdb/gdb/jit.c:850
    #6 0x5650ef8effe3 in jit_register_code /home/simark/src/binutils-gdb/gdb/jit.c:948
    #7 0x5650ef8f2c92 in jit_event_handler(gdbarch*) /home/simark/src/binutils-gdb/gdb/jit.c:1396
    #8 0x5650ef0d137e in handle_jit_event /home/simark/src/binutils-gdb/gdb/breakpoint.c:5470
    [snip]

gdb/ChangeLog:

	* jit.c (finalize_symtab): Set gdb_block_iter_tmp in loop.

gdb/testsuite/ChangeLog:

	* gdb.base/jit-reader.exp (jit_reader_test): Rename
	jit_function_00 to jit_function_stack_mangle.
	* gdb.base/jithost.c (jit_function_t): Rename to...
	(jit_function_stack_mangle_t): ... this.
	(jit_function_add_t): New typedef.
	(jit_function_00_code): Rename to...
	(jit_function_stack_mangle_code): ... this, make static.
	(jit_function_add_code): New.
	(main): Generate "add" function and call it.  Adjust to changes
	in jithost_abi.
	* gdb.base/jithost.h (struct jithost_abi_bounds): New.
	(struct jithost_abi) <begin, end>: Remove fields.
	<object, function_stack_mangle, function_add>: New fields.
	* gdb.base/jitreader.c (struct reader_state) <code_begin,
	code_end>: Remove fields.
	<func_stack_mangle>: New field.
	(read_debug_info): Adjust to renaming, create block for "add"
	function.
	(read_sp, unwind_frame, get_frame_id): Adjust to other changes.

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index d2660cc711260cdc537252c4244498a886c9a713..c30b60efbfd29577448c8bf8a4e5a2173addca6e 100644 (file)
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,7 @@
+2019-12-16  Simon Marchi  <simon.marchi@polymtl.ca>
+
+       * jit.c (finalize_symtab): Set gdb_block_iter_tmp in loop.
+
 2019-12-16  Tom Tromey  <tromey@adacore.com>
 
        * windows-nat.c (windows_nat_target::attach): Update.

diff --git a/gdb/jit.c b/gdb/jit.c
index 9ea68330212aafe23451a93364ce6c1688a147fc..1cd487502c5e5ff900d980fc3dfd6e2bccacced4 100644 (file)
--- a/gdb/jit.c
+++ b/gdb/jit.c
@@ -765,6 +765,7 @@ finalize_symtab (struct gdb_symtab *stab, struct objfile *objfile)
        gdb_block_iter;
        gdb_block_iter = gdb_block_iter_tmp)
     {
+      gdb_block_iter_tmp = gdb_block_iter->next;
       xfree ((void *) gdb_block_iter->name);
       xfree (gdb_block_iter);
     }

diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index b31e8dd61154667de37a6ac0a9cd5b9a2cf8f062..4fd959171cdc13aee68b7f046e6c0d9bd0111932 100644 (file)
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,25 @@
+2019-12-16  Simon Marchi  <simon.marchi@polymtl.ca>
+
+       * gdb.base/jit-reader.exp (jit_reader_test): Rename
+       jit_function_00 to jit_function_stack_mangle.
+       * gdb.base/jithost.c (jit_function_t): Rename to...
+       (jit_function_stack_mangle_t): ... this.
+       (jit_function_add_t): New typedef.
+       (jit_function_00_code): Rename to...
+       (jit_function_stack_mangle_code): ... this, make static.
+       (jit_function_add_code): New.
+       (main): Generate "add" function and call it.  Adjust to changes
+       in jithost_abi.
+       * gdb.base/jithost.h (struct jithost_abi_bounds): New.
+       (struct jithost_abi) <begin, end>: Remove fields.
+       <object, function_stack_mangle, function_add>: New fields.
+       * gdb.base/jitreader.c (struct reader_state) <code_begin,
+       code_end>: Remove fields.
+       <func_stack_mangle>: New field.
+       (read_debug_info): Adjust to renaming, create block for "add"
+       function.
+       (read_sp, unwind_frame, get_frame_id): Adjust to other changes.
+
 2019-12-11  Tom Tromey  <tom@tromey.com>
 
        * gdb.tui/resize.exp: Fix regexp.

diff --git a/gdb/testsuite/gdb.base/jit-reader.exp b/gdb/testsuite/gdb.base/jit-reader.exp
index 1ef3341bd253971b5de7af68a45d37174201f2a8..639c95f7402fa2a9caf975fd66182272a94910ab 100644 (file)
--- a/gdb/testsuite/gdb.base/jit-reader.exp
+++ b/gdb/testsuite/gdb.base/jit-reader.exp
@@ -120,7 +120,7 @@ proc jit_reader_test {} {
        with_test_prefix "before mangling" {
            gdb_test "bt" \
                [multi_line \
-                    "#0 ${any} in jit_function_00 ${any}" \
+                    "#0 ${any} in jit_function_stack_mangle ${any}" \
                     "#1 ${any} in main ${any}" \
                    ] \
                "bt works"
@@ -128,7 +128,7 @@ proc jit_reader_test {} {
            set sp_before_mangling \
                [get_hexadecimal_valueof "\$sp" 0 "get sp"]
 
-           gdb_test "up" "#1  $any in main $any\r\n$any  function $any" \
+           gdb_test "up" "#1  $any in main $any\r\n$any  function_stack_mangle $any" \
                "move up to caller"
 
            set caller_sp \
@@ -140,7 +140,7 @@ proc jit_reader_test {} {
        # reader's unwinder understands the mangling and should thus
        # be able to unwind at that location.
        with_test_prefix "after mangling" {
-           gdb_test "si" "in jit_function_00 .*" "step over stack mangling"
+           gdb_test "si" "in jit_function_stack_mangle .*" "step over stack mangling"
 
            set sp_after_mangling \
                [get_hexadecimal_valueof "\$sp" 0 "get sp"]
@@ -152,7 +152,7 @@ proc jit_reader_test {} {
            # the mangled stack pointer.
            gdb_test "bt" \
                [multi_line \
-                    "#0 ${any} in jit_function_00 ${any}" \
+                    "#0 ${any} in jit_function_stack_mangle ${any}" \
                     "#1 ${any} in main ${any}" \
                    ] \
                "bt works"
@@ -161,11 +161,11 @@ proc jit_reader_test {} {
                info_registers_current_frame $sp_after_mangling
 
                gdb_test "info frame" \
-                   "Stack level 0, frame at $sp_before_mangling.*in jit_function_00.*"
+                   "Stack level 0, frame at $sp_before_mangling.*in jit_function_stack_mangle.*"
            }
 
            with_test_prefix "caller frame" {
-               gdb_test "up" "#1  $any in main $any\r\n$any  function $any" \
+               gdb_test "up" "#1  $any in main $any\r\n$any  function_stack_mangle $any" \
                    "up to caller"
 
                # Since the JIT unwinder only provides RIP/RSP/RBP,
@@ -243,7 +243,7 @@ proc jit_reader_test {} {
        # the mangled stack pointer.
        gdb_test "bt" \
            [multi_line \
-                "#0 ${any} in jit_function_00 ${any}" \
+                "#0 ${any} in jit_function_stack_mangle ${any}" \
                 "#1 ${any} in main ${any}" \
                ]
     }

diff --git a/gdb/testsuite/gdb.base/jithost.c b/gdb/testsuite/gdb.base/jithost.c
index 28eb10421987f0152faf97ce3f9ca343dc3d4672..5b9834ab53597c2fada920a871baec5ad93b1d0d 100644 (file)
--- a/gdb/testsuite/gdb.base/jithost.c
+++ b/gdb/testsuite/gdb.base/jithost.c
@@ -31,7 +31,8 @@ void __attribute__((noinline)) __jit_debug_register_code () { }
 struct jit_descriptor __jit_debug_descriptor = { 1, 0, 0, 0 };
 struct jit_code_entry only_entry;
 
-typedef void (jit_function_t) ();
+typedef void (jit_function_stack_mangle_t) (void);
+typedef long (jit_function_add_t) (long a, long b);
 
 /* The code of the jit_function_00 function that is copied into an
    mmapped buffer in the inferior at run time.
@@ -39,26 +40,47 @@ typedef void (jit_function_t) ();
    The second instruction mangles the stack pointer, meaning that when
    stopped at the third instruction, GDB needs assistance from the JIT
    unwinder in order to be able to unwind successfully.  */
-const unsigned char jit_function_00_code[] = {
+static const unsigned char jit_function_stack_mangle_code[] = {
   0xcc,                                /* int3 */
   0x48, 0x83, 0xf4, 0xff,      /* xor $0xffffffffffffffff, %rsp */
   0x48, 0x83, 0xf4, 0xff,      /* xor $0xffffffffffffffff, %rsp */
   0xc3                         /* ret */
 };
 
+/* And another "JIT-ed" function, with the prototype `jit_function_add_t`.  */
+static const unsigned char jit_function_add_code[] = {
+  0x48, 0x01, 0xfe,            /* add %rdi,%rsi */
+  0x48, 0x89, 0xf0,            /* mov %rsi,%rax */
+  0xc3,                                /* retq */
+};
+
 int
 main (int argc, char **argv)
 {
-  struct jithost_abi *symfile;
+  struct jithost_abi *symfile = malloc (sizeof (struct jithost_abi));
   char *code = mmap (NULL, getpagesize (), PROT_WRITE | PROT_EXEC,
                     MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
-  jit_function_t *function = (jit_function_t *) code;
-
-  memcpy (code, jit_function_00_code, sizeof (jit_function_00_code));
-
-  symfile = malloc (sizeof (struct jithost_abi));
-  symfile->begin = code;
-  symfile->end = code + sizeof (jit_function_00_code);
+  char *code_end = code;
+
+  /* "JIT" function_stack_mangle.  */
+  memcpy (code_end, jit_function_stack_mangle_code,
+         sizeof (jit_function_stack_mangle_code));
+  jit_function_stack_mangle_t *function_stack_mangle
+    = (jit_function_stack_mangle_t *) code_end;
+  symfile->function_stack_mangle.begin = code_end;
+  code_end += sizeof (jit_function_stack_mangle_code);
+  symfile->function_stack_mangle.end = code_end;
+
+  /* "JIT" function_add.  */
+  memcpy (code_end, jit_function_add_code, sizeof (jit_function_add_code));
+  jit_function_add_t *function_add = (jit_function_add_t *) code_end;
+  symfile->function_add.begin = code_end;
+  code_end += sizeof (jit_function_add_code);
+  symfile->function_add.end = code_end;
+
+  /* Bounds of the whole object.  */
+  symfile->object.begin = code;
+  symfile->object.end = code_end;
 
   only_entry.symfile_addr = symfile;
   only_entry.symfile_size = sizeof (struct jithost_abi);
@@ -69,7 +91,8 @@ main (int argc, char **argv)
   __jit_debug_descriptor.version = 1;
   __jit_debug_register_code ();
 
-  function ();
+  function_stack_mangle ();
+  function_add (5, 6);
 
   return 0;
 }

diff --git a/gdb/testsuite/gdb.base/jithost.h b/gdb/testsuite/gdb.base/jithost.h
index 806593197f26538be923247a0e20367641e8475c..a01d84605db8980b2e90854515ffc8b75a940648 100644 (file)
--- a/gdb/testsuite/gdb.base/jithost.h
+++ b/gdb/testsuite/gdb.base/jithost.h
@@ -18,10 +18,21 @@
 #ifndef JITHOST_H
 #define JITHOST_H
 
+struct jithost_abi_bounds
+{
+  const char *begin, *end;
+};
+
 struct jithost_abi
 {
-  const char *begin;
-  const char *end;
+  /* Beginning and past-the-end for the whole object. */
+  struct jithost_abi_bounds object;
+
+  /* Beginning and past-the-end for function_stack_mangle.  */
+  struct jithost_abi_bounds function_stack_mangle;
+
+  /* Beginning and past-the-end for function_add.  */
+  struct jithost_abi_bounds function_add;
 };
 
 #endif /* JITHOST_H */

diff --git a/gdb/testsuite/gdb.base/jitreader.c b/gdb/testsuite/gdb.base/jitreader.c
index 05d53fdbb3dcfbd0f958966caa34fac5acd9c7e6..6716c5b0f70e181f93a488477e214eb431993635 100644 (file)
--- a/gdb/testsuite/gdb.base/jitreader.c
+++ b/gdb/testsuite/gdb.base/jitreader.c
@@ -34,8 +34,10 @@ enum register_mapping
 
 struct reader_state
 {
-  uintptr_t code_begin;
-  uintptr_t code_end;
+  struct {
+    uintptr_t begin;
+    uintptr_t end;
+  } func_stack_mangle;
 };
 
 static enum gdb_status
@@ -46,15 +48,24 @@ read_debug_info (struct gdb_reader_funcs *self,
   struct jithost_abi *symfile = memory;
   struct gdb_object *object = cbs->object_open (cbs);
   struct gdb_symtab *symtab = cbs->symtab_open (cbs, object, "");
-  GDB_CORE_ADDR begin = (GDB_CORE_ADDR) symfile->begin;
-  GDB_CORE_ADDR end = (GDB_CORE_ADDR) symfile->end;
+
   struct reader_state *state = (struct reader_state *) self->priv_data;
 
-  /* Record the function's range, for the unwinder.  */
-  state->code_begin = begin;
-  state->code_end = end;
+  /* Record the stack mangle function's range, for the unwinder.  */
+  state->func_stack_mangle.begin
+    = (uintptr_t) symfile->function_stack_mangle.begin;
+  state->func_stack_mangle.end
+    = (uintptr_t) symfile->function_stack_mangle.end;
+
+  cbs->block_open (cbs, symtab, NULL,
+                  (GDB_CORE_ADDR) symfile->function_stack_mangle.begin,
+                  (GDB_CORE_ADDR) symfile->function_stack_mangle.end,
+                  "jit_function_stack_mangle");
 
-  cbs->block_open (cbs, symtab, NULL, begin, end, "jit_function_00");
+  cbs->block_open (cbs, symtab, NULL,
+                  (GDB_CORE_ADDR) symfile->function_add.begin,
+                  (GDB_CORE_ADDR) symfile->function_add.end,
+                  "jit_function_add");
 
   cbs->symtab_close (cbs, symtab);
   cbs->object_close (cbs, object);
@@ -113,7 +124,7 @@ read_sp (struct gdb_reader_funcs *self, struct gdb_unwind_callbacks *cbs,
 
   /* If stopped at the instruction after the "xor $-1, %rsp", demangle
      the stack pointer back.  */
-  if (ip == state->code_begin + 5)
+  if (ip == state->func_stack_mangle.begin + 5)
     sp ^= (uintptr_t) -1;
 
   *value = sp;
@@ -132,7 +143,8 @@ unwind_frame (struct gdb_reader_funcs *self, struct gdb_unwind_callbacks *cbs)
   if (!read_register (cbs, AMD64_RA, &this_ip))
     return GDB_FAIL;
 
-  if (this_ip >= state->code_end || this_ip < state->code_begin)
+  if (this_ip >= state->func_stack_mangle.end
+      || this_ip < state->func_stack_mangle.begin)
     return GDB_FAIL;
 
   /* Unwind RBP in order to make the unwinder that tries to unwind
@@ -168,7 +180,7 @@ get_frame_id (struct gdb_reader_funcs *self, struct gdb_unwind_callbacks *cbs)
   read_register (cbs, AMD64_RA, &ip);
   read_sp (self, cbs, ip, &sp);
 
-  frame_id.code_address = (GDB_CORE_ADDR) state->code_begin;
+  frame_id.code_address = (GDB_CORE_ADDR) state->func_stack_mangle.begin;
   frame_id.stack_address = (GDB_CORE_ADDR) sp;
 
   return frame_id;