Messing typedefs?

Messing typedefs?

[Thomas Schmid]

Following code

        typedef unsigned char  plcbit;
        typedef unsigned char  USINT;

        USINT usVar1;

leads to a symbol "usVar1" with base_type->ident named "plcbit".
Is this behaviour expected?

Best regards,
Thomas Schmid

Re: Messing typedefs?

[Pavel Roskin]

Hello Thomas,

On Mon, 2007-06-11 at 13:59 +0200, Thomas Schmid wrote:
> Following code
> 
>         typedef unsigned char  plcbit;
>         typedef unsigned char  USINT;
> 
>         USINT usVar1;
> 
> leads to a symbol "usVar1" with base_type->ident named "plcbit".
> Is this behaviour expected?

This looks like a serious bug to me, although I'm surprised that it
doesn't seem to affect the sparse functionality (at least on the
surface).

What happens is the base types like uchar_ctype, which are supposed to
be initialized once and never changed again, are actually initialized by
the first typedef, so they are sort of "imprinted" with the new name.

This sparse was compiled without optimization to avoid any additional
weirdness.

(gdb) set args usvar.c
(gdb) tb main
Breakpoint 1 at 0x4017bc: file sparse.c, line 278.
(gdb) r
Starting program: /home/proski/src/sparse/sparse usvar.c
main (argc=2, argv=0x7fff6616d7e8) at sparse.c:278
278             struct string_list *filelist = NULL;
(gdb) watch uchar_ctype->ident
Hardware watchpoint 2: uchar_ctype->ident
(gdb) c
Continuing.
Hardware watchpoint 2: uchar_ctype->ident

Old value = (struct ident *) 0x0
New value = (struct ident *) 0x2b3e44959008
0x00000000004257bd in external_declaration (token=0x2b3e44988088, list=0x65ad70)
    at parse.c:2108
2108                            base_type->ident = ident;
(gdb) p *ident
$1 = {next = 0x0, symbols = 0x2b3e4497d548, len = 6 '\006', tainted = 0 '\0', 
  reserved = 0 '\0', keyword = 0 '\0', name = 0x2b3e4495901a "plcbit"}
(gdb)

And the code is:

        if (is_typedef) {
                if (base_type && !base_type->ident)
                        base_type->ident = ident;
        } else if (base_type && base_type->type == SYM_FN) {

I don't know what base_type->ident needs to be updated at all.  At least
SYM_BASETYPE should be exempt from this retroactive update.

-- 
Regards,
Pavel Roskin

Antwort: Re: Messing typedefs?

[Thomas Schmid]

Hi,

Pavel Roskin <proski@gnu.org> schrieb am 13.06.2007 07:22:02:

> Hello Thomas,
> 
> On Mon, 2007-06-11 at 13:59 +0200, Thomas Schmid wrote:
> > Following code
> > 
> >         typedef unsigned char  plcbit;
> >         typedef unsigned char  USINT;
> > 
> >         USINT usVar1;
> > 
> > leads to a symbol "usVar1" with base_type->ident named "plcbit".
> > Is this behaviour expected?
> 
> What happens is the base types like uchar_ctype, which are supposed to
> be initialized once and never changed again, are actually initialized by
> the first typedef, so they are sort of "imprinted" with the new name.

> And the code is:
> 
>         if (is_typedef) {
>                 if (base_type && !base_type->ident)
>                         base_type->ident = ident;
>         } else if (base_type && base_type->type == SYM_FN) {
> 

If I get it right, every typedef pointing to a base type should lead into 
a new base type to get its right ident?

> I don't know what base_type->ident needs to be updated at all.  At least
> SYM_BASETYPE should be exempt from this retroactive update.

Any idea how to fix this?

Best regards,
Thomas Schmid

Re: Antwort: Re: Messing typedefs?

[Pavel Roskin]

On Tue, 2007-06-26 at 10:23 +0200, Thomas Schmid wrote:
> Pavel Roskin <proski@gnu.org> schrieb am 13.06.2007 07:22:02:

> > What happens is the base types like uchar_ctype, which are supposed to
> > be initialized once and never changed again, are actually initialized by
> > the first typedef, so they are sort of "imprinted" with the new name.
> 
> > And the code is:
> > 
> >         if (is_typedef) {
> >                 if (base_type && !base_type->ident)
> >                         base_type->ident = ident;
> >         } else if (base_type && base_type->type == SYM_FN) {
> > 
> 
> If I get it right, every typedef pointing to a base type should lead into 
> a new base type to get its right ident?

I don't understand your question.

My interpretation of the code is following.  Types may have idents,
which keep information where and how the type was defined.  Base types
don't have idents.

Suppose we have a typedef which uses a new type using a type without an
ident.  In this case, the quotes code snippet would set the ident for
the original type, as if it's also defined by the same typedef.

If other words,

typedef int handle_t;

would be considered as a definition for both "handle_t" and "int".
That's obviously wrong for base types.

The code can be traced back to the commit
d0d20047fb01047beff2beb39c1f4286791196f7 by Oleg Nesterov.  That's the
description:

    [PATCH] de-anonymize typedefs
    
    Code:
        atomic_t v;
        v.xxxx = 0;
    
    without patch:
        warning: no member 'xxxx' in struct <unnamed>
    
    with patch:
        warning: no member 'xxxx' in struct atomic_t
    
    Actually I want this patch because I started the simple libsparse
    client, and I don't see the simple way to resolve SYM_STRUCT's
    name in typedef case.

That's an example that doesn't rely on kernel headers:

typedef struct { int counter; } atomic_t;
int main (int args, char **argv)
{
        atomic_t v;
        v.xxxx = 0;
}

Commenting out the retroactive ident assignment makes this issue
reappear.

> > I don't know what base_type->ident needs to be updated at all.  At least
> > SYM_BASETYPE should be exempt from this retroactive update.
> 
> Any idea how to fix this?

I think a struct defined inside a typedef (and probably inside some
other constructs) should get its own ident.  Relying on typedef seems
wrong to me.  Every struct or union type should have an ident from the
beginning.

-- 
Regards,
Pavel Roskin

Antwort: Re: Antwort: Re: Messing typedefs?

[Thomas Schmid]

Pavel Roskin <proski@gnu.org> schrieb am 27.06.2007 07:01:22:

> On Tue, 2007-06-26 at 10:23 +0200, Thomas Schmid wrote:
> > Pavel Roskin <proski@gnu.org> schrieb am 13.06.2007 07:22:02:
> 
> > > What happens is the base types like uchar_ctype, which are supposed 
to
> > > be initialized once and never changed again, are actually 
initialized by
> > > the first typedef, so they are sort of "imprinted" with the new 
name.
> > 
> > > And the code is:
> > > 
> > >         if (is_typedef) {
> > >                 if (base_type && !base_type->ident)
> > >                         base_type->ident = ident;
> > >         } else if (base_type && base_type->type == SYM_FN) {
> > > 
> > 
> > If I get it right, every typedef pointing to a base type should lead 
into 
> > a new base type to get its right ident?
> 
> I don't understand your question.

I see, sorry, i only meant that every typedef (pointing to a base type) 
should get its own symbol->ctype->base_type with a new ident.

> My interpretation of the code is following.  Types may have idents,
> which keep information where and how the type was defined.  Base types
> don't have idents.

But unfortunately they get one. 

Best regards.
Thomas Schmid

Re: Antwort: Re: Antwort: Re: Messing typedefs?

[Pavel Roskin]

On Wed, 2007-06-27 at 08:51 +0200, Thomas Schmid wrote:
> > My interpretation of the code is following.  Types may have idents,
> > which keep information where and how the type was defined.  Base types
> > don't have idents.
> 
> But unfortunately they get one. 

I understand it better now.  Suppose we have:

typedef struct {int a;} foo;
int main(int argc, char **argv)
{
        foo bar;
        bar.x = 1;
}

The structure is indeed unnamed.  If the error message is going to call
the _structure_ by name, it's correct to call it unnamed.

One possible fix would be to have an "inherited ident", which would be
set only by that code in external_declaration().  This would leave basic
types alone.

Then we need to come with a message that would be printed if only the
inherited ident is present.  gcc prints:

test.c:5: error: 'foo' has no member named 'x'

No "struct" is mentioned.  If we want to be more verbose, we could print
something like:

test.c:5:5: error: no member 'x' in struct type foo

Another solution would be to remove the ident setting code and try to
find the typedef name directly in evaluate_member_dereference().

-- 
Regards,
Pavel Roskin