* [PATCH 1/3] struct_union_enum_specifier: set MOD_TOPLEVEL if toplevel(sym->scope)
@ 2020-02-14 11:33 Oleg Nesterov
2020-02-14 11:33 ` [PATCH 2/3] dissect: fix sym_is_local(SYM_STRUCT/UNION/ENUM) Oleg Nesterov
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Oleg Nesterov @ 2020-02-14 11:33 UTC (permalink / raw)
To: Luc Van Oostenryck; +Cc: Alexey Gladkov, linux-sparse
With this change dissect can know the scope of SYM_STRUCT/UNION/ENUM,
see the next patch.
Note that MOD_TOPLEVEL can be set even if struct/union/enum type is
private and bind_symbol() is not called.
IIUC nobody else looks at SYM_STRUCT->ctype.modifiers, "make check"
doesn't show any difference.
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
parse.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/parse.c b/parse.c
index a08165a..4492586 100644
--- a/parse.c
+++ b/parse.c
@@ -741,6 +741,8 @@ static struct token *struct_union_enum_specifier(enum type type,
// symbol being redefined.
sym = alloc_symbol(token->pos, type);
bind_symbol(sym, token->ident, NS_STRUCT);
+ if (toplevel(sym->scope))
+ sym->ctype.modifiers |= MOD_TOPLEVEL;
}
if (sym->type != type)
error_die(token->pos, "invalid tag applied to %s", show_typename (sym));
@@ -772,6 +774,8 @@ static struct token *struct_union_enum_specifier(enum type type,
}
sym = alloc_symbol(token->pos, type);
+ if (toplevel(block_scope))
+ sym->ctype.modifiers |= MOD_TOPLEVEL;
token = parse(token->next, sym);
ctx->ctype.base_type = sym;
token = expect(token, '}', "at end of specifier");
--
2.5.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/3] dissect: fix sym_is_local(SYM_STRUCT/UNION/ENUM)
2020-02-14 11:33 [PATCH 1/3] struct_union_enum_specifier: set MOD_TOPLEVEL if toplevel(sym->scope) Oleg Nesterov
@ 2020-02-14 11:33 ` Oleg Nesterov
2020-02-14 11:34 ` [PATCH 3/3] dissect: enforce MOD_TOPLEVEL if SYM_STRUCT was not defined Oleg Nesterov
2020-02-17 21:46 ` [PATCH 1/3] struct_union_enum_specifier: set MOD_TOPLEVEL if toplevel(sym->scope) Luc Van Oostenryck
2 siblings, 0 replies; 6+ messages in thread
From: Oleg Nesterov @ 2020-02-14 11:33 UTC (permalink / raw)
To: Luc Van Oostenryck; +Cc: Alexey Gladkov, linux-sparse
Now that struct_union_enum_specifier() sets MOD_TOPLEVEL we can
simplify sym_is_local(sym) and rely on it even if "sym" is type.
Test-case:
// copied from linux kernel
# define __force __attribute__((force))
#define WRITE_ONCE(x, val) \
({ \
union { typeof(x) __val; char __c[1]; } __u = \
{ .__val = (__force typeof(x)) (val) }; \
__write_once_size(&(x), __u.__c, sizeof(x)); \
__u.__val; \
})
void func(int *p)
{
WRITE_ONCE(*p, 0);
}
before this patch the widely used WRITE_ONCE() generates a lot of spam which
can't be filtered out using sym_is_local(),
11:6 def f func void ( ... )
11:11 func def . v p int *
13:9 def s :__u
13:9 --- . v p int *
13:9 def m :__u.__val int
13:9 def m :__u.__c char [1]
13:9 func def . v __u union :__u
13:9 func -w- . v __u union :__u
13:9 func -w- m :__u.__val int
13:9 func --- . v p int *
13:9 func --r f __write_once_size bad type
13:9 func -r- . v p int *
13:9 func -r- . v __u union :__u
13:9 func m-- m :__u.__c char [1]
13:9 func --- . v p int *
13:9 func --- . v __u union :__u
13:9 func --- m :__u.__val int
plus it triggers warning("no context") in test-dissect.c. With this patch
the only "nonlocal" report is __write_once_size() call.
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
dissect.c | 3 ++-
dissect.h | 2 +-
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/dissect.c b/dissect.c
index 40baf64..1465760 100644
--- a/dissect.c
+++ b/dissect.c
@@ -238,7 +238,8 @@ static void examine_sym_node(struct symbol *node, struct symbol *parent)
return;
dctx = dissect_ctx;
- dissect_ctx = NULL;
+ if (base->ctype.modifiers & MOD_TOPLEVEL)
+ dissect_ctx = NULL;
if (base->ident || deanon(base, name, parent))
reporter->r_symdef(base);
diff --git a/dissect.h b/dissect.h
index 326d3dc..38ac877 100644
--- a/dissect.h
+++ b/dissect.h
@@ -29,7 +29,7 @@ extern struct symbol *dissect_ctx;
static inline bool sym_is_local(struct symbol *sym)
{
- return sym->kind == 'v' && !(sym->ctype.modifiers & MOD_TOPLEVEL);
+ return !(sym->ctype.modifiers & MOD_TOPLEVEL);
}
extern void dissect(struct reporter *, struct string_list *);
--
2.5.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 3/3] dissect: enforce MOD_TOPLEVEL if SYM_STRUCT was not defined
2020-02-14 11:33 [PATCH 1/3] struct_union_enum_specifier: set MOD_TOPLEVEL if toplevel(sym->scope) Oleg Nesterov
2020-02-14 11:33 ` [PATCH 2/3] dissect: fix sym_is_local(SYM_STRUCT/UNION/ENUM) Oleg Nesterov
@ 2020-02-14 11:34 ` Oleg Nesterov
2020-02-17 21:46 ` [PATCH 1/3] struct_union_enum_specifier: set MOD_TOPLEVEL if toplevel(sym->scope) Luc Van Oostenryck
2 siblings, 0 replies; 6+ messages in thread
From: Oleg Nesterov @ 2020-02-14 11:34 UTC (permalink / raw)
To: Luc Van Oostenryck; +Cc: Alexey Gladkov, linux-sparse
A separate change for documentation purposes.
Test-case:
void func(void)
{
struct UNDEFINED x;
x.member = 0;
}
output:
1:6 def f func void ( ... )
3:26 func def . v x struct UNDEFINED
4:9 func -w- . v x struct UNDEFINED
4:10 func -w- . m UNDEFINED.member bad type
but in this case is_sym_local(UNDEFINED) = F makes more sense, most
probably this struct was defined somewhere else but __sparse() didn't
see its definition.
Change lookup_member() to set MOD_TOPLEVEL if !SYM_STRUCT symbol_list.
This is not 100% correct, but struct_union_enum_specifier() does the
same with the following comment:
// The following test is actually wrong for empty
// structs, but (1) they are not C99, (2) gcc does
// the same thing, and (3) it's easier.
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
dissect.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/dissect.c b/dissect.c
index 1465760..f60999e 100644
--- a/dissect.c
+++ b/dissect.c
@@ -305,6 +305,8 @@ static struct symbol *lookup_member(struct symbol *type, struct ident *name, int
.kind = 'm',
};
+ if (!type->symbol_list)
+ type->ctype.modifiers |= MOD_TOPLEVEL;
mem = &bad_member;
mem->ident = name;
}
--
2.5.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/3] struct_union_enum_specifier: set MOD_TOPLEVEL if toplevel(sym->scope)
2020-02-14 11:33 [PATCH 1/3] struct_union_enum_specifier: set MOD_TOPLEVEL if toplevel(sym->scope) Oleg Nesterov
2020-02-14 11:33 ` [PATCH 2/3] dissect: fix sym_is_local(SYM_STRUCT/UNION/ENUM) Oleg Nesterov
2020-02-14 11:34 ` [PATCH 3/3] dissect: enforce MOD_TOPLEVEL if SYM_STRUCT was not defined Oleg Nesterov
@ 2020-02-17 21:46 ` Luc Van Oostenryck
2020-02-18 10:38 ` Oleg Nesterov
2 siblings, 1 reply; 6+ messages in thread
From: Luc Van Oostenryck @ 2020-02-17 21:46 UTC (permalink / raw)
To: Oleg Nesterov; +Cc: Alexey Gladkov, linux-sparse
On Fri, Feb 14, 2020 at 12:33:20PM +0100, Oleg Nesterov wrote:
> With this change dissect can know the scope of SYM_STRUCT/UNION/ENUM,
> see the next patch.
>
> Note that MOD_TOPLEVEL can be set even if struct/union/enum type is
> private and bind_symbol() is not called.
I don't like that very much. For example: why this is needed for
struct/union/enum and not other types?
Should it be possible to use the function toplevel() or add and
helper for it in scope.c?
> IIUC nobody else looks at SYM_STRUCT->ctype.modifiers, "make check"
> doesn't show any difference.
Yes, it's true and it shouldn't make any difference but still I
would prefer to not mix symbols and types more than they already are.
-- Luc
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/3] struct_union_enum_specifier: set MOD_TOPLEVEL if toplevel(sym->scope)
2020-02-17 21:46 ` [PATCH 1/3] struct_union_enum_specifier: set MOD_TOPLEVEL if toplevel(sym->scope) Luc Van Oostenryck
@ 2020-02-18 10:38 ` Oleg Nesterov
2020-02-18 15:59 ` Luc Van Oostenryck
0 siblings, 1 reply; 6+ messages in thread
From: Oleg Nesterov @ 2020-02-18 10:38 UTC (permalink / raw)
To: Luc Van Oostenryck; +Cc: Alexey Gladkov, linux-sparse
On 02/17, Luc Van Oostenryck wrote:
>
> On Fri, Feb 14, 2020 at 12:33:20PM +0100, Oleg Nesterov wrote:
> > With this change dissect can know the scope of SYM_STRUCT/UNION/ENUM,
> > see the next patch.
> >
> > Note that MOD_TOPLEVEL can be set even if struct/union/enum type is
> > private and bind_symbol() is not called.
>
> I don't like that very much. For example: why this is needed for
> struct/union/enum and not other types?
Do you mean builtin types like int_ctype? OK, I agree, this is slightly
inconsistent.
> Should it be possible to use the function toplevel() or add and
> helper for it in scope.c?
Well, toplevel() won't work if SYM_STRUCT/etc is anonymous, in this
case bind_symbol() is not called and thus sym->scope = NULL.
Consider
struct { int m; } x;
void func(void)
{
struct { int m; } x;
}
we want to report the 2nd struct definition as "local"
1:8 def s :x
1:14 def m :x.m int
1:19 def v x struct :x
3:6 def f func void ( ... )
5:16 func def . s :x
5:22 func def . m :x.m int
5:27 func def . v x struct :x
so that this spam can be filtered out, but base->scope is NULL in both
cases.
> > IIUC nobody else looks at SYM_STRUCT->ctype.modifiers, "make check"
> > doesn't show any difference.
>
> Yes, it's true and it shouldn't make any difference but still I
> would prefer to not mix symbols and types more than they already are.
OK, will you agree with one-liner below? This should make toplevel() work.
Oleg.
--- a/parse.c
+++ b/parse.c
@@ -772,6 +772,7 @@ static struct token *struct_union_enum_specifier(enum type type,
}
sym = alloc_symbol(token->pos, type);
+ sym->scope = block_scope;
token = parse(token->next, sym);
ctx->ctype.base_type = sym;
token = expect(token, '}', "at end of specifier");
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/3] struct_union_enum_specifier: set MOD_TOPLEVEL if toplevel(sym->scope)
2020-02-18 10:38 ` Oleg Nesterov
@ 2020-02-18 15:59 ` Luc Van Oostenryck
0 siblings, 0 replies; 6+ messages in thread
From: Luc Van Oostenryck @ 2020-02-18 15:59 UTC (permalink / raw)
To: Oleg Nesterov; +Cc: Alexey Gladkov, linux-sparse
On Tue, Feb 18, 2020 at 11:38:38AM +0100, Oleg Nesterov wrote:
> On 02/17, Luc Van Oostenryck wrote:
> >
> > On Fri, Feb 14, 2020 at 12:33:20PM +0100, Oleg Nesterov wrote:
> > > With this change dissect can know the scope of SYM_STRUCT/UNION/ENUM,
> > > see the next patch.
> > >
> > > Note that MOD_TOPLEVEL can be set even if struct/union/enum type is
> > > private and bind_symbol() is not called.
> >
> > I don't like that very much. For example: why this is needed for
> > struct/union/enum and not other types?
>
> Do you mean builtin types like int_ctype? OK, I agree, this is slightly
> inconsistent.
I was thinking of the other constructed types: arrays & pointers.
> > Should it be possible to use the function toplevel() or add and
> > helper for it in scope.c?
>
> Well, toplevel() won't work if SYM_STRUCT/etc is anonymous, in this
> case bind_symbol() is not called and thus sym->scope = NULL.
>
> Consider
>
> struct { int m; } x;
>
> void func(void)
> {
> struct { int m; } x;
>
> }
>
> we want to report the 2nd struct definition as "local"
>
> 1:8 def s :x
> 1:14 def m :x.m int
> 1:19 def v x struct :x
> 3:6 def f func void ( ... )
> 5:16 func def . s :x
> 5:22 func def . m :x.m int
> 5:27 func def . v x struct :x
>
> so that this spam can be filtered out, but base->scope is NULL in both
> cases.
OK, I see.
> > > IIUC nobody else looks at SYM_STRUCT->ctype.modifiers, "make check"
> > > doesn't show any difference.
> >
> > Yes, it's true and it shouldn't make any difference but still I
> > would prefer to not mix symbols and types more than they already are.
>
> OK, will you agree with one-liner below? This should make toplevel() work.
This will still be inconsistent with the other types but I can live
with this. If you could just add a comment explaining why it is needed
and using an helper instead of directly using 'block_scope' (like
set_[current_]scope() or something).
-- Luc
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-02-18 15:59 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-02-14 11:33 [PATCH 1/3] struct_union_enum_specifier: set MOD_TOPLEVEL if toplevel(sym->scope) Oleg Nesterov
2020-02-14 11:33 ` [PATCH 2/3] dissect: fix sym_is_local(SYM_STRUCT/UNION/ENUM) Oleg Nesterov
2020-02-14 11:34 ` [PATCH 3/3] dissect: enforce MOD_TOPLEVEL if SYM_STRUCT was not defined Oleg Nesterov
2020-02-17 21:46 ` [PATCH 1/3] struct_union_enum_specifier: set MOD_TOPLEVEL if toplevel(sym->scope) Luc Van Oostenryck
2020-02-18 10:38 ` Oleg Nesterov
2020-02-18 15:59 ` Luc Van Oostenryck
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).