| Message ID | 1410157803-3658-1-git-send-email-ard.biesheuvel@linaro.org (mailing list archive) |
|---|---|
| State | Mainlined, archived |
| Headers | show |
On 8 September 2014 08:30, Ard Biesheuvel <ard.biesheuvel@linaro.org> 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. > > To avoid producing a warning on the inverse case, i.e., > > static void bar(void) > { > void *foo = bar; > } > > we only address the case where the function pointer is the target of > an assignment. > > Reviewed-by: Josh Triplett <josh@joshtriplett.org> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > --- > v3: add rationale for restriction to assignments to commit message > add R-b > > v2: only treat function pointers as pointers to const data when they are the > target of an assignment > > 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 > Ping? -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Oct 6, 2014 at 10:44 PM, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: >> we only address the case where the function pointer is the target of >> an assignment. I apply the patch in my private branch but haven't push out yet. I just want to confirm, you didn't find this kind of assignment in the kernel, right? Chris -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 8 October 2014 20:00, Christopher Li <sparse@chrisli.org> wrote: > On Mon, Oct 6, 2014 at 10:44 PM, Ard Biesheuvel > <ard.biesheuvel@linaro.org> wrote: >>> we only address the case where the function pointer is the target of >>> an assignment. > > I apply the patch in my private branch but haven't push out yet. > > I just want to confirm, you didn't find this kind of assignment in the > kernel, right? > I noticed this patch https://git.kernel.org/cgit/linux/kernel/git/kvmarm/kvmarm.git/commit/?h=queue&id=de56fb1923ca11f428bf557870e0faa99f38762e and saw that it was bogus: changing the return type of the pointed-to function to 'const int' makes no sense at all, and yet it shuts up sparse. With my sparse patch, that patch could have been dropped (although I think it was merged after all)
On 8 October 2014 20:06, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > On 8 October 2014 20:00, Christopher Li <sparse@chrisli.org> wrote: >> On Mon, Oct 6, 2014 at 10:44 PM, Ard Biesheuvel >> <ard.biesheuvel@linaro.org> wrote: >>>> we only address the case where the function pointer is the target of >>>> an assignment. >> >> I apply the patch in my private branch but haven't push out yet. >> >> I just want to confirm, you didn't find this kind of assignment in the >> kernel, right? >> > > I noticed this patch > > https://git.kernel.org/cgit/linux/kernel/git/kvmarm/kvmarm.git/commit/?h=queue&id=de56fb1923ca11f428bf557870e0faa99f38762e > > and saw that it was bogus: changing the return type of the pointed-to > function to 'const int' makes no sense at all, and yet it shuts up > sparse. > > With my sparse patch, that patch could have been dropped (although I > think it was merged after all) > Ping? -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Oct 14, 2014 at 8:55 PM, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > > Ping? It is already applied and pushed as c1763a249aba0a40bd326c845c2a146132a02448 Chris -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 15 October 2014 03:34, Christopher Li <sparse@chrisli.org> wrote: > On Tue, Oct 14, 2014 at 8:55 PM, Ard Biesheuvel > <ard.biesheuvel@linaro.org> wrote: >> >> Ping? > > It is already applied and pushed as c1763a249aba0a40bd326c845c2a146132a02448 > OK, thanks! Which upstream repo is that? I couldn't find it on git://git.kernel.org/pub/scm/devel/sparse/sparse.git regards, Ard. -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Oct 15, 2014 at 1:16 PM, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > > Which upstream repo is that? I couldn't find it on > git://git.kernel.org/pub/scm/devel/sparse/sparse.git https://git.kernel.org/cgit/devel/sparse/chrisl/sparse.git/ Chris -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 15 October 2014 09:06, Christopher Li <sparse@chrisli.org> wrote: > On Wed, Oct 15, 2014 at 1:16 PM, Ard Biesheuvel > <ard.biesheuvel@linaro.org> wrote: >> >> Which upstream repo is that? I couldn't find it on >> git://git.kernel.org/pub/scm/devel/sparse/sparse.git > > https://git.kernel.org/cgit/devel/sparse/chrisl/sparse.git/ > OK, thanks! -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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;
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. To avoid producing a warning on the inverse case, i.e., static void bar(void) { void *foo = bar; } we only address the case where the function pointer is the target of an assignment. Reviewed-by: Josh Triplett <josh@joshtriplett.org> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> --- v3: add rationale for restriction to assignments to commit message add R-b v2: only treat function pointers as pointers to const data when they are the target of an assignment evaluate.c | 9 +++++++++ 1 file changed, 9 insertions(+)