[PATCH 1/3] struct_union_enum_specifier: set MOD_TOPLEVEL if toplevel(sym->scope)

[PATCH 1/3] struct_union_enum_specifier: set MOD_TOPLEVEL if toplevel(sym->scope)

[Oleg Nesterov]

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

[PATCH 2/3] dissect: fix sym_is_local(SYM_STRUCT/UNION/ENUM)

[Oleg Nesterov]

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

[PATCH 3/3] dissect: enforce MOD_TOPLEVEL if SYM_STRUCT was not defined

[Oleg Nesterov]

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

Re: [PATCH 1/3] struct_union_enum_specifier: set MOD_TOPLEVEL if toplevel(sym->scope)

[Luc Van Oostenryck]

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

Re: [PATCH 1/3] struct_union_enum_specifier: set MOD_TOPLEVEL if toplevel(sym->scope)

[Oleg Nesterov]

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");

Re: [PATCH 1/3] struct_union_enum_specifier: set MOD_TOPLEVEL if toplevel(sym->scope)

[Luc Van Oostenryck]

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