[Crash-utility] fix gdb_get_datatype cannot handle integer type

[Dave Anderson]

----- Original Message -----
> Dave,
> 
> On Fri, Mar 8, 2013 at 4:12 AM, Dave Anderson <anderson at redhat.com>
> wrote:
> >
> >
> > ----- Original Message -----
> >> Dave,
> >>
> >> On Thu, Mar 7, 2013 at 12:56 AM, Dave Anderson
> >> <anderson at redhat.com>
> >> wrote:
> >> >
> >> >
> >> > ----- Original Message -----
> >> >> Dave,
> >> >>
> >> >> On Wed, Mar 6, 2013 at 10:04 PM, Dave Anderson
> >> >> <anderson at redhat.com>
> >> >> wrote:
> >> >> >
> >> >> >
> >> >> > ----- Original Message -----
> >> >> >> Hi Dave,
> >> >> >>
> >> >> >> Current when pass integer type to gdb_get_datatype in crash,
> >> >> >> it would return
> >> >> >> req->typecode=0 and req->length=0.
> >> >> >>
> >> >> >> As it only allow TYPE_CODE_ENUM to be passed. here is a
> >> >> >> patch for fixing it.
> >> >> >> Do you think it could be merged?
> >> >> >
> >> >> > That's the OP_VAR_VALUE section -- what is the command that
> >> >> > you're using that
> >> >> > ends up passing an integer type to the function?  And what
> >> >> > problem does it
> >> >> > cause?
> >> >>
> >> >> I enhance whatis command in my local code base to show the
> >> >> function's original variant name.
> >> >> If not with my change, the original result is:
> >> >> int schedule_delayed_work_on(int , struct delayed_work * , long
> >> >> unsigned int );
> >> >> With my change, the result becomes:
> >> >> int schedule_delayed_work_on(int cpu, struct delayed_work *
> >> >> dwork, long unsigned int delay);
> >> >>
> >> >> So it look nicer, right? :)
> >> >
> >> > I guess -- but when I add the patch, it doesn't look any
> >> > different, so presumably
> >> > it only applies w/respect to your enhanced whatis command...
> >> >
> >> >>
> >> >> But I meet an issue that whatis is not intent for showing
> >> >> function prototype only, but for
> >> >> structure/union/integer type.
> >> >>
> >> >> So I need to add arg_to_datatype in whatis_variable to tell the
> >> >> passed data's type to whether
> >> >> call gdb's function for exacting out the function parameter's
> >> >> name.
> >> >>
> >> >> Then I face the bug as I reported...
> >> >
> >> > It's not really a bug because that code path was meant for usage
> >> > by the
> >> > enumerator_value() function.  So it makes me a bit nervous to
> >> > modify it
> >> > for code that only your enhanced whatis command would ever see.
> >> >
> >> > Would your code work if only the req->typecode and req->length
> >> > fields where
> >> > passed back?  (i.e., without tinkering with the req->value and
> >> > req->tagname
> >> > fields in the non-enum case?)  Like this:
> >> >
> >> > --- gdb-7.3.1/gdb/symtab.c.orig
> >> > +++ gdb-7.3.1/gdb/symtab.c
> >> > @@ -5054,8 +5054,9 @@ gdb_get_datatype(struct gnu_request *req
> >> >                 if (gdb_CRASHDEBUG(2))
> >> >                         console("expr->elts[0].opcode:
> >> >                         OP_VAR_VALUE\n");
> >> >                 type = expr->elts[2].symbol->type;
> >> > +               req->typecode = TYPE_CODE(type);
> >> > +               req->length = TYPE_LENGTH(type);
> >> >                 if (TYPE_CODE(type) == TYPE_CODE_ENUM) {
> >> > -                       req->typecode = TYPE_CODE(type);
> >> >                         req->value =
> >> >                         SYMBOL_VALUE(expr->elts[2].symbol);
> >> >                         req->tagname = TYPE_TAG_NAME(type);
> >> >                         if (!req->tagname) {
> >> >
> >>
> >> This seems also works for me.
> >
> > All right, I'll merge the patch as shown above.  I cannot seem to
> > find
> > another pathway to get it other than via enumerator_value(), so it
> > should be safe.
> >
> 
> Thanks for accepting the patch.
> Do you have any interest for my whatis changes, that is add parameter name showing for function symbol?
> If yes, I also could make a patch for it.

I'll take a look at it.  It depends upon how intrusive it is, and
whether it can be self-contained to text symbols only, and not spill
over into the other type paths.

Dave