This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


Hi Eli,

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.)


On Wed, 14 Sep 2011 11:28:10 +0200, Eli Zaretskii wrote:
> > +set print entry-values (both|compact|default|if-needed|no|only|preferred)
> > +show print entry-values
> > +  Set printing of frame arguments values at function entry.  In some cases
>                      ^^^^^^^^^^^^^^^^^^^^^^
> "frame arguments' values" (with the apostrophe), or "frame argument
> values" (in single).

done, the latter.


> > +  GDB can determine the value of function argument which was passed by the
> > +  function caller, despite the argument value may be already modified.
>                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> "even if the value was modified inside the called function", I think.

done.


> > +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.


Some samples of `Virtual tail call frames' were shown in:
	[patch 04/12] entryval: Virtual tail call frames
	http://sourceware.org/ml/gdb-patches/2011-07/msg00434.html

Sometimes it is not clear why GDB did not find some virtual tail call frame
while one thinks it could.  With `set debug tailcall 1' GDB will print also:

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))

which clearly shows why the `amb' frame is not listed:
#0  d (i=125, j=125.5) at gdb.arch/amd64-entry-value.cc:30
#1  0x00000000004006e4 in amb_z (i=<optimized out>) at gdb.arch/amd64-entry-value.cc:65
#2  0x00000000004006f5 in amb_y (i=<optimized out>) at gdb.arch/amd64-entry-value.cc:71
#3  0x0000000000400705 in amb_x (i=<optimized out>) at gdb.arch/amd64-entry-value.cc:77
#4  0x0000000000400725 in amb_b (i=i@entry=101) at gdb.arch/amd64-entry-value.cc:92
#5  0x0000000000400735 in amb_a (i=i@entry=100) at gdb.arch/amd64-entry-value.cc:98
#6  0x000000000040052e in main () at gdb.arch/amd64-entry-value.cc:230

The `tailcall: *' lines are normally (`set debug tailcall 0') not shown to the
user.


> > -If you append @code{@@entry} string to a function parameter name you get its
> > +If you append @kbd{@@entry} string to a function parameter name you get its
> >  value at the time the function got called.  If the value is not available an
> > -error message is printed.  Entry values are available only since @value{NGCC}
> > -version 4.7.
> > +error message is printed.  Entry values are available only with some compilers.
> 
> I think it would be good to have a cross-reference here to where you
> describe "set print entry-values".

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


> > +the function caller, despite the argument value may be already modified by the
>                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> Same correction as above.

done.


> > +current function and therefore different.
>                                   ^^^^^^^^^
> "are different"

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.


> >                                         For optimized code also the current
> > +value may be possibly not available and the entry value may be still known,
> > +which aids the debugging of production code.
> 
>   With optimized code, the current value could be unavailable, but the
>   entry value may still be known.

done.


> > +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.  There can be:
	set foo (bar|baz|default)
but still the GDB factory setting can be:
	set foo baz

In this case the factory setting of GDB is:
	set print entry-values default
from the choices:
	set print entry-values (both|compact|default|if-needed|no|only|preferred)

We could for example chooce since gdb-8.0 the default one is "compact";
although in such case the setting name "default" would be confusing then.


> > +the compiler has to produce @samp{DW_TAG_GNU_call_site} tags.  For example for
> > +@value{NGCC} additionally optimization and debugging compilation options must
> > +be enabled (@option{-O -g}).
> 
>   With @value{NGCC}, you need to specify @option{-O -g} during
>   compilation, to get this information.

done.


> >                             Still even with the required debug info there
> > +exist many reasons why the code path analysis by @value{GDBN} may fail in
> > +specific cases.
> 
> I would omit this sentence, it isn't useful.

done.


> > +@table @code
> > +@item no
> 
> This table needs to be preceded by a sentence saying something like
> 
>   The @{value} parameter can be one of the following:

done:
[...] this information.

The @{value} parameter can be one of the following:

@table @code
@item no


> > +@item only
> > +set print entry-values only
> 
> I think this second line should be deleted.

Yes; and the same for "both".


> > +Print only parameter values from function entry point.  If value from function
> > +entry point is not known while the actual value is known print at least the
>                                                            ^
> Comma here.

done, 3 times.


> This example is identical to the previous one and does not show the
> effect of the `preferred' setting.

There were some errors in the doc, I have synced it with the GDB output and
I agree with the differences/fixes, the changes were:
preferred:
-#0  born (val@@entry=<optimized out>)
+#0  born (val=10)
if-needed:
-#0  equal (val@@entry=5)
+#0  equal (val=5)
-#0  different (val@@entry=5)
+#0  different (val=6)
-#0  invalid (val@@entry=<optimized out>)
+#0  invalid (val=<optimized out>)


> > +@item if-needed
> > +Print actual parameter values.  If actual parameter value is not known while
> > +value from function entry point is known print at least the entry point value
>                                            ^
> A comma is missing.  Also, what do you mean by "at least"?

It was only a language construct without technical meaning, like that 'if we
do not know what we were primarily asked for (actual parameter value), provide
at least least the inferior substitution (the entry value).'.

I have removed "at least" from both `preferred' and `if-needed'.


> > +@smallexample
> > +#0  equal (val@@entry=5)
> > +#0  different (val@@entry=5)
> > +#0  lost (val@@entry=5)
> > +#0  born (val=10)
> > +#0  invalid (val@@entry=<optimized out>)
> > +@end smallexample
> 
> I think this example should show almost all values NOT @entry.

I agree, fixed above, thanks for reading what I wrote.


> > +Always print both the actual parameter value and its value from function entry
> > +point.  Still print both even if one of them or both are @code{<optimized out>}.
> 
>   Always print both the actual parameter value and its value from
>   function entry point, even if values of one or both are not
>   available due to compiler optimizations.

done.


> > +@item compact
> 
> I cannot say I like the "compact" name.  How about "both-if-known"
> (and "both-always" for the previous one)?

I do not find it close to "both", it is more a variant of "default".  Just
I could not make "default" the optimal (shortest) output as I think replacing
former:
	f (val=<optimized out>)
by the `default' variant:
	f (val=<optimized out>, val@entry=5)
is more backward compatible than the radical change of `compact' variant:
	f (val@entry=5)

If there were no users of existing/previous GDB I would propose `compact' as
the default/only output kind.


> > +Print the actual parameter value if it is know and also its value from
> > +function entry point if it is known.  If neither is known print for the actual
> > +value @code{<optimized out>}.  If not in MI mode (@pxref{GDB/MI}) and if both
> > +values are known and they are equal print the shortened
>               ^^^^^^^^^^^^^^^^^^^^^^^^
> "known and identical".  And a comma after that.

done.


> > +@item default
> > +Always print the actual parameter value.  Print also its value from function
> > +entry point but only if it is known.  If not in MI mode (@pxref{GDB/MI}) and if
>               ^
> Comma.

done.


> > +both values are known and they are equal print the shortened
> > +@code{param=param@@entry=VALUE} notation.
> 
> Not quite clear how this differs from the previous one.

The difference is small, described above
"default":
	f (val=<optimized out>, val@entry=5)
"compact":
	f (val@entry=5)


> Maybe it would be better to say "like compact, but...".

I find it easy to find out from the sample output if there are concerns about
the text.  Important is there "Always print the actual parameter value." which
is not true for the "compact" mode and this IMO breaks backward compatibility
of the "compact" mode.


> > +@item show print entry-values
> > +Show printing of frame arguments values at function entry.
> 
> "Show the method being used for printing of..."

done.


> > +Function @code{B} can call function @code{C} by its very last statement.  In
>                                                 ^^
> "in"

done.


> > +instead.  Such use of a jump instruction is called tail call.
> 
> "@dfn{tail call}" -- you are introducing new terminology.

done.


> > +During execution of function @code{C} there will remain no indication it has
> > +been tail called from function @code{B}.
> 
>   During execution of function @code{C}, there will be no indication
>   in the generated code that it was tail-called from @code{B}.

I was trying to express the opposite.  There remains no indication in the
stack frames.  The generated code contains the only indication but it is a bit
difficult to decode out - this is the whole new tail call frames logic in GDB.

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


> > +function @code{B} which tail calls function @code{C} then @value{GDBN} sees as
>                            ^^^^^^^^^^
> "tail-calls", and a comma before "then".

done.


> > +the caller of function @code{C} the function @code{A}.
> 
>   then @value{GDBN} will see @code{A} as the caller of @{C}.

done.


> >                                                       @value{GDBN} can in
> > +some cases search all the possible code paths and if it determintes there
> > +exists an unambiguous code path it will create call frames for it (in this case
> > +a frame with @code{$pc} in function @code{B}).  The virtual return address into
> > +such tail call frame will be pointing pointing right after the jump instruction
> > +(as if it would be a call instructions).
> 
>   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"?


> > +the compiler has to produce @samp{DW_TAG_GNU_call_site} tags.  For example for
> > +@value{NGCC} additionally optimization and debugging compilation options must
> > +be enabled (@option{-O -g}).  Still even with the required debug info there
> > +exist many reasons why the code path analysis by @value{GDBN} may fail in
> > +specific cases.
> 
> Please modify this as I did with a similar text above.

done.


> > +@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?

@kbd{info frame} command (@pxref{Frame Info}) will indicate the tail call frame
kind by text @code{tail call frame} such as in this sample @value{GDBN} output:

@smallexample
(gdb) x/i $pc - 2
   0x40066b <b(int, double)+11>:        jmp    0x400640 <c(int, double)>
(gdb) info frame
Stack level 1, frame at 0x7fffffffda30:
 rip = 0x40066d in b (./gdb.arch/amd64-entry-value.cc:59); saved rip 0x4004c5
 tail call frame, caller of frame at 0x7fffffffda30
 source language c++.
 Arglist at unknown address.
 Locals at unknown address, Previous frame's sp is 0x7fffffffda30
@end smallexample


> > +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))

In this case both address 0x40071e or 0x400719 could be in the backtrace so
GDB will rather omit `amb(int)' completely.  In this case both addresses
belong to the same function `amb(int)' but in more complicated examples they
could be also from different functions (and have different number of virtual
frames etc.).


> > +@kbd{set verbose} command (@pxref{Messages/Warnings, ,Optional Warnings and
> > +Messages}.) can show some reasons why the complete code path analysis did not
> > +succeed in a specific case.
> 
> Ditto.

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
[...]


> Also, @pxref should not have a period at its end, before the
> closing parenthesis, it generates such punctuation automatically (so
> the above will have 2 periods in the manual).

In fact I know, fixed, thanks.


> > +@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.


> > +@item gdb.TAILCALL_FRAME
> > +A frame representing a tail call.  Tail calls are used if the last statement of
> > +an inferior function is a call of a (usually different) function.  Such call
> > +immediately followed by return instruction is in optimized code converted to
> > +just one jump instruction.  @value{GDBN} can in some cases guess such jump has
> > +been executed and it creates a @code{gdb.TAILCALL_FRAME} for it.
> 
> Instead of all this description, just add a cross-reference to where
> tail-calls are described.

done:
@item gdb.TAILCALL_FRAME
A frame representing a tail call.  @xref{Tail Call Frames}.


> > +  add_setshow_enum_cmd ("entry-values", class_stack,
> > +			print_entry_values_choices, &print_entry_values,
> > +			_("Set printing of frame arguments values at function "
> 
> I suggest
> 
>   Set printing of function arguments at function entry.

done:
                        _("Set printing of function arguments at function "
                          "entry"),
                        _("Show printing of function arguments at function "
                          "entry"),


> > +GDB can print in some cases besides frame arguments values also the values\n\
> > +they had at function entry (marked as `NAME@entry').  The value itself and/or\n\
> > +the entry value may be <optimized out>.  Which of this current or entry\n\
> > +values get printed in which case can be set by this option."),
> 
>  GDB can sometimes determine the values of function arguments at entry,
>  in addition to their current values.  This option tells GDB whether
>  to print the current value, the value at entry (marked as val@entry),
>  or both.  Note that one or both of these values may be <optimized out>.

done.


Thanks,
Jan


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]