[PATCH v2 0/8] fix uses of killed instructions

[PATCH v2 0/8] fix uses of killed instructions

[Luc Van Oostenryck]

The series fixes some issues regarding the operands of killed
instructions not having their usage adjusted.
The main apparent consequence of this situation is that
some instructions which should be considered as unneeded are
not removed from the code as expected. This can sometimes have
a cascading effect that can be "quite interesting".

There is some closely related issues that remains,
for example the situation of the exotic instructions.
It's also possible that something is missing with OP_SYMADDR.

OP_LOAD, OP_STORE & OP_CALL are completly ignored and their
correct handling need another approach, so they're also
ignored by this series.


*ATTENTION*
The test cases in this series depends on some
features I added to the test-suite that I posted the
10th November with the "fix discarded label statement" serie.


Change since V1:
- change the testing of conditional vs uncondiational branches (patch 1)
- add a missing default case in patch 8
- drop patch 2 ("fix killing of OP_SETVAL instructions") as it causes
  a strange unexpected change at a single place when testing on a kernel
  allyesconfig on x86.

Luc Van Oostenryck (8):
  fix crash while testing between conditional & unconditional OP_BR
  kill uses of replaced instructions
  fix killing OP_PHI instructions
  fix killing OP_CAST & friends
  fix killing OP_SELECT
  fix killing OP_COMPUTEDGOTO
  explicitely ignore killing OP_ENTRY
  cleanup kill_instruction()

 simplify.c                      | 74 ++++++++++++++++++++++++++++-------------
 validation/kill-casts.c         | 22 ++++++++++++
 validation/kill-computedgoto.c  | 17 ++++++++++
 validation/kill-phi-node.c      | 18 ++++++++++
 validation/kill-replaced-insn.c | 54 ++++++++++++++++++++++++++++++
 validation/kill-select.c        | 16 +++++++++
 6 files changed, 177 insertions(+), 24 deletions(-)
 create mode 100644 validation/kill-casts.c
 create mode 100644 validation/kill-computedgoto.c
 create mode 100644 validation/kill-phi-node.c
 create mode 100644 validation/kill-replaced-insn.c
 create mode 100644 validation/kill-select.c

-- 
2.11.0

[PATCH v2 1/8] fix crash while testing between conditional & unconditional OP_BR

[Luc Van Oostenryck]

It seems that testing for a NULL insn->cond is not the right test,
what must be done is to test if either of ->bb_{true,false} is NULL.

Fixes: 556dbc8d75 ("Update usage chain for dead instructions")
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 simplify.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/simplify.c b/simplify.c
index b5cd0ea77..3dea03b5e 100644
--- a/simplify.c
+++ b/simplify.c
@@ -221,7 +221,7 @@ void kill_instruction(struct instruction *insn)
 	case OP_BR:
 		insn->bb = NULL;
 		repeat_phase |= REPEAT_CSE;
-		if (insn->cond)
+		if (insn->bb_true && insn->bb_false)
 			kill_use(&insn->cond);
 		return;
 	}
-- 
2.11.0

[PATCH v2 2/8] kill uses of replaced instructions

[Luc Van Oostenryck]

When an instruction is replaced by a pseudo, the 'usage' of its
operands should be removed too but it's not the case.

The fix consists in calling kill_use() for each operands after
the pseudo is replaced.
Not all types of instruction are considered, only those which
can be replaced by a pseudo.

The following example illustrate the situation. When looking at
the output of test-linearize, the following function:

	static int kill_add(int a, int b)
	{
		return (a + b) && 0;
	}

without the patch, gives this output:

	kill_add:
		add.32      %r3 <- %arg1, %arg2
		ret.32      $0

The 'add' instruction is obviously unneeded but nevertheless present.

Before any optimization the code was something like:

	kill_add:
		add.32      %r3 <- %arg1, %arg2
		and_bool.32 %r4 <- %r3, $0
		ret.32      %r4

During the simplification phase, the result of the 'and' instruction (%r4)
have been replaced by '0' and the instruction itself is discarded.
But '%r3' usage has not been adjusted and the further phases are not
aware that '%r3' is not needed anymore and so the 'add' instruction is kept
while not needed by anything.

With the patch the 'add' instruction is correctly discarded, giving the
expected output:

	kill_add:
		ret.32      $0

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 simplify.c                      | 20 +++++++++++++++
 validation/kill-replaced-insn.c | 54 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 74 insertions(+)
 create mode 100644 validation/kill-replaced-insn.c

diff --git a/simplify.c b/simplify.c
index 3dea03b5e..ed41e441a 100644
--- a/simplify.c
+++ b/simplify.c
@@ -253,6 +253,26 @@ static inline int constant(pseudo_t pseudo)
 static int replace_with_pseudo(struct instruction *insn, pseudo_t pseudo)
 {
 	convert_instruction_target(insn, pseudo);
+
+	switch (insn->opcode) {
+	case OP_SEL:
+	case OP_RANGE:
+		kill_use(&insn->src3);
+	case OP_BINARY ... OP_BINCMP_END:
+		kill_use(&insn->src2);
+	case OP_NOT:
+	case OP_NEG:
+	case OP_SYMADDR:
+	case OP_CAST:
+	case OP_SCAST:
+	case OP_FPCAST:
+	case OP_PTRCAST:
+		kill_use(&insn->src1);
+		break;
+
+	default:
+		assert(0);
+	}
 	insn->bb = NULL;
 	return REPEAT_CSE;
 }
diff --git a/validation/kill-replaced-insn.c b/validation/kill-replaced-insn.c
new file mode 100644
index 000000000..be031b6c1
--- /dev/null
+++ b/validation/kill-replaced-insn.c
@@ -0,0 +1,54 @@
+// See if the replaced operation is effectively killed or not
+
+static int kill_add(int a, int b)
+{
+	return (a + b) && 0;
+}
+
+static int kill_scast(short a)
+{
+	return ((int) a) && 0;
+}
+
+static int kill_ucast(unsigned char a)
+{
+	return ((int) a) && 0;
+}
+
+static int kill_pcast(int *a)
+{
+	return ((void*) a) && 0;
+}
+
+static int kill_fcast(double a)
+{
+	return ((int) a) && 0;
+}
+
+static int kill_select(int a)
+{
+	return (a ? 1 : 0) && 0;
+}
+
+static int kill_load(int *a)
+{
+	return *a && 0;
+}
+
+static int kill_store(int *a)
+{
+	return (*a = 1) && 0;
+}
+
+/*
+ * check-name: kill-replaced-insn
+ * check-command: test-linearize $file
+ *
+ * check-output-ignore
+ * check-output-excludes: add\\.
+ * check-output-excludes: scast\\.
+ * check-output-excludes: \\<cast\\.
+ * check-output-excludes: ptrcast\\.
+ * check-output-excludes: fpcast\\.
+ * check-output-excludes: sel\\.
+ */
-- 
2.11.0

[PATCH v2 3/8] fix killing OP_PHI instructions

[Luc Van Oostenryck]

Currently kill_instruction() doesn't do anything with the
sources of OP_PHI instructions. But when these instructions
are removed the 'usage' of the associated sources must also
be removed. This is not done and as result the instructions
producing the phi-sources are not optimized away as expected.

This patch fixes that by calling clear_phi() when killing a
phi-instruction.

For example, when looking at the output of test-linearize,
the following function:
	void foo(int a, int *b, unsigned int g);
	void foo(int a, int *b, unsigned int g)
	{
		int d = 0;

		if ((!a || *b) && g)
			d = 16;
		else
			d = 8;
	}

gives this output without the patch:
	foo:
		br          %arg1, .L1, .L2
	.L1:
		phisrc.32   %phi1 <- $1
		br          .L3
	.L2:
		load.32     %r3 <- 0[%arg2]
		phisrc.32   %phi2 <- %r3
		br          .L3
	.L3:
		ret

The 'phisrc' instructions are obviously unneeded but nevertheless present.

With the patch, the output is much closer to what's expected:
	foo:
		br          %arg1, .L3, .L2
	.L2:
		load.32     %r3 <- 0[%arg2]
		br          .L3
	.L3:
		ret

Note 1) The 'load' instruction is also dead and should have been removed
but it's separate problem.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 simplify.c                 |  1 +
 validation/kill-phi-node.c | 18 ++++++++++++++++++
 2 files changed, 19 insertions(+)
 create mode 100644 validation/kill-phi-node.c

diff --git a/simplify.c b/simplify.c
index ed41e441a..90998021d 100644
--- a/simplify.c
+++ b/simplify.c
@@ -202,6 +202,7 @@ void kill_instruction(struct instruction *insn)
 		return;
 
 	case OP_PHI:
+		clear_phi(insn);
 		insn->bb = NULL;
 		repeat_phase |= REPEAT_CSE;
 		return;
diff --git a/validation/kill-phi-node.c b/validation/kill-phi-node.c
new file mode 100644
index 000000000..88de9f962
--- /dev/null
+++ b/validation/kill-phi-node.c
@@ -0,0 +1,18 @@
+void foo(int a, int *b, unsigned int g);
+void foo(int a, int *b, unsigned int g)
+{
+	int d = 0;
+
+	if ((!a || *b) && g)
+		d = 16;
+	else
+		d = 8;
+}
+
+/*
+ * check-name: kill-phi-node
+ * check-command: test-linearize $file
+ *
+ * check-output-ignore
+ * check-output-excludes: phisrc\\.
+ */
-- 
2.11.0

[PATCH v2 4/8] fix killing OP_CAST & friends

[Luc Van Oostenryck]

Currently  kill_instruction() doesn't do anything with the
operands of casts instructions. But when theses instructions
are removed the operands 'usage' must be adjusted and this is
not done and as result the instructions producing the operands
of these casts are not optimized away as expected.

This patch fixes that by killing these casts the same way as others
unary instructions (OP_NOT & OP_NEG).

To illustrate the situation, the output of test-linearize
on the following code:
	extern void __abort(void);
	struct s {
		int elem:3;
	};
	void foo(struct s *x);
	void foo(struct s *x)
	{
		if (x->elem == 0) {
			if (x->elem != 0 && x->elem != 1)
				__abort();
		}
	}

gives this output without the patch:
	foo:
		load.32     %r2 <- 0[%arg1]
		cast.32     %r3 <- (3) %r2
		br          .L1
	.L1:
		ret

Since x->elem can't be at the same time == 0 & != 0, the inner if is never
true and the whole code should have been optimized away.
The 'cast' instruction is obviously not needed but nevertheless present.

With the patch, the output is much closer to what's expected:
	foo:
		load.32     %r2 <- 0[%arg1]
		br          .L1
	.L1:
		ret

Note 1) The 'load' instruction is also dead but it's a separate problem.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 simplify.c              |  4 ++++
 validation/kill-casts.c | 22 ++++++++++++++++++++++
 2 files changed, 26 insertions(+)
 create mode 100644 validation/kill-casts.c

diff --git a/simplify.c b/simplify.c
index 90998021d..fc6bae791 100644
--- a/simplify.c
+++ b/simplify.c
@@ -195,6 +195,10 @@ void kill_instruction(struct instruction *insn)
 		repeat_phase |= REPEAT_CSE;
 		return;
 
+	case OP_CAST:
+	case OP_SCAST:
+	case OP_FPCAST:
+	case OP_PTRCAST:
 	case OP_NOT: case OP_NEG:
 		insn->bb = NULL;
 		kill_use(&insn->src1);
diff --git a/validation/kill-casts.c b/validation/kill-casts.c
new file mode 100644
index 000000000..cf52f2460
--- /dev/null
+++ b/validation/kill-casts.c
@@ -0,0 +1,22 @@
+extern void __abort(void);
+
+struct s {
+	int elem:3;
+};
+
+void foo(struct s *x);
+void foo(struct s *x)
+{
+	if (x->elem == 0) {
+		if (x->elem != 0 && x->elem != 1)
+			__abort();
+	}
+}
+
+/*
+ * check-name: kill-casts
+ * check-command: test-linearize $file
+ *
+ * check-output-ignore
+ * check-output-excludes: cast\\.
+ */
-- 
2.11.0

[PATCH v2 5/8] fix killing OP_SELECT

[Luc Van Oostenryck]

Currently kill_instruction() doesn't do anything with the
operands of select instructions (OP_SELECT). But when these
instructions are removed we must also remove the operands 'usage'.
Without this the instructions which provides the select's
operands are not optimized away as expected.

This patch fixes this by doing for OP_SELECTs the basic
kill_instruction() for ternary instruction, like OP_RANGE.

As an example, when looking at the output of test-linearize,
the following code:

	void foo(int x)
	{
		unsigned int ui;

		ui = x + 1;
		ui = ui ? 0 : 1;
	}

gives this output:

	foo:
		add.32      %r2 <- %arg1, $1
		ret

Since the result of the ?: is never used, the whole code should be
optimized away. The 'select' instruction itself is indeed discarded
but the 'add' is not.

With the patch, the output is much closer to what's expected:

	foo:
		ret

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 simplify.c               |  1 +
 validation/kill-select.c | 16 ++++++++++++++++
 2 files changed, 17 insertions(+)
 create mode 100644 validation/kill-select.c

diff --git a/simplify.c b/simplify.c
index fc6bae791..1e4aa63b4 100644
--- a/simplify.c
+++ b/simplify.c
@@ -216,6 +216,7 @@ void kill_instruction(struct instruction *insn)
 		repeat_phase |= REPEAT_CSE | REPEAT_SYMBOL_CLEANUP;
 		return;
 
+	case OP_SEL:
 	case OP_RANGE:
 		insn->bb = NULL;
 		repeat_phase |= REPEAT_CSE;
diff --git a/validation/kill-select.c b/validation/kill-select.c
new file mode 100644
index 000000000..445472be8
--- /dev/null
+++ b/validation/kill-select.c
@@ -0,0 +1,16 @@
+void foo(int x);
+void foo(int x)
+{
+	unsigned int ui;
+
+	ui = x + 1;
+	ui = ui ? 0 : 1;
+}
+
+/*
+ * check-name: kill-select
+ * check-command: test-linearize $file
+ *
+ * check-output-ignore
+ * check-output-excludes: add\\.
+ */
-- 
2.11.0

[PATCH v2 6/8] fix killing OP_COMPUTEDGOTO

[Luc Van Oostenryck]

Currently kill_instruction() doesn't do anything with the
operands of computed gotos (OP_COMPUTEDGOTO). But when these
instructions are removed we must also remove the operands 'usage'.
Without this some instructions, which provides the select's
operands, are not optimized away as expected.

The fix consists by killing it's operand much like what is done for
conditional branches.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 simplify.c                     |  1 +
 validation/kill-computedgoto.c | 17 +++++++++++++++++
 2 files changed, 18 insertions(+)
 create mode 100644 validation/kill-computedgoto.c

diff --git a/simplify.c b/simplify.c
index 1e4aa63b4..690fdc4e8 100644
--- a/simplify.c
+++ b/simplify.c
@@ -225,6 +225,7 @@ void kill_instruction(struct instruction *insn)
 		kill_use(&insn->src3);
 		return;
 	case OP_BR:
+	case OP_COMPUTEDGOTO:
 		insn->bb = NULL;
 		repeat_phase |= REPEAT_CSE;
 		if (insn->bb_true && insn->bb_false)
diff --git a/validation/kill-computedgoto.c b/validation/kill-computedgoto.c
new file mode 100644
index 000000000..3b3ed8ff2
--- /dev/null
+++ b/validation/kill-computedgoto.c
@@ -0,0 +1,17 @@
+void foo(int a);
+void foo(int a)
+{
+	void *l = &&end + 3;
+
+end:
+	if (a * 0)
+		goto *l;
+}
+
+/*
+ * check-name: kill-computedgoto
+ * check-command: test-linearize $file
+ *
+ * check-output-ignore
+ * check-output-excludes: add\\.
+ */
-- 
2.11.0

[PATCH v2 7/8] explicitely ignore killing OP_ENTRY

[Luc Van Oostenryck]

It doesn't change anything, it's more for documentation
purpose.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 simplify.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/simplify.c b/simplify.c
index 690fdc4e8..66550fa3d 100644
--- a/simplify.c
+++ b/simplify.c
@@ -231,6 +231,10 @@ void kill_instruction(struct instruction *insn)
 		if (insn->bb_true && insn->bb_false)
 			kill_use(&insn->cond);
 		return;
+
+	case OP_ENTRY:
+		/* ignore */
+		return;
 	}
 }
 
-- 
2.11.0

[PATCH v2 8/8] cleanup kill_instruction()

[Luc Van Oostenryck]

No functional changes:
- factorize out the '->bb = NULL' and '|= REPEAT_CSE'.
- fall through the switch cases for ternary/bunary/unary ops.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 simplify.c | 47 +++++++++++++++++++++--------------------------
 1 file changed, 21 insertions(+), 26 deletions(-)

diff --git a/simplify.c b/simplify.c
index 66550fa3d..f62fc83a1 100644
--- a/simplify.c
+++ b/simplify.c
@@ -188,54 +188,49 @@ void kill_instruction(struct instruction *insn)
 		return;
 
 	switch (insn->opcode) {
+	case OP_SEL:
+	case OP_RANGE:
+		kill_use(&insn->src3);
+		/* fall through */
+
 	case OP_BINARY ... OP_BINCMP_END:
-		insn->bb = NULL;
-		kill_use(&insn->src1);
 		kill_use(&insn->src2);
-		repeat_phase |= REPEAT_CSE;
-		return;
+		/* fall through */
 
 	case OP_CAST:
 	case OP_SCAST:
 	case OP_FPCAST:
 	case OP_PTRCAST:
 	case OP_NOT: case OP_NEG:
-		insn->bb = NULL;
 		kill_use(&insn->src1);
-		repeat_phase |= REPEAT_CSE;
-		return;
+		break;
 
 	case OP_PHI:
 		clear_phi(insn);
-		insn->bb = NULL;
-		repeat_phase |= REPEAT_CSE;
-		return;
+		break;
 
 	case OP_SYMADDR:
-		insn->bb = NULL;
-		repeat_phase |= REPEAT_CSE | REPEAT_SYMBOL_CLEANUP;
-		return;
+		repeat_phase |= REPEAT_SYMBOL_CLEANUP;
+		break;
 
-	case OP_SEL:
-	case OP_RANGE:
-		insn->bb = NULL;
-		repeat_phase |= REPEAT_CSE;
-		kill_use(&insn->src1);
-		kill_use(&insn->src2);
-		kill_use(&insn->src3);
-		return;
 	case OP_BR:
+		if (!insn->bb_true || !insn->bb_false)
+			break;
+		/* fall through */
+
 	case OP_COMPUTEDGOTO:
-		insn->bb = NULL;
-		repeat_phase |= REPEAT_CSE;
-		if (insn->bb_true && insn->bb_false)
-			kill_use(&insn->cond);
-		return;
+		kill_use(&insn->cond);
+		break;
 
 	case OP_ENTRY:
+	default:
 		/* ignore */
 		return;
 	}
+
+	insn->bb = NULL;
+	repeat_phase |= REPEAT_CSE;
+	return;
 }
 
 /*
-- 
2.11.0

status of sparse-next

[Luc Van Oostenryck]

I've tested reatively extensively the current sparse-next.
There was a crash problem triggered by one of my patches and
two others minors problems, all three addressed by this serie.

With this changes, sparse-next behave correctly on:
- the testsuite
- using sparse on an allyesconfig of the kernel on x86
- no crashes on the GCC testsuite (where sparse or test-linearizes
  make sense).
- my collection of test files (200+ files but consisting mainly
  of simple cases concerning linearization).


Luc

Re: [PATCH v2 0/8] fix uses of killed instructions

[Van Oostenryck Luc]

On Sun, Jan 29, 2017 at 11:48 AM, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
> - drop patch 2 ("fix killing of OP_SETVAL instructions") as it causes
>   a strange unexpected change at a single place when testing on a kernel
>   allyesconfig on x86.

I've investigated what was happening with this one.
Everything is all right.
What was happening was that the absence of the patch
"fix value of label statement" created a lot of dead OP_SETVAL.
The patch "fix killing of OP_SETVAL instructions" did its job
but the code was wrong anyway and created a lot of changes
in kernel's allyesconfig's output.

So now that "fix value of label statement" is applied, you can
safely apply "fix killing of OP_SETVAL instructions" somewhere
after it, it won't create useless noise.

Luc

Re: [PATCH v2 0/8] fix uses of killed instructions

[Luc Van Oostenryck]

On Tue, Feb 07, 2017 at 08:30:19PM +0100, Van Oostenryck Luc wrote:
> On Sun, Jan 29, 2017 at 11:48 AM, Luc Van Oostenryck
> <luc.vanoostenryck@gmail.com> wrote:
> > - drop patch 2 ("fix killing of OP_SETVAL instructions") as it causes
> >   a strange unexpected change at a single place when testing on a kernel
> >   allyesconfig on x86.
> 
> I've investigated what was happening with this one.
> Everything is all right.
> What was happening was that the absence of the patch
> "fix value of label statement" created a lot of dead OP_SETVAL.
> The patch "fix killing of OP_SETVAL instructions" did its job
> but the code was wrong anyway and created a lot of changes
> in kernel's allyesconfig's output.
> 
> So now that "fix value of label statement" is applied, you can
> safely apply "fix killing of OP_SETVAL instructions" somewhere
> after it, it won't create useless noise.

I just noticed that you have added it at the top of sparse-next
but what you applied (basically adding "case OP_SETVAL" in 
simplify_instruction()) is *not* what the patch I sent did
(adding "case OP_SETVAL" in kill_instruction())s and causes
quick crashes..
I suppose there was a conflict or so.

Do you want that I sent a new patch or can you solve it
directly?

Luc

Re: [PATCH v2 0/8] fix uses of killed instructions

[Christopher Li]

On Thu, Feb 9, 2017 at 12:50 AM, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
>> So now that "fix value of label statement" is applied, you can
>> safely apply "fix killing of OP_SETVAL instructions" somewhere
>> after it, it won't create useless noise.
>
> I just noticed that you have added it at the top of sparse-next
> but what you applied (basically adding "case OP_SETVAL" in
> simplify_instruction()) is *not* what the patch I sent did
> (adding "case OP_SETVAL" in kill_instruction())s and causes
> quick crashes..
> I suppose there was a conflict or so.
>
> Do you want that I sent a new patch or can you solve it
> directly?

Strange. That is the only patch I found in my series.

Sure, go ahead and send me the new one. I will take out the
one in sparse-next and replace to your new one.

Chris

[PATCH] fix killing OP_SETVAL instructions

[Luc Van Oostenryck]

Currently, kill_instruction() ignore OP_SETVAL instructions
with the result that some instructions are not optimized away
as expected.

For example, when looking at the output of test-linearize,
the following function:

	static int kill_setval(void)
	{
	l:
		return &&l && 0;
	}

gives the following output:

	kill_setval:
		set.64      %r6 <- .L1
		ret.32      $0

The 'set' instruction is obviously unneeded but nevertheless present.

With the patch, the output is the expected:

	kill_set:
		ret.32      $0

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 simplify.c                      | 1 +
 validation/kill-replaced-insn.c | 7 +++++++
 2 files changed, 8 insertions(+)

diff --git a/simplify.c b/simplify.c
index cbb1844e2..a7e74ae0e 100644
--- a/simplify.c
+++ b/simplify.c
@@ -201,6 +201,7 @@ void kill_instruction(struct instruction *insn)
 	case OP_SCAST:
 	case OP_FPCAST:
 	case OP_PTRCAST:
+	case OP_SETVAL:
 	case OP_NOT: case OP_NEG:
 		kill_use(&insn->src1);
 		break;
diff --git a/validation/kill-replaced-insn.c b/validation/kill-replaced-insn.c
index be031b6c1..920218778 100644
--- a/validation/kill-replaced-insn.c
+++ b/validation/kill-replaced-insn.c
@@ -30,6 +30,12 @@ static int kill_select(int a)
 	return (a ? 1 : 0) && 0;
 }
 
+static int kill_setval(int a)
+{
+l:
+	return &&l && 0;
+}
+
 static int kill_load(int *a)
 {
 	return *a && 0;
@@ -51,4 +57,5 @@ static int kill_store(int *a)
  * check-output-excludes: ptrcast\\.
  * check-output-excludes: fpcast\\.
  * check-output-excludes: sel\\.
+ * check-output-excludes: set\\.
  */
-- 
2.11.0

Re: [PATCH v2 0/8] fix uses of killed instructions

[Luc Van Oostenryck]

On Thu, Feb 09, 2017 at 04:40:07AM +0800, Christopher Li wrote:
> On Thu, Feb 9, 2017 at 12:50 AM, Luc Van Oostenryck
> <luc.vanoostenryck@gmail.com> wrote:
> >> So now that "fix value of label statement" is applied, you can
> >> safely apply "fix killing of OP_SETVAL instructions" somewhere
> >> after it, it won't create useless noise.
> >
> > I just noticed that you have added it at the top of sparse-next
> > but what you applied (basically adding "case OP_SETVAL" in
> > simplify_instruction()) is *not* what the patch I sent did
> > (adding "case OP_SETVAL" in kill_instruction())s and causes
> > quick crashes..
> > I suppose there was a conflict or so.
> >
> > Do you want that I sent a new patch or can you solve it
> > directly?
> 
> Strange. That is the only patch I found in my series.

Well, it was the rigth patch in the sense that it had the right
log message and stuff but the resulting diff was only correct
within a 1 line context. With a 3 lines context 'git am' or
a pure 'patch < ...' should have given a conflict as one of
the patch that was initialy just after it
(1856b3461 "fix killing OP_CAST & friends") made also some
changes in the same lines and these two patch have now been
exchanged.

> Sure, go ahead and send me the new one. I will take out the
> one in sparse-next and replace to your new one.

Done.

Luc 

Re: [PATCH v2 0/8] fix uses of killed instructions

[Christopher Li]

On Thu, Feb 9, 2017 at 5:35 AM, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
>> Strange. That is the only patch I found in my series.
>
> Well, it was the rigth patch in the sense that it had the right
> log message and stuff but the resulting diff was only correct
> within a 1 line context. With a 3 lines context 'git am' or
> a pure 'patch < ...' should have given a conflict as one of
> the patch that was initialy just after it
> (1856b3461 "fix killing OP_CAST & friends") made also some
> changes in the same lines and these two patch have now been
> exchanged.

I see. So what happen is that the git am rejects it while
the patch program has not problem apply the patch. So I take
the patched version, which was wrong in terms of conflict resolution.

I go through my logs, similar things happen in the following two commit.

684710647 testsuite: check patterns presence or absence in output
49118f27e2 testsuite: check the nbr of times a pattern should be present

"git am" initial rejects it but the patch program accepts it.

Can you double check this two changes sounds correct to you?
I did take a second look, seems normal to me.

Any way, your new version of patch is applied in sparse-next.

Chris

Re: [PATCH v2 0/8] fix uses of killed instructions

[Luc Van Oostenryck]

On Thu, Feb 09, 2017 at 06:13:46AM +0800, Christopher Li wrote:
> On Thu, Feb 9, 2017 at 5:35 AM, Luc Van Oostenryck
> <luc.vanoostenryck@gmail.com> wrote:
> >> Strange. That is the only patch I found in my series.
> >
> > Well, it was the rigth patch in the sense that it had the right
> > log message and stuff but the resulting diff was only correct
> > within a 1 line context. With a 3 lines context 'git am' or
> > a pure 'patch < ...' should have given a conflict as one of
> > the patch that was initialy just after it
> > (1856b3461 "fix killing OP_CAST & friends") made also some
> > changes in the same lines and these two patch have now been
> > exchanged.
> 
> I see. So what happen is that the git am rejects it while
> the patch program has not problem apply the patch. So I take
> the patched version, which was wrong in terms of conflict resolution.
> 
> I go through my logs, similar things happen in the following two commit.
> 
> 684710647 testsuite: check patterns presence or absence in output
> 49118f27e2 testsuite: check the nbr of times a pattern should be present
> 
> "git am" initial rejects it but the patch program accepts it.
> 
> Can you double check this two changes sounds correct to you?
> I did take a second look, seems normal to me.

I just checked, two chunks of the second of these patches are in
another place than what I have here in my original tree but
it's equivalent regarding functionality.

Luc

Re: [PATCH v2 0/8] fix uses of killed instructions

[Luc Van Oostenryck]

On Thu, Feb 09, 2017 at 06:13:46AM +0800, Christopher Li wrote:
> 
> Any way, your new version of patch is applied in sparse-next.
> 
> Chris

Thanks.
I've just finished to test everything again and everything is
fine. Linux's kernel x86 allyesconfig output give exactly the
same output as two days ago as it was expected.

Luc

Re: [PATCH v2 0/8] fix uses of killed instructions

[Christopher Li]

On Thu, Feb 9, 2017 at 7:45 AM, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
> I've just finished to test everything again and everything is
> fine. Linux's kernel x86 allyesconfig output give exactly the
> same output as two days ago as it was expected.

Great.

Thanks for the hard work.

Chris

Re: [PATCH v2 0/8] fix uses of killed instructions

[Luc Van Oostenryck]

On Wed, Feb 8, 2017 at 11:28 PM, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
> On Thu, Feb 09, 2017 at 06:13:46AM +0800, Christopher Li wrote:
>> I see. So what happen is that the git am rejects it while
>> the patch program has not problem apply the patch. So I take
>> the patched version, which was wrong in terms of conflict resolution.
>>
>> I go through my logs, similar things happen in the following two commit.
>>
>> 684710647 testsuite: check patterns presence or absence in output
>> 49118f27e2 testsuite: check the nbr of times a pattern should be present
>>
>> "git am" initial rejects it but the patch program accepts it.
>>
>> Can you double check this two changes sounds correct to you?
>> I did take a second look, seems normal to me.
>
> I just checked, two chunks of the second of these patches are in
> another place than what I have here in my original tree but
> it's equivalent regarding functionality.

Sigh. I was a bit short on time when checking this and the diff I looked
at was somehow misleading but in fact the functionality is not at all equivalent
although all the tests did the expected result.

I'm resending a respin of the two concerned series.

Luc

[PATCH v2 00/14] testsuite improvements

[Luc Van Oostenryck]

This serie aimes to improve the test suite in two ways:
1) make effective use of the 'known-to-fail' tag
2) check the presence or absence of some patterns in the output file

The patches are organised like this:
- patches 1 & 3 are cleanup some tests
- patch 4 adds a test case for -Wenum-mismatch
- patch 2 & 6 make effective use of the 'known-to-fail' tag
- patches 5 & 8 check the presence or absence of the patterns
- patch 9 adds some self checking 
- patch 10 lets check the number of times a pattern occurs
- patch 11: somehow correct a message from the testsuite
- patches 12-14: make the testsuite quieter regarding the 'known-to-fail'

Changes since V1
- none, it's only a consolidation of two sub-series
- add some selfchecking (patch 9)

Luc Van Oostenryck (14):
  testsuite: give a proper name to the 'binary-constant' test
  testsuite: make tests known to fail effectively fail
  testsuite: simplify the ioc-typecheck case
  testsuite: add a simple test for -Wenum-mismatch
  testsuite: add tag to ignore the output/error
  testsuite: report as error tests known to fail but which succeed
  allow to launch the test suite from the project root dir
  testsuite: check patterns presence or absence in output
  testsuite: add some selfchecking
  testsuite: check the nbr of times a pattern should be present
  testsuite: use 'error' instead of 'info' for successful tests known to fail
  testsuite: get 'check-known-to-fail' earlier
  testsuite: allow quieter error reporting
  testsuite: quieter error reporting for 'known-to-fail'

 Documentation/test-suite          |  20 +++++++
 validation/badtype1.c             |   4 ++
 validation/binary-constant.c      |   2 +-
 validation/enum-mismatch.c        |  19 ++++++
 validation/ioc-typecheck.c        |  12 +---
 validation/struct-ns2.c           |   6 ++
 validation/test-suite             | 119 ++++++++++++++++++++++++++++++++++----
 validation/testsuite-selfcheck1.c |  10 ++++
 validation/testsuite-selfcheck2.c |  10 ++++
 validation/testsuite-selfcheck3.c |  10 ++++
 10 files changed, 192 insertions(+), 20 deletions(-)
 create mode 100644 validation/enum-mismatch.c
 create mode 100644 validation/testsuite-selfcheck1.c
 create mode 100644 validation/testsuite-selfcheck2.c
 create mode 100644 validation/testsuite-selfcheck3.c

-- 
2.11.0

[PATCH v2 01/14] testsuite: give a proper name to the 'binary-constant' test

[Luc Van Oostenryck]

The template for this test was visibly copied from another one
and totally irrelevant to what is tested.

Give it another name, briefly describing the subject of the test.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 validation/binary-constant.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/validation/binary-constant.c b/validation/binary-constant.c
index a589403a9..c4ae04546 100644
--- a/validation/binary-constant.c
+++ b/validation/binary-constant.c
@@ -3,5 +3,5 @@ extern int x;
 int x = 0b11;
 
 /*
- * check-name: inline compound literals
+ * check-name: binary constant
  */
-- 
2.11.0

[PATCH v2 02/14] testsuite: make tests known to fail effectively fail

[Luc Van Oostenryck]

These tests must fail because some errors which should
be detected are not.
But the test suite lack to see that because those tests
don't have the check-error-start/end section.

Add such a section (with madeup error message) so that the
testsuite now properly see and report the failure.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 validation/badtype1.c   | 4 ++++
 validation/struct-ns2.c | 6 ++++++
 2 files changed, 10 insertions(+)

diff --git a/validation/badtype1.c b/validation/badtype1.c
index ced7f9f16..b15cb50e8 100644
--- a/validation/badtype1.c
+++ b/validation/badtype1.c
@@ -3,4 +3,8 @@ static void foo(enum bar baz);
 /*
  * check-name: enum not in scope
  * check-known-to-fail
+ *
+ * check-error-start
+badtype1.c:1:22: warning: bad scope for 'enum bar'
+ * check-error-end
  */
diff --git a/validation/struct-ns2.c b/validation/struct-ns2.c
index 4dd2c3bf0..c5afbb71a 100644
--- a/validation/struct-ns2.c
+++ b/validation/struct-ns2.c
@@ -16,4 +16,10 @@ h (void)
 /*
  * check-name: struct not in scope
  * check-known-to-fail
+ *
+ * check-error-start
+struct-ns2.c:2:11: warning: bad scope for 'struct Bar'
+struct-ns2.c:12:14: error: incomplete type/unknown size for 'y'
+struct-ns2.c:13:5: error: using member 'i' in incomplete 'struct Bar'
+ * check-error-end
  */
-- 
2.11.0

[PATCH v2 03/14] testsuite: simplify the ioc-typecheck case

[Luc Van Oostenryck]

The test was relatively convoluted and it wasn't very clear what
was the problem and waht was tested.

The real problem is simply a problem about a non-constant
expression in an array designator.

Simplify the test to clearly show that.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 validation/ioc-typecheck.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/validation/ioc-typecheck.c b/validation/ioc-typecheck.c
index aa060f7be..34b37d310 100644
--- a/validation/ioc-typecheck.c
+++ b/validation/ioc-typecheck.c
@@ -1,16 +1,10 @@
 extern unsigned int __invalid_size_argument_for_IOC;
-#define _IOC_TYPECHECK(t) \
-                ((sizeof(t) == sizeof(t[1]) && \
-                    sizeof(t) < (1 << 14)) ? \
-                   sizeof(t) : __invalid_size_argument_for_IOC)
-
-#define TEST_IOCTL (50 | (_IOC_TYPECHECK(unsigned) << 8))

[PATCH v2 04/14] testsuite: add a simple test for -Wenum-mismatch

[Luc Van Oostenryck]

A nice feature
No tests existed for it
Create one now.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 validation/enum-mismatch.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)
 create mode 100644 validation/enum-mismatch.c

diff --git a/validation/enum-mismatch.c b/validation/enum-mismatch.c
new file mode 100644
index 000000000..9a929d24c
--- /dev/null
+++ b/validation/enum-mismatch.c
@@ -0,0 +1,19 @@
+enum ea { A = 0, };
+enum eb { B = 1, };
+
+
+static enum eb foo(enum ea a)
+{
+	return a;
+}
+
+/*
+ * check-name: enum-mismatch
+ * check-command: sparse -Wenum-mismatch $file
+ *
+ * check-error-start
+enum-mismatch.c:7:16: warning: mixing different enum types
+enum-mismatch.c:7:16:     int enum ea  versus
+enum-mismatch.c:7:16:     int enum eb 
+ * check-error-end
+ */
-- 
2.11.0

[PATCH v2 05/14] testsuite: add tag to ignore the output/error

[Luc Van Oostenryck]

Currently the test suite always check the exit value and the output
of the command used for the test. This is fine and allow use to catch
the most common situations:
- failure or crash (via the exit value)
- (un)expected output (like when testing the result of the preprocessor)
- (un)expected errors & warnings (like when testing sparse's warnings)

But sometimes, we're not interested in the output or the output (as is)
is simply not meaningful for the test or can't be compared textually
to some reference.

This patch add two new tags (check-output-ignore & check-error-ignore),
telling to test suite to ignore the content of stdout or stderr when
testing this file.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 Documentation/test-suite | 6 ++++++
 validation/test-suite    | 2 ++
 2 files changed, 8 insertions(+)

diff --git a/Documentation/test-suite b/Documentation/test-suite
index 6c4f24f65..6936feeb1 100644
--- a/Documentation/test-suite
+++ b/Documentation/test-suite
@@ -29,6 +29,12 @@ check-output-start / check-output-end (optional)
 	The expected output (stdout and stderr) of check-command lies between
 	those two tags. It defaults to no output.
 
+check-output-ignore / check-error-ignore (optional)
+	Don't check the expected output (stdout or stderr) of check-command
+	(usefull when this output is not comparable or if you're only interested
+	in the exit value).
+	By default this check is done.
+
 check-known-to-fail (optional)
 	Mark the test as being known to fail.
 
diff --git a/validation/test-suite b/validation/test-suite
index df5a7c60d..0d874e075 100755
--- a/validation/test-suite
+++ b/validation/test-suite
@@ -146,6 +146,8 @@ do_test()
 	actual_exit_value=$?
 
 	for stream in output error; do
+		grep -s -q "check-$stream-ignore" $file && continue
+
 		diff -u "$file".$stream.expected "$file".$stream.got > "$file".$stream.diff
 		if [ "$?" -ne "0" ]; then
 			error "actual $stream text does not match expected $stream text."
-- 
2.11.0

[PATCH v2 06/14] testsuite: report as error tests known to fail but which succeed

[Luc Van Oostenryck]

Such situation may simply show that what was tested is now fixed
and that it's juste the test annotation which need to be adapted,
but can be a sign that something else is broken.

Reporting the exact result (failure/success, known-to-fail/expect-to-succeed)
make the testsuite more useful and allow to use more efficiently
git-bisect or other automated testing tools.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 validation/test-suite | 29 +++++++++++++++++++++--------
 1 file changed, 21 insertions(+), 8 deletions(-)

diff --git a/validation/test-suite b/validation/test-suite
index 0d874e075..8692bfe98 100755
--- a/validation/test-suite
+++ b/validation/test-suite
@@ -11,6 +11,10 @@ if [ ! -x "$default_path/sparse-llvm" ]; then
 	disabled_cmds="sparsec sparsei sparse-llvm"
 fi
 
+# flags:
+#	- some tests gave an unexpected result
+failed=0
+
 # counts:
 #	- tests that have not been converted to test-suite format
 #	- tests that are disabled
@@ -163,18 +167,27 @@ do_test()
 		test_failed=1
 	fi
 
-	if [ "$test_failed" -eq "1" ]; then
-		ko_tests=`expr $ko_tests + 1`
-		get_tag "check-known-to-fail" $file
-		if [ "$?" -eq "0" ]; then
+	get_tag "check-known-to-fail" $file
+	must_fail=`expr "$?" = 0`
+	known_ko_tests=`expr $known_ko_tests + $must_fail`
+
+	[ "$test_failed" -eq "$must_fail" ] || failed=1
+
+	if [ "$must_fail" -eq "1" ]; then
+		if [ "$test_failed" -eq "1" ]; then
 			echo "info: test '$file' is known to fail"
-			known_ko_tests=`expr $known_ko_tests + 1`
+		else
+			echo "info: test '$file' is known to fail but succeed!"
+			test_failed=1
 		fi
-		return 1
+	fi
+
+	if [ "$test_failed" -eq "1" ]; then
+		ko_tests=`expr $ko_tests + 1`
 	else
 		ok_tests=`expr $ok_tests + 1`
-		return 0
 	fi
+	return $test_failed
 }
 
 do_test_suite()
@@ -273,5 +286,5 @@ case "$1" in
 		;;
 esac
 
-exit 0
+exit $failed
 
-- 
2.11.0

[PATCH v2 07/14] allow to launch the test suite from the project root dir

[Luc Van Oostenryck]

The test suite must be run from the validation/ dir
which is sometimes slightly annoying.

Change that by letting the script to first do a cd to
the validation dir.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 validation/test-suite | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/validation/test-suite b/validation/test-suite
index 8692bfe98..b6ec5655e 100755
--- a/validation/test-suite
+++ b/validation/test-suite
@@ -2,6 +2,8 @@
 
 #set -x
 
+cd $(dirname "$0")
+
 default_path=".."
 default_cmd="sparse \$file"
 tests_list=`find . -name '*.c' | sed -e 's#^\./\(.*\)#\1#' | sort`
-- 
2.11.0

[PATCH v2 08/14] testsuite: check patterns presence or absence in output

[Luc Van Oostenryck]

Currently the test suite always check the exit value and the output
of the command used for the test. This is fine and allow use to catch
the most common situations:
- failure or crash (via the exit value)
- (un)expected output (like when testing the result of the preprocessor)
- (un)expected errors & warnings (like when testing sparse's warnings)

But sometimes, we're not interested in the output as such because it
can't be compared textually to some reference. This occurs systematically
when testing the output of test-linearize or test-unssa which emits
labels names which are in fact pointer values and which exact output
is very sensitive to any change in processing order, optimizations, ...
But useful tests can be easily made by just checking for the presence
or absence of some identifiers, or more generally some patterns.

This patch allow to do that by adding support for two  new tags
(check-output-contains & check-output-excludes), telling to test suite
to verifiy that the given patterns are effectively present ot absent
from the output of the tested file.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 Documentation/test-suite | 10 ++++++++++
 validation/test-suite    | 52 ++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 62 insertions(+)

diff --git a/Documentation/test-suite b/Documentation/test-suite
index 6936feeb1..a0f205f43 100644
--- a/Documentation/test-suite
+++ b/Documentation/test-suite
@@ -38,6 +38,16 @@ check-output-ignore / check-error-ignore (optional)
 check-known-to-fail (optional)
 	Mark the test as being known to fail.
 
+check-output-contains: <pattern> (optional)
+	Check that the output (stdout) contains the given pattern.
+	Several such tags can be given, in which case the output
+	must contains all the patterns.
+
+check-output-excludes: <pattern> (optional)
+	Similar than the above one, but with opposite logic.
+	Check that the output (stdout) doesn't contain the given pattern.
+	Several such tags can be given, in which case the output
+	must contains none of the patterns.
 
 	Using test-suite
 	~~~~~~~~~~~~~~~~
diff --git a/validation/test-suite b/validation/test-suite
index b6ec5655e..e5317109d 100755
--- a/validation/test-suite
+++ b/validation/test-suite
@@ -53,6 +53,46 @@ get_tag()
 	return $?
 }
 
+##
+# helper for has_(each|none)_patterns()
+has_patterns()
+{
+	ifile="$1"
+	patt="$2"
+	ofile="$3"
+	cmp="$4"
+	grep "$patt:" "$ifile" | \
+	sed -e "s/^.*$patt: *\(.*\)$/\1/" | \
+	while read val; do
+		grep -s -q "$val" "$ofile"
+		if [ "$?" $cmp 0 ]; then
+			return 1
+		fi
+	done
+
+	return $?
+}
+
+##
+# has_each_patterns(ifile tag ofile) - does ofile contains some
+#                        of the patterns given by ifile's tags?
+#
+# returns 0 if all present, 1 otherwise
+has_each_patterns()
+{
+	has_patterns "$1" "$2" "$3" -ne
+}
+
+##
+# has_none_patterns(ifile tag ofile) - does ofile contains some
+#                        of the patterns given by ifile's tags?
+#
+# returns 1 if any present, 0 otherwise
+has_none_patterns()
+{
+	has_patterns "$1" "$2" "$3" -eq
+}
+
 ##
 # verbose(string) - prints string if we are in verbose mode
 verbose()
@@ -169,6 +209,18 @@ do_test()
 		test_failed=1
 	fi
 
+	# verify the 'check-output-contains/excludes' tags
+	has_each_patterns "$file" 'check-output-contains' $file.output.got
+	if [ "$?" -ne "0" ]; then
+		error "Actual output doesn't contain some of the expected patterns."
+		test_failed=1
+	fi
+	has_none_patterns "$file" 'check-output-excludes' $file.output.got
+	if [ "$?" -ne "0" ]; then
+		error "Actual output contains some patterns which are not expected."
+		test_failed=1
+	fi
+
 	get_tag "check-known-to-fail" $file
 	must_fail=`expr "$?" = 0`
 	known_ko_tests=`expr $known_ko_tests + $must_fail`
-- 
2.11.0

[PATCH v2 09/14] testsuite: add some selfchecking

[Luc Van Oostenryck]

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 validation/testsuite-selfcheck1.c | 10 ++++++++++
 validation/testsuite-selfcheck2.c | 10 ++++++++++
 validation/testsuite-selfcheck3.c | 10 ++++++++++
 3 files changed, 30 insertions(+)
 create mode 100644 validation/testsuite-selfcheck1.c
 create mode 100644 validation/testsuite-selfcheck2.c
 create mode 100644 validation/testsuite-selfcheck3.c

diff --git a/validation/testsuite-selfcheck1.c b/validation/testsuite-selfcheck1.c
new file mode 100644
index 000000000..d927f9961
--- /dev/null
+++ b/validation/testsuite-selfcheck1.c
@@ -0,0 +1,10 @@
+good
+
+/*
+ * check-name: selfcheck1
+ * check-command: sparse -E $file
+ * check-output-ignore
+ *
+ * check-output-contains: good
+ * check-output-excludes: evil
+ */
diff --git a/validation/testsuite-selfcheck2.c b/validation/testsuite-selfcheck2.c
new file mode 100644
index 000000000..5309e32f3
--- /dev/null
+++ b/validation/testsuite-selfcheck2.c
@@ -0,0 +1,10 @@
+evil
+
+/*
+ * check-name: selfcheck2
+ * check-command: sparse -E $file
+ * check-output-ignore
+ * check-known-to-fail
+ *
+ * check-output-contains: good
+ */
diff --git a/validation/testsuite-selfcheck3.c b/validation/testsuite-selfcheck3.c
new file mode 100644
index 000000000..6d834e68d
--- /dev/null
+++ b/validation/testsuite-selfcheck3.c
@@ -0,0 +1,10 @@
+evil
+
+/*
+ * check-name: selfcheck3
+ * check-command: sparse -E $file
+ * check-output-ignore
+ * check-known-to-fail
+ *
+ * check-output-excludes: evil
+ */
-- 
2.11.0

[PATCH v2 10/14] testsuite: check the nbr of times a pattern should be present

[Luc Van Oostenryck]

Complement the 'check-output-contains/excludes' tags to also
be able to specify the number of times a given pattern should
occurs in the output.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 Documentation/test-suite |  4 ++++
 validation/test-suite    | 28 ++++++++++++++++++++++++++++
 2 files changed, 32 insertions(+)

diff --git a/Documentation/test-suite b/Documentation/test-suite
index a0f205f43..2e786bbf3 100644
--- a/Documentation/test-suite
+++ b/Documentation/test-suite
@@ -49,6 +49,10 @@ check-output-excludes: <pattern> (optional)
 	Several such tags can be given, in which case the output
 	must contains none of the patterns.
 
+check-output-pattern-<nbr>-times: <pattern> (optional)
+	Similar than the contains/excludes her above, but with full control
+	of the number of times the pattern should occurs in the output.
+
 	Using test-suite
 	~~~~~~~~~~~~~~~~
 
diff --git a/validation/test-suite b/validation/test-suite
index e5317109d..c14a4c5ab 100755
--- a/validation/test-suite
+++ b/validation/test-suite
@@ -93,6 +93,27 @@ has_none_patterns()
 	has_patterns "$1" "$2" "$3" -eq
 }
 
+##
+# nbr_patterns(ifile tag ofile) - does ofile contains the
+#                        the patterns given by ifile's tags
+#                        the right number of time?
+nbr_patterns()
+{
+	ifile="$1"
+	patt="$2"
+	ofile="$3"
+	grep "$patt-[0-9][0-9]*-times:" "$ifile" | \
+	sed -e "s/^.*$patt-\([0-9][0-9]*\)-times: *\(.*\)/\1 \2/" | \
+	while read nbr pat; do
+		n=$(grep -s "$pat" "$ofile" | wc -l)
+		if [ "$n" -ne "$nbr" ]; then
+			return 1
+		fi
+	done
+
+	return $?
+}
+
 ##
 # verbose(string) - prints string if we are in verbose mode
 verbose()
@@ -221,6 +242,13 @@ do_test()
 		test_failed=1
 	fi
 
+	# verify the 'check-output-pattern-X-times' tags
+	nbr_patterns "$file" 'check-output-pattern' $file.output.got
+	if [ "$?" -ne "0" ]; then
+		error "Actual output doesn't contain the pattern the expected number."
+		test_failed=1
+	fi
+
 	get_tag "check-known-to-fail" $file
 	must_fail=`expr "$?" = 0`
 	known_ko_tests=`expr $known_ko_tests + $must_fail`
-- 
2.11.0

[PATCH v2 11/14] testsuite: use 'error' instead of 'info' for successful tests known to fail

[Luc Van Oostenryck]

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 validation/test-suite | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/validation/test-suite b/validation/test-suite
index c14a4c5ab..852904a4d 100755
--- a/validation/test-suite
+++ b/validation/test-suite
@@ -259,7 +259,7 @@ do_test()
 		if [ "$test_failed" -eq "1" ]; then
 			echo "info: test '$file' is known to fail"
 		else
-			echo "info: test '$file' is known to fail but succeed!"
+			echo "error: test '$file' is known to fail but succeed!"
 			test_failed=1
 		fi
 	fi
-- 
2.11.0

[PATCH v2 12/14] testsuite: get 'check-known-to-fail' earlier

[Luc Van Oostenryck]

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 validation/test-suite | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/validation/test-suite b/validation/test-suite
index 852904a4d..fdb7cbc26 100755
--- a/validation/test-suite
+++ b/validation/test-suite
@@ -212,6 +212,10 @@ do_test()
 	$cmd 1> $file.output.got 2> $file.error.got
 	actual_exit_value=$?
 
+	get_tag "check-known-to-fail" $file
+	must_fail=`expr "$?" = 0`
+	known_ko_tests=`expr $known_ko_tests + $must_fail`
+
 	for stream in output error; do
 		grep -s -q "check-$stream-ignore" $file && continue
 
@@ -249,10 +253,6 @@ do_test()
 		test_failed=1
 	fi
 
-	get_tag "check-known-to-fail" $file
-	must_fail=`expr "$?" = 0`
-	known_ko_tests=`expr $known_ko_tests + $must_fail`

[PATCH v2 13/14] testsuite: allow quieter error reporting

[Luc Van Oostenryck]

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 validation/test-suite | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/validation/test-suite b/validation/test-suite
index fdb7cbc26..47015c61f 100755
--- a/validation/test-suite
+++ b/validation/test-suite
@@ -126,7 +126,7 @@ verbose()
 # error(string[, die]) - prints an error and exits with value die if given
 error()
 {
-	echo "error: $1"
+	[ "$quiet" -ne 1 ] && echo "error: $1"
 	[ -n "$2" ] && exit $2
 	return 0
 }
@@ -223,7 +223,7 @@ do_test()
 		if [ "$?" -ne "0" ]; then
 			error "actual $stream text does not match expected $stream text."
 			error  "see $file.$stream.* for further investigation."
-			cat "$file".$stream.diff
+			[ $quiet -ne 1 ] && cat "$file".$stream.diff
 			test_failed=1
 		fi
 	done
-- 
2.11.0

[PATCH v2 14/14] testsuite: quieter error reporting for 'known-to-fail'

[Luc Van Oostenryck]

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 validation/test-suite | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/validation/test-suite b/validation/test-suite
index 47015c61f..a5f5e25e2 100755
--- a/validation/test-suite
+++ b/validation/test-suite
@@ -214,6 +214,8 @@ do_test()
 
 	get_tag "check-known-to-fail" $file
 	must_fail=`expr "$?" = 0`
+	quiet=0
+	[ $must_fail -eq 1 ] && [ $V -eq 0 ] && quiet=1
 	known_ko_tests=`expr $known_ko_tests + $must_fail`
 
 	for stream in output error; do
-- 
2.11.0

Re: [PATCH v2 0/8] fix uses of killed instructions

[Christopher Li]

On Mon, Feb 13, 2017 at 7:25 AM, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
> Sigh. I was a bit short on time when checking this and the diff I looked
> at was somehow misleading but in fact the functionality is not at all equivalent
> although all the tests did the expected result.
>
> I'm resending a respin of the two concerned series.

Not problem. I will re-apply your new series.

Chris

Re: [PATCH v2 00/14] testsuite improvements

[Christopher Li]

On Mon, Feb 13, 2017 at 7:28 AM, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
> This serie aimes to improve the test suite in two ways:
> 1) make effective use of the 'known-to-fail' tag
> 2) check the presence or absence of some patterns in the output file

Patches refreshed on sparse-next.

Please take a look.

Chris

Re: [PATCH v2 0/8] fix uses of killed instructions

[Luc Van Oostenryck]

On Mon, Feb 13, 2017 at 12:38 AM, Christopher Li <sparse@chrisli.org> wrote:
> On Mon, Feb 13, 2017 at 7:25 AM, Luc Van Oostenryck
> <luc.vanoostenryck@gmail.com> wrote:
>> Sigh. I was a bit short on time when checking this and the diff I looked
>> at was somehow misleading but in fact the functionality is not at all equivalent
>> although all the tests did the expected result.
>>
>> I'm resending a respin of the two concerned series.
>
> Not problem. I will re-apply your new series.

Thanks. It's really OK now, it passes all the tests *and* is able to
detect errors now.

Luc