Re: [patch/RFA] multiarch INSTRUCTION_NULLIFIED

[Andrew Cagney <cagney@gnu.org>]

Randolph Chung wrote:

Just some nits, fix that, and assuming daniel doesn't notice anything 
(for a day or til monday?), commit it.

> +gdb_breakpoint foo
> +gdb_test "run" ".*Breakpoint 1, .* in foo.*" "Breakpoint at foo"

The leading .* in the pattern you've picked up from the existing code 
isn't actually needed (well at least I'm 99% certain of this as I've 
been stripping them out my self and it makes things more efficient) (the 
trailing .*s are needed :-):

> +  -re ".*in foo.*$gdb_prompt $" {

ditto here and for others.  Can you strip them out and check things are 
ok (i'd do it incrementally first :-).

> +    send_gdb "stepi\n"

Good one ...

> +    exp_continue -continue_timer

I can never remember to do this :-(

> +  }
> +  -re ".*in bar.*$gdb_prompt $" {
> +    fail "stepped into bar"

The pass / fail messages should all be identical (except perhaps for a 
fail where a trailing comment in paren can be added).  So:

	fail "$test (stepped into bar)"

> +  }
> +  -re ".*in main.*$gdb_prompt $" {
> +    pass "stepped into main"

and just:

	pass "$test"

check for the same problem elsewhere.

> +  gdb_test_multiple "print $sym" "Get address of $sym" {
> +    -re ".*($hex) <$sym>.*$gdb_prompt $" {
> +      set addr $expect_out(1,string)
> +      pass "got address of $sym = $addr"
> +    }
> +  }

Remove the address from the test result (I'm always being slapped for 
doing that one ;-).  It can make the comparison of subsequent runs, or 
runs on different systems, harder.

> +  return $addr
> +}
> +
> +if { ! [ runto_main ] } then { gdb_suppress_tests; }
> +
> +set foo [get_addr_of_sym "foo"]
> +set bar [get_addr_of_sym "bar"]
> +set foo_last [expr $bar - 4]
> +
> +gdb_breakpoint "*$foo_last"
> +
> +gdb_test "continue" "Breakpoint \[0-9\]*,.* in foo.*"
> +gdb_test "backtrace" ".*in foo.*in main.*" "Backtrace from last insn in foo"
> +gdb_test "stepi" ".*in foo.*" "stepi to nullified instruction stays in foo"
> +gdb_test "backtrace" ".*in foo.*in main.*" "Backtrace from nullified insn"
> +gdb_test "stepi" ".*in main.*" "stepi to main"
> +
> +# In the third test, we verify that backtraces from nullified instructions
> +# work even in coredumps
> +
> +proc gen_core { test } {
> +  global gcorefile
> +  global gdb_prompt
> +  set gcore_works 0
> +  set escapedfilename [string_to_regexp $gcorefile]
> +
> +  gdb_test_multiple "gcore $gcorefile" "gcore" {
> +    -re "Saved corefile ${escapedfilename}\[\r\n\]+$gdb_prompt $" {
> +      pass "$test: gcore"
> +      set gcore_works 1
> +    }
> +    -re "Can't create a corefile\[\r\n\]+$gdb_prompt $" {
> +      unsupported "gcore"
> +    }
> +  }

I'd kfail this.  Not supporting gcore is a bug.

If, on your system, gcore doesn't work then it might be better to steal 
code from bigcore.exp that uses SIGABRT to generate the core file.  Your 
call though.

Anyway, given the follow-on code, I guess it's working for you.

Finally, as a quick sanity check, can you take the gdb.sum output from 
just this test and check it for duplicate test names (sort < gdb.sum | 
uniq -d).  They should be unique.

Thanks!

Andrew