Re: [PING][PATCH v2 PR gdb/21870] aarch64: Leftover uncleared debug registers

Pedro Alves
2019-02-14T13:02:03Z

On 02/13/2019 09:56 PM, Wei-min Pan wrote:
>
> On 2/13/2019 3:40 AM, Pedro Alves wrote:
>> On 02/12/2019 01:10 AM, Weimin Pan wrote:
>>> +clean_restart $testfile
>>> +
>>> +set test "run to exit"
>>> +gdb_test_multiple "run" "$test" {
>>> +Â Â Â  -re "exited with code 01.*$gdb_prompt $" {
>>> +Â Â Â Â Â Â Â  pass "$test"
>>> +Â Â Â  }
>>> +Â Â Â  -re "exited normally.*$gdb_prompt $" {
>>> +Â Â Â Â Â Â Â  pass "$test"
>>> +Â Â Â  }
>>> +}
>> A naked "run" command doesn't work when testing against
>> gdbserver with --target_board=native-gdbserver.
>>
>> Is "run" important here?Â  Could this use runto_main + "continue" instead?
>
> As long as the test run doesn't assert, we could instead use
> "runto_main + "continue". Thanks for pointing this out.

OOC, why does asserting make a difference?

>
>> Also, the comment at the top of the file says:
>>
>> Â  # This test checks that GDB does not alter watchpoints set by an inferior.
>> Â  # It sets a watchpoint on memory then writes to the watched memory.
>> Â  # It will exit with 1 if the watchpoint is not reached.
>>
>> But I couldn't spot where that "exit with 1" happens in the .c file.
>
> You are right, it should be "exit with 2".
>
>> Also, when that happens, we're issuing a pass, as seen above.
>> Is that intended?
>
> "Exit with 1" could happen if the PTRACE_SETREGSET call should
> fail which is ok as long as it doesn't cause assertion.

I see. Could you add some comments, please?

Could you send a patch?

BTW:

- why is the SET_WATCHPOINT macro necessary?

- what does the ??? below mean?

dreg_state.dbg_regs[0].ctrl |= 2 << 1; // GDB: ???: enabled at el0

- The code has:

atexit (cleanup);

(...)

but then also calls cleanup() return returning (twice, end of main),
which seems unnecessary?

- While at it, the gdb_compile + clean_restart in the .exp file
could be replaced with a single prepare_for_testing call.

>
> Thanks for your comments.
>

Thanks,
Pedro Alves