[PATCH] do not ignore attribute 'noreturn'...

[PATCH] do not ignore attribute 'noreturn'...

[Kamil Dudka]

[-- Attachment #1: Type: text/plain, Size: 617 bytes --]

Hello,

enclosed is a simple patch adding support for attribute 'noreturn' to the 
parser. The enhancement makes it possible to optimize walk through CFG and 
thus help us to fight with the state explosion. The benefit is demonstrated 
on a simple real-world example.

Generated CFG before patch:
http://dudka.cz/devel/html/slsparse-before/slplug.c-handle_stmt_assign.svg

Generated CFG after patch:
http://dudka.cz/devel/html/slsparse-after/slplug.c-handle_stmt_assign.svg

It's one of the key features I am currently missing in SPARSE in contrast
to gcc used as parser. Thanks in advance for considering it!

Kamil

[-- Attachment #2: 0001-do-not-ignore-attribute-noreturn.patch --]
[-- Type: text/x-diff, Size: 2134 bytes --]

From 892f19915cbcbe26b3a7ad2c3baf1a1420f3c954 Mon Sep 17 00:00:00 2001
From: Kamil Dudka <kdudka@redhat.com>
Date: Fri, 28 Aug 2009 21:49:16 +0200
Subject: [PATCH] do not ignore attribute 'noreturn'...

... and thus make it possible to optimize walk through CFG.

Signed-off-by: Kamil Dudka <kdudka@redhat.com>
---
 parse.c  |    4 ++--
 symbol.h |    4 +++-
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/parse.c b/parse.c
index e5ad867..215c4da 100644
--- a/parse.c
+++ b/parse.c
@@ -480,8 +480,8 @@ static struct init_keyword {
 	{ "const",	NS_KEYWORD,	.op = &ignore_attr_op },
 	{ "__const",	NS_KEYWORD,	.op = &ignore_attr_op },
 	{ "__const__",	NS_KEYWORD,	.op = &ignore_attr_op },
-	{ "noreturn",	NS_KEYWORD,	.op = &ignore_attr_op },
-	{ "__noreturn__",	NS_KEYWORD,	.op = &ignore_attr_op },
+	{ "noreturn",	NS_KEYWORD,	MOD_NORETURN,	.op = &attr_mod_op },
+	{ "__noreturn__",	NS_KEYWORD,	MOD_NORETURN,	.op = &attr_mod_op },
 	{ "no_instrument_function",	NS_KEYWORD,	.op = &ignore_attr_op },
 	{ "__no_instrument_function__",	NS_KEYWORD,	.op = &ignore_attr_op },
 	{ "sentinel",	NS_KEYWORD,	.op = &ignore_attr_op },
diff --git a/symbol.h b/symbol.h
index 42d69d6..d8d1793 100644
--- a/symbol.h
+++ b/symbol.h
@@ -216,6 +216,8 @@ struct symbol {
 #define MOD_EXPLICITLY_SIGNED	0x40000000
 #define MOD_BITWISE	0x80000000
 
+#define MOD_NORETURN	0x100000000
+
 #define MOD_NONLOCAL	(MOD_EXTERN | MOD_TOPLEVEL)
 #define MOD_STORAGE	(MOD_AUTO | MOD_REGISTER | MOD_STATIC | MOD_EXTERN | MOD_INLINE | MOD_TOPLEVEL)
 #define MOD_SIGNEDNESS	(MOD_SIGNED | MOD_UNSIGNED | MOD_EXPLICITLY_SIGNED)
@@ -223,7 +225,7 @@ struct symbol {
 #define MOD_SIZE	(MOD_CHAR | MOD_SHORT | MOD_LONG | MOD_LONGLONG)
 #define MOD_IGNORE (MOD_TOPLEVEL | MOD_STORAGE | MOD_ADDRESSABLE |	\
 	MOD_ASSIGNED | MOD_USERTYPE | MOD_ACCESSED | MOD_EXPLICITLY_SIGNED)
-#define MOD_PTRINHERIT (MOD_VOLATILE | MOD_CONST | MOD_NODEREF | MOD_STORAGE)
+#define MOD_PTRINHERIT (MOD_VOLATILE | MOD_CONST | MOD_NODEREF | MOD_STORAGE | MOD_NORETURN)
 
 
 /* Current parsing/evaluation function */
-- 
1.6.4.1

Re: [PATCH] do not ignore attribute 'noreturn'...

[Christopher Li]

On Fri, Aug 28, 2009 at 2:30 PM, Kamil Dudka<kdudka@redhat.com> wrote:
> enclosed is a simple patch adding support for attribute 'noreturn' to the
> parser. The enhancement makes it possible to optimize walk through CFG and
> thus help us to fight with the state explosion. The benefit is demonstrated
> on a simple real-world example.
>
> Generated CFG before patch:
> http://dudka.cz/devel/html/slsparse-before/slplug.c-handle_stmt_assign.svg
>
> Generated CFG after patch:
> http://dudka.cz/devel/html/slsparse-after/slplug.c-handle_stmt_assign.svg
>
> It's one of the key features I am currently missing in SPARSE in contrast
> to gcc used as parser. Thanks in advance for considering it!

Yes, no return is kind of useful. I think we need to do some thing about the
MOD_XXX eventually. It is very easy to run out of bits there.

Thanks for the patch. Applied.

Chris

Re: [PATCH] do not ignore attribute 'noreturn'...

[Christopher Li]

On Fri, Aug 28, 2009 at 2:44 PM, Christopher Li <sparse@chrisli.org> wrote:
> Yes, no return is kind of useful. I think we need to do some thing about the
> MOD_XXX eventually. It is very easy to run out of bits there.
>
Ah, the MOD_NORETURN is bigger than size of long in 32 bit systems.
I just get a few warning from GCC.

I just relocate the MOD_NORETURN bits.

Chris

diff --git a/symbol.h b/symbol.h
index 80ef363..9f5a32f 100644
--- a/symbol.h
+++ b/symbol.h
@@ -214,10 +214,10 @@ struct symbol {
 #define MOD_SAFE       0x8000000       // non-null/non-trapping pointer

 #define MOD_USERTYPE   0x10000000
+#define MOD_NORETURN   0x20000000
 #define MOD_EXPLICITLY_SIGNED  0x40000000
 #define MOD_BITWISE    0x80000000

-#define MOD_NORETURN   0x100000000

 #define MOD_NONLOCAL   (MOD_EXTERN | MOD_TOPLEVEL)
 #define MOD_STORAGE    (MOD_AUTO | MOD_REGISTER | MOD_STATIC | MOD_EXTERN | MOD

Re: [PATCH] do not ignore attribute 'noreturn'...

[Kamil Dudka]

On Monday 14 of September 2009 19:27:40 Christopher Li wrote:
> On Fri, Aug 28, 2009 at 2:44 PM, Christopher Li <sparse@chrisli.org> wrote:
> > Yes, no return is kind of useful. I think we need to do some thing about
> > the MOD_XXX eventually. It is very easy to run out of bits there.
>
> Ah, the MOD_NORETURN is bigger than size of long in 32 bit systems.
> I just get a few warning from GCC.
>
> I just relocate the MOD_NORETURN bits.

Thanks! It makes sense to me.

As for the long term solution, I don't think it's possible without breaking 
the current API/ABI. What about using a bitfield instead? I can see it solved 
this way in <gcc/tree.h>.

Kamil

Re: [PATCH] do not ignore attribute 'noreturn'...

[Christopher Li]

On Mon, Sep 14, 2009 at 10:32 AM, Kamil Dudka <kdudka@redhat.com> wrote:
> As for the long term solution, I don't think it's possible without breaking
> the current API/ABI. What about using a bitfield instead? I can see it solved
> this way in <gcc/tree.h>.

I am actually not afraid of breaking current API/ABI because there is not
enough sparse user application to worry about. Just recompile the apps should
be fine. Please warn me if any one disagree.

As for long term, here is my plan, it will go will with Al's replace same
type with same type pointer plan as well.

We need to have a different structure to support the extended attributes
for ctype. Current "contexts" and "as" should go into that extended attribute
as well. Then ctype should have one pointer for extended attributes.
For most common case, the extended attribute pointer is just NULL.

Because the extended attribute pointer is possibles to be shared between
different ctypes. It is not allow to modify the extended attribute struct
from ctype directly. Instead it should replace with a new one.
We can even hash the extended attribute struct so same attribute struct
will have the same pointer, just like ident. That will make comparing attribute
is the same much easier.

I try it before, it is  hard to do because current code does implicit
assign for the ctype.

Chris

Re: [PATCH] do not ignore attribute 'noreturn'...

[Kamil Dudka]

On Monday 14 of September 2009 19:55:49 Christopher Li wrote:
> On Mon, Sep 14, 2009 at 10:32 AM, Kamil Dudka <kdudka@redhat.com> wrote:
> > As for the long term solution, I don't think it's possible without
> > breaking the current API/ABI. What about using a bitfield instead? I can
> > see it solved this way in <gcc/tree.h>.
>
> I am actually not afraid of breaking current API/ABI because there is not
> enough sparse user application to worry about. Just recompile the apps
> should be fine. Please warn me if any one disagree.
>
> As for long term, here is my plan, it will go will with Al's replace same
> type with same type pointer plan as well.
>
> We need to have a different structure to support the extended attributes
> for ctype. Current "contexts" and "as" should go into that extended
> attribute as well. Then ctype should have one pointer for extended
> attributes. For most common case, the extended attribute pointer is just
> NULL.
>
> Because the extended attribute pointer is possibles to be shared between
> different ctypes. It is not allow to modify the extended attribute struct
> from ctype directly. Instead it should replace with a new one.
> We can even hash the extended attribute struct so same attribute struct
> will have the same pointer, just like ident. That will make comparing
> attribute is the same much easier.
>
> I try it before, it is  hard to do because current code does implicit
> assign for the ctype.

It sounds like a tough challenge to me :-)

I am playing with the SPARSE API now. Any chance to pass in a unique ID
to each symbol? (as equivalent to DECL_UID from <gcc/tree.h>) I can of course 
manage an ptr/ID mapping at application level. I am only curious if this is 
possible to place directly into SPARSE, so that other apps could gain
from it. Please let me know if such functionality is already implemented
in SPARSE.

Kamil

Re: [PATCH] do not ignore attribute 'noreturn'...

[Christopher Li]

On Mon, Sep 14, 2009 at 12:44 PM, Kamil Dudka <kdudka@redhat.com> wrote:
> It sounds like a tough challenge to me :-)
>
> I am playing with the SPARSE API now. Any chance to pass in a unique ID
> to each symbol? (as equivalent to DECL_UID from <gcc/tree.h>) I can of course
> manage an ptr/ID mapping at application level. I am only curious if this is

Can you clarify why do you need to use the UID instead of the pointer of
symbol? One reason I can see is that if you want to dump the AST tree
into objects on disk. Other application can read it back and load into
memory. If the symbol already exist in the memory, you should be able to
use pointer directly.

BTW, I have code to dump most of the struct into disk already. I think
I send to the list before. Of course I can create a branch for it. Reading
is not completed yet.

Chris

Re: [PATCH] do not ignore attribute 'noreturn'...

[Kamil Dudka]

On Monday 14 of September 2009 22:47:40 Christopher Li wrote:
> Can you clarify why do you need to use the UID instead of the pointer of
> symbol? One reason I can see is that if you want to dump the AST tree
> into objects on disk. Other application can read it back and load into
> memory. If the symbol already exist in the memory, you should be able to
> use pointer directly.

The only reason (for now) is that I already use them with the gcc plug-in and 
the code_listener interace is based on them. I don't think it *needs* to be 
implemented in SPARSE since the workaround is easy. I've only raised the idea
in hope such enumeration might be generally useful.

Adding Petr Peringer to CC. He came with UID idea to our project and might be 
able to give us better clarification for that approach.

> BTW, I have code to dump most of the struct into disk already. I think
> I send to the list before. Of course I can create a branch for it. Reading
> is not completed yet.

Thanks! Good to know something like that is available. But I don't think our 
project can gain from this functionality right now.

Kamil