[PATCH 0/3] fix discarded label statement

[PATCH 0/3] fix discarded label statement

[Luc Van Oostenryck]

This series fixes a bug occurring when code contains an unsed statement.
In this case, not only the label is ignored, but the following statement
is also erroneously discarded together with the label.

This can be easily verified by inspecting the output of test-linearize
but can't be tested directly with the current test suite.
So patches 1 & 2 contain two small improvements to the test suite that
allow to test this sort of thing directly, the fix itself and its test
are in patch 3.

Luc Van Oostenryck (3):
  testsuite: add tag to ignore the output/error
  testsuite: check patterns presence or absence in output
  fix discarded label statement

 Documentation/test-suite               | 16 ++++++++++
 linearize.c                            |  2 +-
 validation/discarded-label-statement.c | 24 +++++++++++++++
 validation/test-suite                  | 54 ++++++++++++++++++++++++++++++++++
 4 files changed, 95 insertions(+), 1 deletion(-)
 create mode 100644 validation/discarded-label-statement.c

-- 
2.10.1

[PATCH 1/3] 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 6c4f24f6..6936feeb 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 df5a7c60..0d874e07 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.10.1

Re: [PATCH 1/3] testsuite: add tag to ignore the output/error

[Christopher Li]

Applied.

Chris

On Thu, Nov 10, 2016 at 10:45 PM, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
> 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 6c4f24f6..6936feeb 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 df5a7c60..0d874e07 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.10.1
>

[PATCH 2/3] 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 6936feeb..a0f205f4 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 0d874e07..7b2d3066 100755
--- a/validation/test-suite
+++ b/validation/test-suite
@@ -48,6 +48,46 @@ get_tag()
 }
 
 ##
+# 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()
 {
@@ -163,6 +203,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
+
 	if [ "$test_failed" -eq "1" ]; then
 		ko_tests=`expr $ko_tests + 1`
 		get_tag "check-known-to-fail" $file
-- 
2.10.1

[PATCH 3/3] fix discarded label statement

[Luc Van Oostenryck]

When code contains an unused label, it's not needed to create a new
basic block for the code that follow it but that doesn't mean that
the following code is unreachable.

There is currently a bug related to this when processing a label statement
for a label that is never used: not only the label is ignored (and this
no new basic block is created) but the whol statement is ignored.
In other words the statement directly following an unused label is
simply ignored.

The patch fix this by simply moving the code handling the statement out
of the conditional part processing used labels.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 linearize.c                            |  2 +-
 validation/discarded-label-statement.c | 24 ++++++++++++++++++++++++
 2 files changed, 25 insertions(+), 1 deletion(-)
 create mode 100644 validation/discarded-label-statement.c

diff --git a/linearize.c b/linearize.c
index c6ada1e8..4dc3d04d 100644
--- a/linearize.c
+++ b/linearize.c
@@ -2026,8 +2026,8 @@ pseudo_t linearize_statement(struct entrypoint *ep, struct statement *stmt)
 
 		if (label->used) {
 			add_label(ep, label);
-			linearize_statement(ep, stmt->label_statement);
 		}
+		linearize_statement(ep, stmt->label_statement);
 		break;
 	}
 
diff --git a/validation/discarded-label-statement.c b/validation/discarded-label-statement.c
new file mode 100644
index 00000000..b4e58ac6
--- /dev/null
+++ b/validation/discarded-label-statement.c
@@ -0,0 +1,24 @@
+/*
+ * Verify that the statement following an unused label
+ * is not discarded with the label.
+ */
+
+static int bad(int a, int b)
+{
+	int r = 0;
+
+start:
+	r += a;
+	r += b;
+
+	return r;
+}
+
+/*
+ * check-name: discarded-label-statement
+ * check-command: test-linearize $file
+ *
+ * check-output-ignore
+ * check-output-contains: add
+ * check-output-contains: %arg1
+ */
-- 
2.10.1

Re: [PATCH 3/3] fix discarded label statement

[Christopher Li]

On Thu, Nov 10, 2016 at 10:45 PM, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
> The patch fix this by simply moving the code handling the statement out
> of the conditional part processing used labels.

Great catch.

Applied.

Chris