From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-io1-xd2e.google.com (mail-io1-xd2e.google.com [IPv6:2607:f8b0:4864:20::d2e]) by sourceware.org (Postfix) with ESMTPS id AF7333858D20 for ; Thu, 9 Mar 2023 18:45:08 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org AF7333858D20 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=adacore.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=adacore.com Received: by mail-io1-xd2e.google.com with SMTP id q6so1090088iot.2 for ; Thu, 09 Mar 2023 10:45:08 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=adacore.com; s=google; t=1678387508; h=mime-version:user-agent:message-id:in-reply-to:date:references :subject:cc:to:from:from:to:cc:subject:date:message-id:reply-to; bh=ynWH5Vk4ewa17d1PSpo0MY5pXtYhwogysUxUIpgdV78=; b=cJLipvOCeFCj0JGCHz3G+5NoBzZlfFm09mY3hKp+eNipLKVpu+cALdTiep1ZUJFjUH X6xrL+yp0+AQmcM9MIPhd+J/m1OlwKWnVEkZJZWf+78aWYsqxU+uWemYeLbAY0ziLmPe aD4XXfs4fSnbgCHfeoFjfq7Y1qfpJr2C2v1gJWWtyIDepphE71zYFFQYlg96bYZQPUmB ouuSkR3urMdpOyuuGS5aoo+OPkUWr+euMdhbGOSaw9S7Ip8G0+lcL9RqdK6coa+LzXyA Ph+4B0+NC0HjsLbKk+yAV7Q95jNx/EByWLd5/QZSmsFvsW1zpvXRISOZ6iQpiHMmzYPh pidw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1678387508; h=mime-version:user-agent:message-id:in-reply-to:date:references :subject:cc:to:from:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=ynWH5Vk4ewa17d1PSpo0MY5pXtYhwogysUxUIpgdV78=; b=E+CQiEFVR1G+NPBabk/gW+ZEM0IxOxNj8wkWCTd/US6j0UUSekt9xH5kBl3oEpkhgF DallQ7ZfrtGpZvcHm1yhaZl6Vss3FKKHdJT53hCR1DztIgs9YQwAWQqG//9foG/JL0xf Zlt720UKOexJKZbAHh+Wyv02DRgYE98F60b9tJPCiZnJw7+zqMWZzYNAa1eCjMqFsZ0/ yH8iivGGJ73q14aHajMEU6oJZOC/anvhYEfEdbHzPox94DSlmExtd0D05uAvMg6HJksD X8TRobFH36IlE9r2f3ljgv/OhgmhhnoZCHLNkcJ0MKL5+tFhjTnIEJ/n+ICX43LKC4+X xjNQ== X-Gm-Message-State: AO0yUKXrJ1rqpK3a0XYX7F8nD9y8i/CfqjLGtdQ4gtldIGlH59Y5QRXp rtc0kM8NAPQPUaH1igJUpBQgaA== X-Google-Smtp-Source: AK7set9MNFokyQuDhDogezlvNBMFAgDRnn66UpR+UdJiqYTzsZzHwqhcyL888YBmz5jOg5Mod13BtQ== X-Received: by 2002:a5d:929a:0:b0:74d:770:f4b7 with SMTP id s26-20020a5d929a000000b0074d0770f4b7mr14689635iom.9.1678387507776; Thu, 09 Mar 2023 10:45:07 -0800 (PST) Received: from murgatroyd (71-211-185-113.hlrn.qwest.net. [71.211.185.113]) by smtp.gmail.com with ESMTPSA id o14-20020a92dace000000b00313f1b861b7sm5417423ilq.51.2023.03.09.10.45.07 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 09 Mar 2023 10:45:07 -0800 (PST) From: Tom Tromey To: Tom de Vries Cc: Tom Tromey , Tom Tromey via Gdb-patches Subject: Re: [PATCH] Fix DAP stackTrace through frames without debuginfo References: <20230215194847.3805619-1-tromey@adacore.com> <874jqx7vux.fsf@tromey.com> <87fsaenfj9.fsf@tromey.com> <1c5fbdb3-4307-eba8-9976-673a11655bee@suse.de> X-Attribution: Tom Date: Thu, 09 Mar 2023 11:45:06 -0700 In-Reply-To: <1c5fbdb3-4307-eba8-9976-673a11655bee@suse.de> (Tom de Vries's message of "Thu, 9 Mar 2023 17:10:02 +0100") Message-ID: <87356dojbh.fsf@tromey.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.2 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain 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.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Tom> Sure. Still a FAIL, but a different error. Log attached. Ok, I finally figured it out (I think) and it's a latent bug in frames.py. I have no idea how/why this ever worked. Tom commit 7190893e3563f7353a6ebbf130e7aa6a145c18a1 Author: Tom Tromey Date: Thu Mar 9 07:47:29 2023 -0700 Implement hash function for gdb.Frame Tom de Vries pointed out that one DAP test failed on Python 3.6 because gdb.Frame is not hashable. This error was mildly confusing, because I think the real issue was this: - pair = (gdb.selected_thread().global_num, frame.level) Here, 'frame.level' is a bound method, not just a simple value. This patch adds a hash function to gdb.Frame, then fixes frames.py to use it directly. diff --git a/gdb/frame-id.h b/gdb/frame-id.h index 8ddf7d11408..53abbe203bf 100644 --- a/gdb/frame-id.h +++ b/gdb/frame-id.h @@ -112,6 +112,9 @@ struct frame_id /* Return a string representation of this frame id. */ std::string to_string () const; + /* A hash function for this frame_id. */ + hashval_t hash () const; + /* Returns true when this frame_id and R identify the same frame. */ bool operator== (const frame_id &r) const; diff --git a/gdb/frame.c b/gdb/frame.c index 9b867b6cd9c..478bd881b63 100644 --- a/gdb/frame.c +++ b/gdb/frame.c @@ -228,26 +228,12 @@ frame_addr_hash (const void *ap) { const frame_info *frame = (const frame_info *) ap; const struct frame_id f_id = frame->this_id.value; - hashval_t hash = 0; gdb_assert (f_id.stack_status != FID_STACK_INVALID || f_id.code_addr_p || f_id.special_addr_p); - if (f_id.stack_status == FID_STACK_VALID) - hash = iterative_hash (&f_id.stack_addr, - sizeof (f_id.stack_addr), hash); - if (f_id.code_addr_p) - hash = iterative_hash (&f_id.code_addr, - sizeof (f_id.code_addr), hash); - if (f_id.special_addr_p) - hash = iterative_hash (&f_id.special_addr, - sizeof (f_id.special_addr), hash); - - char user_created_p = f_id.user_created_p; - hash = iterative_hash (&user_created_p, sizeof (user_created_p), hash); - - return hash; + return f_id.hash (); } /* Internal equality function for the hash table. This function @@ -796,6 +782,29 @@ frame_id_artificial_p (frame_id l) return l.artificial_depth != 0; } +/* See frame-id.h. */ + +hashval_t +frame_id::hash () const +{ + hashval_t hash = 0; + + if (stack_status == FID_STACK_VALID) + hash = iterative_hash (&stack_addr, + sizeof (stack_addr), hash); + if (code_addr_p) + hash = iterative_hash (&code_addr, + sizeof (code_addr), hash); + if (special_addr_p) + hash = iterative_hash (&special_addr, + sizeof (special_addr), hash); + + char user_created_p = user_created_p; + hash = iterative_hash (&user_created_p, sizeof (user_created_p), hash); + + return hash; +} + bool frame_id::operator== (const frame_id &r) const { diff --git a/gdb/python/lib/gdb/dap/frames.py b/gdb/python/lib/gdb/dap/frames.py index 337bbedae0f..4b554f3d920 100644 --- a/gdb/python/lib/gdb/dap/frames.py +++ b/gdb/python/lib/gdb/dap/frames.py @@ -18,8 +18,8 @@ import gdb from .startup import in_gdb_thread -# Map from frame (thread,level) pair to frame ID numbers. Note we -# can't use the frame itself here as it is not hashable. +# Map from frame pair to the frame ID number that is passed back to +# the client. _frame_ids = {} # Map from frame ID number back to frames. @@ -42,12 +42,11 @@ gdb.events.cont.connect(_clear_frame_ids) def frame_id(frame): """Return the frame identifier for FRAME.""" global _frame_ids, _id_to_frame - pair = (gdb.selected_thread().global_num, frame.level) - if pair not in _frame_ids: + if frame not in _frame_ids: id = len(_frame_ids) - _frame_ids[pair] = id + _frame_ids[frame] = id _id_to_frame[id] = frame - return _frame_ids[pair] + return _frame_ids[frame] @in_gdb_thread diff --git a/gdb/python/py-frame.c b/gdb/python/py-frame.c index ecd55d2e568..099f39ac44e 100644 --- a/gdb/python/py-frame.c +++ b/gdb/python/py-frame.c @@ -687,6 +687,23 @@ gdbpy_frame_stop_reason_string (PyObject *self, PyObject *args) return PyUnicode_Decode (str, strlen (str), host_charset (), NULL); } +/* Implement the hash method. */ + +static Py_hash_t +frapy_hash (PyObject *self) +{ + frame_object *self_frame = (frame_object *) self; + + hashval_t hash = self_frame->frame_id.hash (); + hash <<= 1; + hash |= self_frame->frame_id_is_next; + + Py_hash_t result = (Py_hash_t) hash; + if (result == -1) + result = 0; + return result; +} + /* Implements the equality comparison for Frame objects. All other comparison operators will throw a TypeError Python exception, as they aren't valid for frames. */ @@ -816,7 +833,7 @@ PyTypeObject frame_object_type = { 0, /* tp_as_number */ 0, /* tp_as_sequence */ 0, /* tp_as_mapping */ - 0, /* tp_hash */ + frapy_hash, /* tp_hash */ 0, /* tp_call */ frapy_str, /* tp_str */ 0, /* tp_getattro */