[PATCH] sparse: option to print compound global data symbol info

[PATCH] sparse: option to print compound global data symbol info

[Randy Dunlap]

From: Randy Dunlap <rdunlap@infradead.org>
with help from Linus. (many moons ago)

sparse addition to print all compound/composite global data symbols
with their sizes and alignment.

usage: -list-symbols
Example: (in kernel tree)
  make C=2 CF="-list-symbols" arch/x86_64/kernel/smpboot.o
  arch/x86/kernel/smpboot.c:99:1: struct cpuinfo_x86 [addressable] [noderef] [toplevel] <asn:3>cpu_info: compound size 240, alignment 8

Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
---
I have had versions of this patch around for about 10 (!) years, so I think
that it's time to try to have it merged -- although there could easily be
better ways to do this, so please tell me.

 lib.c    |    9 +++++++++
 lib.h    |    1 +
 sparse.1 |    5 +++++
 sparse.c |   41 ++++++++++++++++++++++++++++++++++++++++-
 4 files changed, 55 insertions(+), 1 deletion(-)

--- orig/sparse.c
+++ next/sparse.c
@@ -36,6 +36,7 @@
 #include "allocate.h"
 #include "token.h"
 #include "parse.h"
+#include "ptrlist.h"
 #include "symbol.h"
 #include "expression.h"
 #include "linearize.h"
@@ -292,6 +293,39 @@ static void check_symbols(struct symbol_
 		exit(1);
 }
 
+extern int list_symbols;
+
+static void list_all_symbols(struct symbol_list *list)
+{
+	struct symbol *sym;
+
+	FOR_EACH_PTR(list, sym) {
+		/* Only show arrays, structures, unions, enums, & typedefs */
+		if (!(sym->namespace & (NS_STRUCT | NS_TYPEDEF | NS_SYMBOL)))
+			continue;
+		/* Only show types we actually examined (ie used) */
+		if (!sym->bit_size)
+			continue;
+		if (sym->type == SYM_FN || sym->type == SYM_ENUM)
+			continue;
+		if (!sym->ctype.base_type)
+			continue;
+		if (sym->ctype.base_type->type == SYM_FN)
+			continue;
+		if (sym->ctype.base_type->type == SYM_ENUM)
+			continue;
+		if (sym->ctype.base_type->type == SYM_BASETYPE)
+			continue;
+		/* Don't show unnamed types */
+		if (!sym->ident)
+			continue;
+		info(sym->pos, "%s: compound size %u, alignment %lu",
+			show_typename(sym),
+			sym->bit_size >> 3,
+			sym->ctype.alignment);
+	} END_FOR_EACH_PTR(sym);
+}
+
 int main(int argc, char **argv)
 {
 	struct string_list *filelist = NULL;
@@ -300,7 +334,12 @@ int main(int argc, char **argv)
 	// Expand, linearize and show it.
 	check_symbols(sparse_initialize(argc, argv, &filelist));
 	FOR_EACH_PTR_NOTAG(filelist, file) {
-		check_symbols(sparse(file));
+		struct symbol_list *res = sparse(file);
+
+		check_symbols(res);
+
+		if (list_symbols)
+			list_all_symbols(res);
 	} END_FOR_EACH_PTR_NOTAG(file);
 
 	report_stats();
--- orig/lib.c
+++ next/lib.c
@@ -258,6 +258,7 @@ int dump_macro_defs = 0;
 int dbg_entry = 0;
 int dbg_dead = 0;
 
+int list_symbols = 0;
 int fmem_report = 0;
 int fdump_linearize;
 unsigned long long fmemcpy_max_count = 100000;
@@ -393,6 +394,13 @@ static char **handle_switch_i(char *arg,
 	return next;
 }
 
+static char **handle_switch_l(char *arg, char **next)
+{
+	if (!strcmp(arg, "list-symbols"))
+		list_symbols = 1;
+	return next;
+}
+
 static char **handle_switch_M(char *arg, char **next)
 {
 	if (!strcmp(arg, "MF") || !strcmp(arg,"MQ") || !strcmp(arg,"MT")) {
@@ -903,6 +911,7 @@ static char **handle_switch(char *arg, c
 	case 'G': return handle_switch_G(arg, next);
 	case 'I': return handle_switch_I(arg, next);
 	case 'i': return handle_switch_i(arg, next);
+	case 'l': return handle_switch_l(arg, next);
 	case 'M': return handle_switch_M(arg, next);
 	case 'm': return handle_switch_m(arg, next);
 	case 'n': return handle_switch_n(arg, next);
--- orig/lib.h
+++ next/lib.h
@@ -151,6 +151,7 @@ extern int dump_macro_defs;
 extern int dbg_entry;
 extern int dbg_dead;
 
+extern int list_symbols;
 extern int fmem_report;
 extern int fdump_linearize;
 extern unsigned long long fmemcpy_max_count;
--- orig/sparse.1
+++ next/sparse.1
@@ -387,6 +387,11 @@ Set the distance between tab stops.  Thi
 column numbers in warnings or errors.  If the value is less than 1 or
 greater than 100, the option is ignored.  The default is 8.
 .
+.TP
+.B \-list-symbols
+Print all compound/composite global data symbols along with their
+compound size and alignment.
+.
 .SH SEE ALSO
 .BR cgcc (1)
 .

Re: [PATCH] sparse: option to print compound global data symbol info

[Luc Van Oostenryck]

On Sat, Jan 20, 2018 at 07:36:24PM -0800, Randy Dunlap wrote:
> From: Randy Dunlap <rdunlap@infradead.org>
> with help from Linus. (many moons ago)
> 
> sparse addition to print all compound/composite global data symbols
> with their sizes and alignment.
> 
> usage: -list-symbols
> Example: (in kernel tree)
>   make C=2 CF="-list-symbols" arch/x86_64/kernel/smpboot.o
>   arch/x86/kernel/smpboot.c:99:1: struct cpuinfo_x86 [addressable] [noderef] [toplevel] <asn:3>cpu_info: compound size 240, alignment 8

If this only lists compound symbols, it seems a bit strange to me
to use '-list-symbols' as the option name. Maybe you could go one
step further an have '-list-symbols=compound' and if needed it can
be extended to '-list-symbols=all'.

> --- orig/sparse.c
> +++ next/sparse.c
> @@ -36,6 +36,7 @@
>  #include "allocate.h"
>  #include "token.h"
>  #include "parse.h"
> +#include "ptrlist.h"

Not really needed as it's already included indirectly but it
won't hurt, of course.

> +extern int list_symbols;

Not needed since already declared in lib.h.

> +static void list_all_symbols(struct symbol_list *list)
> +{
> +	struct symbol *sym;
> +
> +	FOR_EACH_PTR(list, sym) {
> +		/* Only show arrays, structures, unions, enums, & typedefs */
> +		if (!(sym->namespace & (NS_STRUCT | NS_TYPEDEF | NS_SYMBOL)))
> +			continue;
> +		/* Only show types we actually examined (ie used) */
> +		if (!sym->bit_size)
> +			continue;

Even after being examined, 'bit_size' can stay zero if there is
some errors. You can directly use 'if (!sym->examined)' if you
want but I think it's better to adjust the comment.

> +		if (sym->type == SYM_FN || sym->type == SYM_ENUM)
> +			continue;
> +		if (!sym->ctype.base_type)
> +			continue;
> +		if (sym->ctype.base_type->type == SYM_FN)
> +			continue;
> +		if (sym->ctype.base_type->type == SYM_ENUM)
> +			continue;
> +		if (sym->ctype.base_type->type == SYM_BASETYPE)
> +			continue;

It's a bit dangereous here. You should check for SYM_NODE.
I also think it would be better and simpler to use the helpers
is_func_type(), is_int_type(), ... or add is_compound_type().

> +		/* Don't show unnamed types */
> +		if (!sym->ident)
> +			continue;
> +		info(sym->pos, "%s: compound size %u, alignment %lu",
> +			show_typename(sym),
> +			sym->bit_size >> 3,

bits_to_bytes() should be used here.


Regards,
Luc Van Oostenryck

Re: [PATCH] sparse: option to print compound global data symbol info

[Christopher Li]

On Sat, Jan 20, 2018 at 8:27 PM, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
> On Sat, Jan 20, 2018 at 07:36:24PM -0800, Randy Dunlap wrote:
>> From: Randy Dunlap <rdunlap@infradead.org>
>> with help from Linus. (many moons ago)
>>
>> sparse addition to print all compound/composite global data symbols
>> with their sizes and alignment.
>>
>> usage: -list-symbols
>> Example: (in kernel tree)
>>   make C=2 CF="-list-symbols" arch/x86_64/kernel/smpboot.o
>>   arch/x86/kernel/smpboot.c:99:1: struct cpuinfo_x86 [addressable] [noderef] [toplevel] <asn:3>cpu_info: compound size 240, alignment 8
>
> If this only lists compound symbols, it seems a bit strange to me
> to use '-list-symbols' as the option name. Maybe you could go one
> step further an have '-list-symbols=compound' and if needed it can
> be extended to '-list-symbols=all'.

I think maybe it can be group with the "-v<debug>" options. e.g. "-vcompound".
After all it is showing some debug information very similar to "-ventry".

>
> Not really needed as it's already included indirectly but it
> won't hurt, of course.
>...
>
> Not needed since already declared in lib.h.

Agree here and other Luc's feedback, so I skip the duplication.

>
>> +static void list_all_symbols(struct symbol_list *list)
>> +{
>> +     struct symbol *sym;
>> +
>> +     FOR_EACH_PTR(list, sym) {
>> +             /* Only show arrays, structures, unions, enums, & typedefs */

This comment is a bit confusing for me. It mention "Only show ... *enums*"

>
>> +             if (sym->type == SYM_FN || sym->type == SYM_ENUM)
>> +                     continue;
>> +             if (!sym->ctype.base_type)
>> +                     continue;
>> +             if (sym->ctype.base_type->type == SYM_FN)
>> +                     continue;
>> +             if (sym->ctype.base_type->type == SYM_ENUM)
>> +                     continue;

Here it skips enums. Not consistent with previous comment
about showing enums. Am I missing something obvious?

Chris

Re: [PATCH] sparse: option to print compound global data symbol info

[Randy Dunlap]

On 01/23/2018 02:46 AM, Christopher Li wrote:
> On Sat, Jan 20, 2018 at 8:27 PM, Luc Van Oostenryck
> <luc.vanoostenryck@gmail.com> wrote:
>> On Sat, Jan 20, 2018 at 07:36:24PM -0800, Randy Dunlap wrote:
>>> From: Randy Dunlap <rdunlap@infradead.org>
>>> with help from Linus. (many moons ago)
>>>
>>> sparse addition to print all compound/composite global data symbols
>>> with their sizes and alignment.
>>>
>>> usage: -list-symbols
>>> Example: (in kernel tree)
>>>   make C=2 CF="-list-symbols" arch/x86_64/kernel/smpboot.o
>>>   arch/x86/kernel/smpboot.c:99:1: struct cpuinfo_x86 [addressable] [noderef] [toplevel] <asn:3>cpu_info: compound size 240, alignment 8
>>
>> If this only lists compound symbols, it seems a bit strange to me
>> to use '-list-symbols' as the option name. Maybe you could go one
>> step further an have '-list-symbols=compound' and if needed it can
>> be extended to '-list-symbols=all'.
> 
> I think maybe it can be group with the "-v<debug>" options. e.g. "-vcompound".
> After all it is showing some debug information very similar to "-ventry".
> 

OK, thanks for that.  I was a bit hung up on where to go with that.

>>
>> Not really needed as it's already included indirectly but it
>> won't hurt, of course.
>> ...
>>
>> Not needed since already declared in lib.h.
> 
> Agree here and other Luc's feedback, so I skip the duplication.
> 
>>
>>> +static void list_all_symbols(struct symbol_list *list)
>>> +{
>>> +     struct symbol *sym;
>>> +
>>> +     FOR_EACH_PTR(list, sym) {
>>> +             /* Only show arrays, structures, unions, enums, & typedefs */
> 
> This comment is a bit confusing for me. It mention "Only show ... *enums*"
> 
>>
>>> +             if (sym->type == SYM_FN || sym->type == SYM_ENUM)
>>> +                     continue;
>>> +             if (!sym->ctype.base_type)
>>> +                     continue;
>>> +             if (sym->ctype.base_type->type == SYM_FN)
>>> +                     continue;
>>> +             if (sym->ctype.base_type->type == SYM_ENUM)
>>> +                     continue;
> 
> Here it skips enums. Not consistent with previous comment
> about showing enums. Am I missing something obvious?

Nope.  I have already corrected that locally a few days ago.

thanks.
-- 
~Randy