[SPARSE v2 00/28] detect invalid branches

[SPARSE v2 00/28] detect invalid branches

[Luc Van Oostenryck]

It's not allowed to do a goto into an expression statement.
For example, it's not well defined what should happen if such
an expression is not otherwise reachable and/or can be optimized
away. For such situations GCC issues an error, clang doesn't
and produce a valid IR but Spare produce an invalid IR with
branches to unexisting BBs.

The goals of the patches in this series are:
*) to detect such gotos at evaluation time;
*) issue a sensible error message;
*) avoid the linearization of functions with invalid gotos.

The implementation principle behind these is to add a new kind
of scope (label_scope), one for the usual function scope of
labels one for each statement expressions. This new scope,
instead of being used as a real scope for the visibility of
labels, is used to mark where labels are defined and where
they're used. 

Using this label scope as a real scope controling the
visibility of labels was quite appealing and was the initial
drive for this implementation but has the problem of inner
scope shadowing earlier occurence of labels identically
named. This is of course desired for 'normal' symbols but for
labels (which are normally visible in the whole function
and which may be used before being declared/defined)
it has the disadvantage of:
*) inhibiting the detecting of misuses once an inner scope
   is closed
*) allowing several distinct labels with the same name
   in a single function (this can be regarded as a feature
   but __label__ at block scope should be used for this)
*) create diffrences about what is permssble or not between
   sparse and GCC or clang.


Changes since v1:
* move most of the checks from eveluation time to parsing time
* add warnings for unused labels based on code from Linus
* add support for label attributes 'unused'
* add to label-expressions the same kind of checks than
  those done for gotos
* use the correct position in warnings
* lots of small improvements


Luc Van Oostenryck (28):
  misc: fix testcase typeof-safe
  misc: s/fntype/rettype/
  misc: always use the node for current_fn
  bad-goto: add testcase for 'jump inside discarded expression statement'
  bad-goto: add testcases for linearization of invalid labels
  bad-goto: reorganize testcases and add some more
  bad-goto: do not linearize if the IR will be invalid
  bad-goto: reorg test in evaluate_goto_statement()
  bad-goto: simplify testing of undeclared labels
  bad-goto: do not linearize function with undeclared labels
  bad-goto: catch labels with reserved names
  scope: no memset() needed after __alloc_scope()
  scope: move scope opening/ending inside compound_statement()
  scope: extract bind_symbol_with_scope() from bind_symbol()
  scope: __func__ is special
  scope: __label__ is special
  scope: s/{start,end}_symbol_scope/{start,end}_block_scope/
  scope: make function_scope invalid outside functions
  scope: let labels have their own scope
  scope: add is_in_scope()
  scope: give a scope for labels & gotos
  bad-goto: jumping inside a statemet expression is an error
  bad-goto: label expression inside a statement expression is UB
  bad-goto: extract check_label_declaration()
  bad-goto: check declaration of label expressions
  bad-label: check for unused labels
  bad-label: mark labels as used when needed
  bad-label: respect attribute((unused))

 evaluate.c                                    | 36 ++++++--
 expand.c                                      |  2 +-
 expression.c                                  | 12 +--
 linearize.c                                   |  2 +-
 parse.c                                       | 73 +++++++++++++----
 parse.h                                       |  4 +-
 scope.c                                       | 46 +++++++++--
 scope.h                                       | 10 ++-
 symbol.c                                      | 13 ++-
 symbol.h                                      |  7 ++
 validation/__func__-scope.c                   |  8 ++
 .../{asm-goto-lables.c => asm-goto-labels.c}  |  0
 validation/label-asm.c                        |  1 +
 validation/label-attr.c                       |  2 +-
 validation/label-scope-cgoto.c                | 82 +++++++++++++++++++
 validation/label-scope.c                      |  5 +-
 validation/label-scope1.c                     | 42 ++++++++++
 validation/label-scope2.c                     | 31 +++++++
 validation/label-stmt-expr0.c                 | 38 +++++++++
 validation/label-stmt-expr1.c                 | 28 +++++++
 validation/label-stmt-expr2.c                 | 46 +++++++++++
 validation/label-unused.c                     | 29 +++++++
 validation/linear/goto-invalid.c              | 18 ++++
 .../linear/goto-stmt-expr-conditional.c       | 27 ++++++
 .../linear/goto-stmt-expr-short-circuit.c     | 31 +++++++
 validation/linear/label-scope-cgoto.c         | 10 +++
 .../label-stmt-dropped.c}                     |  4 +-
 .../label-stmt-expr0.c}                       |  4 +-
 ...reachable-label0.c => label-unreachable.c} |  3 +-
 validation/typeof-safe.c                      | 26 ++++--
 30 files changed, 576 insertions(+), 64 deletions(-)
 create mode 100644 validation/__func__-scope.c
 rename validation/{asm-goto-lables.c => asm-goto-labels.c} (100%)
 create mode 100644 validation/label-scope-cgoto.c
 create mode 100644 validation/label-scope1.c
 create mode 100644 validation/label-scope2.c
 create mode 100644 validation/label-stmt-expr0.c
 create mode 100644 validation/label-stmt-expr1.c
 create mode 100644 validation/label-stmt-expr2.c
 create mode 100644 validation/label-unused.c
 create mode 100644 validation/linear/goto-invalid.c
 create mode 100644 validation/linear/goto-stmt-expr-conditional.c
 create mode 100644 validation/linear/goto-stmt-expr-short-circuit.c
 create mode 100644 validation/linear/label-scope-cgoto.c
 rename validation/{discarded-label-statement.c => linear/label-stmt-dropped.c} (84%)
 rename validation/{label-expr.c => linear/label-stmt-expr0.c} (75%)
 rename validation/linear/{unreachable-label0.c => label-unreachable.c} (80%)


base-commit: 146e6a63e715e0c3e08aacbcaa79ff8930289297
-- 
2.26.2

[PATCH v1 01/28] misc: fix testcase typeof-safe

[Luc Van Oostenryck]

This testcase was marked as known-to-fail but it was
simply the expected error messages that were missing.

So, slightly reorganize the test a little bit, add the
expected messages and remove the 'known-to-fail' tag.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 validation/typeof-safe.c | 26 ++++++++++++++++++++------
 1 file changed, 20 insertions(+), 6 deletions(-)

diff --git a/validation/typeof-safe.c b/validation/typeof-safe.c
index 614863fba381..508bd39204c5 100644
--- a/validation/typeof-safe.c
+++ b/validation/typeof-safe.c
@@ -2,16 +2,24 @@
 
 static void test_safe(void)
 {
-	int __safe obj, *ptr;
-	typeof(obj) var = obj;
-	typeof(ptr) ptr2 = ptr;
+	int obj;
+	int __safe *ptr;
+
+	int __safe *ptr2 = ptr;
+	typeof(ptr) ptr3 = ptr;
 	typeof(*ptr) var2 = obj;
-	typeof(*ptr) *ptr3 = ptr;
-	typeof(obj) *ptr4 = ptr;
+	int __safe  var3 = obj;
+	int *ptr4 = &obj;
+	int *ptr4 = ptr;		// KO
+
+	typeof(*ptr) sobj;
+	typeof(&sobj) ptr5 = &obj;
+	typeof(&sobj) ptr6 = ptr;	// KO
+
 	obj = obj;
 	ptr = ptr;
-	ptr = &obj;
 	obj = *ptr;
+	ptr = (int __safe *) &obj;
 }
 
 /*
@@ -19,5 +27,11 @@ static void test_safe(void)
  * check-known-to-fail
  *
  * check-error-start
+typeof-safe.c:13:21: warning: incorrect type in initializer (different modifiers)
+typeof-safe.c:13:21:    expected int *ptr4
+typeof-safe.c:13:21:    got int [safe] *ptr
+typeof-safe.c:17:30: warning: incorrect type in initializer (different modifiers)
+typeof-safe.c:17:30:    expected int *ptr6
+typeof-safe.c:17:30:    got int [safe] *ptr
  * check-error-end
  */
-- 
2.26.2

Re: [PATCH v1 01/28] misc: fix testcase typeof-safe

[Ramsay Jones]

On 19/05/2020 01:57, Luc Van Oostenryck wrote:
> This testcase was marked as known-to-fail but it was
> simply the expected error messages that were missing.
> 
> So, slightly reorganize the test a little bit, add the
> expected messages and remove the 'known-to-fail' tag.
> 
> Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
> ---
>  validation/typeof-safe.c | 26 ++++++++++++++++++++------
>  1 file changed, 20 insertions(+), 6 deletions(-)
> 
> diff --git a/validation/typeof-safe.c b/validation/typeof-safe.c
> index 614863fba381..508bd39204c5 100644
> --- a/validation/typeof-safe.c
> +++ b/validation/typeof-safe.c
> @@ -2,16 +2,24 @@
>  
>  static void test_safe(void)
>  {
> -	int __safe obj, *ptr;
> -	typeof(obj) var = obj;
> -	typeof(ptr) ptr2 = ptr;
> +	int obj;
> +	int __safe *ptr;
> +
> +	int __safe *ptr2 = ptr;
> +	typeof(ptr) ptr3 = ptr;
>  	typeof(*ptr) var2 = obj;
> -	typeof(*ptr) *ptr3 = ptr;
> -	typeof(obj) *ptr4 = ptr;
> +	int __safe  var3 = obj;
> +	int *ptr4 = &obj;
> +	int *ptr4 = ptr;		// KO

ptr4 declared twice - and sparse didn't complain?

ATB,
Ramsay Jones

> +
> +	typeof(*ptr) sobj;
> +	typeof(&sobj) ptr5 = &obj;
> +	typeof(&sobj) ptr6 = ptr;	// KO
> +
>  	obj = obj;
>  	ptr = ptr;
> -	ptr = &obj;
>  	obj = *ptr;
> +	ptr = (int __safe *) &obj;
>  }
>  
>  /*
> @@ -19,5 +27,11 @@ static void test_safe(void)
>   * check-known-to-fail
>   *
>   * check-error-start
> +typeof-safe.c:13:21: warning: incorrect type in initializer (different modifiers)
> +typeof-safe.c:13:21:    expected int *ptr4
> +typeof-safe.c:13:21:    got int [safe] *ptr
> +typeof-safe.c:17:30: warning: incorrect type in initializer (different modifiers)
> +typeof-safe.c:17:30:    expected int *ptr6
> +typeof-safe.c:17:30:    got int [safe] *ptr
>   * check-error-end
>   */
> 

Re: [PATCH v1 01/28] misc: fix testcase typeof-safe

[Ramsay Jones]

On 20/05/2020 01:33, Ramsay Jones wrote:
> On 19/05/2020 01:57, Luc Van Oostenryck wrote:
>> This testcase was marked as known-to-fail but it was
>> simply the expected error messages that were missing.
>>
>> So, slightly reorganize the test a little bit, add the
>> expected messages and remove the 'known-to-fail' tag.
>>
>> Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
>> ---
>>  validation/typeof-safe.c | 26 ++++++++++++++++++++------
>>  1 file changed, 20 insertions(+), 6 deletions(-)
>>
>> diff --git a/validation/typeof-safe.c b/validation/typeof-safe.c
>> index 614863fba381..508bd39204c5 100644
>> --- a/validation/typeof-safe.c
>> +++ b/validation/typeof-safe.c
>> @@ -2,16 +2,24 @@
>>  
>>  static void test_safe(void)
>>  {
>> -	int __safe obj, *ptr;
>> -	typeof(obj) var = obj;
>> -	typeof(ptr) ptr2 = ptr;
>> +	int obj;
>> +	int __safe *ptr;
>> +
>> +	int __safe *ptr2 = ptr;
>> +	typeof(ptr) ptr3 = ptr;
>>  	typeof(*ptr) var2 = obj;
>> -	typeof(*ptr) *ptr3 = ptr;
>> -	typeof(obj) *ptr4 = ptr;
>> +	int __safe  var3 = obj;
>> +	int *ptr4 = &obj;
>> +	int *ptr4 = ptr;		// KO
> 
> ptr4 declared twice - and sparse didn't complain?

Heh, I had a slightly different example in the test case
for my '{0}' initializer patch (but involving different
types as well).

I had a quick look at this and tried to use 'git-bisect' to
isolate the change which broke this. However, I couldn't find
a version of sparse that worked correctly! :D (I went all the
way back to v0.4.2 before giving up - several tagged releases
didn't even compile without some fix-ups, including v0.4.2).

Just FYI, this was my test-case:

  $ cat -n test-dup-decl.c
       1	#ifdef WORKS_OK
       2	static int sobj;
       3	static int *sptr4 = &sobj;
       4	static int *sptr4 = 0;
       5	#endif
       6	
       7	static void func(void)
       8	{
       9		int obj, *ptr;
      10		int *ptr4 = &obj;
      11		int *ptr4 = ptr;
      12		int a;
      13		float a;
      14	}
  $ 

  $ gcc -c test-dup-decl.c
  test-dup-decl.c: In function ‘func’:
  test-dup-decl.c:11:7: error: redefinition of ‘ptr4’
    int *ptr4 = ptr;
         ^~~~
  test-dup-decl.c:10:7: note: previous definition of ‘ptr4’ was here
    int *ptr4 = &obj;
         ^~~~
  test-dup-decl.c:13:8: error: conflicting types for ‘a’
    float a;
          ^
  test-dup-decl.c:12:6: note: previous declaration of ‘a’ was here
    int a;
        ^
  $ 

  $ ./sparse test-dup-decl.c
  $ 

ATB,
Ramsay Jones

Re: [PATCH v1 01/28] misc: fix testcase typeof-safe

[Luc Van Oostenryck]

On Wed, May 20, 2020 at 04:34:53PM +0100, Ramsay Jones wrote:
> On 20/05/2020 01:33, Ramsay Jones wrote:
> > On 19/05/2020 01:57, Luc Van Oostenryck wrote:
> >> +	int __safe  var3 = obj;
> >> +	int *ptr4 = &obj;
> >> +	int *ptr4 = ptr;		// KO
> > 
> > ptr4 declared twice - and sparse didn't complain?

Yes, I was surprised by this too.
 
> Heh, I had a slightly different example in the test case
> for my '{0}' initializer patch (but involving different
> types as well).
> 
> I had a quick look at this and tried to use 'git-bisect' to
> isolate the change which broke this. However, I couldn't find
> a version of sparse that worked correctly! :D (I went all the
> way back to v0.4.2 before giving up - several tagged releases
> didn't even compile without some fix-ups, including v0.4.2).

Yes, it's quite annoying when bisecting, but well ...
 
> Just FYI, this was my test-case:
> 
>   $ cat -n test-dup-decl.c
>        1	#ifdef WORKS_OK
>        2	static int sobj;
>        3	static int *sptr4 = &sobj;
>        4	static int *sptr4 = 0;
>        5	#endif
>        6	
>        7	static void func(void)
>        8	{
>        9		int obj, *ptr;
>       10		int *ptr4 = &obj;
>       11		int *ptr4 = ptr;
>       12		int a;
>       13		float a;
>       14	}
>   $ 
> 
>   $ gcc -c test-dup-decl.c
>   test-dup-decl.c: In function ‘func’:
>   test-dup-decl.c:11:7: error: redefinition of ‘ptr4’
>     int *ptr4 = ptr;
>          ^~~~
>   test-dup-decl.c:10:7: note: previous definition of ‘ptr4’ was here
>     int *ptr4 = &obj;
>          ^~~~
>   test-dup-decl.c:13:8: error: conflicting types for ‘a’
>     float a;
>           ^
>   test-dup-decl.c:12:6: note: previous declaration of ‘a’ was here
>     int a;
>         ^
>   $ 
> 
>   $ ./sparse test-dup-decl.c
>   $ 

It seems that sparse detect the redefinition when the symbols are global
but not when they're local.

Thanks for noticing this.
-- Luc

[PATCH v1 02/28] misc: s/fntype/rettype/

[Luc Van Oostenryck]

In evaluate_return_expression(), it's checked if the type of
the return statement match the function return type.

But, the variable used to hold this type is named 'fntype'
which is slightly confusing.

So, rename the variable holding the return type to 'rettype'
and only use 'fntype' for the one hoding the full function type.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 evaluate.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/evaluate.c b/evaluate.c
index b7bb1f52aa91..54cd5fa136e6 100644
--- a/evaluate.c
+++ b/evaluate.c
@@ -3450,13 +3450,14 @@ void evaluate_symbol_list(struct symbol_list *list)
 static struct symbol *evaluate_return_expression(struct statement *stmt)
 {
 	struct expression *expr = stmt->expression;
-	struct symbol *fntype;
+	struct symbol *fntype, *rettype;
 
 	evaluate_expression(expr);
-	fntype = current_fn->ctype.base_type;
-	if (!fntype || fntype == &void_ctype) {
+	fntype = current_fn;
+	rettype = fntype->ctype.base_type;
+	if (!rettype || rettype == &void_ctype) {
 		if (expr && expr->ctype != &void_ctype)
-			expression_error(expr, "return expression in %s function", fntype?"void":"typeless");
+			expression_error(expr, "return expression in %s function", rettype?"void":"typeless");
 		if (expr && Wreturn_void)
 			warning(stmt->pos, "returning void-valued expression");
 		return NULL;
@@ -3468,7 +3469,7 @@ static struct symbol *evaluate_return_expression(struct statement *stmt)
 	}
 	if (!expr->ctype)
 		return NULL;
-	compatible_assignment_types(expr, fntype, &stmt->expression, "return expression");
+	compatible_assignment_types(expr, rettype, &stmt->expression, "return expression");
 	return NULL;
 }
 
-- 
2.26.2

Re: [PATCH v1 02/28] misc: s/fntype/rettype/

[Ramsay Jones]

On 19/05/2020 01:57, Luc Van Oostenryck wrote:
> In evaluate_return_expression(), it's checked if the type of
> the return statement match the function return type.
> 
> But, the variable used to hold this type is named 'fntype'
> which is slightly confusing.
> 
> So, rename the variable holding the return type to 'rettype'
> and only use 'fntype' for the one hoding the full function type.

s/hoding/holding/

> 
> Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
> ---
>  evaluate.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/evaluate.c b/evaluate.c
> index b7bb1f52aa91..54cd5fa136e6 100644
> --- a/evaluate.c
> +++ b/evaluate.c
> @@ -3450,13 +3450,14 @@ void evaluate_symbol_list(struct symbol_list *list)
>  static struct symbol *evaluate_return_expression(struct statement *stmt)
>  {
>  	struct expression *expr = stmt->expression;
> -	struct symbol *fntype;
> +	struct symbol *fntype, *rettype;
>  
>  	evaluate_expression(expr);
> -	fntype = current_fn->ctype.base_type;
> -	if (!fntype || fntype == &void_ctype) {
> +	fntype = current_fn;
> +	rettype = fntype->ctype.base_type;

So, do you need to keep the 'fntype' variable?

ATB,
Ramsay Jones

> +	if (!rettype || rettype == &void_ctype) {
>  		if (expr && expr->ctype != &void_ctype)
> -			expression_error(expr, "return expression in %s function", fntype?"void":"typeless");
> +			expression_error(expr, "return expression in %s function", rettype?"void":"typeless");
>  		if (expr && Wreturn_void)
>  			warning(stmt->pos, "returning void-valued expression");
>  		return NULL;
> @@ -3468,7 +3469,7 @@ static struct symbol *evaluate_return_expression(struct statement *stmt)
>  	}
>  	if (!expr->ctype)
>  		return NULL;
> -	compatible_assignment_types(expr, fntype, &stmt->expression, "return expression");
> +	compatible_assignment_types(expr, rettype, &stmt->expression, "return expression");
>  	return NULL;
>  }
>  
> 

Re: [PATCH v1 02/28] misc: s/fntype/rettype/

[Luc Van Oostenryck]

On Wed, May 20, 2020 at 01:35:13AM +0100, Ramsay Jones wrote:
> On 19/05/2020 01:57, Luc Van Oostenryck wrote:
> > So, rename the variable holding the return type to 'rettype'
> > and only use 'fntype' for the one hoding the full function type.
> 
> s/hoding/holding/

Thanks for this and the ones in the other patches.
-- Luc 

[PATCH v1 03/28] misc: always use the node for current_fn

[Luc Van Oostenryck]

At evaluation time and at expansion time, current_fn is set
to the function's base type (SYM_FN) but at parse time it's
set to its parent type (SYM_NODE).

Since current_fn is used to access the corresponding ident,
it should be set to the node type, not the base.

So, always set current_fn to the node type.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 evaluate.c | 4 ++--
 expand.c   | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/evaluate.c b/evaluate.c
index 54cd5fa136e6..c18ae81df5ad 100644
--- a/evaluate.c
+++ b/evaluate.c
@@ -3422,7 +3422,7 @@ static struct symbol *evaluate_symbol(struct symbol *sym)
 		if (sym->definition && sym->definition != sym)
 			return evaluate_symbol(sym->definition);
 
-		current_fn = base_type;
+		current_fn = sym;
 
 		examine_fn_arguments(base_type);
 		if (!base_type->stmt && base_type->inline_stmt)
@@ -3453,7 +3453,7 @@ static struct symbol *evaluate_return_expression(struct statement *stmt)
 	struct symbol *fntype, *rettype;
 
 	evaluate_expression(expr);
-	fntype = current_fn;
+	fntype = current_fn->ctype.base_type;
 	rettype = fntype->ctype.base_type;
 	if (!rettype || rettype == &void_ctype) {
 		if (expr && expr->ctype != &void_ctype)
diff --git a/expand.c b/expand.c
index e75598781b6c..ab296c730efd 100644
--- a/expand.c
+++ b/expand.c
@@ -918,7 +918,7 @@ static int expand_symbol_call(struct expression *expr, int cost)
 			struct symbol *fn = def->ctype.base_type;
 			struct symbol *curr = current_fn;
 
-			current_fn = fn;
+			current_fn = def;
 			evaluate_statement(expr->statement);
 			current_fn = curr;
 
-- 
2.26.2

Re: [PATCH v1 03/28] misc: always use the node for current_fn

[Ramsay Jones]

On 19/05/2020 01:57, Luc Van Oostenryck wrote:
> At evaluation time and at expansion time, current_fn is set
> to the function's base type (SYM_FN) but at parse time it's
> set to its parent type (SYM_NODE).
> 
> Since current_fn is used to access the corresponding ident,
> it should be set to the node type, not the base.
> 
> So, always set current_fn to the node type.
> 
> Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
> ---
>  evaluate.c | 4 ++--
>  expand.c   | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/evaluate.c b/evaluate.c
> index 54cd5fa136e6..c18ae81df5ad 100644
> --- a/evaluate.c
> +++ b/evaluate.c
> @@ -3422,7 +3422,7 @@ static struct symbol *evaluate_symbol(struct symbol *sym)
>  		if (sym->definition && sym->definition != sym)
>  			return evaluate_symbol(sym->definition);
>  
> -		current_fn = base_type;
> +		current_fn = sym;
>  
>  		examine_fn_arguments(base_type);
>  		if (!base_type->stmt && base_type->inline_stmt)
> @@ -3453,7 +3453,7 @@ static struct symbol *evaluate_return_expression(struct statement *stmt)
>  	struct symbol *fntype, *rettype;
>  
>  	evaluate_expression(expr);
> -	fntype = current_fn;
> +	fntype = current_fn->ctype.base_type;

Ah, OK, question answered!

ATB,
Ramsay Jones

>  	rettype = fntype->ctype.base_type;
>  	if (!rettype || rettype == &void_ctype) {
>  		if (expr && expr->ctype != &void_ctype)
> diff --git a/expand.c b/expand.c
> index e75598781b6c..ab296c730efd 100644
> --- a/expand.c
> +++ b/expand.c
> @@ -918,7 +918,7 @@ static int expand_symbol_call(struct expression *expr, int cost)
>  			struct symbol *fn = def->ctype.base_type;
>  			struct symbol *curr = current_fn;
>  
> -			current_fn = fn;
> +			current_fn = def;
>  			evaluate_statement(expr->statement);
>  			current_fn = curr;
>  
> 

[PATCH v1 04/28] bad-goto: add testcase for 'jump inside discarded expression statement'

[Luc Van Oostenryck]

A goto done into an piece of code discarded at expand or
linearize time will produce an invalid IR.

Add a testcase for it.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 validation/label-stmt-expr1.c           | 29 +++++++++++++++++++++++++
 validation/linear/goto-and-expr-stmt0.c | 28 ++++++++++++++++++++++++
 2 files changed, 57 insertions(+)
 create mode 100644 validation/label-stmt-expr1.c
 create mode 100644 validation/linear/goto-and-expr-stmt0.c

diff --git a/validation/label-stmt-expr1.c b/validation/label-stmt-expr1.c
new file mode 100644
index 000000000000..339217dcb999
--- /dev/null
+++ b/validation/label-stmt-expr1.c
@@ -0,0 +1,29 @@
+static int foo(void)
+{
+	goto l;
+	({
+l:
+		0;
+	});
+}
+
+static void bar(void)
+{
+	({
+l:
+		0;
+	});
+	goto l;
+}
+
+/*
+ * check-name: label-stmt-expr1
+ * check-known-to-fail
+ *
+ * check-error-start
+label-stmt-expr1.c:3:9: error: label 'l' used outside statement expression
+label-stmt-expr1.c:5:1:    label 'l' defined here
+label-stmt-expr1.c:16:9: error: label 'l' used outside statement expression
+label-stmt-expr1.c:13:1:    label 'l' defined here
+ * check-error-end
+ */
diff --git a/validation/linear/goto-and-expr-stmt0.c b/validation/linear/goto-and-expr-stmt0.c
new file mode 100644
index 000000000000..548813531779
--- /dev/null
+++ b/validation/linear/goto-and-expr-stmt0.c
@@ -0,0 +1,28 @@
+int t(void)
+{
+	goto inside;
+	return 1 ? 2 : ({
+inside:
+			return 3;
+			4;
+		    });
+}
+
+void f(int x, int y)
+{
+	1 ? x : ({
+a:
+		 y;
+	});
+	goto a;
+}
+
+/*
+ * check-name: goto-and-expr-stmt0
+ * check-command: test-linearize -Wno-decl $file
+ * check-known-to-fail
+ *
+ * check-output-ignore
+ * check-output-excludes: END
+ * check-error-ignore
+ */
-- 
2.26.2

[PATCH v1 05/28] bad-goto: add testcases for linearization of invalid labels

[Luc Van Oostenryck]

A goto to a reserved or a undeclared label will generate
an IR with a branch to a non-existing BB. Bad.

Add a testcase for these.

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

diff --git a/validation/linear/invalid-labels0.c b/validation/linear/invalid-labels0.c
new file mode 100644
index 000000000000..ae3bf7283fb8
--- /dev/null
+++ b/validation/linear/invalid-labels0.c
@@ -0,0 +1,19 @@
+static void foo(void)
+{
+	goto return;
+}
+
+void bar(void)
+{
+	goto neverland;
+}
+
+/*
+ * check-name: invalid-labels0
+ * check-command: test-linearize -Wno-decl $file
+ * check-known-to-fail
+ *
+ * check-output-ignore
+ * check-output-excludes: END
+ * check-error-ignore
+ */
-- 
2.26.2

[PATCH v1 06/28] bad-goto: reorganize testcases and add some more

[Luc Van Oostenryck]

Reorganize the testcases related to the 'scope' of labels
and add a few new ones.

Also, some related testcases have some unreported errors other
than the features being tested. This is a problem since such
tescases can still fail after the feature being tested is fixed
or implemented. So, fix these testcases or split them so that
they each test a unique feature.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 validation/__func__-scope.c                   |  8 ++
 .../{asm-goto-lables.c => asm-goto-labels.c}  |  0
 validation/label-asm.c                        |  1 +
 validation/label-attr.c                       |  2 +-
 validation/label-scope-cgoto.c                | 83 +++++++++++++++++++
 validation/label-scope.c                      |  5 +-
 validation/label-scope1.c                     | 42 ++++++++++
 validation/label-scope2.c                     | 32 +++++++
 validation/label-stmt-expr0.c                 | 39 +++++++++
 validation/label-stmt-expr2.c                 | 47 +++++++++++
 validation/label-unused.c                     | 24 ++++++
 .../{invalid-labels0.c => goto-invalid.c}     |  4 +-
 ...r-stmt0.c => goto-stmt-expr-conditional.c} |  4 +-
 .../linear/goto-stmt-expr-short-circuit.c     | 32 +++++++
 validation/linear/label-scope-cgoto.c         | 11 +++
 .../label-stmt-dropped.c}                     |  4 +-
 .../label-stmt-expr0.c}                       |  4 +-
 ...reachable-label0.c => label-unreachable.c} |  3 +-
 18 files changed, 332 insertions(+), 13 deletions(-)
 create mode 100644 validation/__func__-scope.c
 rename validation/{asm-goto-lables.c => asm-goto-labels.c} (100%)
 create mode 100644 validation/label-scope-cgoto.c
 create mode 100644 validation/label-scope1.c
 create mode 100644 validation/label-scope2.c
 create mode 100644 validation/label-stmt-expr0.c
 create mode 100644 validation/label-stmt-expr2.c
 create mode 100644 validation/label-unused.c
 rename validation/linear/{invalid-labels0.c => goto-invalid.c} (88%)
 rename validation/linear/{goto-and-expr-stmt0.c => goto-stmt-expr-conditional.c} (87%)
 create mode 100644 validation/linear/goto-stmt-expr-short-circuit.c
 create mode 100644 validation/linear/label-scope-cgoto.c
 rename validation/{discarded-label-statement.c => linear/label-stmt-dropped.c} (84%)
 rename validation/{label-expr.c => linear/label-stmt-expr0.c} (75%)
 rename validation/linear/{unreachable-label0.c => label-unreachable.c} (80%)

diff --git a/validation/__func__-scope.c b/validation/__func__-scope.c
new file mode 100644
index 000000000000..508a8b91d407
--- /dev/null
+++ b/validation/__func__-scope.c
@@ -0,0 +1,8 @@
+static void foo(void)
+{
+	const char *name = ({ __func__; });
+}
+/*
+ * check-name: __func__'s scope
+ * check-command: sparse -Wall $file
+ */
diff --git a/validation/asm-goto-lables.c b/validation/asm-goto-labels.c
similarity index 100%
rename from validation/asm-goto-lables.c
rename to validation/asm-goto-labels.c
diff --git a/validation/label-asm.c b/validation/label-asm.c
index 411020ac361b..b58d1e52e5fd 100644
--- a/validation/label-asm.c
+++ b/validation/label-asm.c
@@ -3,6 +3,7 @@
 static void f(void)
 {
 	barrier();
+	goto l;
 l:
 	barrier();
 }
diff --git a/validation/label-attr.c b/validation/label-attr.c
index a82d7bc98b39..81c4ac3c6aef 100644
--- a/validation/label-attr.c
+++ b/validation/label-attr.c
@@ -1,6 +1,6 @@
 static int foo(void)
 {
-       return 0;
+       goto rtattr_failure;
 rtattr_failure: __attribute__ ((unused))
        return -1;
 }
diff --git a/validation/label-scope-cgoto.c b/validation/label-scope-cgoto.c
new file mode 100644
index 000000000000..c5d278d3d654
--- /dev/null
+++ b/validation/label-scope-cgoto.c
@@ -0,0 +1,83 @@
+void foo(void)
+{
+	void *p = &&l;
+	{
+l:		 ;
+	}
+	goto *p;			// OK
+}
+
+void bar(void)
+{
+	void *p = &&l;			// KO: 'jump' inside
+	({
+l:		 1;
+	});
+	goto *p;
+}
+
+void baz(void)
+{
+	void *p = &&l;			// KO: 'jump' inside
+	0 ? 1 : ({
+l:		 1;
+		 });
+	goto *p;
+}
+
+void qux(void)
+{
+	void *p = &&l;			// KO: 'jump' inside + removed
+	1 ? 1 : ({
+l:		 1;
+		 });
+	goto *p;
+}
+
+void quz(void)
+{
+	void *p;
+	p = &&l;			// KO: undeclared
+	goto *p;
+}
+
+void qxu(void)
+{
+	void *p;
+	({
+l:		1;
+	 });
+	p = &&l;			// KO: 'jump' inside
+	goto *p;
+}
+
+void qzu(void)
+{
+	void *p;
+	1 ? 1 : ({
+l:		 1;
+		 });
+	p = &&l;			// KO: 'jump' inside + removed
+	goto *p;
+}
+
+
+/*
+ * check-name: label-scope-cgoto
+ * check-command: sparse -Wno-decl $file
+ * check-known-to-fail
+ *
+ * check-error-start
+label-scope-cgoto.c:12:19: error: label 'l' used outside statement expression
+label-scope-cgoto.c:14:1:    label 'l' defined here
+label-scope-cgoto.c:21:19: error: label 'l' used outside statement expression
+label-scope-cgoto.c:23:1:    label 'l' defined here
+label-scope-cgoto.c:30:19: error: label 'l' used outside statement expression
+label-scope-cgoto.c:32:1:    label 'l' defined here
+label-scope-cgoto.c:50:13: error: label 'l' used outside statement expression
+label-scope-cgoto.c:48:1:    label 'l' defined here
+label-scope-cgoto.c:60:13: error: label 'l' used outside statement expression
+label-scope-cgoto.c:58:1:    label 'l' defined here
+label-scope-cgoto.c:40:13: error: label 'l' was not declared
+ * check-error-end
+ */
diff --git a/validation/label-scope.c b/validation/label-scope.c
index 7af3d916c347..0ffaaf4a4ccc 100644
--- a/validation/label-scope.c
+++ b/validation/label-scope.c
@@ -3,10 +3,7 @@ static int f(int n)
 	__label__ n;
 n:	return n;
 }
-static int g(int n)
-{
-n:	return n;
-}
+
 /*
  * check-name: __label__ scope
  */
diff --git a/validation/label-scope1.c b/validation/label-scope1.c
new file mode 100644
index 000000000000..f2b1ae9b158a
--- /dev/null
+++ b/validation/label-scope1.c
@@ -0,0 +1,42 @@
+static void ok_top(void)
+{
+	__label__ l;
+l:
+	goto l;
+}
+
+static void ko_undecl(void)
+{
+	__label__ l;
+	goto l;				// KO: undeclared
+}
+
+static void ok_local(void)
+{
+l:
+	{
+		__label__ l;
+l:
+		goto l;
+	}
+goto l;
+}
+
+static void ko_scope(void)
+{
+	{
+		__label__ l;
+l:
+		goto l;
+	}
+goto l;					// KO: undeclared
+}
+
+/*
+ * check-name: label-scope1
+ *
+ * check-error-start
+label-scope1.c:11:9: error: label 'l' was not declared
+label-scope1.c:32:1: error: label 'l' was not declared
+ * check-error-end
+ */
diff --git a/validation/label-scope2.c b/validation/label-scope2.c
new file mode 100644
index 000000000000..8c04ac6525e5
--- /dev/null
+++ b/validation/label-scope2.c
@@ -0,0 +1,32 @@
+static void ok_lvl2(void)
+{
+	__label__ l;
+
+	{
+	l:
+		goto l;
+	}
+}
+
+static void ko_expr2(void)
+{
+	{
+		__label__ a;
+
+		({
+a:
+			 0;
+		});
+		goto a;
+	}
+}
+
+/*
+ * check-name: label-scope2
+ * check-known-to-fail
+ *
+ * check-error-start
+label-scope2.c:20:17: error: label 'a' used outside statement expression
+label-scope2.c:17:1:    label 'a' defined here
+ * check-error-end
+ */
diff --git a/validation/label-stmt-expr0.c b/validation/label-stmt-expr0.c
new file mode 100644
index 000000000000..66a6490241bd
--- /dev/null
+++ b/validation/label-stmt-expr0.c
@@ -0,0 +1,39 @@
+void aft(void)
+{
+	({
+l:		 1;
+	});
+	goto l;				// KO
+}
+
+void bef(void)
+{
+	goto l;				// KO
+	({
+l:		 1;
+	});
+}
+
+void lab(void)
+{
+	__label__ l;
+	({
+l:		 1;
+	});
+	goto l;				// KO
+}
+
+/*
+ * check-name: label-stmt-expr0
+ * check-command: sparse -Wno-decl $file
+ * check-known-to-fail
+ *
+ * check-error-start
+label-stmt-expr0.c:6:9: error: label 'l' used outside statement expression
+label-stmt-expr0.c:4:1:    label 'l' defined here
+label-stmt-expr0.c:11:9: error: label 'l' used outside statement expression
+label-stmt-expr0.c:13:1:    label 'l' defined here
+label-stmt-expr0.c:23:9: error: label 'l' used outside statement expression
+label-stmt-expr0.c:21:1:    label 'l' defined here
+ * check-error-end
+ */
diff --git a/validation/label-stmt-expr2.c b/validation/label-stmt-expr2.c
new file mode 100644
index 000000000000..7a38e3799c55
--- /dev/null
+++ b/validation/label-stmt-expr2.c
@@ -0,0 +1,47 @@
+static int foo(void)
+{
+	goto l;
+	({
+l:
+		0;
+	});
+	goto l;
+}
+
+static void bar(void)
+{
+	goto l;
+	goto l;
+	({
+l:
+		0;
+	});
+}
+
+static void baz(void)
+{
+	({
+l:
+		0;
+	});
+	goto l;
+	goto l;
+}
+
+/*
+ * check-name: label-stmt-expr2
+ * check-known-to-fail
+ *
+ * check-error-start
+label-stmt-expr2.c:3:9: error: label 'l' used outside statement expression
+label-stmt-expr2.c:5:1:    label 'l' defined here
+label-stmt-expr2.c:8:9: error: label 'l' used outside statement expression
+label-stmt-expr2.c:5:1:    label 'l' defined here
+label-stmt-expr2.c:13:9: error: label 'l' used outside statement expression
+label-stmt-expr2.c:16:1:    label 'l' defined here
+label-stmt-expr2.c:27:9: error: label 'l' used outside statement expression
+label-stmt-expr2.c:24:1:    label 'l' defined here
+label-stmt-expr2.c:28:9: error: label 'l' used outside statement expression
+label-stmt-expr2.c:24:1:    label 'l' defined here
+ * check-error-end
+ */
diff --git a/validation/label-unused.c b/validation/label-unused.c
new file mode 100644
index 000000000000..c136c7a8813e
--- /dev/null
+++ b/validation/label-unused.c
@@ -0,0 +1,24 @@
+static void foo(void)
+{
+l:
+	return;
+}
+
+static int bar(void)
+{
+	return  ({
+l:
+		;
+		0;
+	});
+}
+
+/*
+ * check-name: label-unused
+ * check-known-to-fail
+ *
+ * check-error-start
+label-unused.c:3:1: warning: unused label 'l'
+label-unused.c:10:1: warning: unused label 'l'
+ * check-error-end
+ */
diff --git a/validation/linear/invalid-labels0.c b/validation/linear/goto-invalid.c
similarity index 88%
rename from validation/linear/invalid-labels0.c
rename to validation/linear/goto-invalid.c
index ae3bf7283fb8..569d0b0d2db1 100644
--- a/validation/linear/invalid-labels0.c
+++ b/validation/linear/goto-invalid.c
@@ -9,11 +9,11 @@ void bar(void)
 }
 
 /*
- * check-name: invalid-labels0
+ * check-name: goto-invalid
  * check-command: test-linearize -Wno-decl $file
  * check-known-to-fail
  *
+ * check-error-ignore
  * check-output-ignore
  * check-output-excludes: END
- * check-error-ignore
  */
diff --git a/validation/linear/goto-and-expr-stmt0.c b/validation/linear/goto-stmt-expr-conditional.c
similarity index 87%
rename from validation/linear/goto-and-expr-stmt0.c
rename to validation/linear/goto-stmt-expr-conditional.c
index 548813531779..6576052b50ac 100644
--- a/validation/linear/goto-and-expr-stmt0.c
+++ b/validation/linear/goto-stmt-expr-conditional.c
@@ -18,11 +18,11 @@ a:
 }
 
 /*
- * check-name: goto-and-expr-stmt0
+ * check-name: goto-stmt-expr-conditional
  * check-command: test-linearize -Wno-decl $file
  * check-known-to-fail
  *
+ * check-error-ignore
  * check-output-ignore
  * check-output-excludes: END
- * check-error-ignore
  */
diff --git a/validation/linear/goto-stmt-expr-short-circuit.c b/validation/linear/goto-stmt-expr-short-circuit.c
new file mode 100644
index 000000000000..426315e69fbd
--- /dev/null
+++ b/validation/linear/goto-stmt-expr-short-circuit.c
@@ -0,0 +1,32 @@
+int foo(int p)
+{
+	goto inside;
+	if (0 && ({
+inside:
+		return 1;
+		2;
+		}))
+		return 3;
+	return 4;
+}
+
+int bar(int p)
+{
+	if (0 && ({
+inside:
+		return 1;
+		2;
+		}))
+		return 3;
+	goto inside;
+}
+
+/*
+ * check-name: goto-stmt-expr-short-circuit
+ * check-command: test-linearize -Wno-decl $file
+ * check-known-to-fail
+ *
+ * check-error-ignore
+ * check-output-ignore
+ * check-output-excludes: END
+ */
diff --git a/validation/linear/label-scope-cgoto.c b/validation/linear/label-scope-cgoto.c
new file mode 100644
index 000000000000..592f1ce4f664
--- /dev/null
+++ b/validation/linear/label-scope-cgoto.c
@@ -0,0 +1,11 @@
+#include <label-scope-cgoto.c>
+
+/*
+ * check-name: linear/label-scope-cgoto
+ * check-command: test-linearize -Wno-decl -I. $file
+ * check-known-to-fail
+ *
+ * check-error-ignore
+ * check-output-ignore
+ * check-output-excludes: END
+ */
diff --git a/validation/discarded-label-statement.c b/validation/linear/label-stmt-dropped.c
similarity index 84%
rename from validation/discarded-label-statement.c
rename to validation/linear/label-stmt-dropped.c
index b4e58ac64e1d..74e0f2e63aff 100644
--- a/validation/discarded-label-statement.c
+++ b/validation/linear/label-stmt-dropped.c
@@ -11,11 +11,13 @@ start:
 	r += a;
 	r += b;
 
+	if (!r)
+		goto start;
 	return r;
 }
 
 /*
- * check-name: discarded-label-statement
+ * check-name: label-stmt-dropped
  * check-command: test-linearize $file
  *
  * check-output-ignore
diff --git a/validation/label-expr.c b/validation/linear/label-stmt-expr0.c
similarity index 75%
rename from validation/label-expr.c
rename to validation/linear/label-stmt-expr0.c
index e578ed0042e6..ff3c098077d3 100644
--- a/validation/label-expr.c
+++ b/validation/linear/label-stmt-expr0.c
@@ -3,12 +3,12 @@ int foo(void)
 {
 	int r;
 
-	r = ({ label: 1; });
+	r = ({ goto label; label: 1; });
 	return r;
 }
 
 /*
- * check-name: label-expr
+ * check-name: label-stmt-expr0
  * check-command: test-linearize $file
  * check-output-ignore
  *
diff --git a/validation/linear/unreachable-label0.c b/validation/linear/label-unreachable.c
similarity index 80%
rename from validation/linear/unreachable-label0.c
rename to validation/linear/label-unreachable.c
index 695e5cb072d0..a44e121154d0 100644
--- a/validation/linear/unreachable-label0.c
+++ b/validation/linear/label-unreachable.c
@@ -10,9 +10,10 @@ label:
 }
 
 /*
- * check-name: unreachable-label0
+ * check-name: label-unreachable
  * check-command: test-linearize $file
  *
+ * check-error-ignore
  * check-output-ignore
  * check-output-contains: ret\\.
  * check-output-excludes: END
-- 
2.26.2

[PATCH v1 07/28] bad-goto: do not linearize if the IR will be invalid

[Luc Van Oostenryck]

In some error cases, it's not possible to produce a valid &
correct IR for the concerned function. For exemple, if the
AST contains invalid gotos, the CFG will either be invalid
or won't correspond to the erroneous source code.

So, refuse to linearize such functions.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 linearize.c | 2 +-
 symbol.h    | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/linearize.c b/linearize.c
index b040d345d469..4927468183b0 100644
--- a/linearize.c
+++ b/linearize.c
@@ -2480,7 +2480,7 @@ static struct entrypoint *linearize_fn(struct symbol *sym, struct symbol *base_t
 	pseudo_t result;
 	int i;
 
-	if (!stmt)
+	if (!stmt || sym->bogus_linear)
 		return NULL;
 
 	ep = alloc_entrypoint();
diff --git a/symbol.h b/symbol.h
index 7241f13df4e4..50dba78a654a 100644
--- a/symbol.h
+++ b/symbol.h
@@ -171,6 +171,7 @@ struct symbol {
 			unsigned long	offset;
 			int		bit_size;
 			unsigned int	bit_offset:8,
+					bogus_linear:1,
 					variadic:1,
 					initialized:1,
 					examined:1,
-- 
2.26.2

[PATCH v1 08/28] bad-goto: reorg test in evaluate_goto_statement()

[Luc Van Oostenryck]

No functional changes here, only changing the code structure
to prepare more incoming changes.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 evaluate.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/evaluate.c b/evaluate.c
index c18ae81df5ad..4bdd5ed05842 100644
--- a/evaluate.c
+++ b/evaluate.c
@@ -3742,10 +3742,14 @@ static void evaluate_goto_statement(struct statement *stmt)
 {
 	struct symbol *label = stmt->goto_label;
 
-	if (label && !label->stmt && label->ident && !lookup_keyword(label->ident, NS_KEYWORD))
+	if (!label) {
+		// no label associated, may be a computed goto
+		evaluate_expression(stmt->goto_expression);
+		return;
+	}
+	if (!label->stmt && label->ident && !lookup_keyword(label->ident, NS_KEYWORD)) {
 		sparse_error(stmt->pos, "label '%s' was not declared", show_ident(label->ident));

[PATCH v1 08/28] bad-goto: reorg test in evaluate_goto_statement()

[Luc Van Oostenryck]

No functional changes here, only changing the code structure
to prepare more incoming changes.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 evaluate.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/evaluate.c b/evaluate.c
index c18ae81df5ad..4bdd5ed05842 100644
--- a/evaluate.c
+++ b/evaluate.c
@@ -3742,10 +3742,14 @@ static void evaluate_goto_statement(struct statement *stmt)
 {
 	struct symbol *label = stmt->goto_label;
 
-	if (label && !label->stmt && label->ident && !lookup_keyword(label->ident, NS_KEYWORD))
+	if (!label) {
+		// no label associated, may be a computed goto
+		evaluate_expression(stmt->goto_expression);
+		return;
+	}
+	if (!label->stmt && label->ident && !lookup_keyword(label->ident, NS_KEYWORD)) {
 		sparse_error(stmt->pos, "label '%s' was not declared", show_ident(label->ident));
-
-	evaluate_expression(stmt->goto_expression);
+	}
 }
 
 struct symbol *evaluate_statement(struct statement *stmt)
-- 
2.26.2

[PATCH v1 09/28] bad-goto: simplify testing of undeclared labels

[Luc Van Oostenryck]

There is no need to do a lookup: checking if the label's
symbol is in the NS_LABEL namespace and is lacking an
associated statement is enough and much simpler.

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

diff --git a/evaluate.c b/evaluate.c
index 4bdd5ed05842..d4b462274add 100644
--- a/evaluate.c
+++ b/evaluate.c
@@ -3747,7 +3747,8 @@ static void evaluate_goto_statement(struct statement *stmt)
 		evaluate_expression(stmt->goto_expression);
 		return;
 	}
-	if (!label->stmt && label->ident && !lookup_keyword(label->ident, NS_KEYWORD)) {
+
+	if (label->namespace == NS_LABEL && !label->stmt) {
 		sparse_error(stmt->pos, "label '%s' was not declared", show_ident(label->ident));
 	}
 }
-- 
2.26.2

[PATCH v1 10/28] bad-goto: do not linearize function with undeclared labels

[Luc Van Oostenryck]

It's not possible to produce a valid & correct IR if
the function contains a goto to an undeclared label.

So, try to catch these situations and mark the function
as such, the linearization will then simply ignore it.

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

diff --git a/evaluate.c b/evaluate.c
index d4b462274add..c757fc82b204 100644
--- a/evaluate.c
+++ b/evaluate.c
@@ -3750,6 +3750,7 @@ static void evaluate_goto_statement(struct statement *stmt)
 
 	if (label->namespace == NS_LABEL && !label->stmt) {
 		sparse_error(stmt->pos, "label '%s' was not declared", show_ident(label->ident));
+		current_fn->bogus_linear = 1;
 	}
 }
 
-- 
2.26.2

[PATCH v1 11/28] bad-goto: catch labels with reserved names

[Luc Van Oostenryck]

If a reserved name is used as the destination of a goto,
its associated label won't be valid and at linearization
time no BB will can be created for it, resulting in an
invalid IR.

So, catch such gotos at evaluation time and mark the
function to not be linearized.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 evaluate.c                       | 2 ++
 validation/linear/goto-invalid.c | 1 -
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/evaluate.c b/evaluate.c
index c757fc82b204..21d5d761627f 100644
--- a/evaluate.c
+++ b/evaluate.c
@@ -3752,6 +3752,8 @@ static void evaluate_goto_statement(struct statement *stmt)
 		sparse_error(stmt->pos, "label '%s' was not declared", show_ident(label->ident));
 		current_fn->bogus_linear = 1;
 	}
+	if (label->namespace == NS_NONE)
+		current_fn->bogus_linear = 1;
 }
 
 struct symbol *evaluate_statement(struct statement *stmt)
diff --git a/validation/linear/goto-invalid.c b/validation/linear/goto-invalid.c
index 569d0b0d2db1..860b32a41ef9 100644
--- a/validation/linear/goto-invalid.c
+++ b/validation/linear/goto-invalid.c
@@ -11,7 +11,6 @@ void bar(void)
 /*
  * check-name: goto-invalid
  * check-command: test-linearize -Wno-decl $file
- * check-known-to-fail
  *
  * check-error-ignore
  * check-output-ignore
-- 
2.26.2

[PATCH v1 12/28] scope: no memset() needed after __alloc_scope()

[Luc Van Oostenryck]

When starting some scopes, the newly allocated struct is
memset'ed with zero but this is unneeded since the allocator
always returns zeroed memory.

Remove the unneeded call to memset().

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 scope.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/scope.c b/scope.c
index 420c0f5a3f51..0e4fb3b42150 100644
--- a/scope.c
+++ b/scope.c
@@ -68,7 +68,6 @@ void rebind_scope(struct symbol *sym, struct scope *new)
 static void start_scope(struct scope **s)
 {
 	struct scope *scope = __alloc_scope(0);
-	memset(scope, 0, sizeof(*scope));
 	scope->next = *s;
 	*s = scope;
 }
@@ -77,7 +76,6 @@ void start_file_scope(void)
 {
 	struct scope *scope = __alloc_scope(0);
 
-	memset(scope, 0, sizeof(*scope));
 	scope->next = &builtin_scope;
 	file_scope = scope;
 
-- 
2.26.2

[PATCH v1 13/28] scope: move scope opening/ending inside compound_statement()

[Luc Van Oostenryck]

A compound statement starts and ends a block scope, so
it's better to start & end this scope inside the function
parsing the statement: compound_statement.
The only exception is for the body of a function where
the scope also enclose the parameter declaration but that
is fine since the function is special anyway.

Move the calls to start & close the block scope inside
compound_statement() and directly call statement_list()
for the function body.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 expression.c |  2 --
 parse.c      | 13 ++++++-------
 2 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/expression.c b/expression.c
index 5b9bddfe456e..78e577cf10a1 100644
--- a/expression.c
+++ b/expression.c
@@ -71,9 +71,7 @@ struct token *parens_expression(struct token *token, struct expression **expr, c
 		struct statement *stmt = alloc_statement(token->pos, STMT_COMPOUND);
 		*expr = e;
 		e->statement = stmt;
-		start_symbol_scope();
 		token = compound_statement(token->next, stmt);
-		end_symbol_scope();
 		token = expect(token, '}', "at end of statement expression");
 	} else
 		token = parse_expression(token, expr);
diff --git a/parse.c b/parse.c
index 9e7b74f98638..e23c5b64e8be 100644
--- a/parse.c
+++ b/parse.c
@@ -2555,11 +2555,7 @@ static struct token *statement(struct token *token, struct statement **tree)
 	}
 
 	if (match_op(token, '{')) {
-		stmt->type = STMT_COMPOUND;
-		start_symbol_scope();
 		token = compound_statement(token->next, stmt);
-		end_symbol_scope();
-		
 		return expect(token, '}', "at end of compound statement");
 	}
 			
@@ -2666,7 +2662,10 @@ static struct token *parameter_type_list(struct token *token, struct symbol *fn)
 
 struct token *compound_statement(struct token *token, struct statement *stmt)
 {
+	stmt->type = STMT_COMPOUND;
+	start_symbol_scope();
 	token = statement_list(token, &stmt->stmts);
+	end_symbol_scope();
 	return token;
 }
 
@@ -2818,15 +2817,15 @@ static struct token *parse_function_body(struct token *token, struct symbol *dec
 		decl->ctype.modifiers |= MOD_EXTERN;
 
 	stmt = start_function(decl);
-
 	*p = stmt;
+
 	FOR_EACH_PTR (base_type->arguments, arg) {
 		declare_argument(arg, base_type);
 	} END_FOR_EACH_PTR(arg);
 
-	token = compound_statement(token->next, stmt);

[PATCH v1 13/28] scope: move scope opening/ending inside compound_statement()

[Luc Van Oostenryck]

A compound statement starts and ends a block scope, so
it's better to start & end this scope inside the function
parsing the statement: compound_statement.
The only exception is for the body of a function where
the scope also enclose the parameter declaration but that
is fine since the function is special anyway.

Move the calls to start & close the block scope inside
compound_statement() and directly call statement_list()
for the function body.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 expression.c |  2 --
 parse.c      | 13 ++++++-------
 2 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/expression.c b/expression.c
index 5b9bddfe456e..78e577cf10a1 100644
--- a/expression.c
+++ b/expression.c
@@ -71,9 +71,7 @@ struct token *parens_expression(struct token *token, struct expression **expr, c
 		struct statement *stmt = alloc_statement(token->pos, STMT_COMPOUND);
 		*expr = e;
 		e->statement = stmt;
-		start_symbol_scope();
 		token = compound_statement(token->next, stmt);
-		end_symbol_scope();
 		token = expect(token, '}', "at end of statement expression");
 	} else
 		token = parse_expression(token, expr);
diff --git a/parse.c b/parse.c
index 9e7b74f98638..e23c5b64e8be 100644
--- a/parse.c
+++ b/parse.c
@@ -2555,11 +2555,7 @@ static struct token *statement(struct token *token, struct statement **tree)
 	}
 
 	if (match_op(token, '{')) {
-		stmt->type = STMT_COMPOUND;
-		start_symbol_scope();
 		token = compound_statement(token->next, stmt);
-		end_symbol_scope();
-		
 		return expect(token, '}', "at end of compound statement");
 	}
 			
@@ -2666,7 +2662,10 @@ static struct token *parameter_type_list(struct token *token, struct symbol *fn)
 
 struct token *compound_statement(struct token *token, struct statement *stmt)
 {
+	stmt->type = STMT_COMPOUND;
+	start_symbol_scope();
 	token = statement_list(token, &stmt->stmts);
+	end_symbol_scope();
 	return token;
 }
 
@@ -2818,15 +2817,15 @@ static struct token *parse_function_body(struct token *token, struct symbol *dec
 		decl->ctype.modifiers |= MOD_EXTERN;
 
 	stmt = start_function(decl);
-
 	*p = stmt;
+
 	FOR_EACH_PTR (base_type->arguments, arg) {
 		declare_argument(arg, base_type);
 	} END_FOR_EACH_PTR(arg);
 
-	token = compound_statement(token->next, stmt);
-
+	token = statement_list(token->next, &stmt->stmts);
 	end_function(decl);
+
 	if (!(decl->ctype.modifiers & MOD_INLINE))
 		add_symbol(list, decl);
 	check_declaration(decl);
-- 
2.26.2

[PATCH v1 14/28] scope: extract bind_symbol_with_scope() from bind_symbol()

[Luc Van Oostenryck]

In most cases, the scope that must be used for a symbol is
given by its namespace.

However, in some situations a different scope must be used.
This is then set, for exemple by doing the lookup with
the wrong namespace (but corresponding to the desired scope)
and changing it just after to its correct value.

To avoid these contortions, extract from bind_symbol() a version
where the scope can be explicitly given: bind_symbol_with_scope().

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 symbol.c | 13 +++++++++----
 symbol.h |  1 +
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/symbol.c b/symbol.c
index c2e6f0b426b3..7044ab3f78ce 100644
--- a/symbol.c
+++ b/symbol.c
@@ -671,9 +671,8 @@ static void inherit_static(struct symbol *sym)
 	}
 }
 
-void bind_symbol(struct symbol *sym, struct ident *ident, enum namespace ns)
+void bind_symbol_with_scope(struct symbol *sym, struct ident *ident, enum namespace ns, struct scope *scope)
 {
-	struct scope *scope;
 	if (sym->bound) {
 		sparse_error(sym->pos, "internal error: symbol type already bound");
 		return;
@@ -690,7 +689,6 @@ void bind_symbol(struct symbol *sym, struct ident *ident, enum namespace ns)
 	sym->ident = ident;
 	sym->bound = 1;
 
-	scope = block_scope;
 	if (ns == NS_SYMBOL && toplevel(scope)) {
 		unsigned mod = MOD_ADDRESSABLE | MOD_TOPLEVEL;
 
@@ -704,11 +702,18 @@ void bind_symbol(struct symbol *sym, struct ident *ident, enum namespace ns)
 		}
 		sym->ctype.modifiers |= mod;
 	}
+	bind_scope(sym, scope);
+}
+
+void bind_symbol(struct symbol *sym, struct ident *ident, enum namespace ns)
+{
+	struct scope *scope = block_scope;;
+
 	if (ns == NS_MACRO)
 		scope = file_scope;
 	if (ns == NS_LABEL)
 		scope = function_scope;
-	bind_scope(sym, scope);
+	bind_symbol_with_scope(sym, ident, ns, scope);
 }
 
 struct symbol *create_symbol(int stream, const char *name, int type, int namespace)
diff --git a/symbol.h b/symbol.h
index 50dba78a654a..c297c778dfdf 100644
--- a/symbol.h
+++ b/symbol.h
@@ -332,6 +332,7 @@ extern void show_type_list(struct symbol *);
 extern void show_symbol_list(struct symbol_list *, const char *);
 extern void add_symbol(struct symbol_list **, struct symbol *);
 extern void bind_symbol(struct symbol *, struct ident *, enum namespace);
+extern void bind_symbol_with_scope(struct symbol *, struct ident *, enum namespace, struct scope *);
 
 extern struct symbol *examine_symbol_type(struct symbol *);
 extern struct symbol *examine_pointer_target(struct symbol *);
-- 
2.26.2

Re: [PATCH v1 14/28] scope: extract bind_symbol_with_scope() from bind_symbol()

[Ramsay Jones]

On 19/05/2020 01:57, Luc Van Oostenryck wrote:
> In most cases, the scope that must be used for a symbol is
> given by its namespace.
> 
> However, in some situations a different scope must be used.
> This is then set, for exemple by doing the lookup with

s/exemple/example/

ATB,
Ramsay Jones

> the wrong namespace (but corresponding to the desired scope)
> and changing it just after to its correct value.
> 
> To avoid these contortions, extract from bind_symbol() a version
> where the scope can be explicitly given: bind_symbol_with_scope().
> 
> Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
> ---
>  symbol.c | 13 +++++++++----
>  symbol.h |  1 +
>  2 files changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/symbol.c b/symbol.c
> index c2e6f0b426b3..7044ab3f78ce 100644
> --- a/symbol.c
> +++ b/symbol.c
> @@ -671,9 +671,8 @@ static void inherit_static(struct symbol *sym)
>  	}
>  }
>  
> -void bind_symbol(struct symbol *sym, struct ident *ident, enum namespace ns)
> +void bind_symbol_with_scope(struct symbol *sym, struct ident *ident, enum namespace ns, struct scope *scope)
>  {
> -	struct scope *scope;
>  	if (sym->bound) {
>  		sparse_error(sym->pos, "internal error: symbol type already bound");
>  		return;
> @@ -690,7 +689,6 @@ void bind_symbol(struct symbol *sym, struct ident *ident, enum namespace ns)
>  	sym->ident = ident;
>  	sym->bound = 1;
>  
> -	scope = block_scope;
>  	if (ns == NS_SYMBOL && toplevel(scope)) {
>  		unsigned mod = MOD_ADDRESSABLE | MOD_TOPLEVEL;
>  
> @@ -704,11 +702,18 @@ void bind_symbol(struct symbol *sym, struct ident *ident, enum namespace ns)
>  		}
>  		sym->ctype.modifiers |= mod;
>  	}
> +	bind_scope(sym, scope);
> +}
> +
> +void bind_symbol(struct symbol *sym, struct ident *ident, enum namespace ns)
> +{
> +	struct scope *scope = block_scope;;
> +
>  	if (ns == NS_MACRO)
>  		scope = file_scope;
>  	if (ns == NS_LABEL)
>  		scope = function_scope;
> -	bind_scope(sym, scope);
> +	bind_symbol_with_scope(sym, ident, ns, scope);
>  }
>  
>  struct symbol *create_symbol(int stream, const char *name, int type, int namespace)
> diff --git a/symbol.h b/symbol.h
> index 50dba78a654a..c297c778dfdf 100644
> --- a/symbol.h
> +++ b/symbol.h
> @@ -332,6 +332,7 @@ extern void show_type_list(struct symbol *);
>  extern void show_symbol_list(struct symbol_list *, const char *);
>  extern void add_symbol(struct symbol_list **, struct symbol *);
>  extern void bind_symbol(struct symbol *, struct ident *, enum namespace);
> +extern void bind_symbol_with_scope(struct symbol *, struct ident *, enum namespace, struct scope *);
>  
>  extern struct symbol *examine_symbol_type(struct symbol *);
>  extern struct symbol *examine_pointer_target(struct symbol *);
> 

[PATCH v1 15/28] scope: __func__ is special

[Luc Van Oostenryck]

__func__ needs to be in the namepsace for symbols: NS_SYMBOL
but doesn't follow the usual scope rules of them: it always
needs to be declared in the function scope.

So, use bind_symbol_scoped() instead of first using bind_symbol()
and then changing the namespace.
Also change the comment to better express that it's the scope
that is the unusual thing.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 expression.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/expression.c b/expression.c
index 78e577cf10a1..ffb3c2dce4d5 100644
--- a/expression.c
+++ b/expression.c
@@ -122,9 +122,8 @@ static struct symbol *handle_func(struct token *token)
 	decl->ctype.modifiers = MOD_STATIC;
 	decl->endpos = token->pos;
 
-	/* function-scope, but in NS_SYMBOL */
-	bind_symbol(decl, ident, NS_LABEL);
-	decl->namespace = NS_SYMBOL;
+	/* NS_SYMBOL but in function-scope */
+	bind_symbol_with_scope(decl, ident, NS_SYMBOL, function_scope);
 
 	len = current_fn->ident->len;
 	string = __alloc_string(len + 1);
-- 
2.26.2

Re: [PATCH v1 15/28] scope: __func__ is special

[Ramsay Jones]

On 19/05/2020 01:57, Luc Van Oostenryck wrote:
> __func__ needs to be in the namepsace for symbols: NS_SYMBOL
> but doesn't follow the usual scope rules of them: it always
> needs to be declared in the function scope.
> 
> So, use bind_symbol_scoped() instead of first using bind_symbol()

s/bind_symbol_scoped/bind_symbol_with_scope/

ATB,
Ramsay Jones

> and then changing the namespace.
> Also change the comment to better express that it's the scope
> that is the unusual thing.
> 
> Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
> ---
>  expression.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/expression.c b/expression.c
> index 78e577cf10a1..ffb3c2dce4d5 100644
> --- a/expression.c
> +++ b/expression.c
> @@ -122,9 +122,8 @@ static struct symbol *handle_func(struct token *token)
>  	decl->ctype.modifiers = MOD_STATIC;
>  	decl->endpos = token->pos;
>  
> -	/* function-scope, but in NS_SYMBOL */
> -	bind_symbol(decl, ident, NS_LABEL);
> -	decl->namespace = NS_SYMBOL;
> +	/* NS_SYMBOL but in function-scope */
> +	bind_symbol_with_scope(decl, ident, NS_SYMBOL, function_scope);
>  
>  	len = current_fn->ident->len;
>  	string = __alloc_string(len + 1);
> 

[PATCH v1 16/28] scope: __label__ is special

[Luc Van Oostenryck]

Labels declared wth __label__ are special because they must follow
the block scope normally used for variables instad of using the
scope used for labels.

So, use bind_symbol_scoped() instead of first using bind_symbol()
and then changing the namespace.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 parse.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/parse.c b/parse.c
index e23c5b64e8be..29e3f939166d 100644
--- a/parse.c
+++ b/parse.c
@@ -2569,8 +2569,7 @@ static struct token *label_statement(struct token *token)
 	while (token_type(token) == TOKEN_IDENT) {
 		struct symbol *sym = alloc_symbol(token->pos, SYM_LABEL);
 		/* it's block-scope, but we want label namespace */
-		bind_symbol(sym, token->ident, NS_SYMBOL);
-		sym->namespace = NS_LABEL;
+		bind_symbol_with_scope(sym, token->ident, NS_LABEL, block_scope);
 		fn_local_symbol(sym);
 		token = token->next;
 		if (!match_op(token, ','))
-- 
2.26.2

Re: [PATCH v1 16/28] scope: __label__ is special

[Ramsay Jones]

On 19/05/2020 01:57, Luc Van Oostenryck wrote:
> Labels declared wth __label__ are special because they must follow
> the block scope normally used for variables instad of using the

s/instad/instead/

> scope used for labels.
> 
> So, use bind_symbol_scoped() instead of first using bind_symbol()

s/bind_symbol_scoped/bind/symbol_with_scope/

ATB,
Ramsay Jones

> and then changing the namespace.
> 
> Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
> ---
>  parse.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/parse.c b/parse.c
> index e23c5b64e8be..29e3f939166d 100644
> --- a/parse.c
> +++ b/parse.c
> @@ -2569,8 +2569,7 @@ static struct token *label_statement(struct token *token)
>  	while (token_type(token) == TOKEN_IDENT) {
>  		struct symbol *sym = alloc_symbol(token->pos, SYM_LABEL);
>  		/* it's block-scope, but we want label namespace */
> -		bind_symbol(sym, token->ident, NS_SYMBOL);
> -		sym->namespace = NS_LABEL;
> +		bind_symbol_with_scope(sym, token->ident, NS_LABEL, block_scope);
>  		fn_local_symbol(sym);
>  		token = token->next;
>  		if (!match_op(token, ','))
> 

[PATCH v1 17/28] scope: s/{start,end}_symbol_scope/{start,end}_block_scope/

[Luc Van Oostenryck]

The functions {start,end}_symbol_scope() haven't been renamed
when separated scope have been introduced for functions & blocks.
But these functions only start & end a block scope.

So, rename them to their more direct name: {start,end}_block_scope().

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 parse.c | 12 ++++++------
 scope.c |  4 ++--
 scope.h |  4 ++--
 3 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/parse.c b/parse.c
index 29e3f939166d..ecc33765e1ef 100644
--- a/parse.c
+++ b/parse.c
@@ -2230,7 +2230,7 @@ static void start_iterator(struct statement *stmt)
 {
 	struct symbol *cont, *brk;
 
-	start_symbol_scope();
+	start_block_scope();
 	cont = alloc_symbol(stmt->pos, SYM_NODE);
 	bind_symbol(cont, &continue_ident, NS_ITERATOR);
 	brk = alloc_symbol(stmt->pos, SYM_NODE);
@@ -2245,7 +2245,7 @@ static void start_iterator(struct statement *stmt)
 
 static void end_iterator(struct statement *stmt)
 {
-	end_symbol_scope();
+	end_block_scope();
 }
 
 static struct statement *start_function(struct symbol *sym)
@@ -2290,7 +2290,7 @@ static void start_switch(struct statement *stmt)
 {
 	struct symbol *brk, *switch_case;
 
-	start_symbol_scope();
+	start_block_scope();
 	brk = alloc_symbol(stmt->pos, SYM_NODE);
 	bind_symbol(brk, &break_ident, NS_ITERATOR);
 
@@ -2310,7 +2310,7 @@ static void end_switch(struct statement *stmt)
 {
 	if (!stmt->switch_case->symbol_list)
 		warning(stmt->pos, "switch with no cases");
-	end_symbol_scope();
+	end_block_scope();
 }
 
 static void add_case_statement(struct statement *stmt)
@@ -2662,9 +2662,9 @@ static struct token *parameter_type_list(struct token *token, struct symbol *fn)
 struct token *compound_statement(struct token *token, struct statement *stmt)
 {
 	stmt->type = STMT_COMPOUND;
-	start_symbol_scope();
+	start_block_scope();
 	token = statement_list(token, &stmt->stmts);
-	end_symbol_scope();
+	end_block_scope();
 	return token;
 }
 
diff --git a/scope.c b/scope.c
index 0e4fb3b42150..cc54f1e1760b 100644
--- a/scope.c
+++ b/scope.c
@@ -84,7 +84,7 @@ void start_file_scope(void)
 	block_scope = scope;
 }
 
-void start_symbol_scope(void)
+void start_block_scope(void)
 {
 	start_scope(&block_scope);
 }
@@ -129,7 +129,7 @@ void new_file_scope(void)
 	start_file_scope();
 }
 
-void end_symbol_scope(void)
+void end_block_scope(void)
 {
 	end_scope(&block_scope);
 }
diff --git a/scope.h b/scope.h
index 3cad514ac128..83741459eb6a 100644
--- a/scope.h
+++ b/scope.h
@@ -47,8 +47,8 @@ extern void start_file_scope(void);
 extern void end_file_scope(void);
 extern void new_file_scope(void);
 
-extern void start_symbol_scope(void);
-extern void end_symbol_scope(void);
+extern void start_block_scope(void);
+extern void end_block_scope(void);
 
 extern void start_function_scope(void);
 extern void end_function_scope(void);
-- 
2.26.2

[PATCH v1 18/28] scope: make function_scope invalid outside functions

[Luc Van Oostenryck]

The scopes are mainly used for symbols corresponding to variables
and functions with this hiearchy:
* builtin
* global
* function
* block

But the function_scope is only used for labels and __func__ and
is meaningless outside a function.

So, mainly in preparation for some incoming changes, let
function_scope's parent be NULL instead of the builtin scope.

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

diff --git a/scope.c b/scope.c
index cc54f1e1760b..83cc34c44bf5 100644
--- a/scope.c
+++ b/scope.c
@@ -36,7 +36,7 @@
 static struct scope builtin_scope = { .next = &builtin_scope };
 
 struct scope	*block_scope = &builtin_scope,		// regular automatic variables etc
-		*function_scope = &builtin_scope,	// labels, arguments etc
+		*function_scope = NULL,			// __fun__, labels
 		*file_scope = &builtin_scope,		// static
 		*global_scope = &builtin_scope;		// externally visible
 
@@ -80,7 +80,6 @@ void start_file_scope(void)
 	file_scope = scope;
 
 	/* top-level stuff defaults to file scope, "extern" etc will choose global scope */
-	function_scope = scope;
 	block_scope = scope;
 }
 
@@ -138,6 +137,7 @@ void end_function_scope(void)
 {
 	end_scope(&block_scope);
 	end_scope(&function_scope);
+	function_scope = NULL;
 }
 
 int is_outer_scope(struct scope *scope)
-- 
2.26.2

Re: [PATCH v1 18/28] scope: make function_scope invalid outside functions

[Linus Torvalds]

On Mon, May 18, 2020 at 5:57 PM Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
>
> So, mainly in preparation for some incoming changes, let
> function_scope's parent be NULL instead of the builtin scope.

Hmm. Gcc nested functions?

Sparse doesn't _support_ them, but the symbol nesting part actually
does work, afaik. This looks like it might break it.

Yes, gcc function nesting is disgusting. But it's a thing.

Stupid test-case that almost works with sparse:

    int test(int a, int b)
    {
        inline int nested(int i)
        {
                return a*2 + i;
        }
        return nested(b);
    }

and by "almost works with sparse" I mean that it actually linearizes
correctly, even if it's mostly by mistake, and you get a warning about

    t.c:5:24: warning: unreplaced symbol 'a'

because sparse really doesn't support nested functions and it really
only works because of inlining.

            Linus

Re: [PATCH v1 18/28] scope: make function_scope invalid outside functions

[Luc Van Oostenryck]

On Mon, May 18, 2020 at 06:38:17PM -0700, Linus Torvalds wrote:
> On Mon, May 18, 2020 at 5:57 PM Luc Van Oostenryck
> <luc.vanoostenryck@gmail.com> wrote:
> >
> > So, mainly in preparation for some incoming changes, let
> > function_scope's parent be NULL instead of the builtin scope.
> 
> Hmm. Gcc nested functions?
> 
> Sparse doesn't _support_ them, but the symbol nesting part actually
> does work, afaik. This looks like it might break it.

Hmm yes. I was barely aware that nested functions were at least parsed
but I had absolutely not internalized that and indeed this patch
assumes that leaving the function will return to the file scope.

But well this patch maybe made some sense at some stage but was
probably a bad idea, even more so with the nested functions.
So I'm dropping it and making the corresponding adjustments
in the next 2 patches.

Thank you for noticing this.

> Yes, gcc function nesting is disgusting. But it's a thing.
> 
> Stupid test-case that almost works with sparse:
> 
>     int test(int a, int b)
>     {
>         inline int nested(int i)
>         {
>                 return a*2 + i;
>         }
>         return nested(b);
>     }
> 
> and by "almost works with sparse" I mean that it actually linearizes
> correctly, even if it's mostly by mistake, and you get a warning about
> 
>     t.c:5:24: warning: unreplaced symbol 'a'
> 
> because sparse really doesn't support nested functions and it really
> only works because of inlining.

I see, interesting. I only saw this "unreplaced symbol" message a very
few tmes and never understood its cause (I don't think it was related
to a nested function but it makes more sense now).

-- Luc

[PATCH v1 19/28] scope: let labels have their own scope

[Luc Van Oostenryck]

It's invalid to jump inside a statement expression.
So, concerning labels & gotos, a statement expression is
like a kind of scope.

So, in preparation for the detection of such jumps, create
these new scopes and open/close them when entering/leaving
statement expressions.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 expression.c |  2 ++
 scope.c      | 19 ++++++++++++++++---
 scope.h      |  4 ++++
 3 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/expression.c b/expression.c
index ffb3c2dce4d5..f8a8f03e7402 100644
--- a/expression.c
+++ b/expression.c
@@ -71,7 +71,9 @@ struct token *parens_expression(struct token *token, struct expression **expr, c
 		struct statement *stmt = alloc_statement(token->pos, STMT_COMPOUND);
 		*expr = e;
 		e->statement = stmt;
+		start_label_scope();
 		token = compound_statement(token->next, stmt);
+		end_label_scope();
 		token = expect(token, '}', "at end of statement expression");
 	} else
 		token = parse_expression(token, expr);
diff --git a/scope.c b/scope.c
index 83cc34c44bf5..2e3a1c37ec15 100644
--- a/scope.c
+++ b/scope.c
@@ -36,7 +36,8 @@
 static struct scope builtin_scope = { .next = &builtin_scope };
 
 struct scope	*block_scope = &builtin_scope,		// regular automatic variables etc
-		*function_scope = NULL,			// __fun__, labels
+		*label_scope = NULL,			// expr-stmt labels
+		*function_scope = NULL,			// __fun__
 		*file_scope = &builtin_scope,		// static
 		*global_scope = &builtin_scope;		// externally visible
 
@@ -90,8 +91,9 @@ void start_block_scope(void)
 
 void start_function_scope(void)
 {
-	start_scope(&function_scope);
 	start_scope(&block_scope);
+	start_scope(&label_scope);
+	function_scope = label_scope;
 }
 
 static void remove_symbol_scope(struct symbol *sym)
@@ -136,8 +138,19 @@ void end_block_scope(void)
 void end_function_scope(void)
 {
 	end_scope(&block_scope);
-	end_scope(&function_scope);
+	end_label_scope();
 	function_scope = NULL;
+	label_scope = NULL;
+}
+
+void start_label_scope(void)
+{
+	start_scope(&label_scope);
+}
+
+void end_label_scope(void)
+{
+	end_scope(&label_scope);
 }
 
 int is_outer_scope(struct scope *scope)
diff --git a/scope.h b/scope.h
index 83741459eb6a..ddcb90bd146b 100644
--- a/scope.h
+++ b/scope.h
@@ -34,6 +34,7 @@ struct scope {
 
 extern struct scope
 		*block_scope,
+		*label_scope,
 		*function_scope,
 		*file_scope,
 		*global_scope;
@@ -53,6 +54,9 @@ extern void end_block_scope(void);
 extern void start_function_scope(void);
 extern void end_function_scope(void);
 
+extern void start_label_scope(void);
+extern void end_label_scope(void);
+
 extern void set_current_scope(struct symbol *);
 extern void bind_scope(struct symbol *, struct scope *);
 extern void rebind_scope(struct symbol *, struct scope *);
-- 
2.26.2

[PATCH v1 20/28] scope: add is_in_scope()

[Luc Van Oostenryck]

Add an helper to check if a scope is included into another one.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 scope.c | 9 +++++++++
 scope.h | 2 ++
 2 files changed, 11 insertions(+)

diff --git a/scope.c b/scope.c
index 2e3a1c37ec15..017c0dcd8600 100644
--- a/scope.c
+++ b/scope.c
@@ -162,3 +162,12 @@ int is_outer_scope(struct scope *scope)
 	return 1;
 }
 
+int is_in_scope(struct scope *outer, struct scope *inner)
+{
+	while (inner != outer) {
+		if (!inner)
+			return 0;
+		inner = inner->next;
+	}
+	return 1;
+}
diff --git a/scope.h b/scope.h
index ddcb90bd146b..36a56d6adf1d 100644
--- a/scope.h
+++ b/scope.h
@@ -62,4 +62,6 @@ extern void bind_scope(struct symbol *, struct scope *);
 extern void rebind_scope(struct symbol *, struct scope *);
 
 extern int is_outer_scope(struct scope *);
+extern int is_in_scope(struct scope *outer, struct scope *inner);
+
 #endif
-- 
2.26.2

[PATCH v1 21/28] scope: give a scope for labels & gotos

[Luc Van Oostenryck]

One way of detecting gotos inside an statement expression
is to use a new kind of scope for the gotos & labels.
Since gotos don't need to have their label predeclared,
nothing can be checked at parsing time but later it can
be checked that a goto doesn't jump inside one of the
label scope created by statement expressions.

So, add additional scope information to gotos and labels
to allow such check to be done.

Note: the label's symbols are still created in the function
      scope since they belong to a single namespace.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 parse.c  | 8 +++++++-
 parse.h  | 1 +
 symbol.h | 4 ++++
 3 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/parse.c b/parse.c
index ecc33765e1ef..bf45e3b0ea44 100644
--- a/parse.c
+++ b/parse.c
@@ -2488,7 +2488,12 @@ static struct token *parse_goto_statement(struct token *token, struct statement
 		token = parse_expression(token->next, &stmt->goto_expression);
 		add_statement(&function_computed_goto_list, stmt);
 	} else if (token_type(token) == TOKEN_IDENT) {
-		stmt->goto_label = label_symbol(token);
+		struct symbol *label = label_symbol(token);
+		stmt->goto_label = label;
+		if (!label->stmt && !label->label_scope) {
+			label->label_scope = label_scope;
+			label->label_pos = stmt->pos;
+		}
 		token = token->next;
 	} else {
 		sparse_error(token->pos, "Expected identifier or goto expression");
@@ -2549,6 +2554,7 @@ static struct token *statement(struct token *token, struct statement **tree)
 			}
 			stmt->type = STMT_LABEL;
 			stmt->label_identifier = s;
+			stmt->label_scope = label_scope;
 			s->stmt = stmt;
 			return statement(token, &stmt->label_statement);
 		}
diff --git a/parse.h b/parse.h
index 0742a2a87e9d..daef243938b2 100644
--- a/parse.h
+++ b/parse.h
@@ -72,6 +72,7 @@ struct statement {
 		};
 		struct /* labeled_struct */ {
 			struct symbol *label_identifier;
+			struct scope *label_scope;
 			struct statement *label_statement;
 		};
 		struct /* case_struct */ {
diff --git a/symbol.h b/symbol.h
index c297c778dfdf..2293d06dd4fb 100644
--- a/symbol.h
+++ b/symbol.h
@@ -167,6 +167,10 @@ struct symbol {
 			int (*handler)(struct stream *, struct token **, struct token *);
 			int normal;
 		};
+		struct /* NS_LABEL */ {
+			struct scope *label_scope;
+			struct position label_pos;
+		};
 		struct /* NS_SYMBOL */ {
 			unsigned long	offset;
 			int		bit_size;
-- 
2.26.2

[PATCH v1 22/28] bad-goto: jumping inside a statemet expression is an error

[Luc Van Oostenryck]

It's invalid to jump inside a statement expression.

So, detect such jumps, issue an error message and mark the
function as useless for linearization since the resulting IR
would be invalid.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 parse.c                                       | 30 ++++++++++++++++---
 parse.h                                       |  1 +
 validation/label-scope2.c                     |  1 -
 validation/label-stmt-expr0.c                 |  1 -
 validation/label-stmt-expr1.c                 |  1 -
 validation/label-stmt-expr2.c                 |  1 -
 .../linear/goto-stmt-expr-conditional.c       |  1 -
 .../linear/goto-stmt-expr-short-circuit.c     |  1 -
 8 files changed, 27 insertions(+), 10 deletions(-)

diff --git a/parse.c b/parse.c
index bf45e3b0ea44..b9d4940e77fb 100644
--- a/parse.c
+++ b/parse.c
@@ -2480,6 +2480,27 @@ static struct token *parse_switch_statement(struct token *token, struct statemen
 	return token;
 }
 
+static void warn_label_usage(struct position def, struct position use, struct ident *ident)
+{
+	const char *id = show_ident(ident);
+	sparse_error(use, "label '%s' used outside statement expression", id);
+	info(def, "   label '%s' defined here", id);
+	current_fn->bogus_linear = 1;
+}
+
+void check_label_usage(struct symbol *label, struct position use_pos)
+{
+	struct statement *def = label->stmt;
+
+	if (def) {
+		if (!is_in_scope(def->label_scope, label_scope))
+			warn_label_usage(def->pos, use_pos, label->ident);
+	} else if (!label->label_scope) {
+		label->label_scope = label_scope;
+		label->label_pos = use_pos;
+	}
+}
+
 static struct token *parse_goto_statement(struct token *token, struct statement *stmt)
 {
 	stmt->type = STMT_GOTO;
@@ -2490,10 +2511,7 @@ static struct token *parse_goto_statement(struct token *token, struct statement
 	} else if (token_type(token) == TOKEN_IDENT) {
 		struct symbol *label = label_symbol(token);
 		stmt->goto_label = label;
-		if (!label->stmt && !label->label_scope) {
-			label->label_scope = label_scope;
-			label->label_pos = stmt->pos;
-		}
+		check_label_usage(label, stmt->pos);
 		token = token->next;
 	} else {
 		sparse_error(token->pos, "Expected identifier or goto expression");
@@ -2555,6 +2573,10 @@ static struct token *statement(struct token *token, struct statement **tree)
 			stmt->type = STMT_LABEL;
 			stmt->label_identifier = s;
 			stmt->label_scope = label_scope;
+			if (s->label_scope) {
+				if (!is_in_scope(label_scope, s->label_scope))
+					warn_label_usage(stmt->pos, s->label_pos, s->ident);
+			}
 			s->stmt = stmt;
 			return statement(token, &stmt->label_statement);
 		}
diff --git a/parse.h b/parse.h
index daef243938b2..2cfdd872e621 100644
--- a/parse.h
+++ b/parse.h
@@ -125,6 +125,7 @@ extern struct statement_list *function_computed_goto_list;
 
 extern struct token *parse_expression(struct token *, struct expression **);
 extern struct symbol *label_symbol(struct token *token);
+extern void check_label_usage(struct symbol *label, struct position use_pos);
 
 extern int show_statement(struct statement *);
 extern void show_statement_list(struct statement_list *, const char *);
diff --git a/validation/label-scope2.c b/validation/label-scope2.c
index 8c04ac6525e5..448647528dc6 100644
--- a/validation/label-scope2.c
+++ b/validation/label-scope2.c
@@ -23,7 +23,6 @@ a:
 
 /*
  * check-name: label-scope2
- * check-known-to-fail
  *
  * check-error-start
 label-scope2.c:20:17: error: label 'a' used outside statement expression
diff --git a/validation/label-stmt-expr0.c b/validation/label-stmt-expr0.c
index 66a6490241bd..5fc452ab0d15 100644
--- a/validation/label-stmt-expr0.c
+++ b/validation/label-stmt-expr0.c
@@ -26,7 +26,6 @@ l:		 1;
 /*
  * check-name: label-stmt-expr0
  * check-command: sparse -Wno-decl $file
- * check-known-to-fail
  *
  * check-error-start
 label-stmt-expr0.c:6:9: error: label 'l' used outside statement expression
diff --git a/validation/label-stmt-expr1.c b/validation/label-stmt-expr1.c
index 339217dcb999..32a89aad4e1f 100644
--- a/validation/label-stmt-expr1.c
+++ b/validation/label-stmt-expr1.c
@@ -18,7 +18,6 @@ l:
 
 /*
  * check-name: label-stmt-expr1
- * check-known-to-fail
  *
  * check-error-start
 label-stmt-expr1.c:3:9: error: label 'l' used outside statement expression
diff --git a/validation/label-stmt-expr2.c b/validation/label-stmt-expr2.c
index 7a38e3799c55..8c54477a4cc3 100644
--- a/validation/label-stmt-expr2.c
+++ b/validation/label-stmt-expr2.c
@@ -30,7 +30,6 @@ l:
 
 /*
  * check-name: label-stmt-expr2
- * check-known-to-fail
  *
  * check-error-start
 label-stmt-expr2.c:3:9: error: label 'l' used outside statement expression
diff --git a/validation/linear/goto-stmt-expr-conditional.c b/validation/linear/goto-stmt-expr-conditional.c
index 6576052b50ac..bbfcb3ebc039 100644
--- a/validation/linear/goto-stmt-expr-conditional.c
+++ b/validation/linear/goto-stmt-expr-conditional.c
@@ -20,7 +20,6 @@ a:
 /*
  * check-name: goto-stmt-expr-conditional
  * check-command: test-linearize -Wno-decl $file
- * check-known-to-fail
  *
  * check-error-ignore
  * check-output-ignore
diff --git a/validation/linear/goto-stmt-expr-short-circuit.c b/validation/linear/goto-stmt-expr-short-circuit.c
index 426315e69fbd..a5953e73bc93 100644
--- a/validation/linear/goto-stmt-expr-short-circuit.c
+++ b/validation/linear/goto-stmt-expr-short-circuit.c
@@ -24,7 +24,6 @@ inside:
 /*
  * check-name: goto-stmt-expr-short-circuit
  * check-command: test-linearize -Wno-decl $file
- * check-known-to-fail
  *
  * check-error-ignore
  * check-output-ignore
-- 
2.26.2

Re: [PATCH v1 22/28] bad-goto: jumping inside a statemet expression is an error

[Ramsay Jones]

On 19/05/2020 01:57, Luc Van Oostenryck wrote:
> It's invalid to jump inside a statement expression.
> 
> So, detect such jumps, issue an error message and mark the
> function as useless for linearization since the resulting IR
> would be invalid.
> 
> Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
> ---
>  parse.c                                       | 30 ++++++++++++++++---
>  parse.h                                       |  1 +
>  validation/label-scope2.c                     |  1 -
>  validation/label-stmt-expr0.c                 |  1 -
>  validation/label-stmt-expr1.c                 |  1 -
>  validation/label-stmt-expr2.c                 |  1 -
>  .../linear/goto-stmt-expr-conditional.c       |  1 -
>  .../linear/goto-stmt-expr-short-circuit.c     |  1 -
>  8 files changed, 27 insertions(+), 10 deletions(-)
> 
> diff --git a/parse.c b/parse.c
> index bf45e3b0ea44..b9d4940e77fb 100644
> --- a/parse.c
> +++ b/parse.c
> @@ -2480,6 +2480,27 @@ static struct token *parse_switch_statement(struct token *token, struct statemen
>  	return token;
>  }
>  
> +static void warn_label_usage(struct position def, struct position use, struct ident *ident)

you are issuing an error report from this function, so should it be
called, something like, 'label_usage_error'?

ATB,
Ramsay Jones

> +{
> +	const char *id = show_ident(ident);
> +	sparse_error(use, "label '%s' used outside statement expression", id);
> +	info(def, "   label '%s' defined here", id);
> +	current_fn->bogus_linear = 1;
> +}
> +
> +void check_label_usage(struct symbol *label, struct position use_pos)
> +{
> +	struct statement *def = label->stmt;
> +
> +	if (def) {
> +		if (!is_in_scope(def->label_scope, label_scope))
> +			warn_label_usage(def->pos, use_pos, label->ident);
> +	} else if (!label->label_scope) {
> +		label->label_scope = label_scope;
> +		label->label_pos = use_pos;
> +	}
> +}
> +
>  static struct token *parse_goto_statement(struct token *token, struct statement *stmt)
>  {
>  	stmt->type = STMT_GOTO;
> @@ -2490,10 +2511,7 @@ static struct token *parse_goto_statement(struct token *token, struct statement
>  	} else if (token_type(token) == TOKEN_IDENT) {
>  		struct symbol *label = label_symbol(token);
>  		stmt->goto_label = label;
> -		if (!label->stmt && !label->label_scope) {
> -			label->label_scope = label_scope;
> -			label->label_pos = stmt->pos;
> -		}
> +		check_label_usage(label, stmt->pos);
>  		token = token->next;
>  	} else {
>  		sparse_error(token->pos, "Expected identifier or goto expression");
> @@ -2555,6 +2573,10 @@ static struct token *statement(struct token *token, struct statement **tree)
>  			stmt->type = STMT_LABEL;
>  			stmt->label_identifier = s;
>  			stmt->label_scope = label_scope;
> +			if (s->label_scope) {
> +				if (!is_in_scope(label_scope, s->label_scope))
> +					warn_label_usage(stmt->pos, s->label_pos, s->ident);
> +			}
>  			s->stmt = stmt;
>  			return statement(token, &stmt->label_statement);
>  		}
> diff --git a/parse.h b/parse.h
> index daef243938b2..2cfdd872e621 100644
> --- a/parse.h
> +++ b/parse.h
> @@ -125,6 +125,7 @@ extern struct statement_list *function_computed_goto_list;
>  
>  extern struct token *parse_expression(struct token *, struct expression **);
>  extern struct symbol *label_symbol(struct token *token);
> +extern void check_label_usage(struct symbol *label, struct position use_pos);
>  
>  extern int show_statement(struct statement *);
>  extern void show_statement_list(struct statement_list *, const char *);
> diff --git a/validation/label-scope2.c b/validation/label-scope2.c
> index 8c04ac6525e5..448647528dc6 100644
> --- a/validation/label-scope2.c
> +++ b/validation/label-scope2.c
> @@ -23,7 +23,6 @@ a:
>  
>  /*
>   * check-name: label-scope2
> - * check-known-to-fail
>   *
>   * check-error-start
>  label-scope2.c:20:17: error: label 'a' used outside statement expression
> diff --git a/validation/label-stmt-expr0.c b/validation/label-stmt-expr0.c
> index 66a6490241bd..5fc452ab0d15 100644
> --- a/validation/label-stmt-expr0.c
> +++ b/validation/label-stmt-expr0.c
> @@ -26,7 +26,6 @@ l:		 1;
>  /*
>   * check-name: label-stmt-expr0
>   * check-command: sparse -Wno-decl $file
> - * check-known-to-fail
>   *
>   * check-error-start
>  label-stmt-expr0.c:6:9: error: label 'l' used outside statement expression
> diff --git a/validation/label-stmt-expr1.c b/validation/label-stmt-expr1.c
> index 339217dcb999..32a89aad4e1f 100644
> --- a/validation/label-stmt-expr1.c
> +++ b/validation/label-stmt-expr1.c
> @@ -18,7 +18,6 @@ l:
>  
>  /*
>   * check-name: label-stmt-expr1
> - * check-known-to-fail
>   *
>   * check-error-start
>  label-stmt-expr1.c:3:9: error: label 'l' used outside statement expression
> diff --git a/validation/label-stmt-expr2.c b/validation/label-stmt-expr2.c
> index 7a38e3799c55..8c54477a4cc3 100644
> --- a/validation/label-stmt-expr2.c
> +++ b/validation/label-stmt-expr2.c
> @@ -30,7 +30,6 @@ l:
>  
>  /*
>   * check-name: label-stmt-expr2
> - * check-known-to-fail
>   *
>   * check-error-start
>  label-stmt-expr2.c:3:9: error: label 'l' used outside statement expression
> diff --git a/validation/linear/goto-stmt-expr-conditional.c b/validation/linear/goto-stmt-expr-conditional.c
> index 6576052b50ac..bbfcb3ebc039 100644
> --- a/validation/linear/goto-stmt-expr-conditional.c
> +++ b/validation/linear/goto-stmt-expr-conditional.c
> @@ -20,7 +20,6 @@ a:
>  /*
>   * check-name: goto-stmt-expr-conditional
>   * check-command: test-linearize -Wno-decl $file
> - * check-known-to-fail
>   *
>   * check-error-ignore
>   * check-output-ignore
> diff --git a/validation/linear/goto-stmt-expr-short-circuit.c b/validation/linear/goto-stmt-expr-short-circuit.c
> index 426315e69fbd..a5953e73bc93 100644
> --- a/validation/linear/goto-stmt-expr-short-circuit.c
> +++ b/validation/linear/goto-stmt-expr-short-circuit.c
> @@ -24,7 +24,6 @@ inside:
>  /*
>   * check-name: goto-stmt-expr-short-circuit
>   * check-command: test-linearize -Wno-decl $file
> - * check-known-to-fail
>   *
>   * check-error-ignore
>   * check-output-ignore
> 

Re: [PATCH v1 22/28] bad-goto: jumping inside a statemet expression is an error

[Luc Van Oostenryck]

On Wed, May 20, 2020 at 01:53:51AM +0100, Ramsay Jones wrote:
> On 19/05/2020 01:57, Luc Van Oostenryck wrote:
> > It's invalid to jump inside a statement expression.
> > 
> > So, detect such jumps, issue an error message and mark the
> > function as useless for linearization since the resulting IR
> > would be invalid.

...

> > +static void warn_label_usage(struct position def, struct position use, struct ident *ident)
> 
> you are issuing an error report from this function, so should it be
> called, something like, 'label_usage_error'?

Yes, it's a bit confusing. I hesitated on the name when writting it.
The logic is that most functions in this file (and other files too)
are named following the verb+object pattern and I don't have a good
(short) verb for 'issue an diagnostic message'. 'label_usage_error'
sounds to me more like the name for a variable. In standardese
maybe 'diagnose' could be used but ... no, thanks. The way I see it
is that the verb/action 'warn' can be realized in 2 ways:
issue a warning message or issue an error message.

In fact, I really would prefer to fold this function with its check.
It was how it was written at some stage but the function needed 5
arguments and was quite hard to read.
 
Best regards,
-- Luc

[PATCH v1 23/28] bad-goto: label expression inside a statement expression is UB

[Luc Van Oostenryck]

More exactly, what is undefined is to jump inside the statement
expression with a computed goto.

Of course, once the address of such a label is taken, it's generaly
impossible to track if it will be used or not to jump inside the
statement expression.

So, for now, handle taking the address of such a label from outside
the statement expression, exactly as if a computed goto is effectively
done from there and so issue an error message and also mark the function
as useless for linearization.

Note: this is only partially correct since:
      1) the address could be taken from outside the statement
         and never used for a computed goto.
      2) the address could be taken from outside the statement
         but the corresponding computed goto only done from
         inside, which is perfectly fine.
      3) the address could be taken from inside but a computed
         goto done from outside.

Note: the real problem, like for the regular goto, is that the
      statement expression can be eliminated before linearization,
      the correspondng gotos corresponding then to branches to
      unexistent BBs.

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

diff --git a/expression.c b/expression.c
index f8a8f03e7402..bbbc24e6b561 100644
--- a/expression.c
+++ b/expression.c
@@ -691,6 +691,7 @@ static struct token *unary_expression(struct token *token, struct expression **t
 				sym->ctype.modifiers |= MOD_ADDRESSABLE;
 				add_symbol(&function_computed_target_list, sym);
 			}
+			check_label_usage(sym, token->pos);
 			label->flags = CEF_ADDR;
 			label->label_symbol = sym;
 			*tree = label;
-- 
2.26.2

[PATCH v1 24/28] bad-goto: extract check_label_declaration()

[Luc Van Oostenryck]

Extract this helper from evaluate_goto_statement().

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 evaluate.c | 22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/evaluate.c b/evaluate.c
index 21d5d761627f..b272e3f642b2 100644
--- a/evaluate.c
+++ b/evaluate.c
@@ -3257,6 +3257,21 @@ static struct symbol *evaluate_offsetof(struct expression *expr)
 	return size_t_ctype;
 }
 
+static void check_label_declaration(struct position pos, struct symbol *label)
+{
+	switch (label->namespace) {
+	case NS_LABEL:
+		if (label->stmt)
+			break;
+		sparse_error(pos, "label '%s' was not declared", show_ident(label->ident));
+		/* fallthrough */
+	case NS_NONE:
+		current_fn->bogus_linear = 1;
+	default:
+		break;
+	}
+}
+
 struct symbol *evaluate_expression(struct expression *expr)
 {
 	if (!expr)
@@ -3748,12 +3763,7 @@ static void evaluate_goto_statement(struct statement *stmt)
 		return;
 	}
 
-	if (label->namespace == NS_LABEL && !label->stmt) {
-		sparse_error(stmt->pos, "label '%s' was not declared", show_ident(label->ident));
-		current_fn->bogus_linear = 1;
-	}
-	if (label->namespace == NS_NONE)
-		current_fn->bogus_linear = 1;
+	check_label_declaration(stmt->pos, label);
 }
 
 struct symbol *evaluate_statement(struct statement *stmt)
-- 
2.26.2

[PATCH v1 25/28] bad-goto: check declaration of label expressions

[Luc Van Oostenryck]

Issue an error when taking the address of an undeclared label
and mark the mark the function as improper for linearization
since the resulting IR would be invalid.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 evaluate.c                            | 1 +
 validation/label-scope-cgoto.c        | 1 -
 validation/linear/label-scope-cgoto.c | 1 -
 3 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/evaluate.c b/evaluate.c
index b272e3f642b2..63d75d9031d1 100644
--- a/evaluate.c
+++ b/evaluate.c
@@ -3344,6 +3344,7 @@ struct symbol *evaluate_expression(struct expression *expr)
 
 	case EXPR_LABEL:
 		expr->ctype = &ptr_ctype;
+		check_label_declaration(expr->pos, expr->label_symbol);
 		return &ptr_ctype;
 
 	case EXPR_TYPE:
diff --git a/validation/label-scope-cgoto.c b/validation/label-scope-cgoto.c
index c5d278d3d654..1edb9948d8cf 100644
--- a/validation/label-scope-cgoto.c
+++ b/validation/label-scope-cgoto.c
@@ -65,7 +65,6 @@ l:		 1;
 /*
  * check-name: label-scope-cgoto
  * check-command: sparse -Wno-decl $file
- * check-known-to-fail
  *
  * check-error-start
 label-scope-cgoto.c:12:19: error: label 'l' used outside statement expression
diff --git a/validation/linear/label-scope-cgoto.c b/validation/linear/label-scope-cgoto.c
index 592f1ce4f664..0eba05aea3c7 100644
--- a/validation/linear/label-scope-cgoto.c
+++ b/validation/linear/label-scope-cgoto.c
@@ -3,7 +3,6 @@
 /*
  * check-name: linear/label-scope-cgoto
  * check-command: test-linearize -Wno-decl -I. $file
- * check-known-to-fail
  *
  * check-error-ignore
  * check-output-ignore
-- 
2.26.2

Re: [PATCH v1 25/28] bad-goto: check declaration of label expressions

[Ramsay Jones]

On 19/05/2020 01:57, Luc Van Oostenryck wrote:
> Issue an error when taking the address of an undeclared label
> and mark the mark the function as improper for linearization

s/mark the mark the/mark the/

ATB,
Ramsay Jones

> since the resulting IR would be invalid.
> 
> Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
> ---
>  evaluate.c                            | 1 +
>  validation/label-scope-cgoto.c        | 1 -
>  validation/linear/label-scope-cgoto.c | 1 -
>  3 files changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/evaluate.c b/evaluate.c
> index b272e3f642b2..63d75d9031d1 100644
> --- a/evaluate.c
> +++ b/evaluate.c
> @@ -3344,6 +3344,7 @@ struct symbol *evaluate_expression(struct expression *expr)
>  
>  	case EXPR_LABEL:
>  		expr->ctype = &ptr_ctype;
> +		check_label_declaration(expr->pos, expr->label_symbol);
>  		return &ptr_ctype;
>  
>  	case EXPR_TYPE:
> diff --git a/validation/label-scope-cgoto.c b/validation/label-scope-cgoto.c
> index c5d278d3d654..1edb9948d8cf 100644
> --- a/validation/label-scope-cgoto.c
> +++ b/validation/label-scope-cgoto.c
> @@ -65,7 +65,6 @@ l:		 1;
>  /*
>   * check-name: label-scope-cgoto
>   * check-command: sparse -Wno-decl $file
> - * check-known-to-fail
>   *
>   * check-error-start
>  label-scope-cgoto.c:12:19: error: label 'l' used outside statement expression
> diff --git a/validation/linear/label-scope-cgoto.c b/validation/linear/label-scope-cgoto.c
> index 592f1ce4f664..0eba05aea3c7 100644
> --- a/validation/linear/label-scope-cgoto.c
> +++ b/validation/linear/label-scope-cgoto.c
> @@ -3,7 +3,6 @@
>  /*
>   * check-name: linear/label-scope-cgoto
>   * check-command: test-linearize -Wno-decl -I. $file
> - * check-known-to-fail
>   *
>   * check-error-ignore
>   * check-output-ignore
> 

[PATCH v1 26/28] bad-label: check for unused labels

[Luc Van Oostenryck]

Issue a warning if a label is defined but not used.

Note: this should take in account the attribute 'unused'.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 scope.c                   | 8 ++++++++
 validation/label-unused.c | 1 -
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/scope.c b/scope.c
index 017c0dcd8600..03593d823d6d 100644
--- a/scope.c
+++ b/scope.c
@@ -150,6 +150,14 @@ void start_label_scope(void)
 
 void end_label_scope(void)
 {
+	struct symbol *sym;
+
+	FOR_EACH_PTR(label_scope->symbols, sym) {
+		if (!sym->stmt || sym->used)
+			continue;
+		warning(sym->pos, "unused label '%s'", show_ident(sym->ident));
+	} END_FOR_EACH_PTR(sym);
+
 	end_scope(&label_scope);
 }
 
diff --git a/validation/label-unused.c b/validation/label-unused.c
index c136c7a8813e..a654ef7742be 100644
--- a/validation/label-unused.c
+++ b/validation/label-unused.c
@@ -15,7 +15,6 @@ l:
 
 /*
  * check-name: label-unused
- * check-known-to-fail
  *
  * check-error-start
 label-unused.c:3:1: warning: unused label 'l'
-- 
2.26.2

[PATCH v1 27/28] bad-label: mark labels as used when needed

[Luc Van Oostenryck]

In most cases symbols are automatically marked as being used via
a successfull call to lookup_symbols(), the idea being that the
symbol will be created at its declaration and then any (successfull)
lookup will correspond to an use.

For labels, things are slightly different because labels are
created on-demand via label_symbol() and their use can precede their
'declaration'. And of, course, label_symbol() has no ways to know
if it is used for a definition or an use.

So, fix this by adding an argument to label_symbol(), explictly
telling if the call correspond to an use or not.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 expression.c |  2 +-
 parse.c      | 10 ++++++----
 parse.h      |  2 +-
 3 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/expression.c b/expression.c
index bbbc24e6b561..99a6d7568222 100644
--- a/expression.c
+++ b/expression.c
@@ -686,7 +686,7 @@ static struct token *unary_expression(struct token *token, struct expression **t
 		if (match_op(token, SPECIAL_LOGICAL_AND) &&
 		    token_type(token->next) == TOKEN_IDENT) {
 			struct expression *label = alloc_expression(token->pos, EXPR_LABEL);
-			struct symbol *sym = label_symbol(token->next);
+			struct symbol *sym = label_symbol(token->next, 1);
 			if (!(sym->ctype.modifiers & MOD_ADDRESSABLE)) {
 				sym->ctype.modifiers |= MOD_ADDRESSABLE;
 				add_symbol(&function_computed_target_list, sym);
diff --git a/parse.c b/parse.c
index b9d4940e77fb..a8e4a02e90e4 100644
--- a/parse.c
+++ b/parse.c
@@ -726,12 +726,14 @@ static struct symbol * alloc_indirect_symbol(struct position pos, struct ctype *
  * it also ends up using function scope instead of the
  * regular symbol scope.
  */
-struct symbol *label_symbol(struct token *token)
+struct symbol *label_symbol(struct token *token, int used)
 {
 	struct symbol *sym = lookup_symbol(token->ident, NS_LABEL);
 	if (!sym) {
 		sym = alloc_symbol(token->pos, SYM_LABEL);
 		bind_symbol(sym, token->ident, NS_LABEL);
+		if (used)
+			sym->used = 1;
 		fn_local_symbol(sym);
 	}
 	return sym;
@@ -2139,7 +2141,7 @@ static struct token *parse_asm_labels(struct token *token, struct statement *stm
 		token = token->next; /* skip ':' and ',' */
 		if (token_type(token) != TOKEN_IDENT)
 			return token;
-		label = label_symbol(token);
+		label = label_symbol(token, 1);
 		add_symbol(labels, label);
 		token = token->next;
 	} while (match_op(token, ','));
@@ -2509,7 +2511,7 @@ static struct token *parse_goto_statement(struct token *token, struct statement
 		token = parse_expression(token->next, &stmt->goto_expression);
 		add_statement(&function_computed_goto_list, stmt);
 	} else if (token_type(token) == TOKEN_IDENT) {
-		struct symbol *label = label_symbol(token);
+		struct symbol *label = label_symbol(token, 1);
 		stmt->goto_label = label;
 		check_label_usage(label, stmt->pos);
 		token = token->next;
@@ -2563,7 +2565,7 @@ static struct token *statement(struct token *token, struct statement **tree)
 			return s->op->statement(token, stmt);
 
 		if (match_op(token->next, ':')) {
-			struct symbol *s = label_symbol(token);
+			struct symbol *s = label_symbol(token, 0);
 			token = skip_attributes(token->next->next);
 			if (s->stmt) {
 				sparse_error(stmt->pos, "label '%s' redefined", show_ident(s->ident));
diff --git a/parse.h b/parse.h
index 2cfdd872e621..5ac9a23ba363 100644
--- a/parse.h
+++ b/parse.h
@@ -124,7 +124,7 @@ extern struct symbol_list *function_computed_target_list;
 extern struct statement_list *function_computed_goto_list;
 
 extern struct token *parse_expression(struct token *, struct expression **);
-extern struct symbol *label_symbol(struct token *token);
+extern struct symbol *label_symbol(struct token *token, int used);
 extern void check_label_usage(struct symbol *label, struct position use_pos);
 
 extern int show_statement(struct statement *);
-- 
2.26.2

[PATCH v1 28/28] bad-label: respect attribute((unused))

[Luc Van Oostenryck]

Currently, attributes on labels were simply ignored. This was fine
since nothing was done wth them anyway.

But now that Sparse can give a warning for unused labels it would
be nice to also support the attribute 'unused' not to issues the
warning when not desired.

So, add a small helper around handle_attributes() and use this
instead of skipping the attributes.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 parse.c                   | 11 ++++++++++-
 scope.c                   |  2 ++
 symbol.h                  |  1 +
 validation/label-unused.c |  6 ++++++
 4 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/parse.c b/parse.c
index a8e4a02e90e4..3cd84a3c7703 100644
--- a/parse.c
+++ b/parse.c
@@ -2554,6 +2554,15 @@ static struct token *parse_range_statement(struct token *token, struct statement
 	return expect(token, ';', "after range statement");
 }
 
+static struct token *handle_label_attributes(struct token *token, struct symbol *label)
+{
+	struct decl_state ctx = { };
+
+	token = handle_attributes(token, &ctx, KW_ATTRIBUTE);
+	label->label_modifiers = ctx.ctype.modifiers;
+	return token;
+}
+
 static struct token *statement(struct token *token, struct statement **tree)
 {
 	struct statement *stmt = alloc_statement(token->pos, STMT_NONE);
@@ -2566,7 +2575,7 @@ static struct token *statement(struct token *token, struct statement **tree)
 
 		if (match_op(token->next, ':')) {
 			struct symbol *s = label_symbol(token, 0);
-			token = skip_attributes(token->next->next);
+			token = handle_label_attributes(token->next->next, s);
 			if (s->stmt) {
 				sparse_error(stmt->pos, "label '%s' redefined", show_ident(s->ident));
 				// skip the label to avoid multiple definitions
diff --git a/scope.c b/scope.c
index 03593d823d6d..4c1badb2c135 100644
--- a/scope.c
+++ b/scope.c
@@ -155,6 +155,8 @@ void end_label_scope(void)
 	FOR_EACH_PTR(label_scope->symbols, sym) {
 		if (!sym->stmt || sym->used)
 			continue;
+		if (sym->label_modifiers & MOD_UNUSED)
+			continue;
 		warning(sym->pos, "unused label '%s'", show_ident(sym->ident));
 	} END_FOR_EACH_PTR(sym);
 
diff --git a/symbol.h b/symbol.h
index 2293d06dd4fb..6f90479505cc 100644
--- a/symbol.h
+++ b/symbol.h
@@ -170,6 +170,7 @@ struct symbol {
 		struct /* NS_LABEL */ {
 			struct scope *label_scope;
 			struct position label_pos;
+			unsigned long label_modifiers;
 		};
 		struct /* NS_SYMBOL */ {
 			unsigned long	offset;
diff --git a/validation/label-unused.c b/validation/label-unused.c
index a654ef7742be..e3f255e1b5de 100644
--- a/validation/label-unused.c
+++ b/validation/label-unused.c
@@ -13,6 +13,12 @@ l:
 	});
 }
 
+static void baz(void)
+{
+l: __attribute__((unused));
+	return;
+}
+
 /*
  * check-name: label-unused
  *
-- 
2.26.2

Re: [SPARSE v2 00/28] detect invalid branches

[Linus Torvalds]

On Mon, May 18, 2020 at 5:57 PM Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
>
> The goals of the patches in this series are:
> *) to detect such gotos at evaluation time;
> *) issue a sensible error message;
> *) avoid the linearization of functions with invalid gotos.

Ack. Apart from that one question I had, which I didn't actually
verify whether it was a problem for the insane test-case I posted.

I only _read_ the patches, I didn't actually apply and test them in any way.

               Linus

Re: [SPARSE v2 00/28] detect invalid branches

[Luc Van Oostenryck]

On Mon, May 18, 2020 at 06:41:07PM -0700, Linus Torvalds wrote:
> On Mon, May 18, 2020 at 5:57 PM Luc Van Oostenryck
> <luc.vanoostenryck@gmail.com> wrote:
> >
> > The goals of the patches in this series are:
> > *) to detect such gotos at evaluation time;
> > *) issue a sensible error message;
> > *) avoid the linearization of functions with invalid gotos.
> 
> Ack. Apart from that one question I had, which I didn't actually
> verify whether it was a problem for the insane test-case I posted.
> 
> I only _read_ the patches, I didn't actually apply and test them in any way.

Sure, I understand that very well. I tend to give to this kind of
series a decent amount of testing and its mostly fully automatic
anyway (and, I think, the tests cover well most situations),
but having another eye and another opinion on the patches is very
much appreciated.

-- Luc