This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH V4] Add negative repeat count to 'x' command
- From: Toshihito Kikuchi <k dot toshihito at yahoo dot de>
- To: Pedro Alves <palves at redhat dot com>, gdb-patches at sourceware dot org
- Cc: Simon Marchi <simon dot marchi at ericsson dot com>
- Date: Thu, 2 Jun 2016 01:50:04 -0700
- Subject: Re: [PATCH V4] Add negative repeat count to 'x' command
- Authentication-results: sourceware.org; auth=none
- References: <1464373082-14678-1-git-send-email-k dot toshihito at yahoo dot de> <e24423b8-5420-2b9b-f7e3-c26c99f2e21e at redhat dot com>
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
>