Bug 10633 - std::string pretty printer should respect 'print elements' limit
Summary: std::string pretty printer should respect 'print elements' limit
Status: RESOLVED FIXED
Alias: None
Product: gdb
Classification: Unclassified
Component: python (show other bugs)
Version: unknown
: P2 normal
Target Milestone: 6.8
Assignee: Phil Muldoon
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-09-12 00:37 UTC by Paul Pluzhnikov
Modified: 2009-09-21 11:12 UTC (History)
2 users (show)

See Also:
Host: x86_64-unknown-linux-gnu
Target: x86_64-unknown-linux-gnu
Build: x86_64-unknown-linux-gnu
Last reconfirmed:


Attachments
Rough patch limiting string print to options->print_max (381 bytes, patch)
2009-09-14 13:15 UTC, Phil Muldoon
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Paul Pluzhnikov 2009-09-12 00:37:08 UTC
There is inconsistency between e.g. std::vector and std::string pretty
printers: the former respects 'print elements' limit, the latter doesn't.

Example:

--- cut ---
#include <string>
#include <vector>

using namespace std;
int main()
{
  //                      01234567890123456789012
  const char *const cs = "Mary had a little lamb.";
  vector<int> v;
  string s;

  for (int i = 0; i < 10; ++i)
    {
      v.push_back(i);
      s += cs;
    }
  return 0;  // break here
}
--- cut ---

g++ -g -o long-str long-str.cc

gdb64-cvs -nx long-str
GNU gdb (GDB) 6.8.50.20090911-cvs
Copyright (C) 2009 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "x86_64-unknown-linux-gnu".
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>...
Reading symbols from /home/ppluzhnikov/src/tmp/long-str...done.
(gdb) b 17
Breakpoint 1 at 0x4008d6: file long-str.cc, line 17.
(gdb) run
Starting program: /home/ppluzhnikov/src/tmp/long-str 

Breakpoint 1, main () at long-str.cc:17
17        return 0;  // break here

## This is fresh svn sync of the latest pretty printers as of 5 minutes ago:
(gdb) py sys.path.insert(0, '/home/ppluzhnikov/Archive/gcc-svn/libstdc++-v3/python')
(gdb) py from libstdcxx.v6.printers import register_libstdcxx_printers
(gdb) py register_libstdcxx_printers (None)
(gdb) set print elem 5
(gdb) print v
$1 = std::vector of length 10, capacity 16 = {0, 1, 2, 3, 4...}
(gdb) print cs
$2 = 0x40138e "Mary "...
(gdb) print s
$3 = 
    "Mary had a little lamb.Mary had a little lamb.Mary had a little lamb.Mary
had a little lamb.Mary had a little lamb.Mary had a little lamb.Mary had a
little lamb.Mary had a little lamb.Mary had a little lamb.Mary had a little lamb."
(gdb) q

Note how 'print elem 5' applies to both vector and 'const char *cs',
but the string is printed "in full".
Comment 1 Phil Muldoon 2009-09-14 09:20:26 UTC
I took a very brief look at this bug this morning. The reason the std::string
printer is not respecting the "set print element n" directive is that it does
not implement a children(self ...) function. It returns the contents of the
std::string in the to_string(self) pretty printer api.

The elements length code is only checked in the print_children function within
py-prettyprint.c, and so this code immediately returns:

  if (! PyObject_HasAttr (printer, gdbpy_children_cst))
    return;


Whether std::string should treat each character as a child, or the parent
to_string() should respect options->print_max, I'm not sure. (Though I can't see
a workable solution with to_string gated by print_max, as that would also affect
all kinds of helper/descriptive text that is not a part of the actual value)
Comment 2 Phil Muldoon 2009-09-14 12:41:42 UTC
Comment #2 is a complete red herring, sorry for going off half-cocked!

print cs observes the options->print_max as follows.

in val_print_string
1410	  fetchlimit = (len == -1 ? options->print_max : min (len,
options->print_max));
1412	  errcode = read_string (addr, len, width, fetchlimit, byte_order,

And then LA_PRINT_STRING is called with read results length of that call (which
will always be gated with fetchlimit, which is always gated with print_max).

But in print_string_repr in py-prettyprint.c we don't pay attention to
options->print_max before we call LA_PRINT_STRING:

	  if (hint && !strcmp (hint, "string"))
	    LA_PRINT_STRING (stream, builtin_type (gdbarch)->builtin_char,
			     output, len, 0, options);

I'm not sure why the function pointed to (in this case c_printstr in c-lang.c)
does not gate the string print. But there you have it
Comment 3 Phil Muldoon 2009-09-14 13:15:25 UTC
Created attachment 4195 [details]
Rough patch limiting string print to options->print_max

Something like this patch would limit the fetch. If this is acceptable, I'll
submit a full patch with regression test.
Comment 4 Phil Muldoon 2009-09-15 13:29:43 UTC
After looking at this again this afternoon, looks like this is a bug in
c-lang.c:c_printstr after all. Attempt 3 .. ;)

I'm not sure why val_print pre-sized the string to conform with print_max before
passing it to LA_PRINT_STRING, but it just happened to do so. Anyway, not so
important. But from reading the code LA_PRINT_STRING should respect print_max in
all cases. So cases like print_string_repr which do not do any form of pre-size
check with print_max can happily rely on LA_PRINT_STRING to enforce it. In the
current code's case, the code has two loops. One outer which does gate on
print_max, and one inner which doesn't. So the inner loop races past the check,
as far as I can tell. I added a check to the inner loop, and this prints string
to the print_max size, and does the right thing with ellipses too. It passes all
tests in the testsuite.


git diff c-lang.c 
diff --git a/gdb/c-lang.c b/gdb/c-lang.c
index 83a7382..515685a 100644
--- a/gdb/c-lang.c
+++ b/gdb/c-lang.c
@@ -459,7 +459,7 @@ c_printstr (struct ui_file *stream, struct type *type, const
gdb_byte *string,
         single character in isolation.  This makes the code simpler
         and probably does the sensible thing in the majority of
         cases.  */
-      while (num_chars == 1)
+      while (num_chars == 1 && things_printed < options->print_max)
        {
          /* Count the number of repetitions.  */
          unsigned int reps = 0;
Comment 5 Sourceware Commits 2009-09-21 09:40:16 UTC
Subject: Bug 10633

CVSROOT:	/cvs/src
Module name:	src
Changes by:	pmuldoon@sourceware.org	2009-09-21 09:39:53

Modified files:
	gdb            : ChangeLog c-lang.c 
	gdb/testsuite  : ChangeLog 
	gdb/testsuite/gdb.python: py-prettyprint.exp 

Log message:
	2009-09-21  Phil Muldoon <pmuldoon@redhat.com>
	
	PR python/10633
	
	* c-lang.c (c_printstr): Do not loop past  options->print_max when
	iterating with wchar_iterate.
	
	2009-09-21  Phil Muldoon <pmuldoon@redhat.com>
	
	PR python/10633
	
	* gdb.python/py-prettyprint.exp (gdb_py_test_silent_cmd): New
	Function.
	(run_lang_tests): Add print elements test.

Patches:
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/gdb/ChangeLog.diff?cvsroot=src&r1=1.10890&r2=1.10891
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/gdb/c-lang.c.diff?cvsroot=src&r1=1.75&r2=1.76
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/gdb/testsuite/ChangeLog.diff?cvsroot=src&r1=1.1962&r2=1.1963
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/gdb/testsuite/gdb.python/py-prettyprint.exp.diff?cvsroot=src&r1=1.1&r2=1.2

Comment 6 Sourceware Commits 2009-09-21 10:25:50 UTC
Subject: Bug 10633

CVSROOT:	/cvs/src
Module name:	src
Branch: 	gdb_7_0-branch
Changes by:	pmuldoon@sourceware.org	2009-09-21 10:25:30

Modified files:
	gdb            : ChangeLog c-lang.c 
	gdb/testsuite  : ChangeLog 
	gdb/testsuite/gdb.python: py-prettyprint.exp 

Log message:
	2009-09-21  Phil Muldoon  <pmuldoon@redhat.com>
	
	PR python/10633
	
	* c-lang.c (c_printstr): Do not loop past  options->print_max when
	iterating with wchar_iterate.
	
	2009-09-21  Phil Muldoon  <pmuldoon@redhat.com>
	
	PR python/10633
	
	* gdb.python/py-prettyprint.exp (gdb_py_test_silent_cmd): New
	Function.
	(run_lang_tests): Add print elements test.

Patches:
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/gdb/ChangeLog.diff?cvsroot=src&only_with_tag=gdb_7_0-branch&r1=1.10874.2.13&r2=1.10874.2.14
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/gdb/c-lang.c.diff?cvsroot=src&only_with_tag=gdb_7_0-branch&r1=1.75&r2=1.75.4.1
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/gdb/testsuite/ChangeLog.diff?cvsroot=src&only_with_tag=gdb_7_0-branch&r1=1.1960.2.1&r2=1.1960.2.2
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/gdb/testsuite/gdb.python/py-prettyprint.exp.diff?cvsroot=src&only_with_tag=gdb_7_0-branch&r1=1.1&r2=1.1.2.1

Comment 7 Phil Muldoon 2009-09-21 11:12:14 UTC
Commited to cvs head, and to gdb 7.0 branch