From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 3600 invoked by alias); 14 Sep 2011 09:28:29 -0000 Received: (qmail 3586 invoked by uid 22791); 14 Sep 2011 09:28:28 -0000 X-SWARE-Spam-Status: No, hits=-1.0 required=5.0 tests=AWL,BAYES_50,RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from fencepost.gnu.org (HELO fencepost.gnu.org) (140.186.70.10) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 14 Sep 2011 09:28:11 +0000 Received: from eliz by fencepost.gnu.org with local (Exim 4.71) (envelope-from ) id 1R3llK-0004ry-As; Wed, 14 Sep 2011 05:28:10 -0400 Date: Wed, 14 Sep 2011 10:26:00 -0000 Message-Id: From: Eli Zaretskii To: Jan Kratochvil CC: gdb-patches@sourceware.org In-reply-to: <20110913194306.GA12849@host1.jankratochvil.net> (message from Jan Kratochvil on Tue, 13 Sep 2011 21:43:06 +0200) Subject: Re: [patch 00/12] entryval#2: Fix x86_64 parameters, virtual tail call frames Reply-to: Eli Zaretskii References: <20110913194306.GA12849@host1.jankratochvil.net> X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2011-09/txt/msg00248.txt.bz2 > Date: Tue, 13 Sep 2011 21:43:06 +0200 > From: Jan Kratochvil > > re-post of the series: > [patch 00/12] entryval: Fix x86_64 parameters, virtual tail call frames > http://sourceware.org/ml/gdb-patches/2011-07/msg00430.html > > particular excerpt: > The patches are available (merged only) in GIT for more convenience at: > http://sourceware.org/gdb/wiki/ArcherBranchManagement > archer-jankratochvil-entryval > > > Here is attached a diff against the previous patch series. The changes are: Sorry, I'm confused: am I supposed to review only the diffs in this message and forget about the rest of the series? If no, which parts should I review? For now, here are the comments about this part alone: > +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). > + 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. > +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 ;-) > -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". > +the function caller, despite the argument value may be already modified by the ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Same correction as above. > +current function and therefore different. ^^^^^^^^^ "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. > +this feature will behave in the default @code{default} setting the same way as ^^^^^^^ Lose this "default", it's redundant. > +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. > 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. > +@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: > +@item only > +set print entry-values only I think this second line should be deleted. > +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. > +@smallexample > +#0 equal (val@@entry=5) > +#0 different (val@@entry=5) > +#0 lost (val@@entry=5) > +#0 born (val@@entry=) > +#0 invalid (val@@entry=) > +@end smallexample This example is identical to the previous one and does not show the effect of the `preferred' setting. > +@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"? > +@smallexample > +#0 equal (val@@entry=5) > +#0 different (val@@entry=5) > +#0 lost (val@@entry=5) > +#0 born (val=10) > +#0 invalid (val@@entry=) > +@end smallexample I think this example should show almost all values NOT @entry. > +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{}. 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. > +@item compact I cannot say I like the "compact" name. How about "both-if-known" (and "both-always" for the previous one)? > +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{}. 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. > +@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. > +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. Maybe it would be better to say "like compact, but...". > +@item show print entry-values > +Show printing of frame arguments values at function entry. "Show the method being used for printing of..." > +Function @code{B} can call function @code{C} by its very last statement. In ^^ "in" > +instead. Such use of a jump instruction is called tail call. "@dfn{tail call}" -- you are introducing new terminology. > +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}. > +function @code{B} which tail calls function @code{C} then @value{GDBN} sees as ^^^^^^^^^^ "tail-calls", and a comma before "then". > +the caller of function @code{C} the function @code{A}. then @value{GDBN} will see @code{A} as the caller of @{C}. > @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. > +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. > +@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. > +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. > +@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. 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). > +@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. > +@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. > + 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. > +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 . 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 . Thanks.