[PATCH] lib: __builtin_object_size should accept void *

[PATCH] lib: __builtin_object_size should accept void *

[Michael S. Tsirkin]

I'm seeing these warnings with current Linux:
./arch/x86/include/asm/uaccess.h:705:18: warning: incorrect type in argument 1 (different modifiers)
./arch/x86/include/asm/uaccess.h:705:18:    expected void *<noident>
./arch/x86/include/asm/uaccess.h:705:18:    got void const *from

Because of this code:

static __always_inline unsigned long __must_check
copy_to_user(void __user *to, const void *from, unsigned long n)
{
        int sz = __compiletime_object_size(from);

...
}

where we have

# define __compiletime_object_size(obj) __builtin_object_size(obj, 0)

to fix, mark the argument as const void *.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---

Sorry if this has already been reported/fixed.

 lib.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib.c b/lib.c
index d5b56b0..2d66aa0 100644
--- a/lib.c
+++ b/lib.c
@@ -888,7 +888,7 @@ void declare_builtin_functions(void)
 	add_pre_buffer("extern long double __builtin_nanl(const char *);\n");
 
 	/* And some __FORTIFY_SOURCE ones.. */
-	add_pre_buffer ("extern __SIZE_TYPE__ __builtin_object_size(void *, int);\n");
+	add_pre_buffer ("extern __SIZE_TYPE__ __builtin_object_size(const void *, int);\n");
 	add_pre_buffer ("extern void * __builtin___memcpy_chk(void *, const void *, __SIZE_TYPE__, __SIZE_TYPE__);\n");
 	add_pre_buffer ("extern void * __builtin___memmove_chk(void *, const void *, __SIZE_TYPE__, __SIZE_TYPE__);\n");
 	add_pre_buffer ("extern void * __builtin___mempcpy_chk(void *, const void *, __SIZE_TYPE__, __SIZE_TYPE__);\n");
-- 
MST

Re: [PATCH] lib: __builtin_object_size should accept void *

[Christopher Li]

A similar patch has been applied:

Chris

commit f2bf519e1da89779380fd781c0eb28aae415979d
Author: Lance Richardson <lrichard@redhat.com>
Date:   Wed Sep 21 10:13:58 2016 -0400

    sparse: update __builtin_object_size() prototype

    Sparse emits a large number of warnings for the linux kernel source
    tree of the form:
        ./arch/x86/include/asm/uaccess.h:735:18: \
            warning: incorrect type in argument 1 (different modifiers)
        ./arch/x86/include/asm/uaccess.h:735:18:    expected void *<noident>
        ./arch/x86/include/asm/uaccess.h:735:18:    got void const *from

    Fix by making the first parameter to __builtin_object_size()
    type "const void *" instead of "void *", which is consistent with GCC
    behavior (the prototype for this builtin in GCC documentation is evidently
    incorrect).

    Signed-off-by: Lance Richardson <lrichard@redhat.com>
    Acked-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
    Signed-off-by: Christopher Li <sparse@chrisli.org>

diff --git a/lib.c b/lib.c
index d5b56b0..2d66aa0 100644
--- a/lib.c
+++ b/lib.c
@@ -888,7 +888,7 @@ void declare_builtin_functions(void)
        add_pre_buffer("extern long double __builtin_nanl(const char *);\n");

        /* And some __FORTIFY_SOURCE ones.. */
-       add_pre_buffer ("extern __SIZE_TYPE__
__builtin_object_size(void *, int);\n");
+       add_pre_buffer ("extern __SIZE_TYPE__
__builtin_object_size(const void *, int);\n");
        add_pre_buffer ("extern void * __builtin___memcpy_chk(void *,
const void *, __SIZE_TYPE__, __SIZE_TYPE__);\n");
        add_pre_buffer ("extern void * __builtin___memmove_chk(void *,
const void *, __SIZE_TYPE__, __SIZE_TYPE__);\n");
        add_pre_buffer ("extern void * __builtin___mempcpy_chk(void *,
const void *, __SIZE_TYPE__, __SIZE_TYPE__);\n");



On Thu, Nov 24, 2016 at 4:24 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> I'm seeing these warnings with current Linux:
> ./arch/x86/include/asm/uaccess.h:705:18: warning: incorrect type in argument 1 (different modifiers)
> ./arch/x86/include/asm/uaccess.h:705:18:    expected void *<noident>
> ./arch/x86/include/asm/uaccess.h:705:18:    got void const *from
>
> Because of this code:
>
> static __always_inline unsigned long __must_check
> copy_to_user(void __user *to, const void *from, unsigned long n)
> {
>         int sz = __compiletime_object_size(from);
>
> ...
> }
>
> where we have
>
> # define __compiletime_object_size(obj) __builtin_object_size(obj, 0)
>
> to fix, mark the argument as const void *.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>
> Sorry if this has already been reported/fixed.
>
>  lib.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib.c b/lib.c
> index d5b56b0..2d66aa0 100644
> --- a/lib.c
> +++ b/lib.c
> @@ -888,7 +888,7 @@ void declare_builtin_functions(void)
>         add_pre_buffer("extern long double __builtin_nanl(const char *);\n");
>
>         /* And some __FORTIFY_SOURCE ones.. */
> -       add_pre_buffer ("extern __SIZE_TYPE__ __builtin_object_size(void *, int);\n");
> +       add_pre_buffer ("extern __SIZE_TYPE__ __builtin_object_size(const void *, int);\n");
>         add_pre_buffer ("extern void * __builtin___memcpy_chk(void *, const void *, __SIZE_TYPE__, __SIZE_TYPE__);\n");
>         add_pre_buffer ("extern void * __builtin___memmove_chk(void *, const void *, __SIZE_TYPE__, __SIZE_TYPE__);\n");
>         add_pre_buffer ("extern void * __builtin___mempcpy_chk(void *, const void *, __SIZE_TYPE__, __SIZE_TYPE__);\n");
> --
> MST
> --
> 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

Re: [PATCH] lib: __builtin_object_size should accept void *

[Michael S. Tsirkin]

On Thu, Nov 24, 2016 at 08:16:24AM +0800, Christopher Li wrote:
> A similar patch has been applied:
> 
> Chris
> 
> commit f2bf519e1da89779380fd781c0eb28aae415979d
> Author: Lance Richardson <lrichard@redhat.com>
> Date:   Wed Sep 21 10:13:58 2016 -0400
> 
>     sparse: update __builtin_object_size() prototype
> 
>     Sparse emits a large number of warnings for the linux kernel source
>     tree of the form:
>         ./arch/x86/include/asm/uaccess.h:735:18: \
>             warning: incorrect type in argument 1 (different modifiers)
>         ./arch/x86/include/asm/uaccess.h:735:18:    expected void *<noident>
>         ./arch/x86/include/asm/uaccess.h:735:18:    got void const *from
> 
>     Fix by making the first parameter to __builtin_object_size()
>     type "const void *" instead of "void *", which is consistent with GCC
>     behavior (the prototype for this builtin in GCC documentation is evidently
>     incorrect).
> 
>     Signed-off-by: Lance Richardson <lrichard@redhat.com>
>     Acked-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
>     Signed-off-by: Christopher Li <sparse@chrisli.org>

Doesn't seem to be pushed yet - at least I don't see it in master
on git://git.kernel.org/pub/scm/devel/sparse/sparse.git


> diff --git a/lib.c b/lib.c
> index d5b56b0..2d66aa0 100644
> --- a/lib.c
> +++ b/lib.c
> @@ -888,7 +888,7 @@ void declare_builtin_functions(void)
>         add_pre_buffer("extern long double __builtin_nanl(const char *);\n");
> 
>         /* And some __FORTIFY_SOURCE ones.. */
> -       add_pre_buffer ("extern __SIZE_TYPE__
> __builtin_object_size(void *, int);\n");
> +       add_pre_buffer ("extern __SIZE_TYPE__
> __builtin_object_size(const void *, int);\n");
>         add_pre_buffer ("extern void * __builtin___memcpy_chk(void *,
> const void *, __SIZE_TYPE__, __SIZE_TYPE__);\n");
>         add_pre_buffer ("extern void * __builtin___memmove_chk(void *,
> const void *, __SIZE_TYPE__, __SIZE_TYPE__);\n");
>         add_pre_buffer ("extern void * __builtin___mempcpy_chk(void *,
> const void *, __SIZE_TYPE__, __SIZE_TYPE__);\n");
> 
> 
> 
> On Thu, Nov 24, 2016 at 4:24 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> > I'm seeing these warnings with current Linux:
> > ./arch/x86/include/asm/uaccess.h:705:18: warning: incorrect type in argument 1 (different modifiers)
> > ./arch/x86/include/asm/uaccess.h:705:18:    expected void *<noident>
> > ./arch/x86/include/asm/uaccess.h:705:18:    got void const *from
> >
> > Because of this code:
> >
> > static __always_inline unsigned long __must_check
> > copy_to_user(void __user *to, const void *from, unsigned long n)
> > {
> >         int sz = __compiletime_object_size(from);
> >
> > ...
> > }
> >
> > where we have
> >
> > # define __compiletime_object_size(obj) __builtin_object_size(obj, 0)
> >
> > to fix, mark the argument as const void *.
> >
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> >
> > Sorry if this has already been reported/fixed.
> >
> >  lib.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/lib.c b/lib.c
> > index d5b56b0..2d66aa0 100644
> > --- a/lib.c
> > +++ b/lib.c
> > @@ -888,7 +888,7 @@ void declare_builtin_functions(void)
> >         add_pre_buffer("extern long double __builtin_nanl(const char *);\n");
> >
> >         /* And some __FORTIFY_SOURCE ones.. */
> > -       add_pre_buffer ("extern __SIZE_TYPE__ __builtin_object_size(void *, int);\n");
> > +       add_pre_buffer ("extern __SIZE_TYPE__ __builtin_object_size(const void *, int);\n");
> >         add_pre_buffer ("extern void * __builtin___memcpy_chk(void *, const void *, __SIZE_TYPE__, __SIZE_TYPE__);\n");
> >         add_pre_buffer ("extern void * __builtin___memmove_chk(void *, const void *, __SIZE_TYPE__, __SIZE_TYPE__);\n");
> >         add_pre_buffer ("extern void * __builtin___mempcpy_chk(void *, const void *, __SIZE_TYPE__, __SIZE_TYPE__);\n");
> > --
> > MST
> > --
> > 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

Re: [PATCH] lib: __builtin_object_size should accept void *

[Christopher Li]

On Thu, Nov 24, 2016 at 11:28 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> Doesn't seem to be pushed yet - at least I don't see it in master
> on git://git.kernel.org/pub/scm/devel/sparse/sparse.git

That is right. I haven't push them out yet. The last few days have been a bit
hectic for me in terms patch apply and modify.  I am considering setup
a linux-next
like staging repository for sparse. Maybe sparse-next?

Chris

Re: [PATCH] lib: __builtin_object_size should accept void *

[Josh Triplett]

On Thu, Nov 24, 2016 at 12:01:21PM +0800, Christopher Li wrote:
> On Thu, Nov 24, 2016 at 11:28 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> > Doesn't seem to be pushed yet - at least I don't see it in master
> > on git://git.kernel.org/pub/scm/devel/sparse/sparse.git
> 
> That is right. I haven't push them out yet. The last few days have been a bit
> hectic for me in terms patch apply and modify.  I am considering setup
> a linux-next
> like staging repository for sparse. Maybe sparse-next?

Probably doesn't need a separate integration repository; if you want to
stage changes that you don't know you want to ship yet, a "next" branch
in the main repository would be easier for people to find and use.  If
you just want to give changes time to stabilize, you can probably put
them on "master" and just not release them yet.

Re: [PATCH] lib: __builtin_object_size should accept void *

[Christopher Li]

On Thu, Nov 24, 2016 at 12:35 PM, Josh Triplett <josh@kernel.org> wrote:
> Probably doesn't need a separate integration repository; if you want to
> stage changes that you don't know you want to ship yet, a "next" branch
> in the main repository would be easier for people to find and use.  If
> you just want to give changes time to stabilize, you can probably put
> them on "master" and just not release them yet.

I want a branch I can publish for others to review and test.
At the same time I want to be able to go back and modify the history
of the patch, merge and squash fix for the already applied patch.

That "next" branch will not be pull stable, is it acceptable?
I saw linux-next has a separate repository and the master branch
is not pull stable. At the same time there are tag like next-20161121
to reference old branch.

I assume I can do the similar thing. The question is that does it
need to be a separate repository?

As long as people don't expect pull stable from that "next" branch,
I am fine with putting it under the sparse main repository.

I guess there is other implications on the git repository size. If the
repository get rewind and modify very often, it might cause the git
pack file contain a lot of useless head not part of the stable master
branch. I don't think sparse next branch will rewind that often, but
that is some thing to consider as a separate repository.

Chris

Re: [PATCH] lib: __builtin_object_size should accept void *

[Josh Triplett]

On Thu, Nov 24, 2016 at 01:03:32PM +0800, Christopher Li wrote:
> On Thu, Nov 24, 2016 at 12:35 PM, Josh Triplett <josh@kernel.org> wrote:
> > Probably doesn't need a separate integration repository; if you want to
> > stage changes that you don't know you want to ship yet, a "next" branch
> > in the main repository would be easier for people to find and use.  If
> > you just want to give changes time to stabilize, you can probably put
> > them on "master" and just not release them yet.
> 
> I want a branch I can publish for others to review and test.
> At the same time I want to be able to go back and modify the history
> of the patch, merge and squash fix for the already applied patch.
> 
> That "next" branch will not be pull stable, is it acceptable?
> I saw linux-next has a separate repository and the master branch
> is not pull stable. At the same time there are tag like next-20161121
> to reference old branch.

Rebasing a "next" branch, as long as you clearly document that, seems
fine.

> I assume I can do the similar thing. The question is that does it
> need to be a separate repository?

I don't think so, no.

> As long as people don't expect pull stable from that "next" branch,
> I am fine with putting it under the sparse main repository.
> 
> I guess there is other implications on the git repository size. If the
> repository get rewind and modify very often, it might cause the git
> pack file contain a lot of useless head not part of the stable master
> branch. I don't think sparse next branch will rewind that often, but
> that is some thing to consider as a separate repository.

git will automatically discard any unreferenced objects when repacking.
But even if you archive the old versions of "next" using branches or
tags, it still won't take up that much space, compared to linux.git for
instance.

Re: [PATCH] lib: __builtin_object_size should accept void *

[Christopher Li]

On Thu, Nov 24, 2016 at 1:19 PM, Josh Triplett <josh@kernel.org> wrote:
> Rebasing a "next" branch, as long as you clearly document that, seems
> fine.

Great. Here is the sparse-next branch. This branch contain the latest patch
I applied. It can be rebased from time to time.

https://git.kernel.org/cgit/devel/sparse/sparse.git/log/?h=sparse-next

Chris