Hi Pedro,
Thanks for your review and explanation. Here is an updated patch.

On 17/04/13 16:48:07, Pedro Alves wrote:
> We should update the comments and test messages too then:
Done.

> Then might as well make all similar tfinds in the non-circular cases
> use "tfind tracepoint" too for consistency.
Done.

> I feel that we should only use commands.  There's no good reason
> to limit the test to the remote protocol.  As soon as someone  
> implements
> tracepoints on some other target (native or something else), we'd
> need to stop using direct RSP packets anyway.  The test used a direct
> packet because there was no user-accessible support for changing the
> trace buffer size.  It was just a hack.
Done.

> BTW, I realized this is the existing style in the test, but
> these "if (...) then return 1" sprinkled around are really
> unnecessary.
> 
> +    if [setup_tracepoints] then { return 1 }
> +    if [gdb_test "break end" "Breakpoint $decimal.*" \
> +	    "breakpoint at end"] then { return 1 }
> +    if [gdb_test "tstart" \
> +	    "\[\r\n\]*" \
> +	    "start trace experiment"] then { return 1 }
> 
> I'd zap them.
Done.

> I see.  The issue is that when haven't collected any registers in the
> the tracepoint (collect $regs), we end up with a frames that are
> un-unwindable (UNWIND_UNAVAILABLE).  All frames in that case have the  
> same
> id (outer_frame_id), even if they represent different functions.  I  
> started
> out with a local fix in tfind_1, but that got ugly quick.  I think we
> should fix this in the frame machinery itself.  Here's a WIP patch  
> for that.
> It makes us build a frame id that represents a function, with an  
> <unavailable>
> stack address.  I haven't thought every detail through, but I think  
> it's
> on the right path.
Your wip patch seems to solve the problem. I run the testsuite with it  
and no regression.
However, this testcase does not depend on this behavior and both  
patches can go in independantly.

Regards,
Abid


2013-04-18  Hafiz Abid Qadeer  <abidh@codesourcery.com>

	* gdb.trace/circ.exp: Remove unnecessary 'if then' checks.
	(run_trace_experiment): Test setup_tracepoints and 'break end'
	in it.
	(trace_buffer_normal): Remove.
	(gdb_trace_circular_tests): Remove.  Move tests to...
	(top level): ...here.  Call 'runto_main' before checking for
	trace support.  Use commands to check the support for circular
	trace buffer and changing of trace buffer size.  Add test
	to calculate size of single frame.  Use this size to
	calculate the size of trace buffer.  Use 'tfind tracepoint 9'
	instead of 'tfind 9'.  Use 'with_test_prefix'.