From: Jeff Law <law@redhat.com>
To: David Malcolm <dmalcolm@redhat.com>, gcc-patches@gcc.gnu.org
Cc: "Martin Liška" <mliska@suse.cz>
Subject: Re: [PATCH 2/2] RFE: poisoning of invalid memory blocks and obstacks
Date: Fri, 15 Jan 2016 21:10:00 -0000 [thread overview]
Message-ID: <5699604B.7040000@redhat.com> (raw)
In-Reply-To: <1452888274-46515-2-git-send-email-dmalcolm@redhat.com>
On 01/15/2016 01:04 PM, David Malcolm wrote:
> It was difficult to track down the memory corruption bug fixed by the
> previous patch (PR jit/68446). The following patch attempts to make
> it easier to find that kind of thing by adding "poisoning" code:
>
> (A) when memory blocks are returned to the memory_block_pool's free
> list (e.g. by an obstack), fill the content with a garbage value.
>
> (B) When calling
> obstack_free (obstack, NULL);
> which leaves the obstack requiring reinitialization, fill
> the obstack's fields with a garbage value.
>
> in both cases to try fail faster for use-after-free errors.
>
> This patch isn't ready as-is:
> - I couldn't see an equivalent of CHECKING_P for libiberty, so
> case (B) would do it even in a production build.
>
> - this interracts badly with Valgrind; the latter emits messages
> about "Invalid write of size 8"
> "16 bytes inside a block of size 65,536 alloc'd"
> I think that it merely needs some extra uses of the valgrind
> annotation macros to fix.
>
> - the garbage/poison values I picked were rather arbitrary
>
> That said, it's survived bootstrap®rtesting on x86_64-pc-linux-gnu
> (in conjunction with the previous patch).
>
> Thoughts?
>
> gcc/ChangeLog:
> * memory-block.h (memory_block_pool::release): If CHECKING_P,
> fill the released block with a poison value.
>
> libiberty/ChangeLog:
> * obstack.c (_obstack_free): If OBJ is zero, poison the
> obstack to highlight the need for reinitialization.
> ---
> gcc/memory-block.h | 3 +++
> libiberty/obstack.c | 5 +++++
> 2 files changed, 8 insertions(+)
>
> diff --git a/gcc/memory-block.h b/gcc/memory-block.h
> index d7b96a3..52c17f9 100644
> --- a/gcc/memory-block.h
> +++ b/gcc/memory-block.h
> @@ -66,6 +66,9 @@ inline void
> memory_block_pool::release (void *uncast_block)
> {
> block_list *block = new (uncast_block) block_list;
> +#if CHECKING_P
> + memset (block, 0xde, block_size);
> +#endif
Is there some reason this isn't if (flag_checking) instead of a #if?
As you note, we don't currently have the concept of checking mode for
libiberty. If obstacks weren't opaque, we could wrap obstack_free
inside GCC and handle poising there.
We'll definitely want the valgrind annotations.
This feels like something we should add during gcc7's cycle. Note that
we may not be the canonical sources for obstacks -- I'm really not sure
on that one.
jeff
next prev parent reply other threads:[~2016-01-15 21:10 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-01-15 19:43 [PATCH 1/2] fix memory chunk corruption for opts_obstack (PR jit/68446) David Malcolm
2016-01-15 19:43 ` [PATCH 2/2] RFE: poisoning of invalid memory blocks and obstacks David Malcolm
2016-01-15 21:10 ` Jeff Law [this message]
2016-01-18 23:58 ` [PATCH 1/2] fix memory chunk corruption for opts_obstack (PR jit/68446) Jakub Jelinek
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=5699604B.7040000@redhat.com \
--to=law@redhat.com \
--cc=dmalcolm@redhat.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=mliska@suse.cz \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).