Re: [PATCH] CodeSamples/tree: Fix compiler warning on free

Leonardo Bras <[email protected]> Mon, 19 Jun 2023 23:13:32 -0300
Newsgroups	org.kernel.vger.perfbook
Message-ID	<[email protected]>
On Fri, 2023-06-16 at 03:39 -0300, Leonardo Bras wrote:
> While building the CodeSamples/datastruct/Issaquah/ directory, I can
> see
> a couple instances of this warning:
> 
> In function ‘free_treenode_cache’,
>     inlined from ‘tree_remove_all’ at tree.c:102:2,
>     inlined from ‘tree_free’ at tree.c:128:2:
> tree.c:251:9: warning: ‘free’ called on pointer ‘trp’ with nonzero
> offset 96 [-Wfree-nonheap-object]
>   251 |         free(tnp);
>       |         ^~~~~~~~~
> 
> I took a look and tried to understand what was happening:
> - tree_remove_all() calls free_treenode_cache() on it's input, which
> ends
>   up free()'ing it (!BAD_MALLOC)
> - It makes sense in most treenodes, since they are allocated with
>   alloc_treenode_cache() and the malloc() output is the same as the
> free()
>   input.
> - tree_free() calls tree_remove_all() on &trp->max, which ends up
> trying
>   to free() this same address.
> - trp is a struct treeroot, which is composed of 2 treenodes: min &
> max
> - The output of malloc() for trp ends up being different from the
> address
>   used for free(), since &trp->max is used instead, and there is an
> offset
>   since max is the second element of struct treeroot.
> 
> To solve this while keeping the tree_remove_all() generic, move
> struct traceroot->max to be the first element, and guarantee the
> address
> used for free() is the same returned by malloc().

Extra info:

Bug reproduction:
https://gitlab.com/linux-kernel/perfbook/-/jobs/4501216686#L212

With bugfix provided in this patch:
https://gitlab.com/linux-kernel/perfbook/-/jobs/4485986705


Thanks,
Leo

> 
> Signed-off-by: Leonardo Bras <[email protected]>
> ---
>  CodeSamples/datastruct/Issaquah/tree.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/CodeSamples/datastruct/Issaquah/tree.h
> b/CodeSamples/datastruct/Issaquah/tree.h
> index f007558a..bbe5e7c1 100644
> --- a/CodeSamples/datastruct/Issaquah/tree.h
> +++ b/CodeSamples/datastruct/Issaquah/tree.h
> @@ -48,8 +48,8 @@ struct treenode {
>   * Root of a tree.
>   */
>  struct treeroot {
> -       struct treenode min;
>         struct treenode max;
> +       struct treenode min;
>  } __attribute__((__aligned__(CACHE_LINE_SIZE)));
>  
>  void treenode_wire_call_rcu(void);
signature.asc (application/pgp-signature, 833 B)