Re: [PATCH v2] Disable record btrace bts support for AMD processors

[Simon Marchi <simark@simark.ca>]

On 2020-05-14 6:03 p.m., Kevin Buettner via Gdb-patches wrote:
> [Note: The actual patch for v2 is unchanged.  I've fixed a mistake
> in the commit log and expanded it a bit too.]
> 
> Some Intel processors implement a Branch Trace Store (BTS) which GDB
> uses for reverse execution support via the "record btrace bts"
> command.
> 
> I have been unable to find a description of a similar feature in a
> recent (April 2020) AMD64 architecture reference:
> 
>     https://www.amd.com/system/files/TechDocs/40332.pdf
> 
> While it is the case that AMD processors have an LBR (last branch
> record) bit in the DebugCtl MSR, it seems that it affects only four
> MSRs when enabled.  The names of these MSRs are LastBranchToIP,
> LastBranchFromIP, LastIntToIP, and LastIntFromIP.  I can find no
> mention of anything more extensive.  While looking at an Intel
> architecture document, I noticed that Intel's P6 family from the
> mid-90s had registers of the same name.
> 
> Therefore...
> 
> This commit disables "record btrace bts" support in GDB for AMD
> processors.
> 
> Using the test case from gdb.base/break.exp, the sessions
> below show the expected behavior (run on a machine with an
> Intel processor) versus that on a machine with an AMD processor.
> The AMD processor in question is reported as follows by "lscpu":
> AMD Ryzen Threadripper 2950X 16-Core Processor .  Finally, I'll
> note that the AMD machine is actually a VM, but I see similar
> behavior on both the virtualization host and the VM.
> 
> Intel machine - Desired behavior:
> 
> [kevinb@mohave gdb]$ ./gdb -q testsuite/outputs/gdb.base/break/break
> Reading symbols from testsuite/outputs/gdb.base/break/break...
> (gdb) start
> Temporary breakpoint 1 at 0x401179: file /home/kevinb/sourceware-git/native-build/bld/../../binutils-gdb/gdb/testsuite/gdb.base/break.c, line 43.
> Starting program: /home/kevinb/sourceware-git/native-build/bld/gdb/testsuite/outputs/gdb.base/break/break
> 
> Temporary breakpoint 1, main (argc=1, argv=0x7fffffffd748, envp=0x7fffffffd758)
>     at /home/kevinb/sourceware-git/native-build/bld/../../binutils-gdb/gdb/testsuite/gdb.base/break.c:43
> 43	    if (argc == 12345) {  /* an unlikely value < 2^16, in case uninited */ /* set breakpoint 6 here */
> (gdb) record btrace
> (gdb) b factorial
> Breakpoint 2 at 0x40121b: file /home/kevinb/sourceware-git/native-build/bld/../../binutils-gdb/gdb/testsuite/gdb.base/break.c, line 63.
> (gdb) c
> Continuing.
> 
> Breakpoint 2, factorial (value=6)
>     at /home/kevinb/sourceware-git/native-build/bld/../../binutils-gdb/gdb/testsuite/gdb.base/break.c:63
> 63	  if (value > 1) {  /* set breakpoint 7 here */
> (gdb) info record
> Active record target: record-btrace
> Recording format: Branch Trace Store.
> Buffer size: 64kB.
> Recorded 768 instructions in 22 functions (0 gaps) for thread 1 (process 19215).
> (gdb) record function-call-history
> 13	do_lookup_x
> 14	_dl_lookup_symbol_x
> 15	_dl_fixup
> 16	_dl_runtime_resolve_xsavec
> 17	atoi
> 18	strtoq
> 19	____strtoll_l_internal
> 20	atoi
> 21	main
> 22	factorial
> (gdb) record instruction-history
> 759	   0x00007ffff7ce0917 <____strtoll_l_internal+647>:	pop    %r15
> 760	   0x00007ffff7ce0919 <____strtoll_l_internal+649>:	retq
> 761	   0x00007ffff7cdd064 <atoi+20>:	add    $0x8,%rsp
> 762	   0x00007ffff7cdd068 <atoi+24>:	retq
> 763	   0x00000000004011b1 <main+75>:	mov    %eax,%edi
> 764	   0x00000000004011b3 <main+77>:	callq  0x401210 <factorial>
> 765	   0x0000000000401210 <factorial+0>:	push   %rbp
> 766	   0x0000000000401211 <factorial+1>:	mov    %rsp,%rbp
> 767	   0x0000000000401214 <factorial+4>:	sub    $0x10,%rsp
> 768	   0x0000000000401218 <factorial+8>:	mov    %edi,-0x4(%rbp)
> 
> AMD machine - Wrong behavior:
> 
> [kev@f32-1 gdb]$ ./gdb -q testsuite/outputs/gdb.base/break/break
> Reading symbols from testsuite/outputs/gdb.base/break/break...
> (gdb) start
> Temporary breakpoint 1 at 0x401179: file /ironwood1/sourceware-git/f32-master/bld/../../worktree-master/gdb/testsuite/gdb.base/break.c, line 43.
> Starting program: /mesquite2/sourceware-git/f32-master/bld/gdb/testsuite/outputs/gdb.base/break/break
> 
> Temporary breakpoint 1, main (argc=1, argv=0x7fffffffd5b8, envp=0x7fffffffd5c8)
>     at /ironwood1/sourceware-git/f32-master/bld/../../worktree-master/gdb/testsuite/gdb.base/break.c:43
> 43	    if (argc == 12345) {  /* an unlikely value < 2^16, in case uninited */ /* set breakpoint 6 here */
> (gdb) record btrace
> (gdb) b factorial
> Breakpoint 2 at 0x40121b: file /ironwood1/sourceware-git/f32-master/bld/../../worktree-master/gdb/testsuite/gdb.base/break.c, line 63.
> (gdb) c
> Continuing.
> 
> Breakpoint 2, factorial (value=6)
>     at /ironwood1/sourceware-git/f32-master/bld/../../worktree-master/gdb/testsuite/gdb.base/break.c:63
> 63	  if (value > 1) {  /* set breakpoint 7 here */
> (gdb) info record
> Active record target: record-btrace
> Recording format: Branch Trace Store.
> Buffer size: 64kB.
> warning: Recorded trace may be incomplete at instruction 7737 (pc = 0x405000).
> warning: Recorded trace may be incomplete at instruction 7739 (pc = 0x0).
> Recorded 7740 instructions in 46 functions (2 gaps) for thread 1 (process 1402911).
> (gdb) record function-call-history
> 37	??
> 38	values
> 39	some_enum_global
> 40	??
> 41	some_union_global
> 42	some_variable
> 43	??
> 44	[decode error (2): unknown instruction]
> 45	??
> 46	[decode error (2): unknown instruction]
> (gdb) record instruction-history
> 7730	   0x0000000000404ff3:	add    %al,(%rax)
> 7731	   0x0000000000404ff5:	add    %al,(%rax)
> 7732	   0x0000000000404ff7:	add    %al,(%rax)
> 7733	   0x0000000000404ff9:	add    %al,(%rax)
> 7734	   0x0000000000404ffb:	add    %al,(%rax)
> 7735	   0x0000000000404ffd:	add    %al,(%rax)
> 7736	   0x0000000000404fff:	.byte 0x0
> 7737	   0x0000000000405000:	Cannot access memory at address 0x405000
> 
> Lastly, I'll note that I see a lot of gdb.btrace failures without
> this commit.  Worse still, the results aren't always the same which
> causes a lot of noise when comparing test results.
> 
> gdbsupport/ChangeLog:
> 
> 	* btrace-common.h (btrace_cpu_vendor): Add CV_AMD.
> 
> gdb/ChangeLog:
> 
> 	* nat/linux-btrace.c (btrace_this_cpu): Add check for AMD
> 	processors.
> 	(cpu_supports_bts): Add CV_AMD case.

Thanks, this version and your other reply answer my questions.

Simon