Re: [PATCH 2/2] Documentation and testcase

[Sergio Durigan Junior <sergiodj@redhat.com>]

On Monday, March 23 2015, Pedro Alves wrote:

> On 03/22/2015 08:45 PM, Sergio Durigan Junior wrote:
>
>> +# We do not do file-backed mappings in the test program, but it is
>> +# important to test this anyway.  One way of performing the test is to
>> +# load GDB with a corefile but without a binary, and then ask for the
>> +# disassemble of a function (i.e., the binary's .text section).  GDB
>> +# should fail in this case.  However, it must succeed if the binary is
>> +# provided along with the corefile.  This is what we test here.
>
> It seems like we now just miss the case of corefilter that _does_ request
> that the file backed regions are dumped.  In that case, disassembly
> should work without the binary.  Could you add that too, please?  We
> can e.g., pass a boolean parameter to test_disasm to specify whether
> to expect that disassembly works without a program file.

Hm, I'm afraid there's a bit of confusion here, at least from my part.

I am already testing the case when we use a value that requests that
file-backed regions are dumped.  If you take a look at the
"all_anon_corefiles" list, you will see that the each corefile generated
there includes everything *except* for the specific type of mapping we
want to ignore (thus the "non_*" names).  And the result of this test is
that GDB cannot disassemble a function without a binary, even if all the
file-backed pages have been dumped.

Having said that, I made a test with git HEAD without my patch.  I
generated a corefile for the same test program, and then loaded only the
corefile:

  $ ./gdb -q -ex 'core ./core.31118' -ex 'disas 0x4007cb'
  ...
  Program terminated with signal SIGTRAP, Trace/breakpoint trap.
  #0  0x0000000000400905 in ?? ()
  No function contains specified address.
  (gdb) 

Which means that, even without my patch, GDB still cannot disassemble a
function without the binary.

FWIW, I did the same test, but this time using a corefile generated by
the Linux kernel (and with all bits set on coredump_filter), and the
results were the same.

>> +
>> +proc test_disasm { core address } {
>> +    global testfile
>> +
>> +    # Restarting GDB without loading the binary
>> +    gdb_exit
>> +    gdb_start
>> +
>> +    set core_loaded [gdb_core_cmd "$core" "load core"]
>> +    if { $core_loaded == -1 } {
>> +	fail "loading $core"
>> +	return
>> +    }
>> +
>> +    gdb_test "disassemble $address" "No function contains specified address." \
>> +	"disassemble function with corefile and without a binary"
>> +
>> +    clean_restart $testfile
>> +
>> +    set core_loaded [gdb_core_cmd "$core" "load core"]
>> +    if { $core_loaded == -1 } {
>> +	fail "loading $core"
>> +	return
>> +    }
>> +
>> +    gdb_test "disassemble $address" "Dump of assembler code for function.*" \
>> +	"disassemble function with corefile and with a binary"
>
> Looks like there are duplicate test messages here, in the
> cases clean_restart, gdb_core_cmd, etc. fail.  You can fix that
> with e.g.:
>
>        with_test_prefix "no binary" {
>            # Restart GDB without loading the binary.
>            gdb_exit
> 	   gdb_start
>
> 	   set core_loaded [gdb_core_cmd "$core" "load core"]
> 	   if { $core_loaded == -1 } {
> 	      fail "load $core"
> 	      return
> 	   }
>
> 	   gdb_test "disassemble $address" "No function contains specified address." \
> 		"disassemble function"
>        }
>
>        with_test_prefix "with binary" {
> 	   clean_restart $testfile
>
> 	   set core_loaded [gdb_core_cmd "$core" "load core"]
> 	   if { $core_loaded == -1 } {
> 	      fail "load $core"
> 	      return
> 	   }
>
> 	   gdb_test "disassemble $address" "No function contains specified address." \
> 		"disassemble function"
>        }

Thanks, fixed.

>> +# Getting the inferior's PID
>
> "Get".  Period at end.

Fixed.

>> +set infpid ""
>> +gdb_test_multiple "info inferiors" "getting inferior pid" {
>> +    -re "process \($decimal\).*\r\n$gdb_prompt $" {
>> +	set infpid $expect_out(1,string)
>> +    }
>> +}
>> +
>> +# Get the main function's address
>
> Period.

Fixed.

> (I saw a few other similar gerund uses in the file which
> read a bit odd to me, but I didn't point them all out.)

I removed all of them, and also added missing periods all over.  Thanks.

> This is OK with the missing test added.

I'll wait until you clarify that comment above :-).  I won't resubmit
the patch now because it only contains fixes to comments.

> Thanks for the patience and for working on this.

Thank you!

-- 
Sergio
GPG key ID: 0x65FC5E36
Please send encrypted e-mail if possible
http://sergiodj.net/