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 gdb/27994] ASan crash when printing expression involving function call
Date: Fri, 25 Jun 2021 19:51:07 +0000	[thread overview]
Message-ID: <bug-27994-4717-qojYUr6uSm@http.sourceware.org/bugzilla/> (raw)
In-Reply-To: <bug-27994-4717@http.sourceware.org/bugzilla/>

https://sourceware.org/bugzilla/show_bug.cgi?id=27994

--- Comment #9 from cvs-commit at gcc dot gnu.org <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Andrew Burgess <aburgess@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=13221aec0d87c701c463d4fa54aa70096d0f43a7

commit 13221aec0d87c701c463d4fa54aa70096d0f43a7
Author: Andrew Burgess <andrew.burgess@embecosm.com>
Date:   Tue Jun 22 10:17:53 2021 +0100

    gdb: replace NULL terminated array with array_view

    After the previous commit, this commit updates the value_struct_elt
    function to take an array_view rather than a NULL terminated array of
    values.

    The requirement for a NULL terminated array of values actually stems
    from typecmp, so the change from an array to array_view needs to be
    propagated through to this function.

    While making this change I noticed that this fixes another bug, in
    value_x_binop and value_x_unop GDB creates an array of values which
    doesn't have a NULL at the end.  An array_view of this array is passed
    to value_user_defined_op, which then unpacks the array_view and passed
    the raw array to value_struct_elt, but only if the language is not
    C++.

    As value_x_binop and value_x_unop can only request member functions
    with the names of C++ operators, then most of the time, assuming the
    inferior is not a C++ program, then GDB will not find a matching
    member function with the call to value_struct_elt, and so typecmp will
    never be called, and so, GDB will avoid undefined behaviour.

    However, it is worth remembering that, when GDB's language is set to
    "auto", the current language is selected based on the language of the
    current compilation unit.  As C++ programs usually link against libc,
    which is written in C, then, if the inferior is stopped in libc GDB
    will set the language to C.  And so, it is possible that we will end
    up using value_struct_elt to try and lookup, and match, a C++
    operator.  If this occurs then GDB will experience undefined
    behaviour.

    I have extended the test added in the previous commit to also cover
    this case.

    Finally, this commit changes the API from passing around a pointer to
    an array to passing around a pointer to an array_view.  The reason for
    this is that we need to be able to distinguish between the cases where
    we call value_struct_elt with no arguments, i.e. we are looking up a
    struct member, but we either don't have the arguments we want to pass
    yet, or we don't expect there to be any need for GDB to use the
    argument types to resolve any overloading; and the second case where
    we call value_struct_elt looking for a function that takes no
    arguments, that is, the argument list is empty.

    NOTE: While writing this I realise that if we pass an array_view at
    all then it will always have at least one item in it, the `this'
    pointer for the object we are planning to call the method on.  So we
    could, I guess, pass an empty array_view to indicate the case where we
    don't know anything about the arguments, and when the array_view is
    length 1 or more, it means we do have the arguments.  However, though
    we could do this, I don't think this would be better, the length 0 vs
    length 1 difference seems a little too subtle, I think that there's a
    better solution...

    I think a better solution would be to wrap the array_view in a
    gdb::optional, this would make the whole, do we have an array view or
    not question explicit.

    I haven't done this as part of this commit as making that change is
    much more extensive, every user of value_struct_elt will need to be
    updated, and as this commit already contains a bug fix, I wanted to
    keep the large refactoring in a separate commit, so, check out the
    next commit for the use of gdb::optional.

    gdb/ChangeLog:

            PR gdb/27994
            * eval.c (structop_base_operation::evaluate_funcall): Pass
            array_view instead of array to value_struct_elt.
            * valarith.c (value_user_defined_op): Likewise.
            * valops.c (typecmp): Change parameter type from array pointer to
            array_view.  Update header comment, and update body accordingly.
            (search_struct_method): Likewise.
            (value_struct_elt): Likewise.
            * value.h (value_struct_elt): Update declaration.

    gdb/testsuite/ChangeLog:

            PR gdb/27994
            * gdb.cp/method-call-in-c.cc (struct foo_type): Add operator+=,
            change initial value of var member variable.
            (main): Make use of foo_type's operator+=.
            * gdb.cp/method-call-in-c.exp: Test use of operator+=.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

  parent reply	other threads:[~2021-06-25 19:51 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-18  4:17 [Bug gdb/27994] New: " simark at simark dot ca
2021-06-19 12:34 ` [Bug gdb/27994] " ssbssa at sourceware dot org
2021-06-21 16:41 ` simark at simark dot ca
2021-06-21 21:49 ` andrew.burgess at embecosm dot com
2021-06-21 22:40 ` andrew.burgess at embecosm dot com
2021-06-21 23:00 ` andrew.burgess at embecosm dot com
2021-06-21 23:40 ` andrew.burgess at embecosm dot com
2021-06-22  0:13 ` simark at simark dot ca
2021-06-22  8:29 ` andrew.burgess at embecosm dot com
2021-06-22 13:48 ` simark at simark dot ca
2021-06-25 19:51 ` cvs-commit at gcc dot gnu.org
2021-06-25 19:51 ` cvs-commit at gcc dot gnu.org [this message]
2021-06-25 19:52 ` andrew.burgess at embecosm dot com

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-27994-4717-qojYUr6uSm@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: 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).