Re: [PATCH] btrace, vdso: add vdso target sections

[Pedro Alves <palves@redhat.com>]

On 05/19/2014 09:05 AM, Metzger, Markus T wrote:
>> -----Original Message-----
>> From: gdb-patches-owner@sourceware.org [mailto:gdb-patches-
>> owner@sourceware.org] On Behalf Of Pedro Alves
>> Sent: Friday, May 16, 2014 3:00 PM
> 
> Thanks for your review.
> 
> 
>> Looks largely fine, but ...
>>
>> On 04/04/2014 09:53 AM, Markus Metzger wrote:
>>
>>> @@ -131,6 +133,27 @@ symbol_file_add_from_memory (struct bfd
>> *templ, CORE_ADDR addr,
>>>  				   from_tty ? SYMFILE_VERBOSE : 0,
>>>                                     sai, OBJF_SHARED, NULL);
>>>
>>> +  if (add_sections)
>>> +    {
>> ...
>>
>> Why is this conditional?  Why not add the sections if the symbols are
>> added manually with add-symbol-file-from-memory?  The use case for the
>> command was originally exactly to add the vdso's symbols to GDB.
> 
> This function is used by the add-symbol-file-from-memory command, as well.
> 
> I'm not sure if we want to add the target sections there, as well.

If there's no good reason, then I'd rather the command installed
the sections as well.  add-symbol-file-from-memory was invented to
add the vdso manually.  And in that case, we'd want btrace to work
just the same.

>>> +# disassemble the code around the current PC
>>> +gdb_test "disassemble \$pc, +10" [join [list \
>>> +	".*" \
>>> +	"End of assembler dump\." \
>>> +    ] "\r\n"]
>>
>> What is the error one gets without the fix?  Doesn't
>> GDB say "End of assembler dump" in that case too?
> 
> No.  GDB says "Cannot access memory at address ...".

Alright.  I was thinking that that might be a little
brittle: if GDB changes that for some reason, the test will no
longer catch problems.  I think the best test is to make sure the
disassemble output when replaying is the same as when not
replaying at all.

Say, borrow the idea of capture_command_output from gcore.exp,
and do

proc disassemble { what test } {
    global gdb_prompt
    global expect_out

    set output_string ""
    set command "disassemble $what"
    gdb_test_multiple "$command" "$command" {
	-re "${command}\[\r\n\]+(.*)End of assembler dump\.\r\n$gdb_prompt $" {
	    set output_string $expect_out(1,string)
	    pass $command
	}
    }
    return $output_string
}

set pre_btrace_disasm [disassemble "gettimeofday"]

# trace the test code
gdb_test_no_output "record btrace"
gdb_test "next"

# start replaying
gdb_test "reverse-stepi"

with_test_prefix "replay" {
  set post_btrace_disasm [disassemble "gettimeofday"]

  set test "disassembly output matched"
  if ![string pre_btrace_disasm $post_btrace_disasm]  {
    pass $test
  } else {
    fail $test
  }
}

Also, how about making sure we're actually testing the
vdso code, by e.g., expecting "Dump of assembler code for
function __vdso_gettimeofday", or making sure the disassembled
region is within what "info proc maps" shows is the
vdso region?

The idea would be that if the kernel ever changes, we'd
realize that the test is no longer disassembling the
vdso (due to a FAIL/UNSUPPORTED, etc.), thus no longer
exercising what the test was meant to exercise.

-- 
Pedro Alves