On Thu, Jul 06, 2017 at 06:18:48PM -0700, Christopher Li wrote:
> On Thu, Jul 6, 2017 at 5:40 PM, Christopher Li <sparse@xxxxxxxxxxx> wrote:
> >
> > Thanks for catching it. That is a very serious bug.
> > The release will be on hold until this bug has been fix.
> > It is actually more than one of them. I will send out a separate patch
> > catching this kind of behavior.
> >
>
> Here is the patch. There is another offender in simplify.c
>
> Chris
>
> From: Christopher Li <sparse@xxxxxxxxxxx>
> Date: Thu, 6 Jul 2017 16:25:38 -0700
> Subject: [PATCH 2/2] check on delete list entry while parent caller using it.
>
> This is checking for type the bug Luc reported.
> Basiclly, the parent DO_FOR_EACH() has __nr caching the
> current ptr position. When the inner loop function delete
> one entry before the current position. The parent current
> position needs to be adjust, because the entry[] has been
> move forward.
>
> This patch only check usage FOR_EACH_XXX macro. There is
> also PREPARE_PTR_LIST haven't cover by this patch.
> It is already catching bugs left and right.
>
> Most noticablely remove_usage() inside of the kill_use_list()
> loop.
> ---
> ptrlist.h | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/ptrlist.h b/ptrlist.h
> index d09be2f..4f38a4e 100644
> --- a/ptrlist.h
> +++ b/ptrlist.h
> @@ -25,7 +25,8 @@
> #define LIST_NODE_NR (29)
>
> struct ptr_list {
> - int nr;
> + unsigned int nr:16;
> + unsigned int active:16;
Mmmm ...
I really don't like this kind of solution.
The ptrlist is already fragile and now it would contains
yet another field that need to be taken care of *and* stay
coherent with other users. I doubt this will help to reduce bugs.
Also, I don't think the principle of this solution is sound.
In particular, these die() make me nervous.
Can you explain a bit how it is working, how you can guarantee
that these die() won't be called?
-- Luc
--
To unsubscribe from this list: send the line "unsubscribe linux-sparse" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
