[PATCH 00/15] Trivial sparse patches

[PATCH 00/15] Trivial sparse patches

[Alexey Zaytsev]

Hello.

Here are a few sparse patches that I have collected
from the mailing list and from my own repository
and that I consider either trivial or harmless.
There is an other series that might require some
more review/discussion, which I'll send later.

You can pull the patches from brahch 'trivial' of
git://git.zaytsev.su/git/sparse.git

Please check that everything is correct and that
all theese patches are still intended for mainline
inclusion.

Also, we need sign-offs for theese patches:
David Given (2):
      Add type information to struct instruction.
      Unhardcode byte size being 8 bits.
---

Alexey Zaytsev (9):
      Warning should be enough for an unhandled transparent union
      A slightly edited irc discussion with Josh Triplett.
      Gdb macros to get a better look at some sparse data structures.
      Handle missing argument to -D.
      Mark handle_switch as static and don't export it from lib.h
      Looks more evident this way.
      Handle a terminal -o option properly.
      Make show_symbol newline-consistent
      Replace the -specs cgcc option with -target

Blue Swirl (2):
      OpenBSD support
      Sparc64 (Sparc V9, LP64) support

Christopher Li (2):
      Remove pre_buffer
      Evaluate iterator symbols

David Given (2):
      Add type information to struct instruction.
      Unhardcode byte size being 8 bits.


 Documentation/data-structures.txt |   54 ++++++
 Makefile                          |   20 +-
 cgcc                              |   12 +
 compat-bsd.c                      |   20 ++
 compile-i386.c                    |    2 
 evaluate.c                        |   27 ++-
 example.c                         |    2 
 expand.c                          |    2 
 flow.c                            |   10 +
 gdbhelpers                        |  310 +++++++++++++++++++++++++++++++++++++
 lib.c                             |   40 +++--
 lib.h                             |    1 
 linearize.c                       |    4 
 linearize.h                       |    1 
 parse.c                           |    2 
 show-parse.c                      |    9 +
 symbol.c                          |   12 +
 target.h                          |   10 +
 token.h                           |    2 
 tokenize.c                        |   21 +--
 20 files changed, 494 insertions(+), 67 deletions(-)
 create mode 100644 Documentation/data-structures.txt
 create mode 100644 compat-bsd.c
 create mode 100644 gdbhelpers

--

[PATCH 01/15] Evaluate iterator symbols

[Alexey Zaytsev]

From: Christopher Li <sparse@chrisli.org>

evaluate.c forgets to evaluate iterator_syms, which
might have some expression in initializer.

Signed-Off-By: Christopher Li <sparse@chrisli.org>
---
 evaluate.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/evaluate.c b/evaluate.c
index 2a126dd..e17da53 100644
--- a/evaluate.c
+++ b/evaluate.c
@@ -3106,6 +3106,7 @@ static void evaluate_if_statement(struct statement *stmt)
 
 static void evaluate_iterator(struct statement *stmt)
 {
+	evaluate_symbol_list(stmt->iterator_syms);
 	evaluate_conditional(stmt->iterator_pre_condition, 1);
 	evaluate_conditional(stmt->iterator_post_condition,1);
 	evaluate_statement(stmt->iterator_pre_statement);

[PATCH 02/15] Unhardcode byte size being 8 bits.

[Alexey Zaytsev]

From: David Given <dg@cowlark.com>


---
 compile-i386.c |    2 +-
 evaluate.c     |   26 +++++++++++++-------------
 example.c      |    2 +-
 expand.c       |    2 +-
 flow.c         |   10 ++++++----
 show-parse.c   |    2 +-
 symbol.c       |   10 +++++-----
 target.h       |   10 ++++++++++
 8 files changed, 38 insertions(+), 26 deletions(-)

diff --git a/compile-i386.c b/compile-i386.c
index 8526408..37ea52e 100644
--- a/compile-i386.c
+++ b/compile-i386.c
@@ -2081,7 +2081,7 @@ static struct storage *x86_call_expression(struct expression *expr)
 		insn("pushl", new, NULL,
 		     !framesize ? "begin function call" : NULL);
 
-		framesize += size >> 3;
+		framesize += bits_to_bytes(size);
 	} END_FOR_EACH_PTR_REVERSE(arg);
 
 	fn = expr->fn;
diff --git a/evaluate.c b/evaluate.c
index e17da53..c501323 100644
--- a/evaluate.c
+++ b/evaluate.c
@@ -72,7 +72,7 @@ static struct symbol *evaluate_string(struct expression *expr)
 	unsigned int length = expr->string->length;
 
 	sym->array_size = alloc_const_expression(expr->pos, length);
-	sym->bit_size = bits_in_char * length;
+	sym->bit_size = bytes_to_bits(length);
 	sym->ctype.alignment = 1;
 	sym->string = 1;
 	sym->ctype.modifiers = MOD_STATIC;
@@ -83,7 +83,7 @@ static struct symbol *evaluate_string(struct expression *expr)
 	initstr->string = expr->string;
 
 	array->array_size = sym->array_size;
-	array->bit_size = bits_in_char * length;
+	array->bit_size = bytes_to_bits(length);
 	array->ctype.alignment = 1;
 	array->ctype.modifiers = MOD_STATIC;
 	array->ctype.base_type = &char_ctype;
@@ -579,7 +579,7 @@ static struct symbol *evaluate_ptr_add(struct expression *expr, struct symbol *i
 	}
 
 	/* Get the size of whatever the pointer points to */
-	multiply = base->bit_size >> 3;
+	multiply = bits_to_bytes(base->bit_size);
 
 	if (ctype == &null_ctype)
 		ctype = &ptr_ctype;
@@ -831,7 +831,7 @@ static struct symbol *evaluate_ptr_sub(struct expression *expr)
 		struct expression *sub = alloc_expression(expr->pos, EXPR_BINOP);
 		struct expression *div = expr;
 		struct expression *val = alloc_expression(expr->pos, EXPR_VALUE);
-		unsigned long value = lbase->bit_size >> 3;
+		unsigned long value = bits_to_bytes(lbase->bit_size);
 
 		val->ctype = size_t_ctype;
 		val->value = value;
@@ -1591,7 +1591,7 @@ static struct symbol *degenerate(struct expression *expr)
 				e3->op = '+';
 				e3->left = e0;
 				e3->right = alloc_const_expression(expr->pos,
-							expr->r_bitpos >> 3);
+							bits_to_bytes(expr->r_bitpos));
 				e3->ctype = &lazy_ptr_ctype;
 			} else {
 				e3 = e0;
@@ -1727,7 +1727,7 @@ static struct symbol *evaluate_postop(struct expression *expr)
 	} else if (class == TYPE_PTR) {
 		struct symbol *target = examine_pointer_target(ctype);
 		if (!is_function(target))
-			multiply = target->bit_size >> 3;
+			multiply = bits_to_bytes(target->bit_size);
 	}
 
 	if (multiply) {
@@ -1949,7 +1949,7 @@ static struct symbol *evaluate_member_dereference(struct expression *expr)
 			expr->base = deref->base;
 			expr->r_bitpos = deref->r_bitpos;
 		}
-		expr->r_bitpos += offset << 3;
+		expr->r_bitpos += bytes_to_bits(offset);
 		expr->type = EXPR_SLICE;
 		expr->r_nrbits = member->bit_size;
 		expr->r_bitpos += member->bit_offset;
@@ -2037,10 +2037,10 @@ static struct symbol *evaluate_sizeof(struct expression *expr)
 		return NULL;
 
 	size = type->bit_size;
-	if ((size < 0) || (size & 7))
+	if ((size < 0) || (size & (bits_in_char - 1)))
 		expression_error(expr, "cannot size expression");
 	expr->type = EXPR_VALUE;
-	expr->value = size >> 3;
+	expr->value = bits_to_bytes(size);
 	expr->taint = 0;
 	expr->ctype = size_t_ctype;
 	return size_t_ctype;
@@ -2071,10 +2071,10 @@ static struct symbol *evaluate_ptrsizeof(struct expression *expr)
 		return NULL;
 	}
 	size = type->bit_size;
-	if (size & 7)
+	if (size & (bits_in_char-1))
 		size = 0;
 	expr->type = EXPR_VALUE;
-	expr->value = size >> 3;
+	expr->value = bits_to_bytes(size);
 	expr->taint = 0;
 	expr->ctype = size_t_ctype;
 	return size_t_ctype;
@@ -2158,7 +2158,7 @@ static void convert_index(struct expression *e)
 	unsigned from = e->idx_from;
 	unsigned to = e->idx_to + 1;
 	e->type = EXPR_POS;
-	e->init_offset = from * (e->ctype->bit_size>>3);
+	e->init_offset = from * bits_to_bytes(e->ctype->bit_size);
 	e->init_nr = to - from;
 	e->init_expr = child;
 }
@@ -2865,7 +2865,7 @@ static struct symbol *evaluate_offsetof(struct expression *expr)
 			unrestrict(idx, i_class, &i_type);
 			idx = cast_to(idx, size_t_ctype);
 			m = alloc_const_expression(expr->pos,
-						   ctype->bit_size >> 3);
+						   bits_to_bytes(ctype->bit_size));
 			m->ctype = size_t_ctype;
 			m->flags = Int_const_expr;
 			expr->type = EXPR_BINOP;
diff --git a/example.c b/example.c
index ae897dc..24444c6 100644
--- a/example.c
+++ b/example.c
@@ -1830,7 +1830,7 @@ static void set_up_arch_entry(struct entrypoint *ep, struct instruction *entry)
 			in->type = REG_FRAME;
 			in->offset = offset;
 			
-			offset += bits >> 3;
+			offset += bits_to_bytes(bits);
 		}
 		i++;
 		NEXT_PTR_LIST(argtype);
diff --git a/expand.c b/expand.c
index 032f0c5..3e962d1 100644
--- a/expand.c
+++ b/expand.c
@@ -880,7 +880,7 @@ static unsigned long bit_offset(const struct expression *expr)
 {
 	unsigned long offset = 0;
 	while (expr->type == EXPR_POS) {
-		offset += expr->init_offset << 3;
+		offset += bytes_to_bits(expr->init_offset);
 		expr = expr->init_expr;
 	}
 	if (expr && expr->ctype)
diff --git a/flow.c b/flow.c
index 82fb23a..5bd9a1d 100644
--- a/flow.c
+++ b/flow.c
@@ -16,6 +16,7 @@
 #include "expression.h"
 #include "linearize.h"
 #include "flow.h"
+#include "target.h"
 
 unsigned long bb_generation;
 
@@ -265,8 +266,8 @@ void convert_load_instruction(struct instruction *insn, pseudo_t src)
 
 static int overlapping_memop(struct instruction *a, struct instruction *b)
 {
-	unsigned int a_start = a->offset << 3;
-	unsigned int b_start = b->offset << 3;
+	unsigned int a_start = bytes_to_bits(a->offset);
+	unsigned int b_start = bytes_to_bits(b->offset);
 	unsigned int a_size = a->size;
 	unsigned int b_size = b->size;
 
@@ -581,13 +582,14 @@ void check_access(struct instruction *insn)
 	pseudo_t pseudo = insn->src;
 
 	if (insn->bb && pseudo->type == PSEUDO_SYM) {
-		int offset = insn->offset, bit = (offset << 3) + insn->size;
+		int offset = insn->offset, bit = bytes_to_bits(offset) + insn->size;
 		struct symbol *sym = pseudo->sym;
 
 		if (sym->bit_size > 0 && (offset < 0 || bit > sym->bit_size))
 			warning(insn->pos, "invalid access %s '%s' (%d %d)",
 				offset < 0 ? "below" : "past the end of",
-				show_ident(sym->ident), offset, sym->bit_size >> 3);
+				show_ident(sym->ident), offset,
+				bits_to_bytes(sym->bit_size));
 	}
 }
 
diff --git a/show-parse.c b/show-parse.c
index 064af32..c79a288 100644
--- a/show-parse.c
+++ b/show-parse.c
@@ -673,7 +673,7 @@ static int show_call_expression(struct expression *expr)
 		int new = show_expression(arg);
 		int size = arg->ctype->bit_size;
 		printf("\tpush.%d\t\tv%d\n", size, new);
-		framesize += size >> 3;
+		framesize += bits_to_bytes(size);
 	} END_FOR_EACH_PTR_REVERSE(arg);
 
 	fn = expr->fn;
diff --git a/symbol.c b/symbol.c
index 3292907..49560ee 100644
--- a/symbol.c
+++ b/symbol.c
@@ -128,7 +128,7 @@ static void lay_out_struct(struct symbol *sym, struct struct_union_info *info)
 		base_size = 0;
 	}
 
-	align_bit_mask = (sym->ctype.alignment << 3) - 1;
+	align_bit_mask = bytes_to_bits(sym->ctype.alignment) - 1;
 
 	/*
 	 * Bitfields have some very special rules..
@@ -143,7 +143,7 @@ static void lay_out_struct(struct symbol *sym, struct struct_union_info *info)
 			bit_size = (bit_size + align_bit_mask) & ~align_bit_mask;
 			bit_offset = 0;
 		}
-		sym->offset = (bit_size - bit_offset) >> 3;
+		sym->offset = bits_to_bytes(bit_size - bit_offset);
 		sym->bit_offset = bit_offset;
 		sym->ctype.base_type->bit_offset = bit_offset;
 		info->bit_size = bit_size + width;
@@ -156,7 +156,7 @@ static void lay_out_struct(struct symbol *sym, struct struct_union_info *info)
 	 * Otherwise, just align it right and add it up..
 	 */
 	bit_size = (bit_size + align_bit_mask) & ~align_bit_mask;
-	sym->offset = bit_size >> 3;
+	sym->offset = bits_to_bytes(bit_size);
 
 	info->bit_size = bit_size + base_size;
 	// warning (sym->pos, "regular: offset=%d", sym->offset);
@@ -182,7 +182,7 @@ static struct symbol * examine_struct_union_type(struct symbol *sym, int advance
 		sym->ctype.alignment = info.max_align;
 	bit_size = info.bit_size;
 	if (info.align_size) {
-		bit_align = (sym->ctype.alignment << 3)-1;
+		bit_align = bytes_to_bits(sym->ctype.alignment)-1;
 		bit_size = (bit_size + bit_align) & ~bit_align;
 	}
 	sym->bit_size = bit_size;
@@ -877,7 +877,7 @@ void init_ctype(void)
 		struct symbol *sym = ctype->ptr;
 		unsigned long bit_size = ctype->bit_size ? *ctype->bit_size : -1;
 		unsigned long maxalign = ctype->maxalign ? *ctype->maxalign : 0;
-		unsigned long alignment = (bit_size + 7) >> 3;
+		unsigned long alignment = bits_to_bytes(bit_size + bits_in_char - 1);
 
 		if (alignment > maxalign)
 			alignment = maxalign;
diff --git a/target.h b/target.h
index 25f7948..e6b0a12 100644
--- a/target.h
+++ b/target.h
@@ -42,4 +42,14 @@ extern int pointer_alignment;
 extern int bits_in_enum;
 extern int enum_alignment;
 
+/*
+ * Helper functions for converting bits to bytes and vice versa.
+ */
+
+static inline int bits_to_bytes(int bits)
+{ return bits / bits_in_char; }
+
+static inline int bytes_to_bits(int bytes)
+{ return bytes * bits_in_char; }
+
 #endif

[PATCH 03/15] Add type information to struct instruction.

[Alexey Zaytsev]

From: David Given <dg@cowlark.com>

Currently there is no generic way to derive phy
type information from the instruction flow.
---
 linearize.c |    4 +++-
 linearize.h |    1 +
 2 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/linearize.c b/linearize.c
index fce1ae8..526a710 100644
--- a/linearize.c
+++ b/linearize.c
@@ -55,7 +55,9 @@ static inline int type_size(struct symbol *type)
 
 static struct instruction *alloc_typed_instruction(int opcode, struct symbol *type)
 {
-	return alloc_instruction(opcode, type_size(type));
+	struct instruction *insn = alloc_instruction(opcode, type_size(type));
+	insn->type = type;
+	return insn;
 }
 
 static struct entrypoint *alloc_entrypoint(void)
diff --git a/linearize.h b/linearize.h
index 32b1c1a..0c5e4ef 100644
--- a/linearize.h
+++ b/linearize.h
@@ -71,6 +71,7 @@ struct instruction {
 		 size:24;
 	struct basic_block *bb;
 	struct position pos;
+	struct symbol *type;
 	union {
 		pseudo_t target;
 		pseudo_t cond;		/* for branch and switch */

[PATCH 04/15] Replace the -specs cgcc option with -target

[Alexey Zaytsev]

-spesc is just confusing, as gcc takes the same
option for something compeltely different.

Signed-off-by: Alexey Zaytsev <alexey.zaytsev@gmail.com>
---
 cgcc |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/cgcc b/cgcc
index 89adbed..31c219a 100755
--- a/cgcc
+++ b/cgcc
@@ -25,7 +25,7 @@ foreach (@ARGV) {
     $m64 = 1 if /^-m64$/;
     $gendeps = 1 if /^-M$/;
 
-    if (/^-specs=(.*)$/) {
+    if (/^-target=(.*)$/) {
 	$check .= &add_specs ($1);
 	$has_specs = 1;
 	next;

[PATCH 05/15] Remove pre_buffer

[Alexey Zaytsev]

From: Christopher Li <spase@chrisli.org>

This patch removes the pre_buffer completely. Instead,
sparse will tokenized the buffer during add_pre_buffer().
Sparse just tracks the beginning and end of pre_buffer.

Reviewed-by: Alexey Zaytsev <alexey.zaytsev@gmail.com>
Signed-Off-By: Christopher Li <spase@chrisli.org>
---
 lib.c      |   23 ++++++++++++++---------
 token.h    |    2 +-
 tokenize.c |   21 +++++++++++----------
 3 files changed, 26 insertions(+), 20 deletions(-)

diff --git a/lib.c b/lib.c
index 0abcc9a..ea40ce1 100644
--- a/lib.c
+++ b/lib.c
@@ -185,8 +185,8 @@ void die(const char *fmt, ...)
 	exit(1);
 }
 
-static unsigned int pre_buffer_size;
-static char pre_buffer[8192];
+static struct token *pre_buffer_begin = NULL;
+static struct token *pre_buffer_end = NULL;
 
 int Waddress_space = 1;
 int Wbitwise = 0;
@@ -230,14 +230,18 @@ void add_pre_buffer(const char *fmt, ...)
 {
 	va_list args;
 	unsigned int size;
+	struct token *begin, *end;
+	char buffer[4096];
 
 	va_start(args, fmt);
-	size = pre_buffer_size;
-	size += vsnprintf(pre_buffer + size,
-		sizeof(pre_buffer) - size,
-		fmt, args);
-	pre_buffer_size = size;
+	size = vsnprintf(buffer, sizeof(buffer), fmt, args);
 	va_end(args);
+	begin = tokenize_buffer(buffer, size, &end);
+	if (!pre_buffer_begin)
+		pre_buffer_begin = begin;
+	if (pre_buffer_end)
+		pre_buffer_end->next = begin;
+	pre_buffer_end = end;
 }
 
 static char **handle_switch_D(char *arg, char **next)
@@ -819,8 +823,9 @@ static struct symbol_list *sparse_initial(void)
 				 token, includepath);
 
 	// Prepend the initial built-in stream
-	token = tokenize_buffer(pre_buffer, pre_buffer_size, token);
-	return sparse_tokenstream(token);
+	if (token)
+		pre_buffer_end->next = token;
+	return sparse_tokenstream(pre_buffer_begin);
 }
 
 struct symbol_list *sparse_initialize(int argc, char **argv, struct string_list **filelist)
diff --git a/token.h b/token.h
index ba7866d..fda880f 100644
--- a/token.h
+++ b/token.h
@@ -195,7 +195,7 @@ extern const char *show_ident(const struct ident *);
 extern const char *show_string(const struct string *string);
 extern const char *show_token(const struct token *);
 extern struct token * tokenize(const char *, int, struct token *, const char **next_path);
-extern struct token * tokenize_buffer(void *, unsigned long, struct token *);
+extern struct token * tokenize_buffer(void *, unsigned long, struct token **);
 
 extern void show_identifier_stats(void);
 extern struct token *preprocess(struct token *);
diff --git a/tokenize.c b/tokenize.c
index d154882..7c41a56 100644
--- a/tokenize.c
+++ b/tokenize.c
@@ -304,7 +304,7 @@ static inline int nextchar(stream_t *stream)
 
 struct token eof_token_entry;
 
-static void mark_eof(stream_t *stream, struct token *end_token)
+static struct token *mark_eof(stream_t *stream)
 {
 	struct token *end;
 
@@ -315,11 +315,10 @@ static void mark_eof(stream_t *stream, struct token *end_token)
 	eof_token_entry.next = &eof_token_entry;
 	eof_token_entry.pos.newline = 1;
 
-	if (!end_token)
-		end_token =  &eof_token_entry;
-	end->next = end_token;
+	end->next =  &eof_token_entry;
 	*stream->tokenlist = end;
 	stream->tokenlist = NULL;
+	return end;
 }
 
 static void add_token(stream_t *stream)
@@ -912,7 +911,7 @@ static struct token *setup_stream(stream_t *stream, int idx, int fd,
 	return begin;
 }
 
-static void tokenize_stream(stream_t *stream, struct token *endtoken)
+static struct token *tokenize_stream(stream_t *stream)
 {
 	int c = nextchar(stream);
 	while (c != EOF) {
@@ -927,22 +926,22 @@ static void tokenize_stream(stream_t *stream, struct token *endtoken)
 		stream->whitespace = 1;
 		c = nextchar(stream);
 	}
-	mark_eof(stream, endtoken);
+	return mark_eof(stream);
 }
 
-struct token * tokenize_buffer(void *buffer, unsigned long size, struct token *endtoken)
+struct token * tokenize_buffer(void *buffer, unsigned long size, struct token **endtoken)
 {
 	stream_t stream;
 	struct token *begin;
 
 	begin = setup_stream(&stream, 0, -1, buffer, size);
-	tokenize_stream(&stream, endtoken);
+	*endtoken = tokenize_stream(&stream);
 	return begin;
 }
 
 struct token * tokenize(const char *name, int fd, struct token *endtoken, const char **next_path)
 {
-	struct token *begin;
+	struct token *begin, *end;
 	stream_t stream;
 	unsigned char buffer[BUFSIZE];
 	int idx;
@@ -954,6 +953,8 @@ struct token * tokenize(const char *name, int fd, struct token *endtoken, const
 	}
 
 	begin = setup_stream(&stream, idx, fd, buffer, 0);
-	tokenize_stream(&stream, endtoken);
+	end = tokenize_stream(&stream);
+	if (endtoken)
+		end->next = endtoken;
 	return begin;
 }

[PATCH 06/15] Sparc64 (Sparc V9, LP64) support

[Alexey Zaytsev]

From: Blue Swirl <blauwirbel@gmail.com>

This patch adds support for Sparc64 (Sparc V9, LP64).

Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
---
 cgcc |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/cgcc b/cgcc
index 31c219a..98686a7 100755
--- a/cgcc
+++ b/cgcc
@@ -243,6 +243,11 @@ sub add_specs {
 		&integer_types (8, 16, 32, $m64 ? 64 : 32, 64) .
 		&float_types (1, 1, 33, [24,8], [53,11], [113,15]) .
 		&define_size_t ($m64 ? "long unsigned int" : "unsigned int"));
+    } elsif ($spec eq 'sparc64') {
+	return (' -Dsparc=1 -D__sparc=1 -D__sparc__=1 -D__sparcv9__=1 -D__sparc64__=1 -D__arch64__=1 -D__LP64__=1' .
+		&integer_types (8, 16, 32, 64, 64) .
+		&float_types (1, 1, 33, [24,8], [53,11], [113,15]) .
+		&define_size_t ("long unsigned int"));
     } elsif ($spec eq 'x86_64') {
 	return (' -Dx86_64=1 -D__x86_64=1 -D__x86_64__=1' .
 		&integer_types (8, 16, 32, $m32 ? 32 : 64, 64) .
@@ -268,6 +273,8 @@ sub add_specs {
 	    return &add_specs ('x86_64');
 	} elsif ($arch =~ /^(ppc)$/i) {
 	    return &add_specs ('ppc');
+	} elsif ($arch =~ /^(sparc64)$/i) {
+	    return &add_specs ('sparc64');
 	}
     } else {
 	die "$0: invalid specs: $spec\n";

[PATCH 07/15] OpenBSD support

[Alexey Zaytsev]

From: Blue Swirl <blauwirbel@gmail.com>

This patch adds OpenBSD support to sparse.

Acked-by: Christopher Li <sparse@chrisli.org>
Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
---
 cgcc         |    3 +++
 compat-bsd.c |   20 ++++++++++++++++++++
 2 files changed, 23 insertions(+), 0 deletions(-)
 create mode 100644 compat-bsd.c

diff --git a/cgcc b/cgcc
index 98686a7..cda8dab 100755
--- a/cgcc
+++ b/cgcc
@@ -221,6 +221,9 @@ sub add_specs {
     } elsif ($spec eq 'linux') {
 	return &add_specs ('unix') .
 	    ' -D__linux__=1 -D__linux=1 -Dlinux=linux';
+    } elsif ($spec eq 'openbsd') {
+	return &add_specs ('unix') .
+	    ' -D__OpenBSD__=1';
     } elsif ($spec eq 'unix') {
 	return ' -Dunix=1 -D__unix=1 -D__unix__=1';
     } elsif ( $spec =~ /^cygwin/) {
diff --git a/compat-bsd.c b/compat-bsd.c
new file mode 100644
index 0000000..4f3c8c0
--- /dev/null
+++ b/compat-bsd.c
@@ -0,0 +1,20 @@
+/*
+ * BSD Compatibility functions
+ *
+ *
+ *  Licensed under the Open Software License version 1.1
+ */
+
+#include <sys/types.h>
+#include <string.h>
+
+#include "lib.h"
+#include "allocate.h"
+#include "token.h"
+
+#include "compat/mmap-blob.c"
+
+long double string_to_ld(const char *nptr, char **endptr)
+{
+	return strtod(nptr, endptr);
+}

[PATCH 08/15] Make show_symbol newline-consistent

[Alexey Zaytsev]

Signed-off-by: Alexey Zaytsev <alexey.zaytsev@gmail.com>
---
 show-parse.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/show-parse.c b/show-parse.c
index c79a288..99795e8 100644
--- a/show-parse.c
+++ b/show-parse.c
@@ -390,8 +390,10 @@ void show_symbol(struct symbol *sym)
 
 	show_type(sym);
 	type = sym->ctype.base_type;
-	if (!type)
+	if (!type) {
+		printf("\n");
 		return;
+	}
 
 	/*
 	 * Show actual implementation information
@@ -410,9 +412,9 @@ void show_symbol(struct symbol *sym)
 
 	case SYM_FN: {
 		struct statement *stmt = type->stmt;
+		printf("\n");
 		if (stmt) {
 			int val;
-			printf("\n");		
 			val = show_statement(stmt);
 			if (val)
 				printf("\tmov.%d\t\tretval,%d\n", stmt->ret->bit_size, val);
@@ -422,6 +424,7 @@ void show_symbol(struct symbol *sym)
 	}
 
 	default:
+		printf("\n");
 		break;
 	}
 

[PATCH 09/15] Handle a terminal -o option properly.

[Alexey Zaytsev]

Signed-off-by: Alexey Zaytsev <alexey.zaytsev@gmail.com>
---
 lib.c |   11 +++++++----
 1 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/lib.c b/lib.c
index ea40ce1..665a6b7 100644
--- a/lib.c
+++ b/lib.c
@@ -347,10 +347,13 @@ static char **handle_switch_m(char *arg, char **next)
 
 static char **handle_switch_o(char *arg, char **next)
 {
-	if (!strcmp (arg, "o") && *next)
-		return next + 1; // "-o foo"
-	else
-		return next;     // "-ofoo" or (bogus) terminal "-o"
+	if (!strcmp (arg, "o")) {       // "-o foo"
+		if (!*++next)
+			die("argument to '-o' is missing");
+	}
+	// else "-ofoo"
+
+	return next;
 }
 
 static const struct warning {

[PATCH 10/15] Looks more evident this way.

[Alexey Zaytsev]

Signed-off-by: Alexey Zaytsev <alexey.zaytsev@gmail.com>
---
 symbol.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/symbol.c b/symbol.c
index 49560ee..4da253b 100644
--- a/symbol.c
+++ b/symbol.c
@@ -49,7 +49,7 @@ struct symbol *lookup_symbol(struct ident *ident, enum namespace ns)
 			return sym;
 		}
 	}
-	return sym;
+	return NULL;
 }
 
 struct context *alloc_context(void)

[PATCH 11/15] Mark handle_switch as static and don't export it from lib.h

[Alexey Zaytsev]

Signed-off-by: Alexey Zaytsev <alexey.zaytsev@gmail.com>
---
 lib.c |    2 +-
 lib.h |    1 -
 2 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/lib.c b/lib.c
index 665a6b7..965fbfb 100644
--- a/lib.c
+++ b/lib.c
@@ -592,7 +592,7 @@ struct switches {
 	char **(*fn)(char *, char **);
 };
 
-char **handle_switch(char *arg, char **next)
+static char **handle_switch(char *arg, char **next)
 {
 	static struct switches cmd[] = {
 		{ "nostdinc", handle_nostdinc },
diff --git a/lib.h b/lib.h
index 42a3396..b22fa93 100644
--- a/lib.h
+++ b/lib.h
@@ -83,7 +83,6 @@ extern void sparse_error(struct position, const char *, ...) FORMAT_ATTR(2);
 extern void error_die(struct position, const char *, ...) FORMAT_ATTR(2) NORETURN_ATTR;
 extern void expression_error(struct expression *, const char *, ...) FORMAT_ATTR(2);
 
-extern char **handle_switch(char *arg, char **next);
 extern void add_pre_buffer(const char *fmt, ...) FORMAT_ATTR(1);
 
 extern int preprocess_only;

[PATCH 12/15] Handle missing argument to -D.

[Alexey Zaytsev]

Signed-off-by: Alexey Zaytsev <alexey.zaytsev@gmail.com>
---
 lib.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/lib.c b/lib.c
index 965fbfb..8dadfa9 100644
--- a/lib.c
+++ b/lib.c
@@ -248,6 +248,10 @@ static char **handle_switch_D(char *arg, char **next)
 {
 	const char *name = arg + 1;
 	const char *value = "1";
+
+	if (!*name || isspace(*name))
+		die("argument to `-D' is missing");
+
 	for (;;) {
 		char c;
 		c = *++arg;

[PATCH 13/15] Gdb macros to get a better look at some sparse data structures.

[Alexey Zaytsev]

Note that we need to build sparse with -g3 -gdwarf-2 to get
the cpp macros included into the debug ingo.

Signed-off-by: Alexey Zaytsev <alexey.zaytsev@gmail.com>
---
 Makefile   |   20 ++--
 gdbhelpers |  310 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 321 insertions(+), 9 deletions(-)
 create mode 100644 gdbhelpers

diff --git a/Makefile b/Makefile
index 3eab17e..ca33218 100644
--- a/Makefile
+++ b/Makefile
@@ -1,19 +1,21 @@
 VERSION=0.4.1
 
-OS ?= linux
+OS = linux
 
-CC ?= gcc
-CFLAGS ?= -O2 -finline-functions -fno-strict-aliasing -g
-CFLAGS += -Wall -Wwrite-strings
-LDFLAGS ?= -g
-AR ?= ar
 
-HAVE_LIBXML=$(shell pkg-config --exists libxml-2.0 && echo 'yes')
+CC = gcc
+CFLAGS = -O2 -finline-functions -fno-strict-aliasing -g
+CFLAGS += -Wall -Wwrite-strings
+LDFLAGS += -g
+AR = ar
 
 #
 # For debugging, uncomment the next one
 #
-CFLAGS += -DDEBUG
+#CFLAGS += -O0 -DDEBUG -g3 -gdwarf-2
+
+HAVE_LIBXML=$(shell pkg-config --exists libxml-2.0 && echo 'yes')
+
 
 DESTDIR=
 PREFIX=$(HOME)
@@ -191,7 +193,7 @@ pre-process.h:
 	$(QUIET_CC)$(CC) -o $@ -c $(CFLAGS) $<
 
 clean: clean-check
-	rm -f *.[oa] $(PROGRAMS) $(SLIB_FILE) pre-process.h sparse.pc
+	rm -f *.[oa] *.so $(PROGRAMS) $(SLIB_FILE) pre-process.h sparse.pc
 
 dist:
 	@if test "`git describe`" != "$(VERSION)" ; then \
diff --git a/gdbhelpers b/gdbhelpers
new file mode 100644
index 0000000..7223ffd
--- /dev/null
+++ b/gdbhelpers
@@ -0,0 +1,310 @@
+
+# Don't forget to rebuild sparse with uncommented debug options
+# in the Makefile. Also, gcc 3 is known to screw up with the
+# cpp macros in the debugging info.
+
+
+# If a gdb_show_* function is running at a non-zero recursion
+# level, only a short summary is shown, preventing further
+# recursion. Also note that gdb has only one, global, scope
+# for variables, so we need to be careful with recursions.
+
+
+set $showing_token = 0
+set $showing_ident = 0
+set $showing_symbol = 0
+
+set $ntabs = 0
+
+define gdb_tabs
+	set $tmp = $ntabs
+	while ($tmp--)
+		printf "\t"
+	end
+end
+
+
+# Ptr list handling
+define ptr_entry
+	set $ptr = $arg0
+	set $index = $arg1
+	set $entry = &($arg2)
+
+	set *($entry) = (void *) (~3UL & (unsigned long)$ptr->list[$index])
+end
+
+
+# Ptr list looping skeleton
+define gdb_ptr_list_for_each
+
+	set $my_head = (struct ptr_list *) $arg0
+	set $my_list = $my_head
+
+
+	if ($my_head)
+		while (1)
+			set $my_nr = 0
+			while ($my_nr < $my_list->nr)
+
+				# Put your iterator code here
+				set $my_nr++
+			end
+
+			if (($my_list = $my_list->next) == $my_head)
+				loop_break
+			end
+		end
+	end
+end
+
+# Show symbols in a symbol_list. Non-recursive
+define gdb_ptr_list_for_each_show_symbol
+
+	set $my_head = (struct ptr_list *) $arg0
+	set $my_list = $my_head
+
+
+	if ($my_head)
+		while (1)
+			set $my_nr = 0
+			while ($my_nr < ($my_list)->nr)
+				set $my_symbol = (struct symbol *) ((~3UL) & (unsigned long)($my_list)->list[$my_nr])
+				gdb_show_symbol($my_symbol)
+
+				set $my_nr++
+			end
+
+			set $my_list = ($my_list)->next
+			if ($my_list == $my_head)
+				loop_break
+			end
+		end
+	end
+end
+
+
+#define gdb_show_statement
+
+
+# Recursive
+define gdb_show_ctype
+	printf "modifiers: "
+	if ($arg0->modifiers & MOD_AUTO)
+		printf "MOD_AUTO "
+	end
+	if ($arg0->modifiers & MOD_REGISTER)
+		printf "MOD_REGISTER "
+	end
+	if ($arg0->modifiers & MOD_STATIC)
+		printf "MOD_STATIC "
+	end
+	if ($arg0->modifiers & MOD_EXTERN)
+		printf "MOD_EXTERN "
+	end
+	if ($arg0->modifiers & MOD_CONST)
+		printf "MOD_CONST "
+	end
+	if ($arg0->modifiers & MOD_VOLATILE)
+		printf "MOD_VOLATILE "
+	end
+	if ($arg0->modifiers & MOD_SIGNED)
+		printf "MOD_SIGNED "
+	end
+	if ($arg0->modifiers & MOD_UNSIGNED)
+		printf "MOD_UNSIGNED "
+	end
+	if ($arg0->modifiers & MOD_CHAR)
+		printf "MOD_CHAR "
+	end
+	if ($arg0->modifiers & MOD_SHORT)
+		printf "MOD_SHORT "
+	end
+	if ($arg0->modifiers & MOD_LONG)
+		printf "MOD_LONG "
+	end
+	if ($arg0->modifiers & MOD_LONGLONG)
+		printf "MOD_LONGLONG "
+	end
+	if ($arg0->modifiers & MOD_TYPEDEF)
+		printf "MOD_TYPEDEF "
+	end
+	if ($arg0->modifiers & MOD_INLINE)
+		printf "MOD_INLINE "
+	end
+	if ($arg0->modifiers & MOD_ADDRESSABLE)
+		printf "MOD_ADDRESSABLE "
+	end
+	if ($arg0->modifiers & MOD_NOCAST)
+		printf "MOD_NOCAST "
+	end
+	if ($arg0->modifiers & MOD_NODEREF)
+		printf "MOD_NODEREF "
+	end
+	if ($arg0->modifiers & MOD_ACCESSED)
+		printf "MOD_ACCESSED "
+	end
+	if ($arg0->modifiers & MOD_TOPLEVEL)
+		printf "MOD_TOPLEVEL "
+	end
+	if ($arg0->modifiers & MOD_LABEL)
+		printf "MOD_LABEL "
+	end
+	if ($arg0->modifiers & MOD_ASSIGNED)
+		printf "MOD_ASSIGNED "
+	end
+	if ($arg0->modifiers & MOD_TYPE)
+		printf "MOD_TYPE "
+	end
+	if ($arg0->modifiers & MOD_SAFE)
+		printf "MOD_SAFE "
+	end
+	if ($arg0->modifiers & MOD_USERTYPE)
+		printf "MOD_USERTYPE "
+	end
+	if ($arg0->modifiers & MOD_FORCE)
+		printf "MOD_FORCE "
+	end
+	if ($arg0->modifiers & MOD_EXPLICITLY_SIGNED)
+		printf "MOD_EXPLICITLY_SIGNED"
+	end
+	if ($arg0->modifiers & MOD_BITWISE)
+		printf "MOD_BITWISE "
+	end
+	if (!$arg0->modifiers)
+		printf "0"
+	end
+
+	printf ", alignment = %d", $arg0->alignment
+	if ($arg0->as)
+		printf ", address_space = %d", $arg0->as
+	end
+	printf "\n"
+
+
+	set $ntabs++
+	if ($arg0->base_type)
+		gdb_tabs
+		printf "base_type = "
+		gdb_show_symbol($arg0->base_type)
+	end
+
+	set $ntabs--
+
+
+end
+
+define gdb_show_symbol
+	printf "(%x) type = ", $arg0
+	output $arg0->type
+	printf ", namespace = "
+	output $arg0->namespace
+	if ($arg0->ident)
+		printf ", ident = %s\n", show_ident($arg0->ident)
+	else
+		printf ", ident = NULL\n"
+	end
+
+#	print "zz"
+
+	gdb_tabs
+	printf "dump:\n"
+	call show_symbol($arg0)
+
+	set $ntabs++
+	if ($arg0->namespace == NS_SYMBOL)
+		gdb_tabs
+		printf "ctype = "
+		gdb_show_ctype(&($arg0->ctype))
+	end
+	set $ntabs--
+end
+
+
+# non-recursive
+define gdb_show_symbols_next_id
+	set $sym = $arg0
+	printf "{\n"
+	set $ntabs++
+	while ($sym)
+		gdb_tabs
+		printf "symbol = "
+		gdb_show_symbol($sym)
+		set $sym = $sym->next_id
+	end
+	set $ntabs--
+	gdb_tabs
+	printf "}\n"
+end
+
+define gdb_show_ident
+	if ($arg0)
+		printf "(%p) '%s'\n", $arg0, show_ident($arg0)
+	else
+		printf "NULL\n"
+	end
+
+	if (! $showing_ident)
+		set $showing_ident = 1
+		set $ntabs++
+
+		set $ident = $arg0
+
+		if ($ident->symbols)
+			gdb_tabs
+			printf "symbols = "
+			gdb_show_symbols_next_id($ident->symbols)
+		end
+
+		set $ntabs--
+		set $showing_ident = 0
+	end
+end
+
+define gdb_show_token
+	printf "%p: '%s', type = ", $arg0, show_token($arg0)
+	output (enum token_type) ($arg0)->pos.type
+	printf "\n"
+
+	if (! $showing_token)
+		set $showing_token = 1
+		set $ntabs++
+
+		set $token = $arg0
+
+		if ($token->pos.type == TOKEN_IDENT)
+			gdb_tabs
+			printf "ident = "
+			gdb_show_ident $token.ident
+		end
+
+		if ($token->pos.type == TOKEN_MACRO_ARGUMENT)
+			gdb_tabs
+			printf "argnum = %d\n", $token->argnum
+		end
+
+		if ($token->pos.type == TOKEN_SPECIAL)
+			gdb_tabs
+			printf "special = \"%s\"\n", show_special($token.special)
+		end
+
+		set $ntabs--
+		set $showing_token = 0
+	end
+end
+
+# non-recursive
+define gdb_show_tokens
+	set $t = $arg0
+	printf "{\n"
+	set $ntabs++
+	while ($t != &eof_token_entry)
+		gdb_tabs
+		printf "token = "
+		gdb_show_token($t)
+		set $t = ($t)->next
+	end
+	set $ntabs--
+	gdb_tabs
+	printf "}\n"
+end
+

[PATCH 14/15] A slightly edited irc discussion with Josh Triplett.

[Alexey Zaytsev]

Describes most data srtructures used in sparse.
---
 Documentation/data-structures.txt |   54 +++++++++++++++++++++++++++++++++++++
 1 files changed, 54 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/data-structures.txt

diff --git a/Documentation/data-structures.txt b/Documentation/data-structures.txt
new file mode 100644
index 0000000..3745b54
--- /dev/null
+++ b/Documentation/data-structures.txt
@@ -0,0 +1,54 @@
+
+<JoshTriplett> As far as the parsing structures go...
+<JoshTriplett> The C parser exists in two main files: parse.c, which parses statements, and expression.c, which parses expressions.
+<JoshTriplett> parse.h contains the definition of struct statement, which represents a C statement.
+<JoshTriplett> That includes only those things which can't appear as an expression, which primarily includes control flow statements such as if, loops, switch/case, and goto.
+<JoshTriplett> expression.h contains the definition of struct expression, which represents a C expression.  That has a lot more content, since most C constructs can appear in expressions.
+<JoshTriplett> A series of statements forms a compound statement (STMT_COMPOUND).
+<JoshTriplett> That appears as another struct statement which has a statement_list member.
+<JoshTriplett> A function body consists of a compound statement.
+<JoshTriplett> When you look at a loop body, if or else body, or case body, you'll notice that they just have a struct statement, not a statement_list; they can have multiple statements by using a compound statement.
+<JoshTriplett> Also note that all loops get turned into a single "iterator" statement.
+<JoshTriplett> for, while, and do-while.
+<JoshTriplett> A symbol, then, represents a name in a C file.  A symbol might represent a variable, a function, a label, or various other things.
+<JoshTriplett> See symbol.h.
+<JoshTriplett> "struct symbol" represents one symbol.
+<JoshTriplett> As with the various other structures, it has some common data and a union of sub-structures for the parts that differ between different types.
+<JoshTriplett> Most of the interesting bits come in the NS_SYMBOL case.
+<JoshTriplett> Among other things, it has a struct statement for the body of a function (if any), a list of symbols for the arguments, an expression for a variable initializer, and so on.
+<JoshTriplett> Together, struct symbol, struct statement, and struct expression represent most of the abstract syntax tree for C.
+<JoshTriplett> So, that represents most of the "front-end" of Sparse: parsing C and generating that abstract syntax tree.
+<JoshTriplett> That much occurs in pretty much any program using the Sparse frontend.
+<JoshTriplett> The backend varies among programs.
+<JoshTriplett> For instance, the c2xml backend goes that far, then outputs XML.
+<JoshTriplett> The sparse static analysis backend has a few steps: it generates linearized bytecode, does some evaluation on that, and outputs some warnings.
+<JoshTriplett> Several other backends run that linearized bytecode stage.
+<JoshTriplett> The linearized bytecode itself has a set of nested structures.
+<JoshTriplett> linearize.h defines all of them.
+<JoshTriplett> At the top level, it has struct entrypoint.
+<JoshTriplett> That represents an entrypoint to the code, which would normally mean a function.
+<JoshTriplett> An entrypoint has a list of basic blocks.
+<JoshTriplett> struct basic_block.
+<JoshTriplett> A basic block represents a series of instructions with no branches.
+<JoshTriplett> Straight-line code.
+<JoshTriplett> A branch only occurs at the end of a basic block, and branches can only target the beginning of a basic block.
+<JoshTriplett> Typically, a conditional will consist of a basic block leading up to the branch, a basic block for the true case, a basic block for the false case, and a basic block where the two paths merge back together.
+<JoshTriplett> Either the true or the false case may not exist.
+<JoshTriplett> A loop will normally have a basic block for the loop body, which can branch to the top at the end or continue to the next basic block.
+<JoshTriplett> So basic blocks represent a node in the control flow graph.
+<JoshTriplett> The edges in that graph lead from one basic block to a basic block which can follow it in the execution of the program.
+<JoshTriplett> Each basic block has a series of instructions, "struct instruction".
+<JoshTriplett> "enum opcode" lists all the instructions.
+<JoshTriplett> Fairly high-level instruction set, corresponding directly to bits of C.
+<JoshTriplett> So you have an entrypoint, which has a graph of basic blocks, each of which has a list of instructions.
+<JoshTriplett> An entrypoint also has a pointer to the first instruction.
+<JoshTriplett> One last bit of trickiness: struct pseudo.
+<JoshTriplett> Have you ever heard of "static single assignment" or SSA form?
+<JoshTriplett> struct pseudo represents one of those single-assignment variables.
+<JoshTriplett> Each one has a pointer to the symbol it represents (which may have many pseudos referencing it).
+<JoshTriplett> Each one also has a pointer to the instruction that defines it.
+<JoshTriplett> That covers most of the major data structures in Sparse.
+<JoshTriplett> Now, given all that, some of the top-level stuff in sparse.c may make more sense.
+<JoshTriplett> For instance, the context checking works in terms of basic blocks.
+<JoshTriplett> Hopefully some of that helped you understand Sparse better.
+

[PATCH 15/15] Warning should be enough for an unhandled transparent union

[Alexey Zaytsev]

An error would be issued if such union is actually used.

Signed-off-by: Alexey Zaytsev <alexey.zaytsev@gmail.com>
---
 parse.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/parse.c b/parse.c
index 877414c..915ae20 100644
--- a/parse.c
+++ b/parse.c
@@ -987,7 +987,7 @@ static struct token *attribute_conditional_context(struct token *token, struct s
 static struct token *attribute_transparent_union(struct token *token, struct symbol *attr, struct ctype *ctype)
 {
 	if (Wtransparent_union)
-		sparse_error(token->pos, "ignoring attribute __transparent_union__");
+		warning(token->pos, "ignoring attribute __transparent_union__");
 	return token;
 }
 

Re: [PATCH 03/15] Add type information to struct instruction.

[Christopher Li]

I think this patch can have more discussion.

If possible, I would rather no increase the instruction struct size.
It is a very common structure, which responsible for a large part
of the byte code memory usage. There is only one person
(David) can benefit from it so far.

I think what David need is just distinction of int vs pointer.
We can do that by save an array of the known basic types.
Including the abstract pointer type. Then we just use an
index to the array rather than use the raw size directly.

That have the extra benefit of, different architecture can share
the same byte code.

Chris

On Sun, Dec 14, 2008 at 4:26 PM, Alexey Zaytsev
<alexey.zaytsev@gmail.com> wrote:
> From: David Given <dg@cowlark.com>
>
> Currently there is no generic way to derive phy
> type information from the instruction flow.
> ---
>  linearize.c |    4 +++-
>  linearize.h |    1 +
>  2 files changed, 4 insertions(+), 1 deletions(-)
>
> diff --git a/linearize.c b/linearize.c
> index fce1ae8..526a710 100644
> --- a/linearize.c
> +++ b/linearize.c
> @@ -55,7 +55,9 @@ static inline int type_size(struct symbol *type)
>
>  static struct instruction *alloc_typed_instruction(int opcode, struct symbol *type)
>  {
> -       return alloc_instruction(opcode, type_size(type));
> +       struct instruction *insn = alloc_instruction(opcode, type_size(type));
> +       insn->type = type;
> +       return insn;
>  }
>
>  static struct entrypoint *alloc_entrypoint(void)
> diff --git a/linearize.h b/linearize.h
> index 32b1c1a..0c5e4ef 100644
> --- a/linearize.h
> +++ b/linearize.h
> @@ -71,6 +71,7 @@ struct instruction {
>                 size:24;
>        struct basic_block *bb;
>        struct position pos;
> +       struct symbol *type;
>        union {
>                pseudo_t target;
>                pseudo_t cond;          /* for branch and switch */
>
>

Re: [PATCH 03/15] Add type information to struct instruction.

[Alexey Zaytsev]

On Tue, Dec 23, 2008 at 06:21, Christopher Li <sparse@chrisli.org> wrote:
> I think this patch can have more discussion.
>
> If possible, I would rather no increase the instruction struct size.
> It is a very common structure, which responsible for a large part
> of the byte code memory usage. There is only one person
> (David) can benefit from it so far.
>
> I think what David need is just distinction of int vs pointer.
> We can do that by save an array of the known basic types.
> Including the abstract pointer type. Then we just use an
> index to the array rather than use the raw size directly.
>
Are you sre this is worth the effort (and code complication)? Struct
instruction is not exactly a tiny one, and a pointer would bloat it
by about 10%. On the other hand, I don't really unerstand why
the type information is associated with the instructions, and not
with the pseudos. Am I missing something?

> That have the extra benefit of, different architecture can share
> the same byte code.
Didn't understand this comment.

> Chris

Re: [PATCH 03/15] Add type information to struct instruction.

[Christopher Li]

On Mon, Dec 22, 2008 at 8:46 PM, Alexey Zaytsev
<alexey.zaytsev@gmail.com> wrote:
> Are you sre this is worth the effort (and code complication)? Struct
> instruction is not exactly a tiny one, and a pointer would bloat it
> by about 10%. On the other hand, I don't really unerstand why

That complexity is needed for the back end to generated C
code any way. Currently, if you save the full C type into instruction.
The back end needs to look into the C type and determinate which
native machine type it should use.

While if you do that up front, the back end side will have a simple
1 to 1 mapping. It will make David's back end code simpler.

On my machine(64 bit). It jump from 56 to 64.  That is 14%.

> the type information is associated with the instructions, and not
> with the pseudos. Am I missing something?

If you have it in the pseudos, the instruction size will depend on
the type of the instruction. There is no universal pseudo member
can be access for all the instruction types. One instruction
can have more than one pseudo, so you are likely to increase the
over all size.

>> That have the extra benefit of, different architecture can share
>> the same byte code.
> Didn't understand this comment.

I am thinking that, if you dump the byte code into file. Then
different back end will need to have different byte code binary,
because difference in the basic type size.

I just realized that because we store the sizeof operation into
binary values, so they are going to have different binary any way.
So this does not matter.

Chris

Re: [PATCH 03/15] Add type information to struct instruction.

[David Given]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Christopher Li wrote:
[...]
> I think what David need is just distinction of int vs pointer.

Actually, I need to distinguish between ints, doubles, object pointers,
function pointers, structures/arrays and void; the code for doing this
is currently a pretty nasty set of heuristics, which is here if you're
interested:

http://cluecc.svn.sourceforge.net/viewvc/cluecc/clue/src/clue/typestore.c?revision=45&view=markup

I'm sure there's a better way of doing it, but I just couldn't find one!
So if you can think of any way to clean all this up I'd be delighted.
The underlying issue, BTW, is that OP_LOAD doesn't carry enough
information in its arguments to determine the type of the instruction.

Incidentally, speaking of distinguishing arguments, one of the issues I
have with Clue is that sparse can't distinguish between parameters of
the different types but the same size in some situations --- e.g.:

extern void foo(float i);
...
foo(5);

...will call foo() with an *integer* parameter rather than a float one.
This is preventing me doing certain optimisations (like being able to
store ints in ints rather than doubles!). Is this related?

(I vaguely recall mentioning this before, but can't find my message...
is there a bug tracker for sparse?)

- --
David Given
dg@cowlark.com
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFJUMo+f9E0noFvlzgRAqy/AJ40DRC4Cgzjml50KMw+Bk9i/hRcXwCdH0sK
2SSrZI19MCnrOhjcy9kYUbI=
=NywA
-----END PGP SIGNATURE-----

Re: [PATCH 03/15] Add type information to struct instruction.

[Christopher Li]

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

On Tue, Dec 23, 2008 at 3:23 AM, David Given <dg@cowlark.com> wrote:
> I'm sure there's a better way of doing it, but I just couldn't find one!
> So if you can think of any way to clean all this up I'd be delighted.
> The underlying issue, BTW, is that OP_LOAD doesn't carry enough
> information in its arguments to determine the type of the instruction.

OK, if it is just for OP_LOAD. Will this attached patch solve your problem?

Instead of adding type to every instruction. It just add that for the OP_LOAD
instruction in insn->orig_type. If that works for you. I am very glad to reclaim
the space back on instruction structure.

> Incidentally, speaking of distinguishing arguments, one of the issues I
> have with Clue is that sparse can't distinguish between parameters of
> the different types but the same size in some situations --- e.g.:
>
> extern void foo(float i);
> ...
> foo(5);
>
> ...will call foo() with an *integer* parameter rather than a float one.
> This is preventing me doing certain optimisations (like being able to
> store ints in ints rather than doubles!). Is this related?

I think sparse should do the proper cast for you when you call
the function call. If it does not, that is a bug and we can fix it.

Please let me know if you can use the patch.

Chris

[-- Attachment #2: load_type --]
[-- Type: application/octet-stream, Size: 1133 bytes --]

Make type information only for OP_LOAD

Index: sparse/linearize.c
===================================================================
--- sparse.orig/linearize.c
+++ sparse/linearize.c
@@ -55,9 +55,7 @@ static inline int type_size(struct symbo
 
 static struct instruction *alloc_typed_instruction(int opcode, struct symbol *type)
 {
-	struct instruction *insn = alloc_instruction(opcode, type_size(type));
-	insn->type = type;
-	return insn;
+	return alloc_instruction(opcode, type_size(type));
 }
 
 static struct entrypoint *alloc_entrypoint(void)
@@ -915,6 +913,7 @@ static pseudo_t add_load(struct entrypoi
 
 	insn->target = new;
 	insn->offset = ad->offset;
+	insn->orig_type = ad->source_type;
 	use_pseudo(insn, ad->address, &insn->src);
 	add_one_insn(ep, insn);
 	return new;
Index: sparse/linearize.h
===================================================================
--- sparse.orig/linearize.h
+++ sparse/linearize.h
@@ -71,7 +71,6 @@ struct instruction {
 		 size:24;
 	struct basic_block *bb;
 	struct position pos;
-	struct symbol *type;
 	union {
 		pseudo_t target;
 		pseudo_t cond;		/* for branch and switch */

Re: [PATCH 03/15] Add type information to struct instruction.

[Alexey Zaytsev]

On Tue, Dec 23, 2008 at 14:23, David Given <dg@cowlark.com> wrote:

[...]
> (I vaguely recall mentioning this before, but can't find my message...
> is there a bug tracker for sparse?)

No, and if you believe there should be one, I can only second it.
While collecting patches from the list, I saw quite some issues with
no resolution status. I think sparse could have an entry in the kernel
bugzilla, or I could set up a separate tracker (I'd prefer roundup over
bugzilla), or maybe Josh could set one up somewhere.

Josh, what do you think about it?

Re: [PATCH 03/15] Add type information to struct instruction.

[David Given]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Christopher Li wrote:
[...]
> OK, if it is just for OP_LOAD. Will this attached patch solve your problem?
> 
> Instead of adding type to every instruction. It just add that for the OP_LOAD
> instruction in insn->orig_type. If that works for you. I am very glad to reclaim
> the space back on instruction structure.

Sorry, I'm not at home right now and can't test this (and probably won't
until New Year, though I'll have a go).

Also, I don't know if it *is* just of OP_LOAD. That's just the one I
remember from looking back at old email. I think the issue is that
OP_LOAD breaks the chain of pseudo->definer->pseudo->definer that I was
using to determine the intrinsic type of the pseudo, but there may be
other instructions that do this as well.

I'd imagine that nobody else has come across this yet because most
people are happy using the size to distinguish between different types;
but (assuming that I've remembered this correctly) this'll bite anyone
who needs to distinguish floats from int32s of doubles from int64s.

If space is really an issue, would it make sense therefore to replace
the insn->size (and symbol->size etc) field with a ->type field pointing
at the defining symbol for the type? Anyone who needs the type can get
it from the defining symbol, plus all the other information that people
like me need would be available, and may also simplify other code. (I
could throw away a complete source file!)

> I think sparse should do the proper cast for you when you call
> the function call. If it does not, that is a bug and we can fix it.

Okay, at some point I'll try to come up with a proper use case.

- --
David Given
dg@cowlark.com
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFJUr9Mf9E0noFvlzgRAmWCAJ4+Q+XPPn9stf8TWHUiIrAk3kwcTACgtJbd
mVD5E9Cg6a2tOtiTI34Mcz8=
=FrsN
-----END PGP SIGNATURE-----

Re: [PATCH 03/15] Add type information to struct instruction.

[Christopher Li]

On Wed, Dec 24, 2008 at 3:01 PM, David Given <dg@cowlark.com> wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Christopher Li wrote:
> [...]
>> OK, if it is just for OP_LOAD. Will this attached patch solve your problem?
>>
>> Instead of adding type to every instruction. It just add that for the OP_LOAD
>> instruction in insn->orig_type. If that works for you. I am very glad to reclaim
>> the space back on instruction structure.
>
> Sorry, I'm not at home right now and can't test this (and probably won't
> until New Year, though I'll have a go).

No problem. Have a nice Christmas.

>
> Also, I don't know if it *is* just of OP_LOAD. That's just the one I
> remember from looking back at old email. I think the issue is that
> OP_LOAD breaks the chain of pseudo->definer->pseudo->definer that I was
> using to determine the intrinsic type of the pseudo, but there may be
> other instructions that do this as well.

I hope that is the only one. Let me know if you find out there is
more instructions.

>
> I'd imagine that nobody else has come across this yet because most
> people are happy using the size to distinguish between different types;
> but (assuming that I've remembered this correctly) this'll bite anyone
> who needs to distinguish floats from int32s of doubles from int64s.

Right, other than the size, we need to distinguish between:
(sign/unsign) integer, floating point, pointers.
That is why we even have three type of cast instruction.
CAST, CASTFP and CASTPTR. They can generate fairly different
code.


> If space is really an issue, would it make sense therefore to replace
> the insn->size (and symbol->size etc) field with a ->type field pointing
> at the defining symbol for the type? Anyone who needs the type can get
> it from the defining symbol, plus all the other information that people
> like me need would be available, and may also simplify other code. (I
> could throw away a complete source file!)

Right, that is what I am purposing in previous email as well.
It looks have more code up front. But it can simplify the back end
who actually going to use it. It is actually much easier to do it
from front end while initializing those symbols.

Chris