[PATCH v2] sparse: treat function pointers as pointers to const data

[PATCH v2] sparse: treat function pointers as pointers to const data

[Ard Biesheuvel]

This code snippet:

static void bar(void const *arg)
{
	int (*foo)(void) = arg;
}

produces the following warning:

test.c:4:28: warning: incorrect type in initializer (different modifiers)
test.c:4:28:    expected int ( *foo )( ... )
test.c:4:28:    got void const *arg

which is caused by the fact that the function pointer 'foo' is not annotated
as being a pointer to const data. However, dereferencing a function pointer
does not produce an lvalue, so a function pointer points to const data by
definition, and we should treat it accordingly.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---

OK, so while my v1 did solve the example case, it turns out universally treating
function pointers as pointers to const data produces so much fallout that it
does more harm than good.

Instead, this v2 only addresses function pointer initializers and assignments.

-- 
Ard.


 evaluate.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/evaluate.c b/evaluate.c
index 66556150ddac..a5a830978bda 100644
--- a/evaluate.c
+++ b/evaluate.c
@@ -1359,6 +1359,15 @@ static int compatible_assignment_types(struct expression *expr, struct symbol *t
 				typediff = "different address spaces";
 				goto Err;
 			}
+			/*
+			 * If this is a function pointer assignment, it is
+			 * actually fine to assign a pointer to const data to
+			 * it, as a function pointer points to const data
+			 * implicitly, i.e., dereferencing it does not produce
+			 * an lvalue.
+			 */
+			if (b1->type == SYM_FN)
+				mod1 |= MOD_CONST;
 			if (mod2 & ~mod1) {
 				typediff = "different modifiers";
 				goto Err;
-- 
1.8.3.2

Re: [PATCH v2] sparse: treat function pointers as pointers to const data

[Josh Triplett]

On Sun, Sep 07, 2014 at 02:36:53PM +0200, Ard Biesheuvel wrote:
> This code snippet:
> 
> static void bar(void const *arg)
> {
> 	int (*foo)(void) = arg;
> }
> 
> produces the following warning:
> 
> test.c:4:28: warning: incorrect type in initializer (different modifiers)
> test.c:4:28:    expected int ( *foo )( ... )
> test.c:4:28:    got void const *arg
> 
> which is caused by the fact that the function pointer 'foo' is not annotated
> as being a pointer to const data. However, dereferencing a function pointer
> does not produce an lvalue, so a function pointer points to const data by
> definition, and we should treat it accordingly.
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

As a data point: gcc does not warn about this case either, whereas it
does warn about "int *foo = arg".  So, this seems fine to me.

Reviewed-by: Josh Triplett <josh@joshtriplett.org>

> OK, so while my v1 did solve the example case, it turns out universally treating
> function pointers as pointers to const data produces so much fallout that it
> does more harm than good.

Can you elaborate on the fallout, and ideally provide that rationale in
the commit message?

- Josh Triplett

Re: [PATCH v2] sparse: treat function pointers as pointers to const data

[Ard Biesheuvel]

On 7 September 2014 19:14, Josh Triplett <josh@joshtriplett.org> wrote:
> On Sun, Sep 07, 2014 at 02:36:53PM +0200, Ard Biesheuvel wrote:
>> This code snippet:
>>
>> static void bar(void const *arg)
>> {
>>       int (*foo)(void) = arg;
>> }
>>
>> produces the following warning:
>>
>> test.c:4:28: warning: incorrect type in initializer (different modifiers)
>> test.c:4:28:    expected int ( *foo )( ... )
>> test.c:4:28:    got void const *arg
>>
>> which is caused by the fact that the function pointer 'foo' is not annotated
>> as being a pointer to const data. However, dereferencing a function pointer
>> does not produce an lvalue, so a function pointer points to const data by
>> definition, and we should treat it accordingly.
>>
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>
> As a data point: gcc does not warn about this case either, whereas it
> does warn about "int *foo = arg".  So, this seems fine to me.
>
> Reviewed-by: Josh Triplett <josh@joshtriplett.org>
>
>> OK, so while my v1 did solve the example case, it turns out universally treating
>> function pointers as pointers to const data produces so much fallout that it
>> does more harm than good.
>
> Can you elaborate on the fallout, and ideally provide that rationale in
> the commit message?
>

Treating a function pointer as pointer-to-const-data in every
occurrence results in sparse triggering on the inverse case, i.e.

extern int (*foo)(void);
void *arg = foo;

which is a pattern that is widely used in the kernel, arguably
incorrect (considering the nature of a function pointer) but not the
case I intended to address.

I will put something to that effect in the v3 commit message.

Cheers,
Ard.