From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr1-x431.google.com (mail-wr1-x431.google.com [IPv6:2a00:1450:4864:20::431]) by sourceware.org (Postfix) with ESMTPS id 9EE25385802D for ; Mon, 8 Feb 2021 11:58:40 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 9EE25385802D Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=embecosm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=andrew.burgess@embecosm.com Received: by mail-wr1-x431.google.com with SMTP id z6so16678731wrq.10 for ; Mon, 08 Feb 2021 03:58:40 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=embecosm.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=/kjsdj/02Q8LfluwII00ZtH2uY1xmJ3n9WCaZcOUSYA=; b=f+TWpuG+e0RxxmLxDmPe7nAZ4v4Dm79oEKNHAfYDrttONhfqL3pKQiPamefe5xKhqW bYahDwLhzuIG3VVzzpo19TUmWIkamR7hkEMw+kCDoffxP4fMxsahPXHR4E9hiHT4rNkd jQDEjMaAB48qg3sU5cw9PovpuTEw4ck698zHvMl+WyHJW5g/FBR5N7h+90u5JB3w5Vs6 bVX8U5fPMBOTDBBSZot01hPlenp/hkg4WLsXveRJo4h9ICWgW1D11P+7nLqK+TFxDqE4 Pc1DWz9g8Pex4yZ7pfkAnvAhVn+Cx2K+5n35bq0iACGkvWYX6AuFDIFhQ9etAK7PyVli wSqw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=/kjsdj/02Q8LfluwII00ZtH2uY1xmJ3n9WCaZcOUSYA=; b=n4Z1LcFg1gbtEcNzv1usb6X4G1+QO99QlfCHNBlsotthu6xkYuJWtBeOURgZpRFau1 THuYUJQ1Sz/JfkfbUi30NDXKRii54nGfqaOEU36//x4xL6dwQS1+D679VM3OOvN1FMQn po2LGNwlq1+6/geMiDLHQP4NnwaYkn+f7Zd8DIFc1sqsGsWVE6175vJyIUuRm5NUMeej 1LMiSen/lxjuDo3VnGBlJige6dA86SEOcD7eTvUnEXfKPyHELmVP+wHIPnsPKMeeZMKT 20dIYMSANvLxwIUrvddhCftvNwTXH1eQLkN8I4IEd8iKhm+p3bJ0EB/weFVz4ygPZuXX j7NQ== X-Gm-Message-State: AOAM530Er9yIfvi/JKkHrNfZc83xas5RMLxe8i3vnFkPXmQvv5sDjPu1 ZepsNEO/Y6gVtgXZefoHDPxWX2GmkjKbsg== X-Google-Smtp-Source: ABdhPJwGYDaLBrFCSTgHbqRjZ9SjaQCB7sByMhrYUeptbee5Mti5At+gcmrpQP/cvejCRdhzSo1fhw== X-Received: by 2002:a05:6000:1141:: with SMTP id d1mr19364076wrx.47.1612785519738; Mon, 08 Feb 2021 03:58:39 -0800 (PST) Received: from localhost (host109-151-46-64.range109-151.btcentralplus.com. [109.151.46.64]) by smtp.gmail.com with ESMTPSA id i20sm19364753wmq.7.2021.02.08.03.58.39 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 08 Feb 2021 03:58:39 -0800 (PST) Date: Mon, 8 Feb 2021 11:58:38 +0000 From: Andrew Burgess To: Tom Tromey Cc: Hannes Domani , "gdb-patches@sourceware.org" Subject: Re: [PATCH 2/4] Return true in TuiWindow.is_valid only if TUI is enabled Message-ID: <20210208115838.GF265215@embecosm.com> References: <20201229170227.821-1-ssbssa@yahoo.de> <20201229170227.821-2-ssbssa@yahoo.de> <20210115164811.GK265215@embecosm.com> <20210117113102.GM265215@embecosm.com> <2060671008.4946685.1610889581434@mail.yahoo.com> <20210123175442.GC265215@embecosm.com> <20210123175551.GD265215@embecosm.com> <87ft29x9yn.fsf@tromey.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <87ft29x9yn.fsf@tromey.com> X-Operating-System: Linux/5.8.13-100.fc31.x86_64 (x86_64) X-Uptime: 11:57:48 up 61 days, 16:42, X-Editor: GNU Emacs [ http://www.gnu.org/software/emacs ] X-Spam-Status: No, score=-11.6 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 08 Feb 2021 11:58:42 -0000 * Tom Tromey [2021-02-06 12:38:56 -0700]: > >>>>> "Andrew" == Andrew Burgess writes: > > Andrew> gdb/ChangeLog: > > Andrew> * python/py-tui.c (gdbpy_tui_window) : New member > Andrew> function. > Andrew> (REQUIRE_WINDOW): Call is_valid member function. > Andrew> (REQUIRE_WINDOW_FOR_SETTER): New define. > Andrew> (gdbpy_tui_is_valid): Call is_valid member function. > Andrew> (gdbpy_tui_set_title): Remove duplicate error checking, call > Andrew> REQUIRE_WINDOW_FOR_SETTER instead. > Andrew> * tui/tui-data.h (struct tui_win_info) : Check > Andrew> tui_active too. > Andrew> * tui/tui-layout.c (tui_apply_current_layout): Add an assert. > Andrew> * tui/tui.c (tui_enable): Move setting of tui_active earlier in > Andrew> the function. > > Hi. Thanks for the patch. > > I really thought I had covered this scenario somehow, but I guess not! > Oops, sorry about that. Maybe I was thinking that tui disable would > destory the windows. Or maybe I planned to do that and then never did. > I don't recall any more. > > Andrew> - if (win->window == nullptr) > Andrew> - { > Andrew> - PyErr_Format (PyExc_RuntimeError, _("TUI window is invalid.")); > Andrew> - return -1; > Andrew> - } > Andrew> - > Andrew> - if (win->window == nullptr) > Andrew> - { > Andrew> - PyErr_Format (PyExc_TypeError, _("Cannot delete \"title\" attribute.")); > Andrew> - return -1; > Andrew> - } > > This second 'if' has the same condition. It's actually a latent bug, as > the code should read: > > if (newvalue == nullptr) > > ... and then the "if" should remain, rather than being deleted by this > patch. > > I think the patch is ok with this tweak. Thanks for pointing this out. I span this fix out into its own commit and added a test for it. Below is what I pushed. Thanks, Andrew --- commit e0c23e11da18b615c382888da8e978f16428e81b Author: Andrew Burgess Date: Mon Feb 8 11:44:51 2021 +0000 gdb/python: don't allow the user to delete window title attributes There's a bug in the python tui API. If the user tries to delete the window title attribute then this will trigger undefined behaviour in GDB due to a missing nullptr check. gdb/ChangeLog: * python/py-tui.c (gdbpy_tui_set_title): Check that the new value for the title is not nullptr. gdb/testsuite/ChangeLog: * gdb.python/tui-window.exp: Add new tests. * gdb.python/tui-window.py (TestWindow) <__init__>: Store TestWindow object into global the_window. : New method. (delete_window_title): New function. diff --git a/gdb/python/py-tui.c b/gdb/python/py-tui.c index 6e9a1462ec5..73b73f33d4a 100644 --- a/gdb/python/py-tui.c +++ b/gdb/python/py-tui.c @@ -434,7 +434,7 @@ gdbpy_tui_set_title (PyObject *self, PyObject *newvalue, void *closure) return -1; } - if (win->window == nullptr) + if (newvalue == nullptr) { PyErr_Format (PyExc_TypeError, _("Cannot delete \"title\" attribute.")); return -1; diff --git a/gdb/testsuite/gdb.python/tui-window.exp b/gdb/testsuite/gdb.python/tui-window.exp index 13e14be0654..8d86afb1449 100644 --- a/gdb/testsuite/gdb.python/tui-window.exp +++ b/gdb/testsuite/gdb.python/tui-window.exp @@ -47,6 +47,12 @@ Term::check_contents "test title" \ "This Is The Title" Term::check_contents "Window display" "Test: 0" +Term::command "python delete_window_title ()" +Term::check_contents "error message after trying to delete title" \ + "TypeError: Cannot delete \"title\" attribute\\." +Term::check_contents "title is unchanged" \ + "This Is The Title" + Term::resize 51 51 # Remember that a resize request actually does two resizes... Term::check_contents "Window was updated" "Test: 2" diff --git a/gdb/testsuite/gdb.python/tui-window.py b/gdb/testsuite/gdb.python/tui-window.py index 88a1b06f12c..3bea78846ae 100644 --- a/gdb/testsuite/gdb.python/tui-window.py +++ b/gdb/testsuite/gdb.python/tui-window.py @@ -22,7 +22,7 @@ the_window = None class TestWindow: def __init__(self, win): global the_window - the_window = win + the_window = self self.count = 0 self.win = win win.title = "This Is The Title" @@ -34,8 +34,16 @@ class TestWindow: self.win.write("Test: " + str(self.count) + " " + str(w) + "x" + str(h)) self.count = self.count + 1 + # Tries to delete the title attribute. GDB will throw an error. + def remove_title(self): + del self.win.title + gdb.register_window_type("test", TestWindow) +# Call REMOVE_TITLE on the global window object. +def delete_window_title (): + the_window.remove_title () + # A TUI window "constructor" that always fails. def failwin(win): raise RuntimeError("Whoops")