Re: [patch 00/12] entryval#2: Fix x86_64 parameters, virtual tail call frames

Eli Zaretskii
2011-09-17T08:59:12Z

> Date: Fri, 16 Sep 2011 11:35:55 +0200
> From: Jan Kratochvil <jan.kratochvil@redhat.com>
> Cc: gdb-patches@sourceware.org
>
> not yet re-posting, just updated the branch:
> archer-jankratochvil-entryval
> (IIRC GIT not being friendly to you but I guess you do not need it for the
> review follow-up.)

I can use git repositories through Bazaar with the bzr-git plugin (but
committing into git repositories is not yet supported by that plugin).

> > > +set debug tailcall
> > > +show debug tailcall
> > > + Control display of debugging info for determining virtual tail call frames,
> > > + present in inferior debug info together with the @entry values.
> >
> > Is it possible to rephrase this in a less mysterious way? (I cannot
> > suggest a new wording because I don't understand what you meant ;-)
>
> maybe:
> Control display of debugging info for determining virtual tail call frames.
> The tail call frames GDB determines from similar inferior debug info content
> like the @entry parameter values.

Would it be good enough to lose the second sentence entirely? This is
just a NEWS entry, so it doesn't need to go into all the details.

> Added:
> Entry values are normally also printed at the function parameter list
> according to @xref{set print entry-values}.

@ref, nor @xref. The latter produces a capitalized "See", which is
inappropriate in the middle of a sentence.

> the whole sentence is now:
> In some cases @value{GDBN} can determine the value of function argument which
> was passed by the function caller, even if the value was modified inside the
> called function and therefore are different.

"is different" ("argument" is single).

> > > +this feature will behave in the default @code{default} setting the same way as
> > ^^^^^^^
> > Lose this "default", it's redundant.
>
> done although I disagree a bit. The setting is called "default". That it is
> the default choice is a coincidence.

That's true, but consider how it will look in the manual:

this feature will behave in the `default' setting the same way as ...

Do you see any problems with reading and understanding this?

> We could for example chooce since gdb-8.0 the default one is "compact";

The result above will be still correct English and clearly
understandable even after such a change, won't it? And having a
`default' setting that is not the default sounds wrong anyway...

> During execution of function @code{C}, there will be no indication in
> the stack frames that it was tail-called from @code{B}.

How about "in the function call stack frames"?

> > However, in some cases @value{GDBN} can determine that @code{C} was
> > tail-called from @code{B}, and will then create fictitious call
> > frame for that, with the return address set up as if @code{B} called
> > @code{C} normally.
>
> Used as you wrote it.
>
> Really no nominative? ", and will then" -> ", and it will then"?

I don't mind adding "it", if you think it will make the sentence more
clear.

> > > +@kbd{info frame} command (@pxref{Frame Info}) will indicate the tail call frame
> > > +kind by text @code{tail call frame}.
> >
> > An example would be good.
>
> Put there - isn't it too long?

Wrap it at or before column 72 and you are good.

> > > +The detection of all the possible code path executions can find them ambiguous.
> > > +There is no execution history stored (possible @ref{Reverse Execution} is never
> > > +used for this purpose) and the last known caller could have reached the known
> > > +callee by multiple different jump sequences. In such case @value{GDBN} still
> > > +tries to show at least all the unambiguous top tail callers and all the
> > > +unambiguous bottom tail calees, if any.
> >
> > I don't understand what this means in practice. If you show an
> > example of this, I could suggest some rewording.
>
> This is discussed above:
>
> tailcall: initial: 0x400735(amb_a(int)) 0x400725(amb_b(int)) 0x40071e(amb(int)) 0x400705(amb_x(int)) 0x4006f5(amb_y(int)) 0x4006e4(amb_z(int))
> tailcall: compare: 0x400735(amb_a(int)) 0x400725(amb_b(int)) 0x400719(amb(int)) 0x400705(amb_x(int)) 0x4006f5(amb_y(int)) 0x4006e4(amb_z(int))
> tailcall: reduced: 0x400735(amb_a(int)) 0x400725(amb_b(int)) | 0x400705(amb_x(int)) 0x4006f5(amb_y(int)) 0x4006e4(amb_z(int))

But this is output only with "debug tailcall" set, isn't it? The text
I commented on is describing the functionality in general, not the
effect of the "debug tailcall" setting. I was asking what does
"@value{GDBN} still tries to show at least all the unambiguous top
tail callers and all the unambiguous bottom tail calees, if any" mean
when "debug tailcall" is _not_ set? Are you talking about output of
some other command, like "bt"? if so, can you show me its output in
such an ambiguous case?

> It is the testcase "self: bt verbose", without `set verbose' there is:
>
> #0 d (i=<optimized out>, j=<optimized out>) at ./gdb.arch/amd64-entry-value.cc:34
> #1 0x0000000000400715 in self (i=<optimized out>) at ./gdb.arch/amd64-entry-value.cc:120
> #2 0x00000000004004d9 in main () at ./gdb.arch/amd64-entry-value.cc:191
>
> and one can be curious why those <optimized out> were not recovered by the
> `entry values' feature. With `set verbose' it will print the reason:
>
> #0 d (DW_OP_GNU_entry_value resolving has found function "self(int)" at 0x4006e0 can call itself via tail calls

I would suggest adding this example, both without and with "verbose"
setting, to the text. It makes the description much more clear and
understandable.

> > > +@table @code
> > > +@item set debug tailcall
> > > +@kindex set debug tailcall
> > > +When set to on, enables tail calls analysis messages printing. It will show
> > > +all the possible valid tail calls code paths it has considered. It will also
> > > +print the intersection of them with the final unambiguous (possibly partial or
> > > +even empty) code path result.
> >
> > Example, please! Some rewording is in order, but I need an example to
> > suggest it.
>
> Again the example given for NEWS `set debug tailcall' entry above.

That example, together with some explanation of the "initial",
"compare", and "reduced" lines, would go a long way towards making the
docs clear.

Thanks.