This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH V4] Add negative repeat count to 'x' command


Hi Toshihito,

Excellent work.  So this looks good to me now.  Just a few follow
up minor copy/edits needed in the comments, as pointed out below,
to make following the code crystal clear, and this is good to go.

Thanks for the patience, and again thanks for working on this.

On 05/27/2016 07:18 PM, Toshihito Kikuchi wrote:
> This change adds support for specifying a negative repeat count to
> all the formats of the 'x' command to examine memory backward.
> A new testsuite 'examine-backward' is added to cover this new feature.

s/testuite/testcase/

>  Since the letters indicating unit sizes are all distinct from the
>  letters specifying output formats, you do not have to remember whether
>  unit size or format comes first; either order works.  The output
> @@ -9366,6 +9371,13 @@ follow the last instruction that is within the count.  The command
>  @code{disassemble} gives an alternative way of inspecting machine
>  instructions; see @ref{Machine Code,,Source and Machine Code}.
>  
> +If a negative repeat count is specified for the formats @samp{s} or @samp{i},
> +the command displays null-terminated strings or instructions before the given
> +address as many as the absolute value of the given number.  For the @samp{i}
> +format, we use line number information in the debug info to accurately locate
> +procedure boundaries while disassembling backward.  If line info is not

After all the discussions and clarifications, I now think this should
say "instruction boundaries" instead of "procedure boundaries".

> +available, the command stops examining memory with an error message.
> +


>  
> +/* Find the address of the instruction that is INST_COUNT instructions before
> +   the instruction at ADDR.
> +   Since some architectures have variable-length instructions, we can't just
> +   simply subtract INST_COUNT * INSN_LEN from ADDR.  Instead, we use line
> +   number information to locate the nearest known instruction boundary,
> +   and disassemble forward from there.  If we go out of the symbol range
> +   during disassembling, we return the lowest address we've got so far and
> +   set the number of instructions read to INST_READ.  */
> +
> +static CORE_ADDR
> +find_instruction_backward (struct gdbarch *gdbarch, CORE_ADDR addr,
> +                           int inst_count, int *inst_read)
> +{
> +  /* The vector PCS is used to store procedure boundary addresses within

s/procedure boundary/instruction/

> +     a pc range.  */
> +  CORE_ADDR loop_start, loop_end, p;
> +  VEC (CORE_ADDR) *pcs = NULL;
> +  struct symtab_and_line sal;
> +  struct cleanup *cleanup = make_cleanup (VEC_cleanup (CORE_ADDR), &pcs);
> +
> +  *inst_read = 0;
> +  loop_start = loop_end = addr;
> +
> +  /* In each iteration of the outer loop, we get a pc range which ends before

s/which/that/

> +     LOOP_START, then we count and store every instruction address of the range
> +     iterated in the loop.
> +     If the number of instructions counted reaches INST_COUNT, return one of
> +     stored addresses which is located INST_COUNT instructions back from ADDR.

"return one of", or "return the" ?  I think the latter.

Also, s/which/that/.

Result:

     If the number of instructions counted reaches INST_COUNT, return the
     stored address that is located INST_COUNT instructions back from ADDR.


> +     If INST_COUNT is not reached, we subtract the number of counted
> +     instructions from INST_COUNT, and go to the next iteration.  */
> +  do
> +    {
> +      VEC_truncate (CORE_ADDR, pcs, 0);
> +      sal = find_pc_sect_line (loop_start, NULL, 1);
> +      if (sal.line <= 0)
> +        {
> +          /* We reach here when line info is not available.  In this case,
> +             we print a message and just exit the loop.  The return value
> +             is calculated after the loop.  */
> +          printf_filtered (_("No line number information available "
> +                             "for address "));
> +          wrap_here ("  ");
> +          print_address (gdbarch, loop_start - 1, gdb_stdout);
> +          printf_filtered ("\n");
> +          break;
> +        }
> +
> +      loop_end = loop_start;
> +      loop_start = sal.pc;
> +
> +      /* This loop pushes procedure boundaries in a range from


      /* This loop pushes instruction addresses in the range from


> +         LOOP_START to LOOP_END.  */
> +      for (p = loop_start; p < loop_end;)
> +        {
> +          VEC_safe_push (CORE_ADDR, pcs, p);
> +          p += gdb_insn_length (gdbarch, p);
> +        }
> +
> +      inst_count -= VEC_length (CORE_ADDR, pcs);
> +      *inst_read += VEC_length (CORE_ADDR, pcs);
> +    }
> +  while (inst_count > 0);
> +
> +  /* After the loop, the vector PCS has procedure boundaries in the last

s/procedure boundaries in the/instruction addresses of the/

> +     line information we processed, and INST_COUNT has a negative value.

s/line information/source line/

> +     We return an address at the index of -INST_COUNT in the vector for

s/an address/the address/

  /* After the loop, the vector PCS has instruction addresses of the last
     source line we processed, and INST_COUNT has a negative value.
     We return an address at the index of -INST_COUNT in the vector for

> +     the reason below.
> +     Let's assume the following instruction addresses and run 'x/-4i 0x400e'.
> +       Line X of File
> +          0x4000
> +          0x4001
> +          0x4005
> +       Line Y of File
> +          0x4009
> +          0x400c
> +       => 0x400e
> +          0x4011
> +     find_instruction_backward is called with INST_COUNT = 4 and expected to
> +     return 0x4001.  When we reach here, INST_COUNT is set to -1 because
> +     it is subtracted by 2 (from Line Y) and 3 (from Line X).  The value

s/it is/it was/

> +     4001 is located at the index 1 of the last iterated line (= Line X),
> +     which is simply calculated by -INST_COUNT.

> +     The case when the length of PCS is 0 means that we reach an area where
> +     line info is not available.  In such a case, we return LOOP_START which
> +     is the lowest procedure boundary having line info.  */

     The case when the length of PCS is 0 means that we reached an area for
     which line info is not available.  In such case, we return LOOP_START,
     which was the lowest instruction address that had line info.  */

> +  p = VEC_length (CORE_ADDR, pcs) > 0
> +    ? VEC_index (CORE_ADDR, pcs, -inst_count)
> +    : loop_start;
> +
> +  /* INST_READ includes all instruction addresses in a pc range.  Need to
> +     exclude a beginning part, that is {0x4000} in the example above.  */

  /* INST_READ includes all instruction addresses in a pc range.  Need to
     exclude the beginning part up to the address we're returning.  That
     is, exclude {0x4000} in the example above.  */

> +  if (inst_count < 0)
> +    *inst_read += inst_count;
> +
> +  do_cleanups (cleanup);
> +  return p;
> +}


> +  /* Update STRINGS_COUNTED with the actual number of loaded strings.  */
> +  *strings_counted = count_original - count;
> +
> +  if (read_error != 0)
> +    {
> +      /* In error case, STRING_START_ADDR is pointing to the string which
> +         is last successfully loaded.  Rewind the partially loaded string.  */

     s/string which is/string that was/

> +      string_start_addr -= chars_counted * char_size;
> +    }
> +

Thanks,
Pedro Alves


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]