diff mbox

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

Message ID 6a927d6f-e487-c3d3-b938-ff1b041073ff@infradead.org (mailing list archive)
State Superseded, archived
Headers show

Commit Message

Randy Dunlap April 10, 2018, 10:03 p.m. UTC 
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: -vcompound
Example:
  $ sparse -vcompound symbol-sizes.c
  compound-sizes.c:39:17: union un static [toplevel] un: compound size 192, alignment 8
  compound-sizes.c:42:25: struct inventory static [toplevel] inven[100]: compound size 19200, alignment 8
  compound-sizes.c:51:33: struct inventory static [toplevel] [usertype] invent[10]: compound size 1920, alignment 8
  compound-sizes.c:58:25: float static [toplevel] floats[42]: compound size 168, alignment 4
  compound-sizes.c:59:25: double static [toplevel] doubles[84]: compound size 672, alignment 8

and validation:
$ ./test-suite   single compound-sizes.c
     TEST    compound-sizes (compound-sizes.c)
compound-sizes.c passed !

Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
---
I think that I have incorporated the previous feedback from Luc and Chris.
(and then I had a few h/w problems ... and I lost the patch, had to
recreate the changes.  blah blah.  I should use some kind of cloud storage.)


 lib.c                       |    2 
 lib.h                       |    1 
 sparse.c                    |   38 ++++++++++++++
 validation/compound-sizes.c |   87 ++++++++++++++++++++++++++++++++++
 4 files changed, 127 insertions(+), 1 deletion(-)



--
To unsubscribe from this list: send the line "unsubscribe linux-sparse" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Luc Van Oostenryck April 11, 2018, 10 a.m. UTC | #1 
On Tue, Apr 10, 2018 at 03:03:06PM -0700, 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.
>
> ...
> 
> I think that I have incorporated the previous feedback from Luc and Chris.

I don't remember the details but it looks good but for a few details.

>  
> +static void list_compound_symbols(struct symbol_list *list)
> +{
> +	struct symbol *sym;
> +
> +	FOR_EACH_PTR(list, sym) {
> +		/* Only show arrays, structures, unions */
> +		if (!(sym->namespace & (NS_STRUCT | NS_TYPEDEF | NS_SYMBOL)))
> +			continue;

I don't think this is needed: you should only have NS_SYMBOLs here
(NS_STRUCT & NS_TYPEDEF are there for the namespace of structure tag &
typedef name. They are used during parsing but all the types belong
to NS_SYMBOL).

> +		if (sym->type != SYM_NODE)
> +			continue;

I don't think there is any kind of guarantee that a compound type is always
'surrounded' by a SYM_NODE (and I don't think they should be). So, I think
that the two lines here above should be removed.

> +		if (!sym->ctype.base_type)
> +			continue;
> +		if (is_func_type(sym))
> +			continue;
> +		if (is_float_type(sym) || is_scalar_type(sym))
> +			continue;
> +		if (sym->ctype.base_type->type == SYM_BASETYPE)
> +			continue;

I think these two lines are redundant with the two just before
(given the SYM_NODE). I also think that it should be better to
move these tests to an helper is_compound_type().
Even better, if you're only interested in structs, unions & arrays,
would be to do something more explicit, for example, like:
		if (sym->type == SYM_NODE)
			base = sym->ctype.base_type;
		else
			base = sym;
		switch (base->type) {
		case SYM_STRUCT: case SYM_UNION: case SYM_ARRAY:
			break;
		default:
			continue;
		}


Cheers,
-- Luc
--
To unsubscribe from this list: send the line "unsubscribe linux-sparse" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

--- spars-052rc1.orig/sparse.c
+++ spars-052rc1/sparse.c
@@ -292,6 +292,37 @@  static void check_symbols(struct symbol_
 		exit(1);
 }
 
+static void list_compound_symbols(struct symbol_list *list)
+{
+	struct symbol *sym;
+
+	FOR_EACH_PTR(list, sym) {
+		/* Only show arrays, structures, unions */
+		if (!(sym->namespace & (NS_STRUCT | NS_TYPEDEF | NS_SYMBOL)))
+			continue;
+		/* Only show symbols that have a non-zero size */
+		if (!sym->bit_size)
+			continue;
+		if (sym->type != SYM_NODE)
+			continue;
+		if (!sym->ctype.base_type)
+			continue;
+		if (is_func_type(sym))
+			continue;
+		if (is_float_type(sym) || is_scalar_type(sym))
+			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),
+			bits_to_bytes(sym->bit_size),
+			sym->ctype.alignment);
+	} END_FOR_EACH_PTR(sym);
+}
+
 int main(int argc, char **argv)
 {
 	struct string_list *filelist = NULL;
@@ -300,7 +331,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 (dbg_compound)
+			list_compound_symbols(res);
 	} END_FOR_EACH_PTR_NOTAG(file);
 
 	report_stats();
--- spars-052rc1.orig/lib.c
+++ spars-052rc1/lib.c
@@ -257,6 +257,7 @@  int dump_macro_defs = 0;
 
 int dbg_entry = 0;
 int dbg_dead = 0;
+int dbg_compound = 0;
 
 int fmem_report = 0;
 int fdump_linearize;
@@ -598,6 +599,7 @@  static char **handle_switch_W(char *arg,
 static struct warning debugs[] = {
 	{ "entry", &dbg_entry},
 	{ "dead", &dbg_dead},
+	{ "compound", &dbg_compound},
 };
 
 
--- spars-052rc1.orig/lib.h
+++ spars-052rc1/lib.h
@@ -150,6 +150,7 @@  extern int dump_macro_defs;
 
 extern int dbg_entry;
 extern int dbg_dead;
+extern int dbg_compound;
 
 extern int fmem_report;
 extern int fdump_linearize;
--- /dev/null
+++ spars-052rc1/validation/compound-sizes.c
@@ -0,0 +1,87 @@ 
+// This tests sparse "-vcompound" output.
+
+#include <stdlib.h>
+#include <stdint.h>
+
+// Do not list functions.
+static int do_nothing(void)
+{}
+
+// no:
+static inline int zero(void)
+{
+	return 0 / 1;
+}
+
+// no:
+struct inventory {
+	unsigned char	description[64];
+	unsigned char	department[64];
+	uint32_t	dept_number;
+	uint32_t	item_cost;
+	uint64_t	stock_number;
+	uint32_t	tally[12];	// per month
+};
+
+// no
+static struct inventory *get_inv(uint64_t stocknum)
+{
+	return NULL;
+}
+
+// no
+union un {
+	struct inventory inv;
+	unsigned char	bytes[0];
+};
+
+// yes
+static union un un;
+
+// yes
+static struct inventory	inven[100];
+
+// no
+typedef struct inventory	inventory_t;
+
+// no
+static struct inventory	*invptr;
+
+// yes
+static inventory_t		invent[10];
+
+// no
+static float		floater;
+static double		double_float;
+
+// yes
+static float		floats[42];
+static double		doubles[84];
+
+// no
+int main(void)
+{
+	// no, these are not global.
+	struct inventory inv[10];
+	inventory_t	invt[10];
+	// what about statics?
+	static struct inventory invtop;
+	static inventory_t inv_top;
+	static uint64_t stocknums[100];
+
+	invptr = get_inv(42000);
+	return 0;
+}
+
+/*
+ * check-name: compound-sizes
+ * check-command: sparse -vcompound $file
+ *
+ * check-error-start
+compound-sizes.c:39:17: union un static [toplevel] un: compound size 192, alignment 8
+compound-sizes.c:42:25: struct inventory static [toplevel] inven[100]: compound size 19200, alignment 8
+compound-sizes.c:51:33: struct inventory static [toplevel] [usertype] invent[10]: compound size 1920, alignment 8
+compound-sizes.c:58:25: float static [toplevel] floats[42]: compound size 168, alignment 4
+compound-sizes.c:59:25: double static [toplevel] doubles[84]: compound size 672, alignment 8
+ * check-error-end
+ */