public inbox for gdb-prs@sourceware.org help / color / mirror / Atom feed
From: "cvs-commit at gcc dot gnu.org" <sourceware-bugzilla@sourceware.org> To: gdb-prs@sourceware.org Subject: [Bug exp/28980] GDB crashes when using GDB/MI and python pretty printers in some cases Date: Wed, 06 Apr 2022 21:02:14 +0000 [thread overview] Message-ID: <bug-28980-4717-W1pg1XR5Ae@http.sourceware.org/bugzilla/> (raw) In-Reply-To: <bug-28980-4717@http.sourceware.org/bugzilla/> https://sourceware.org/bugzilla/show_bug.cgi?id=28980 --- Comment #4 from cvs-commit at gcc dot gnu.org <cvs-commit at gcc dot gnu.org> --- The gdb-12-branch branch has been updated by Simon Marchi <simark@sourceware.org>: https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=439385561588f12c1df670aaa92af9babd98f2ac commit 439385561588f12c1df670aaa92af9babd98f2ac Author: Simon Marchi <simon.marchi@efficios.com> Date: Mon Apr 4 17:45:59 2022 -0400 gdb: don't copy entirely optimized out values in value_copy Bug 28980 shows that trying to value_copy an entirely optimized out value causes an internal error. The original bug report involves MI and some Python pretty printer, and is quite difficult to reproduce, but another easy way to reproduce (that is believed to be equivalent) was proposed: $ ./gdb -q -nx --data-directory=data-directory -ex "py print(gdb.Value(gdb.Value(5).type.optimized_out()))" /home/smarchi/src/binutils-gdb/gdb/value.c:1731: internal-error: value_copy: Assertion `arg->contents != nullptr' failed. This is caused by 5f8ab46bc691 ("gdb: constify parameter of value_copy"). It added an assertion that the contents buffer is allocated if the value is not lazy: if (!value_lazy (val)) { gdb_assert (arg->contents != nullptr); This was based on the comment on value::contents, which suggest that this is the case: /* Actual contents of the value. Target byte-order. NULL or not valid if lazy is nonzero. */ gdb::unique_xmalloc_ptr<gdb_byte> contents; However, it turns out that it can also be nullptr also if the value is entirely optimized out, for example on exit of allocate_optimized_out_value. That function creates a lazy value, marks the entire value as optimized out, and then clears the lazy flag. But contents remains nullptr. This wasn't a problem for value_copy before, because it was calling value_contents_all_raw on the input value, which caused contents to be allocated before doing the copy. This means that the input value to value_copy did not have its contents allocated on entry, but had it allocated on exit. The result value had it allocated on exit. And that we copied bytes for an entirely optimized out value (i.e. meaningless bytes). From here I see two choices: 1. respect the documented invariant that contents is nullptr only and only if the value is lazy, which means making allocate_optimized_out_value allocate contents 2. extend the cases where contents can be nullptr to also include values that are entirely optimized out (note that you could still have some entirely optimized out values that do have contents allocated, it depends on how they were created) and adjust value_copy accordingly Choice #1 is safe, but less efficient: it's not very useful to allocate a buffer for an entirely optimized out value. It's even a bit less efficient than what we had initially, because values coming out of allocate_optimized_out_value would now always get their contents allocated. Choice #2 would be more efficient than what we had before: giving an optimized out value without allocated contents to value_copy would result in an optimized out value without allocated contents (and the input value would still be without allocated contents on exit). But it's more risky, since it's difficult to ensure that all users of the contents (through the various_contents* accessors) are all fine with that new invariant. In this patch, I opt for choice #2, since I think it is a better direction than choice #1. #1 would be a pessimization, and if we go this way, I doubt that it will ever be revisited, it will just stay that way forever. Add a selftest to test this. I initially started to write it as a Python test (since the reproducer is in Python), but a selftest is more straightforward. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28980 Change-Id: I6e2f5c0ea804fafa041fcc4345d47064b5900ed7 -- You are receiving this mail because: You are on the CC list for the bug.
next prev parent reply other threads:[~2022-04-06 21:02 UTC|newest] Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-03-19 13:06 [Bug exp/28980] New: " ssbssa at sourceware dot org 2022-03-19 13:09 ` [Bug exp/28980] " ssbssa at sourceware dot org 2022-03-19 13:10 ` ssbssa at sourceware dot org 2022-03-22 13:37 ` jan at vrany dot io 2022-04-06 20:11 ` cvs-commit at gcc dot gnu.org 2022-04-06 21:02 ` cvs-commit at gcc dot gnu.org [this message] 2022-04-06 21:02 ` simark at simark dot ca
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=bug-28980-4717-W1pg1XR5Ae@http.sourceware.org/bugzilla/ \ --to=sourceware-bugzilla@sourceware.org \ --cc=gdb-prs@sourceware.org \ /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: linkBe 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).