All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Damian Wrobel" <dwrobel@ertelnet.rybnik.pl>
To: "Seebs" <seebs@seebs.net>
Cc: "openembedded-core" <openembedded-core@lists.openembedded.org>
Subject: Re: [OE-core] [PATCH pseudo 4/4] Do not return address of local variable
Date: Tue, 27 Jul 2021 18:30:33 +0200	[thread overview]
Message-ID: <17ae8cdb530.110af7c7857737.3405175808509729608@ertelnet.rybnik.pl> (raw)
In-Reply-To: <20210727104712.385ad146@seebsdell>




 ---- On Tue, 27 Jul 2021 17:47:12 +0200 Seebs <seebs@seebs.net> wrote ----
 > On Tue, 27 Jul 2021 13:49:06 +0200
 > "Damian Wrobel" <dwrobel@ertelnet.rybnik.pl> wrote:
 > 
 > > Fixes the following warning:
 > >   pseudo_client.c: In function ‘pseudo_client_op’:
 > >   cc1: warning: function may return address of local variable
 > > [-Wreturn-local-addr] pseudo_client.c:1592:22: note: declared here
 > >    1592 |         pseudo_msg_t msg = { .type = PSEUDO_MSG_OP };
 > >         |                      ^~~
 > > 
 > > Signed-off-by: Damian Wrobel <dwrobel@ertelnet.rybnik.pl>
 > > ---
 > >  pseudo_client.c | 2 +-
 > >  1 file changed, 1 insertion(+), 1 deletion(-)
 > > 
 > > diff --git a/pseudo_client.c b/pseudo_client.c
 > > index 2583bca..f1d09ff 100644
 > > --- a/pseudo_client.c
 > > +++ b/pseudo_client.c
 > > @@ -1889,7 +1889,7 @@ pseudo_client_op(pseudo_op_t op, int access,
 > > int fd, int dirfd, const char *path case OP_CHROOT:
 > >          if (pseudo_client_chroot(path) == 0) {
 > >              /* return a non-zero value to show
 > > non-failure */
 > > -            result = &msg;
 > > +            result = pseudo_msg_dup(&msg);
 > 
 > This is a memory leak.

If the function needs to return "pseudo_msg_t *", then it has to return a valid pointer.
Returning a pointer to a local non static variable is an error as this address becomes invalid immediately after this function returns.
The returned pointer has to be freed by the caller not by the callee function itself.

I didn't mention that in the commit message but with this fix I stopped to observing mysterious pseudo abort build failures (at least so far).
So I'm curious about your experience with it. In other words the revert made here[1] didn't help for our cases.

[1] http://git.yoctoproject.org/cgit/cgit.cgi/pseudo/commit/?h=oe-core&id=b988b0a6b8afd8d459bc9a2528e834f63a3d59b2

--
Regards,
Damian

 > 
 > That said, I have no idea how the underlying bug escaped notice all
 > this time, it's definitely a bug. I think it is actually safe to just
 > make msg be static, because pseudo_client_op is protected by a lock
 > and is never executed more than once at a time.
 > 
 > On reflection: I think the way it worked is that in that case, the
 > actual message isn't looked at, just checked for nullness, but this
 > is still undefined behavior because the result is a pointer to storage
 > after the storage's lifetime, and formally you can't even check those
 > for "is or isn't null".
 > 
 > -s
 > 

  reply	other threads:[~2021-07-27 16:45 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-27 11:49 [PATCH pseudo 1/4] Remove -fno-strict-aliasing and -Wno-deprecated-declarations Damian Wrobel
2021-07-27 11:49 ` [PATCH pseudo 2/4] Do not pass null argument to pseudo_diag() Damian Wrobel
2021-07-27 15:48   ` [OE-core] " Seebs
2021-07-27 11:49 ` [PATCH pseudo 3/4] Use -pthread instead of -lpthread Damian Wrobel
2021-07-27 11:49 ` [PATCH pseudo 4/4] Do not return address of local variable Damian Wrobel
2021-07-27 15:47   ` [OE-core] " Seebs
2021-07-27 16:30     ` Damian Wrobel [this message]
2021-07-27 16:52       ` Seebs
2021-07-28  9:36         ` Damian Wrobel
2021-07-28 20:16           ` Seebs
2021-07-28 20:49             ` Andre McCurdy
2021-07-28 20:53               ` Seebs
2021-07-29 12:37             ` [OE-core] [PATCH pseudo 4/4] Do not return address of local variable - unverified Damian Wrobel
2021-07-29 15:08               ` Seebs
2021-07-27 15:49 ` [OE-core] [PATCH pseudo 1/4] Remove -fno-strict-aliasing and -Wno-deprecated-declarations Seebs
2021-07-27 16:35   ` Damian Wrobel
2021-07-27 16:46     ` Seebs

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=17ae8cdb530.110af7c7857737.3405175808509729608@ertelnet.rybnik.pl \
    --to=dwrobel@ertelnet.rybnik.pl \
    --cc=openembedded-core@lists.openembedded.org \
    --cc=seebs@seebs.net \
    --subject='Re: [OE-core] [PATCH pseudo 4/4] Do not return address of local variable' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.