[Bug translator/14431] char * always being printed as, possibly INVALID/NULL "<unknown>", string without giving actual address

["mjw at redhat dot com" <sourceware-bugzilla at sourceware dot org>]

http://sourceware.org/bugzilla/show_bug.cgi?id=14431

--- Comment #3 from Mark Wielaard <mjw at redhat dot com> 2012-08-06 15:32:38 UTC ---
(In reply to comment #2)
> (In reply to comment #1)
> > What about we always also add the address of a char array?
> [...]
> > For our contrived example above that would give:
> > 
> > {.foo=""@0xcb4010, .foo_size=42, .bar=""@0xcb4050, .bar_size=47}
> > {.foo="<unknown>"@0x0, .foo_size=42, .bar="<unknown>"@0x2a, .bar_size=0}
> > 
> > Does that look reasonable or silly?
> 
> It's tough to automagically do The Right Thing without knowing what the user is
> looking for.

Yes, that is why we should just give all information instead of guessing what
the user wants.

>  And in some cases, especially with kernel pointers, the %p may
> overshadow the actual string, something like:
> 
> > {.foo=""@0xffffffffffcb4010, .foo_size=42, .bar=""@0xffffffffffcb4050, .bar_size=47}

What do you mean by overshadow?
I think this example shows nicely why you always want the address, since the
strings look the same (are empty - by accident?), but clearly they are not the
same since they are at different addresses.

> Rather than trying both string value and address every time, maybe we should
> make new kernel/user_string variants that return either a quoted string *or* a
> pointer value, e.g. "\"foo\"" or "0x2a" depending on whether it's readable. 
> Maybe "?@0x2a" to indicate it was tried but couldn't be read.  Your examples
> would be:
> 
> > {.foo="", .foo_size=42, .bar="", .bar_size=47}
> > {.foo=?@0x0, .foo_size=42, .bar=?@0x2a, .bar_size=0}
> 
> We can only try to be just so smart...

Yes, that was what I was going for at first. But that seems "too smart".
We might think we are seeing valid strings because there is some zero character
nearby, but that might not be at all what the user was going for (they just use
char * as generic pointer, not as string pointer).

I do like the usage of plain ? instead of "<unknown>". Updated patch below.

> for people who know they want something
> different, like all pointer addresses, they can always roll their own
> pretty-print.

Is that really that simple? I wasn't able to do that easily. Except by printing
the fields individually with my own identifiers.

Updated patch:

diff --git a/tapsets.cxx b/tapsets.cxx
index 1068a42..f3ffaa8 100644
--- a/tapsets.cxx
+++ b/tapsets.cxx
@@ -2893,18 +2893,20 @@ dwarf_pretty_print::print_chars (Dwarf_Die* start_type,
target_symbol* e,
   const char *name = dwarf_diename (&type);
   if (name && (name == string("char") || name == string("unsigned char")))
     {
-      if (push_deref (pf, "\"%s\"", e))
+      if (push_deref (pf, "\"%s\"@%p", e))
         {
           // steal the last arg for a string access
           assert (!pf->args.empty());
+          expression* expr = pf->args.back();
           functioncall* fcall = new functioncall;
           fcall->tok = e->tok;
           fcall->function = userspace_p ? "user_string2" : "kernel_string2";
-          fcall->args.push_back (pf->args.back());
-          expression *err_msg = new literal_string ("<unknown>");
+          fcall->args.push_back (expr);
+          expression *err_msg = new literal_string ("?");
           err_msg->tok = e->tok;
           fcall->args.push_back (err_msg);
           pf->args.back() = fcall;
+          pf->args.push_back (expr);
         }
       return true;
     }

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.