From patchwork Wed Jun 27 02:24:48 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Simon Marchi X-Patchwork-Id: 28062 Received: (qmail 49527 invoked by alias); 27 Jun 2018 02:25:15 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 49037 invoked by uid 89); 27 Jun 2018 02:24:54 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, SPF_HELO_PASS, SPF_PASS autolearn=ham version=3.3.2 spammy=varobj, cur, HContent-Transfer-Encoding:8bit X-HELO: simark.ca Received: from simark.ca (HELO simark.ca) (158.69.221.121) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 27 Jun 2018 02:24:52 +0000 Received: from [10.0.0.11] (unknown [192.222.164.54]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 12F171E003; Tue, 26 Jun 2018 22:24:49 -0400 (EDT) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=simark.ca; s=mail; t=1530066289; bh=GAdpn2ko7nWYdiI/T9YHYvqYhR98qGWYwh7DOvTN73c=; h=Subject:To:References:From:Date:In-Reply-To:From; b=cgmg+dRQzcawT0692P5bESvDnqWN/1kACzX3y3FkdqatIkChCuB2uKSzjCST70Wqv YW/GXAxCEbYfmi0QI1JePX33cVlxTA2JhNZOFCrmsXdaCdregHJ7BJXNqMDSOTeGuM OApHhgWFX95bE7S0GcE8pnIbSMbTH14qYj7R/BKI= Subject: Re: [PATCH v2] Fix segfault when invoking -var-info-path-expression on a dynamic varobj To: Jan Vrany , gdb-patches@sourceware.org References: <20180625205901.29762-1-jan.vrany@fit.cvut.cz> From: Simon Marchi Message-ID: <93b2a663-2672-10c0-097d-9a9026fa4e00@simark.ca> Date: Tue, 26 Jun 2018 22:24:48 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.8.0 MIME-Version: 1.0 In-Reply-To: <20180625205901.29762-1-jan.vrany@fit.cvut.cz> On 2018-06-25 04:59 PM, Jan Vrany wrote: > Invoking -var-info-path-expression on a dynamic varobj lead either in wrong > (nonsense) result or to a segmentation fault in cplus_describe_child(). > This was caused by the fact that varobj_get_path_expr() called > cplus_path_expr_of_child() ignoring the fact the parent of the variable > is dynamic. Then, cplus_describe_child() accessed the underlaying C type > members by index, causing (i) either wrong (nonsense) expression being > returned (since dynamic child may be completely arbibtrary value) > or (ii) segmentation fault (in case the index higher than number of > underlaying C type members. > > This fixes the problem by checking whether a varobj is a child of a dynamic > varobj and, if so, reporting an error to MI consumer as described in > the documentation. Hi Jan, The patch does not compile for me: CXX varobj.o In file included from /home/simark/src/binutils-gdb/gdb/common/common-defs.h:90, from /home/simark/src/binutils-gdb/gdb/defs.h:28, from /home/simark/src/binutils-gdb/gdb/varobj.c:18: /home/simark/src/binutils-gdb/gdb/varobj.c: In function ‘const char* varobj_get_path_expr(const varobj*)’: /home/simark/src/binutils-gdb/gdb/varobj.c:969:41: error: ‘cur’ was not declared in this scope gdb_assert (!varobj_is_dynamic_p (cur)); ^~~ /home/simark/src/binutils-gdb/gdb/common/gdb_assert.h:33:13: note: in definition of macro ‘gdb_assert’ ((void) ((expr) ? 0 : \ ^~~~ > diff --git a/gdb/mi/mi-cmd-var.c b/gdb/mi/mi-cmd-var.c > index 38c59c7e66..fa47387357 100644 > --- a/gdb/mi/mi-cmd-var.c > +++ b/gdb/mi/mi-cmd-var.c > @@ -438,7 +438,15 @@ mi_cmd_var_info_path_expression (const char *command, char **argv, int argc) > > /* Get varobj handle, if a valid var obj name was specified. */ > var = varobj_get_handle (argv[0]); > - > + > + /* -var-info-path-expression is currently not valid for children of > + a dynamic varobj. */ > + for (struct varobj *cur = var->parent; cur != nullptr; cur = cur->parent) > + { > + if (varobj_is_dynamic_p (cur)) > + error (_("Invalid variable object (child of a dynamic varobj)")); > + } > + To make it simpler, instead of checking all the parents recursively here, have you considered adding a check to the varobj_get_path_expr_parent, like this? I haven't given it a very long though, but it does pass your test :). Thanks, Simon diff --git a/gdb/varobj.c b/gdb/varobj.c index f2c10ddc57ff..e601cf0b9780 100644 --- a/gdb/varobj.c +++ b/gdb/varobj.c @@ -948,6 +948,9 @@ varobj_get_path_expr_parent (const struct varobj *var) while (!is_root_p (parent) && !is_path_expr_parent (parent)) parent = parent->parent; + if (varobj_is_dynamic_p (parent)) + error (_("Invalid variable object (child of a dynamic varobj)")); + return parent; }