ikrabbe/plan9front

Possible member shadowing in anonymous struct members in /sys/include/9p.h

Closed this issue · 7 comments

What happened:
I was reading the Plan 9 source and I noticed that in "/sys/include/9p.h", 
`struct Reqqueue` is declared with two anonymous struct members of types 
`QLock` and `Rendez`, both of which have identically named members `QLp *head;` 
and `QLp *tail;`. Because they declare conflicting members, it seems that one 
of these members must shadow the other, or possibly result in undefined 
behavior, and this doesn't seem intentional. For example, how would you refer 
to the `head` in the QLock member vs the same in the Rendez?

Example:

struct Reqqueue x;
x.head->inuse = 0;
/* Which head is accessed? /
/
How does one access the other? */


Affected code:

/* libc.h:434 */
typedef
struct QLock
{
Lock lock;
int locked;
QLp *head;
QLp *tail;
} QLock;

/* libc.h:465 *?
typedef
struct Rendez
{
QLock *l;
QLp *head;
QLp *tail;
} Rendez;

/* lib9p.h:42 */
struct Reqqueue
{
QLock;
Rendez;
Queueelem;
int pid, flush;
Req *cur;
};

Original issue reported on code.google.com by alexchan...@gmail.com on 3 Oct 2014 at 5:27

the compiler knows what member you refer to based on the signature of the 
function you pass it to. see 
http://doc.cat-v.org/plan_9/4th_edition/papers/comp where it begins "If an 
anonymous structure..."

Original comment by mischief@offblast.org on 3 Oct 2014 at 5:37

  • Changed state: WorksForMe
The section you quoted doesn't mention conflicting member names. Could you 
explain what you mean when you say the compiler knows which member is 
referenced based on the signature of the function? For example in this code, I 
can't see how the compiler could resolve the access, without a shadowing rule:

void
sethead(Reqqueue* x, QLp *newhead)
{
x->head = newhead;
}


Original comment by alexchan...@gmail.com on 3 Oct 2014 at 5:46

it's impossible. the compiler will just complain.

typedef struct x x;
typedef struct y y;
typedef struct z z;

struct x {
    int a;
};

struct y {
    int a;
};

struct z {
    x;
    y;
};

void
usez(z *z)
{
    z->a = 1;
}

void
main(void)
{
    z z;
    usez(&z);
}

6c -FTVw -o shadow.6 shadow.c
shadow.c:21 ambiguous structure element: a

on the other hand, you can pass this struct z to other functions without direct 
reference to the ambiguous member. like so.

void
usex(x *x)
{
    x->a = 1;
}

void
usey(y *y)
{
    y->a = 1;
}

void
main(void)
{
    z z;
    usex(&z);
    usey(&z);
}

i hope that clears it up.

Original comment by mischief@offblast.org on 3 Oct 2014 at 5:57

I see, so it's legal to declare a struct with anonymous struct members which 
result in ambiguously named members, because it's possible to unambiguously 
access these members through a function call involving type promotion.

Is this also the case for non-anonymous members like `Ref readers;` in File? 
Seeing as File also contains another anonymous Ref, and an anonymous RWLock, 
which has a `readers` member too:

typedef
struct RWLock
{
Lock lock;
int readers; /* number of readers /
int writer; /
number of writers /
QLp *head; /
list of waiting processes */
QLp *tail;
} RWLock;

/* 9p.h:132 */
struct File {
Ref;
Dir;
File *parent;
void *aux;

/* below is implementation-specific; don't use */
RWLock;
Filelist *filelist;
Tree *tree;
int nchild;
int allocd;
int nxchild;
Ref readers;
};

void useref(Ref *r);

void
main(void)
{
File f;

/* what happens? */
useref(&f);

/* is this an error? which readers does it access? */
f.readers;

}



Original comment by alexchan...@gmail.com on 3 Oct 2014 at 6:24

install 9front and test it for yourself using the compiler.

this issue tracker should not be for general plan 9 queries. you should instead 
mail 9fans mailinglist. alternatively, 9front has mailinglists and irc for 
questions concerning 9front that are not 'issues'.

Original comment by mischief@offblast.org on 3 Oct 2014 at 6:35

ah ok, thanks for the help.

Original comment by alexchan...@gmail.com on 3 Oct 2014 at 7:22

Original comment by a...@phicode.de on 3 Oct 2014 at 8:22

  • Changed state: ThereIsNoBug