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 Pedro,

I've sent V5, updating all the comments below.  The patch got much
better.  Thanks a lot!

Toshihito

On 2016-05-31 04:51 AM, Pedro Alves wrote:
> 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]