Discussion:
PCC-385: function prototypes as arguments cannot take attributes
Iain Hibbert
2012-03-17 08:36:17 UTC
Permalink
Hi

I was looking at the above issue posted to JIRA by me, and I think that
the problem is that abstract_declarator does not cater for attributes
after a parenthesis, probably the correct fix is below

Index: cgram.y
===================================================================
RCS file: /cvsroot/pcc/cc/ccom/cgram.y,v
retrieving revision 1.341
diff -u -p -4 -r1.341 cgram.y
--- cgram.y 3 Sep 2011 07:43:43 -0000 1.341
+++ cgram.y 17 Mar 2012 08:33:04 -0000
@@ -453,13 +453,13 @@ abstract_declarator:
| '(' ')' { $$ = bdty(UCALL, bdty(NAME, NULL)); }
| '(' ib2 parameter_type_list ')' {
$$ = bdty(CALL, bdty(NAME, NULL), $3);
}
- | abstract_declarator '(' ')' {
- $$ = bdty(UCALL, $1);
+ | abstract_declarator '(' ')' attr_var {
+ $$ = block(UCALL, $1, NULL, INT, 0, gcc_attr_parse($4));
}
- | abstract_declarator '(' ib2 parameter_type_list ')' {
- $$ = bdty(CALL, $1, $4);
+ | abstract_declarator '(' ib2 parameter_type_list ')' attr_var {
+ $$ = block(CALL, $1, $4, INT, 0, gcc_attr_parse($6));
}
;

ib2: { }


..can anybody confirm this is right?

while here though, I wonder when the patterns without the preceding
abstract_declarator will match, do I need to handle attributes there also?

regards,
iain
Anders Magnusson
2012-03-17 15:02:39 UTC
Permalink
Hi Iain,
Post by Iain Hibbert
Hi
I was looking at the above issue posted to JIRA by me, and I think that
the problem is that abstract_declarator does not cater for attributes
after a parenthesis, probably the correct fix is below
Index: cgram.y
===================================================================
RCS file: /cvsroot/pcc/cc/ccom/cgram.y,v
retrieving revision 1.341
diff -u -p -4 -r1.341 cgram.y
--- cgram.y 3 Sep 2011 07:43:43 -0000 1.341
+++ cgram.y 17 Mar 2012 08:33:04 -0000
| '(' ')' { $$ = bdty(UCALL, bdty(NAME, NULL)); }
| '(' ib2 parameter_type_list ')' {
$$ = bdty(CALL, bdty(NAME, NULL), $3);
}
- | abstract_declarator '(' ')' {
- $$ = bdty(UCALL, $1);
+ | abstract_declarator '(' ')' attr_var {
+ $$ = block(UCALL, $1, NULL, INT, 0, gcc_attr_parse($4));
}
- | abstract_declarator '(' ib2 parameter_type_list ')' {
- $$ = bdty(CALL, $1, $4);
+ | abstract_declarator '(' ib2 parameter_type_list ')' attr_var {
+ $$ = block(CALL, $1, $4, INT, 0, gcc_attr_parse($6));
}
;
ib2: { }
..can anybody confirm this is right?
Yep, it is correct. It should be checked though that the attributes
really follows
into the function where it prototypes for and not get tossed away somewhere.
Post by Iain Hibbert
while here though, I wonder when the patterns without the preceding
abstract_declarator will match, do I need to handle attributes there also?
Yep, otherwise declarations like
int x(int (int) __attribute__((unused)));
will fail :-)

-- Ragge
Iain Hibbert
2012-03-17 16:12:04 UTC
Permalink
Post by Anders Magnusson
Hi Iain,
Post by Iain Hibbert
..can anybody confirm this is right?
Yep, it is correct. It should be checked though that the attributes
really follows into the function where it prototypes for and not get
tossed away somewhere.
Ok great thanks but how do I do that, I guess some debug output can show
it?
Post by Anders Magnusson
Post by Iain Hibbert
while here though, I wonder when the patterns without the preceding
abstract_declarator will match, do I need to handle attributes there also?
Yep, otherwise declarations like
int x(int (int) __attribute__((unused)));
will fail :-)
I see.. but what does that even mean, is it some kind of shortcut?

iain

(will also expand the examples and put into tests)
Anders Magnusson
2012-03-17 16:26:37 UTC
Permalink
Post by Iain Hibbert
Post by Anders Magnusson
Hi Iain,
Post by Iain Hibbert
..can anybody confirm this is right?
Yep, it is correct. It should be checked though that the attributes
really follows into the function where it prototypes for and not get
tossed away somewhere.
Ok great thanks but how do I do that, I guess some debug output can show
it?
ccom -Xp will show prototype information. I can check it otherwise.
Post by Iain Hibbert
Post by Anders Magnusson
Post by Iain Hibbert
while here though, I wonder when the patterns without the preceding
abstract_declarator will match, do I need to handle attributes there also?
Yep, otherwise declarations like
int x(int (int) __attribute__((unused)));
will fail :-)
I see.. but what does that even mean, is it some kind of shortcut?
It's a function declaration without argument name, which in a
prototype will become a function pointer. Example will explain:

int x(int (int) __attribute__((unused)));
int x(int a(int) __attribute__((unused))) { return a(1);}
Iain Hibbert
2012-03-17 18:54:47 UTC
Permalink
Post by Anders Magnusson
Post by Iain Hibbert
Post by Anders Magnusson
Hi Iain,
Post by Iain Hibbert
..can anybody confirm this is right?
Yep, it is correct. It should be checked though that the attributes
really follows into the function where it prototypes for and not get
tossed away somewhere.
Ok great thanks but how do I do that, I guess some debug output can show
it?
ccom -Xp will show prototype information. I can check it otherwise.
Hm, there is an unrelated problem there.. with a vanilla pcc (no local
changes)

% cat a.c
void foo ( __attribute__ ((__unused__)) void (*a)() );
% pcc -S -Wc,-Xp a.c
major internal compiler error: a.c, line 1
%

this is only where the parameter list was empty though (in any
configuration); adding a parameter gives

% cat a.c
void foo ( __attribute__ ((__unused__)) void (*a)(int a) );
% pcc -S -Wc,-Xp a.c
% cat a.s
arglist 0xbb901798
0xbb901798) NAME, 0, 0x0, int, 0x0, attributes;
arg 0: int
end arglist
arglist 0xbb9017f0
0xbb9017f0) NAME, 3146782404, 0x0, PTR FTN void, 0xbb90187c, attributes; unused: 0 0 0,
arg 0: PTR FTN void arg 0: int

end arglist
.ident "PCC: pcc 1.1.0.DEVEL 20120307 for netbsd-i386"
%

which I think is normal.. (providing a "void" parameter also works)

(I will investigate the attributes for my changes in a bit)
Post by Anders Magnusson
Post by Iain Hibbert
Post by Anders Magnusson
Post by Iain Hibbert
while here though, I wonder when the patterns without the preceding
abstract_declarator will match, do I need to handle attributes there also?
Yep, otherwise declarations like
int x(int (int) __attribute__((unused)));
will fail :-)
I see.. but what does that even mean, is it some kind of shortcut?
It's a function declaration without argument name, which in a
int x(int (int) __attribute__((unused)));
int x(int a(int) __attribute__((unused))) { return a(1);}
Ok I see, it seems that

int x(int (int));

is equivalent to

int x(int (*)(int));

though perhaps less clear..

iain
Iain Hibbert
2012-03-17 19:13:28 UTC
Permalink
Post by Iain Hibbert
(I will investigate the attributes for my changes in a bit)
So anyway, it appears that something is not complete with my patch..


1. void foo2 ( __a void (*)(int) );

arglist 0xbb90178c
0xbb90178c) NAME, 0, 0x0, int, 0x0, attributes;
arg 0: int
end arglist
arglist 0xbb9017e4
0xbb9017e4) NAME, 3146782408, 0x0, PTR FTN void, 0xbb901870, attributes; unused: 0 0 0,
arg 0: PTR FTN void arg 0: int

end arglist



2. void foo3 ( void (*)(int) __a );

arglist 0xbb901760
0xbb901760) NAME, 0, 0x0, int, 0x0, attributes;
arg 0: int
end arglist
arglist 0xbb9017d0
0xbb9017d0) NAME, 3146782472, 0x0, PTR FTN void, 0xbb901870, attributes;
arg 0: PTR FTN void arg 0: int

end arglist


when the abstract_declarator is matched, the attribute is lost..?

(also, pcc.ludd.ltu.se seems to have gone to sleep :)

iain
Anders Magnusson
2012-03-17 20:55:37 UTC
Permalink
Post by Iain Hibbert
(also, pcc.ludd.ltu.se seems to have gone to sleep :)
I tried to reboot it a few hours ago, but something didn't work out:

ragge-laptop:/home/ragge >date
Sat Mar 17 21:53:35 CET 2012
ragge-laptop:/home/ragge >ssh pcc.ludd.ltu.se


NO LOGINS: System going down at 17:56

I will go to the console tomorrow.

-- Ragge
Iain Hibbert
2012-03-19 19:22:42 UTC
Permalink
Post by Iain Hibbert
Post by Anders Magnusson
Post by Iain Hibbert
Post by Anders Magnusson
Hi Iain,
Post by Iain Hibbert
..can anybody confirm this is right?
Yep, it is correct. It should be checked though that the attributes
really follows into the function where it prototypes for and not get
tossed away somewhere.
Ok great thanks but how do I do that, I guess some debug output can show
it?
ccom -Xp will show prototype information. I can check it otherwise.
Hm, there is an unrelated problem there.. with a vanilla pcc (no local
changes)
% cat a.c
void foo ( __attribute__ ((__unused__)) void (*a)() );
% pcc -S -Wc,-Xp a.c
major internal compiler error: a.c, line 1
%
I fixed that

my other fix though, while it works ok for the syntax, it doesn't seem to
work for the attributes.. but the prior art in this area doesn't seem
right either, eg

void foo ( __a int[] );

arglist 0xbb901720
0xbb901720) NAME, 3146782408, 0x0, ARY int, 0xbb9017e4, attributes; unused: 0 0 0,
arg 0: PTR int
end arglist

void foo ( int[] __a );

arglist 0xbb90174c
0xbb90174c) NAME, 3146782604, 0x0, ARY int, 0xbb901850, attributes;
arg 0: PTR int
end arglist

void foo ( __a int a[] );

arglist 0xbb9016c8
0xbb9016c8) NAME, 3146782540, 0x0, ARY int, 0xbb9018b8, attributes; unused: 0 0 0,
arg 0: PTR int
end arglist

void foo ( int a[] __a );

arglist 0xbb9014ac
0xbb9014ac) NAME, 3146782408, 0x0, ARY int, 0xbb9018e0, attributes; unused: 0 0 0,
arg 0: PTR int
end arglist

so hmm, any suggestions?

iain


*** cgram.y.orig 2011-09-03 08:45:26.000000000 +0100
--- cgram.y 2012-03-17 16:15:09.000000000 +0000
***************
*** 450,464 ****
| abstract_declarator '[' e ']' attr_var {
$$ = block(LB, $1, $3, INT, 0, gcc_attr_parse($5));
}
! | '(' ')' { $$ = bdty(UCALL, bdty(NAME, NULL)); }
! | '(' ib2 parameter_type_list ')' {
! $$ = bdty(CALL, bdty(NAME, NULL), $3);
}
! | abstract_declarator '(' ')' {
! $$ = bdty(UCALL, $1);
}
! | abstract_declarator '(' ib2 parameter_type_list ')' {
! $$ = bdty(CALL, $1, $4);
}
;

--- 450,466 ----
| abstract_declarator '[' e ']' attr_var {
$$ = block(LB, $1, $3, INT, 0, gcc_attr_parse($5));
}
! | '(' ')' attr_var {
! $$ = block(UCALL, bdty(NAME, NULL), NULL, INT, 0, gcc_attr_parse($3));
}
! | '(' ib2 parameter_type_list ')' attr_var {
! $$ = block(CALL, bdty(NAME, NULL), $3, INT, 0, gcc_attr_parse($5));
}
! | abstract_declarator '(' ')' attr_var {
! $$ = block(UCALL, $1, NULL, INT, 0, gcc_attr_parse($4));
! }
! | abstract_declarator '(' ib2 parameter_type_list ')' attr_var {
! $$ = block(CALL, $1, $4, INT, 0, gcc_attr_parse($6));
}
;
Iain Hibbert
2012-03-22 19:08:54 UTC
Permalink
Post by Iain Hibbert
my other fix though, while it works ok for the syntax, it doesn't seem to
work for the attributes.. but the prior art in this area doesn't seem
right either, eg
void foo ( __a int[] );
arglist 0xbb901720
0xbb901720) NAME, 3146782408, 0x0, ARY int, 0xbb9017e4, attributes; unused: 0 0 0,
arg 0: PTR int
end arglist
void foo ( int[] __a );
arglist 0xbb90174c
0xbb90174c) NAME, 3146782604, 0x0, ARY int, 0xbb901850, attributes;
arg 0: PTR int
end arglist
I investigated this some more, and it seems that the attribute is parsed
but when passed to tymerge() the attribute gets lost.. tymerge() does not
take attributes from the NAME though, by design according to the comments,
The attributes are supposed to be on the TYPE part..

iain
Anders Magnusson
2012-03-24 08:22:11 UTC
Permalink
Post by Iain Hibbert
Post by Iain Hibbert
my other fix though, while it works ok for the syntax, it doesn't seem to
work for the attributes.. but the prior art in this area doesn't seem
right either, eg
void foo ( __a int[] );
arglist 0xbb901720
0xbb901720) NAME, 3146782408, 0x0, ARY int, 0xbb9017e4, attributes; unused: 0 0 0,
arg 0: PTR int
end arglist
void foo ( int[] __a );
arglist 0xbb90174c
0xbb90174c) NAME, 3146782604, 0x0, ARY int, 0xbb901850, attributes;
arg 0: PTR int
end arglist
I investigated this some more, and it seems that the attribute is parsed
but when passed to tymerge() the attribute gets lost.. tymerge() does not
take attributes from the NAME though, by design according to the comments,
The attributes are supposed to be on the TYPE part..
The comments are correct, the problem is the abstract declarators. If
there is a variable given in the prototype it works:

void foo ( int[] __a);
arglist 0x800c0a340
0x800c0a340) NAME, 34372362992, 0x0, ARY int, 0x800c0a430, attributes;
arg 0: PTR int
end arglist

void foo ( int t[] __a);
arglist 0x800c099d0
0x800c099d0) NAME, 34372363072, 0x0, ARY int, 0x800c0a530, attributes;
unused: 0 0 0,
arg 0: PTR int
end arglist

Note that an attribute given before the type relates it to the type
itself (and all following declarations), but after a declaration only to
the declaration where it is given. For prototypes it do not matter though.

I'll have a look at it.

-- Ragge
Anders Magnusson
2012-03-24 16:55:12 UTC
Permalink
Post by Iain Hibbert
Post by Iain Hibbert
my other fix though, while it works ok for the syntax, it doesn't seem to
work for the attributes.. but the prior art in this area doesn't seem
right either, eg
void foo ( __a int[] );
arglist 0xbb901720
0xbb901720) NAME, 3146782408, 0x0, ARY int, 0xbb9017e4, attributes; unused: 0 0 0,
arg 0: PTR int
end arglist
void foo ( int[] __a );
arglist 0xbb90174c
0xbb90174c) NAME, 3146782604, 0x0, ARY int, 0xbb901850, attributes;
arg 0: PTR int
end arglist
I investigated this some more, and it seems that the attribute is parsed
but when passed to tymerge() the attribute gets lost.. tymerge() does not
take attributes from the NAME though, by design according to the comments,
The attributes are supposed to be on the TYPE part..
Fixed now. The attributes are moved to the other node before tymerge().

-- Ragge
Iain Hibbert
2012-03-26 08:15:00 UTC
Permalink
Post by Anders Magnusson
Post by Iain Hibbert
I investigated this some more, and it seems that the attribute is parsed
but when passed to tymerge() the attribute gets lost.. tymerge() does not
take attributes from the NAME though, by design according to the comments,
The attributes are supposed to be on the TYPE part..
Fixed now. The attributes are moved to the other node before tymerge().
Ah, thanks.. I think that fixes building stuff on NetBSD again (Jörg was
adding attributes to the header files :)

iain

Loading...