From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from EUR04-VI1-obe.outbound.protection.outlook.com (mail-vi1eur04on2087.outbound.protection.outlook.com [40.107.8.87]) by sourceware.org (Postfix) with ESMTPS id EA1A93858C1F for ; Wed, 22 Mar 2023 10:26:28 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org EA1A93858C1F Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=arm.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=armh.onmicrosoft.com; s=selector2-armh-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=YvvZ5uYyfrIyY9JKBBinYc6qLZLJ6KJMRVhcHgdWD6g=; b=LdNpttwDbygtvmVbD7VVNNgEQcfMHidfN3TVJxxmSpAWFIzhxGrtHCalp1c1OvjArQQn+ICsxrQkmHkFDzvGODJ4QiiUPgNsOJxhVN9IxxZc+vSzS5ixmy6Z938Wdt05rMPefZ4wsiapYMj539oMhMZEyn+VJTrrjiQi0rv9Fwk= Received: from DB6PR07CA0014.eurprd07.prod.outlook.com (2603:10a6:6:2d::24) by DB9PR08MB8340.eurprd08.prod.outlook.com (2603:10a6:10:3db::8) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6178.37; Wed, 22 Mar 2023 10:26:21 +0000 Received: from DBAEUR03FT056.eop-EUR03.prod.protection.outlook.com (2603:10a6:6:2d:cafe::1a) by DB6PR07CA0014.outlook.office365.com (2603:10a6:6:2d::24) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6222.15 via Frontend Transport; Wed, 22 Mar 2023 10:26:21 +0000 X-MS-Exchange-Authentication-Results: spf=pass (sender IP is 63.35.35.123) smtp.mailfrom=arm.com; dkim=pass (signature was verified) header.d=armh.onmicrosoft.com;dmarc=pass action=none header.from=arm.com; Received-SPF: Pass (protection.outlook.com: domain of arm.com designates 63.35.35.123 as permitted sender) receiver=protection.outlook.com; client-ip=63.35.35.123; helo=64aa7808-outbound-1.mta.getcheckrecipient.com; pr=C Received: from 64aa7808-outbound-1.mta.getcheckrecipient.com (63.35.35.123) by DBAEUR03FT056.mail.protection.outlook.com (100.127.142.88) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6222.17 via Frontend Transport; Wed, 22 Mar 2023 10:26:21 +0000 Received: ("Tessian outbound cfb430c87a1e:v135"); Wed, 22 Mar 2023 10:26:21 +0000 X-CheckRecipientChecked: true X-CR-MTA-CID: 1899dac08fb097d5 X-CR-MTA-TID: 64aa7808 Received: from cc6e7401857d.1 by 64aa7808-outbound-1.mta.getcheckrecipient.com id 18DAC483-CD2B-4FCD-9B42-4B93866461A3.1; Wed, 22 Mar 2023 10:26:14 +0000 Received: from EUR05-DB8-obe.outbound.protection.outlook.com by 64aa7808-outbound-1.mta.getcheckrecipient.com with ESMTPS id cc6e7401857d.1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384); Wed, 22 Mar 2023 10:26:14 +0000 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=ZmPaZHxUMm1r0zVkJoeqSCHf6SDjVjyYcf2q12oSTN+rWeHCJ1g2kPbYce9d0q9mejHkdtppHMvCSIaZJg1O4yyqDLR7iZ3G17XBc+pi1ksjGvOkAPQf8bOpMz34aotUIHwPlnvHcOey+tQ8lTbCFwETnz1gMwgqP0XE0ZFOnK3SacfmgeJ8ue9toEN9HrKXDEK/YyU+prJ5NN9dXpW6PCHTCB1h5DYoZ2agejGt7Bpqdkq3zpLNNGqxfEjVjcU3rKwRCu+twx80x0v0MIdeusErHFkbxPWsdYf/bi8uKy5k/xPFmI0iUuph26YPAoeVPEAKXVgp6uF/wVUD+h9RQQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=YvvZ5uYyfrIyY9JKBBinYc6qLZLJ6KJMRVhcHgdWD6g=; b=Vf4kXs9ez/ytjBXDX6LrwpeK1mqLMxlLpeGuFzWQSpzh1vVAa8ttLleO2hUVWjwclPRONFsKawyv4HHxCjfXAvhCbuobUDdHFqG3uNYSZ1Tjk4bFhU8JvoOMynj+mO0vJVzMjPAJVdqhQu//zYihd/p76AVoG3PZ4+UUeY4r9zWv3fvujRRTN24q0ZCYFp4CiENd+L8SYsPAE4AzbAKTpjDNYGuZiTrbzck1Z92x24Bpuj3ZQBwiShFMY2gsNHSfKFbhx/H4K6NHgrt1wJfrUJFrj/tx3F8+CYkqToe2aoIOsWZEZxVcMhMq7B0slMAIg3mdm5CR6J/2FFW/Z2zUKg== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=arm.com; dmarc=pass action=none header.from=arm.com; dkim=pass header.d=arm.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=armh.onmicrosoft.com; s=selector2-armh-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=YvvZ5uYyfrIyY9JKBBinYc6qLZLJ6KJMRVhcHgdWD6g=; b=LdNpttwDbygtvmVbD7VVNNgEQcfMHidfN3TVJxxmSpAWFIzhxGrtHCalp1c1OvjArQQn+ICsxrQkmHkFDzvGODJ4QiiUPgNsOJxhVN9IxxZc+vSzS5ixmy6Z938Wdt05rMPefZ4wsiapYMj539oMhMZEyn+VJTrrjiQi0rv9Fwk= Authentication-Results-Original: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=arm.com; Received: from VI1PR08MB3919.eurprd08.prod.outlook.com (2603:10a6:803:c4::31) by GV2PR08MB8146.eurprd08.prod.outlook.com (2603:10a6:150:74::9) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6178.37; Wed, 22 Mar 2023 10:26:05 +0000 Received: from VI1PR08MB3919.eurprd08.prod.outlook.com ([fe80::bced:32a3:b77e:90a6]) by VI1PR08MB3919.eurprd08.prod.outlook.com ([fe80::bced:32a3:b77e:90a6%5]) with mapi id 15.20.6178.037; Wed, 22 Mar 2023 10:26:04 +0000 Message-ID: Date: Wed, 22 Mar 2023 10:26:00 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.8.0 Subject: Re: [PATCH] aarch64: Check for valid inferior thread/regcache before reading pauth registers Content-Language: en-US To: Andrew Burgess , gdb-patches@sourceware.org, pedro@palves.net Cc: marcan@marcan.st References: <20230316103904.1947447-1-luis.machado@arm.com> <873564g0h3.fsf@redhat.com> <5ce96222-d665-5129-8d65-27c6933a6623@arm.com> <874jqfdw6y.fsf@redhat.com> From: Luis Machado In-Reply-To: <874jqfdw6y.fsf@redhat.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-ClientProxiedBy: LO2P265CA0162.GBRP265.PROD.OUTLOOK.COM (2603:10a6:600:9::30) To VI1PR08MB3919.eurprd08.prod.outlook.com (2603:10a6:803:c4::31) MIME-Version: 1.0 X-MS-TrafficTypeDiagnostic: VI1PR08MB3919:EE_|GV2PR08MB8146:EE_|DBAEUR03FT056:EE_|DB9PR08MB8340:EE_ X-MS-Office365-Filtering-Correlation-Id: 61dd530e-48ac-489b-008c-08db2abfe1a8 x-checkrecipientrouted: true NoDisclaimer: true X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam-Untrusted: BCL:0; X-Microsoft-Antispam-Message-Info-Original: aHx/IomcePCJTKO0wSDZHw3AAK8roorcsP4iv5PUehK5l8FslBAL5Tknz7Iy8COhRw2ZuuxLsmTc/yHMYERyxC+Anfs/fv7SJLXYh5dF0NByc7mBLusdIzNCSYxcWt/O/IClsPloLmfAo8NigQuw1tGt3olQz8tgs6PDFD4q9SrRl7IBWD6BdfNjmlL2Ep3+d1MXtMs3v5ut0DM1xC9FweLazUVt+XpYU4OrFb/uKWVQoFFxSzZXD906iWumZ6AQlHxlyvGDIpHOwq57Moo6Oo78Ql0FkYMtH2V9fQzUl/fb7hLyDIMQtSI7QJAM5P01+nZPa9Jsuu0PCb4Tjqa5nwsB/WNvkwjY5T52cx0AjQ82lsy7zfuJiv2ZmaFhIzYfNA1pPxOq/j/O4t5pnGX6FR36qGFM87wjpo/18UmBUKzdl+4a5SRfi+9vxHyJW3QUdO1usfNnC8lvJZzZWFOiaZewvNNO5yMzoSEaXDlXBWt8xMzKdwLDaDcp19Ar7jxq3WtEZUZTK2eNBauhlsYQ1G7LN/k+ZO9lu7YcqwcitQCmenn1q9WKDppy94Ohz6sxW8jkSwwEeEJV21/90hpkfHAF0bIRokgsXLja71IrJFwmTYKlSiZgLSb1x/zzAz+3vT/id2gZLmUdakc5vP9W4nGdZk5oljlsFRvUBZz7Iwl3ZNaRxKIkyc98/ulQ4lndxZkL/O0uFKdUyw/zRRKl6zEpHFMcYfCQPM3uOME+p+LlrkUwKvXSOCo9IAhqy8fc X-Forefront-Antispam-Report-Untrusted: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:VI1PR08MB3919.eurprd08.prod.outlook.com;PTR:;CAT:NONE;SFS:(13230025)(4636009)(376002)(366004)(39860400002)(136003)(396003)(346002)(451199018)(6512007)(31686004)(26005)(6506007)(6486002)(6666004)(53546011)(8676002)(19627235002)(316002)(186003)(83380400001)(66476007)(2616005)(66946007)(4326008)(478600001)(44832011)(8936002)(41300700001)(5660300002)(30864003)(2906002)(84970400001)(66556008)(38100700002)(31696002)(86362001)(36756003)(43740500002)(45980500001);DIR:OUT;SFP:1101; X-MS-Exchange-Transport-CrossTenantHeadersStamped: GV2PR08MB8146 Original-Authentication-Results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=arm.com; X-EOPAttributedMessage: 0 X-MS-Exchange-Transport-CrossTenantHeadersStripped: DBAEUR03FT056.eop-EUR03.prod.protection.outlook.com X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id-Prvs: 4df49a7a-75b1-4f19-7836-08db2abfd6ca X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: TPgaNlZ7OPLqEjOuSawVs17YnP+srhPp8m38s2jxRM6MxCPo58sZ9yg/XfHqa/KJ8zHTTxbTJfUpgUy7V40Qy+N0iwiJ97NT5z8TNsBc752Jst3MguNZK4JjCllDODFngQE7C/6ad36icAhsO7YeCqEzi7qOuCN+Xn4pud8oHYjWdic/CUFg5/+PeiDhZ64hRYCXmk6kSf2MkRJXLeBJ9vW8Qv8aOrgS0ai74HHbzrdpgIQL7s0Yhhs8Jmr1m2Sj3ZLMbz6foGbvnJsjAhXuEheKC9GjeJEJhyd+6dgEBmLtqnbltlLPYtHMxxHK3XlWMnw+DuQFdddkzvUi0f/vxFTutxbrrQNWAvLBtbO/Ux2foLLpBwwS7rCjxlqW+HigQcX2cC+9qI2yuls3jEX5v9/RbZqndm4X1CZ3E7jpHvplvtGPW1w238kmlI17g1I0OdTN9/R1wFN0H+bIDueLKUPkuJ7BvkD3ru+w6rN6N+GJ8ex1g+BvParPq0jXYYUv7c1hv1bP3um54zaQ0DpivNIPzuk5a5EuMs4mOsuspI2lRUqpbd2x8zPU/7uOfHwdVle4FJmPrZCAQWxglKnMT4s6aiclXLjDSgofpq55gvNMNmltsfVqPWChWqLeLLjb9FNGs9VIe9T69IndidYDPbXaFNzXVW43BwD8VcNAKEkVRZT6CoC2WOCwn4v3ODXTBfU7KD4sYVCBHHbu3R+knal5uZuGmdN/q26zJCAR5jwQs9t377mc8q5JQyr0kKJColBGaAIBjXsmzuCLpftY+A== X-Forefront-Antispam-Report: CIP:63.35.35.123;CTRY:IE;LANG:en;SCL:1;SRV:;IPV:CAL;SFV:NSPM;H:64aa7808-outbound-1.mta.getcheckrecipient.com;PTR:ec2-63-35-35-123.eu-west-1.compute.amazonaws.com;CAT:NONE;SFS:(13230025)(4636009)(346002)(39860400002)(396003)(376002)(136003)(451199018)(36840700001)(46966006)(40470700004)(2616005)(84970400001)(19627235002)(6666004)(186003)(107886003)(6486002)(47076005)(478600001)(83380400001)(336012)(70206006)(26005)(70586007)(8676002)(316002)(31686004)(6512007)(6506007)(53546011)(36860700001)(30864003)(44832011)(5660300002)(41300700001)(8936002)(82740400003)(81166007)(2906002)(4326008)(40460700003)(356005)(86362001)(36756003)(31696002)(82310400005)(40480700001)(43740500002);DIR:OUT;SFP:1101; X-OriginatorOrg: arm.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 22 Mar 2023 10:26:21.5904 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: 61dd530e-48ac-489b-008c-08db2abfe1a8 X-MS-Exchange-CrossTenant-Id: f34e5979-57d9-4aaa-ad4d-b122a662184d X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=f34e5979-57d9-4aaa-ad4d-b122a662184d;Ip=[63.35.35.123];Helo=[64aa7808-outbound-1.mta.getcheckrecipient.com] X-MS-Exchange-CrossTenant-AuthSource: DBAEUR03FT056.eop-EUR03.prod.protection.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Anonymous X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB9PR08MB8340 X-Spam-Status: No, score=-11.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,FORGED_SPF_HELO,GIT_PATCH_0,KAM_DMARC_NONE,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,RCVD_IN_VALIDITY_RPBL,SPF_HELO_PASS,SPF_NONE,TXREP,UNPARSEABLE_RELAY,WEIRD_PORT 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: On 3/20/23 20:06, Andrew Burgess wrote: > Luis Machado writes: > >> On 3/16/23 15:49, Andrew Burgess wrote: >>> Luis Machado writes: >>> >>>> There were reports of gdb throwing internal errors when calling >>>> inferior_thread ()/get_current_regcache () on a system with >>>> Pointer Authentication enabled. >>>> >>>> In such cases, gdb produces the following backtrace: >>>> >>>> ../../../repos/binutils-gdb/gdb/thread.c:86: internal-error: inferior_thread: Assertion `current_thread_ != nullptr' failed. >>>> A problem internal to GDB has been detected, >>>> further debugging may prove unreliable. >>>> ----- Backtrace ----- >>>> 0xaaaae04a571f gdb_internal_backtrace_1 >>>> ../../../repos/binutils-gdb/gdb/bt-utils.c:122 >>>> 0xaaaae04a57f3 _Z22gdb_internal_backtracev >>>> ../../../repos/binutils-gdb/gdb/bt-utils.c:168 >>>> 0xaaaae0b52ccf internal_vproblem >>>> ../../../repos/binutils-gdb/gdb/utils.c:401 >>>> 0xaaaae0b5310b _Z15internal_verrorPKciS0_St9__va_list >>>> ../../../repos/binutils-gdb/gdb/utils.c:481 >>>> 0xaaaae0e24b8f _Z18internal_error_locPKciS0_z >>>> ../../../repos/binutils-gdb/gdbsupport/errors.cc:58 >>>> 0xaaaae0a88983 _Z15inferior_threadv >>>> ../../../repos/binutils-gdb/gdb/thread.c:86 >>>> 0xaaaae0956c87 _Z20get_current_regcachev >>>> ../../../repos/binutils-gdb/gdb/regcache.c:428 >>>> 0xaaaae035223f aarch64_remove_non_address_bits >>>> ../../../repos/binutils-gdb/gdb/aarch64-tdep.c:3572 >>>> 0xaaaae03e8abb _Z31gdbarch_remove_non_address_bitsP7gdbarchm >>>> ../../../repos/binutils-gdb/gdb/gdbarch.c:3109 >>>> 0xaaaae0a692d7 memory_xfer_partial >>>> ../../../repos/binutils-gdb/gdb/target.c:1620 >>>> 0xaaaae0a695e3 _Z19target_xfer_partialP10target_ops13target_objectPKcPhPKhmmPm >>>> ../../../repos/binutils-gdb/gdb/target.c:1684 >>>> 0xaaaae0a69e9f target_read_partial >>>> ../../../repos/binutils-gdb/gdb/target.c:1937 >>>> 0xaaaae0a69fdf _Z11target_readP10target_ops13target_objectPKcPhml >>>> ../../../repos/binutils-gdb/gdb/target.c:1977 >>>> 0xaaaae0a69937 _Z18target_read_memorymPhl >>>> ../../../repos/binutils-gdb/gdb/target.c:1773 >>>> 0xaaaae08be523 ps_xfer_memory >>>> ../../../repos/binutils-gdb/gdb/proc-service.c:90 >>>> 0xaaaae08be6db ps_pdread >>>> ../../../repos/binutils-gdb/gdb/proc-service.c:124 >>>> 0x40001ed7c3b3 _td_fetch_value >>>> /build/glibc-RIFKjK/glibc-2.31/nptl_db/fetch-value.c:115 >>>> 0x40001ed791ef td_ta_map_lwp2thr >>>> /build/glibc-RIFKjK/glibc-2.31/nptl_db/td_ta_map_lwp2thr.c:194 >>>> 0xaaaae07f4473 thread_from_lwp >>>> ../../../repos/binutils-gdb/gdb/linux-thread-db.c:413 >>>> 0xaaaae07f6d6f _ZN16thread_db_target4waitE6ptid_tP17target_waitstatus10enum_flagsI16target_wait_flagE >>>> ../../../repos/binutils-gdb/gdb/linux-thread-db.c:1420 >>>> 0xaaaae0a6b33b _Z11target_wait6ptid_tP17target_waitstatus10enum_flagsI16target_wait_flagE >>>> ../../../repos/binutils-gdb/gdb/target.c:2586 >>>> 0xaaaae0789cf7 do_target_wait_1 >>>> ../../../repos/binutils-gdb/gdb/infrun.c:3825 >>>> 0xaaaae0789e6f operator() >>>> ../../../repos/binutils-gdb/gdb/infrun.c:3884 >>>> 0xaaaae078a167 do_target_wait >>>> ../../../repos/binutils-gdb/gdb/infrun.c:3903 >>>> 0xaaaae078b0af _Z20fetch_inferior_eventv >>>> ../../../repos/binutils-gdb/gdb/infrun.c:4314 >>>> 0xaaaae076652f _Z22inferior_event_handler19inferior_event_type >>>> ../../../repos/binutils-gdb/gdb/inf-loop.c:41 >>>> 0xaaaae07dc68b handle_target_event >>>> ../../../repos/binutils-gdb/gdb/linux-nat.c:4206 >>>> 0xaaaae0e25fbb handle_file_event >>>> ../../../repos/binutils-gdb/gdbsupport/event-loop.cc:573 >>>> 0xaaaae0e264f3 gdb_wait_for_event >>>> ../../../repos/binutils-gdb/gdbsupport/event-loop.cc:694 >>>> 0xaaaae0e24f9b _Z16gdb_do_one_eventi >>>> ../../../repos/binutils-gdb/gdbsupport/event-loop.cc:217 >>>> 0xaaaae080f033 start_event_loop >>>> ../../../repos/binutils-gdb/gdb/main.c:411 >>>> 0xaaaae080f1b7 captured_command_loop >>>> ../../../repos/binutils-gdb/gdb/main.c:475 >>>> 0xaaaae0810b97 captured_main >>>> ../../../repos/binutils-gdb/gdb/main.c:1318 >>>> 0xaaaae0810c1b _Z8gdb_mainP18captured_main_args >>>> ../../../repos/binutils-gdb/gdb/main.c:1337 >>>> 0xaaaae0338453 main >>>> ../../../repos/binutils-gdb/gdb/gdb.c:32 >>>> --------------------- >>>> ../../../repos/binutils-gdb/gdb/thread.c:86: internal-error: inferior_thread: Assertion `current_thread_ != nullptr' failed. >>>> A problem internal to GDB has been detected, >>>> further debugging may prove unreliable. >>>> Quit this debugging session? (y or n) >>>> >>>> This issue started after commit d88cb738e6a7a7179dfaff8af78d69250c852af1, which >>>> enabled more broad use of pointer authentication masks to remove non-address >>>> bits of pointers. >>>> >>>> The above crash happens because gdb is in the middle of handling an event, >>>> and do_target_wait_1 calls switch_to_inferior_no_thread, nullifying the >>>> current thread. This means a call to inferior_thread () will assert, and >>>> attempting to call get_current_regcache () will also call inferior_thread (), >>>> resulting in an assertion as well. >>>> >>>> There is another crash scenario after we kill a previously active inferior, in >>>> which case the gdbarch will still say we support pointer authentication but we >>>> will also have no current thread (inferior_thread () will assert etc). >>>> >>>> I thought about this for a bit, and I'm not sure what is the best way >>>> to address this other than actively checking for a valid current thread >>>> and valid current register cache. >>>> >>>> If the target has support for Pointer Authentication, gdb needs to use >>>> a couple (or 4, for bare-metal) mask registers to mask off some bits of >>>> pointers, and for that it needs to access the registers. >>>> >>>> At some points, like the one from the backtrace above, there is no active >>>> thread/current regcache because gdb is in the middle of doing event handling >>>> and switching between threads. >>>> >>>> The following patch improves this situation by checking for an active thread >>>> and register cache. If those don't exist, gdb falls back to using a default >>>> mask which should be enough for what it is trying to do at the moment. >>>> >>>> I discussed with Pedro the possibility of caching the mask register values >>>> (which are per-process and can change mid-execution), but there isn't a good >>>> spot to cache those values. Besides, the mask registers can change constantly >>>> for bare-metal debugging when switching between exception levels. >>>> >>>> In some cases, it is just not possible to get access to these mask registers, >>>> like the case where threads are running. In those cases, using a default mask >>>> to remove the non-address bits should be enough. >>>> >>>> In addition to the check for a valid current register cache, I also introduced >>>> a try/catch guard against errors reading registers, in case the current thread >>>> is running, also falling back to using a default mask to remove non-address >>>> bits. >>>> >>>> This can happen when we let threads run in the background and then we attempt >>>> to access a memory address (now that gdb is capable of reading memory even >>>> with threads running). Thus gdb will attempt to remove non-address bits >>>> of that memory access, will attempt to access registers, running into errors. >>>> >>>> Regression-tested on aarch64-linux Ubuntu 20.04. >>>> >>>> Thoughts? >>>> --- >>>> gdb/aarch64-tdep.c | 38 +++++++++++++++++++++++++++++--------- >>>> gdb/gdbthread.h | 8 ++++++++ >>>> gdb/regcache.c | 10 ++++++++++ >>>> gdb/regcache.h | 8 ++++++++ >>>> gdb/thread.c | 8 ++++++++ >>> >>> Would it be possible to write a test for this issue? Or maybe some of >>> the existing tests fail due to this issue? If there are some tests that >>> already expose this issue then it would be great to mention them in the >>> commit message. >>> >> >> Existing tests fail due to this. Two example are gdb.base/break.exp and gdb.base/access-mem-running.exp are >> two examples. I'll add those to the commit message for context. >> >> The catch here is that they will only fail on a system that supports pointer authentication natively, and >> there are only a few of those at the moment (Graviton 3, M2 and system >> QEMU are 3 examples I know of). > > I don't think that's a problem, you should just list a few tests that > exhibit failures and under what circumstances you expect to see the > failures, e.g. on a system with pointer authentication. > Done now. It will be in v2. >> >>>> 5 files changed, 63 insertions(+), 9 deletions(-) >>>> >>>> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c >>>> index 5b1b9921f87..8368592a7db 100644 >>>> --- a/gdb/aarch64-tdep.c >>>> +++ b/gdb/aarch64-tdep.c >>>> @@ -3562,13 +3562,17 @@ aarch64_remove_non_address_bits (struct gdbarch *gdbarch, CORE_ADDR pointer) >>>> select bit (55). */ >>>> CORE_ADDR mask = AARCH64_TOP_BITS_MASK; >>>> >>>> - if (tdep->has_pauth ()) >>>> + /* Make sure we have a valid active current register cache to fetch pointer >>>> + authentication masks from. In some situations, there may be no active >>>> + threads to fetch the data from (no inferiors, gdb in the middle of >>>> + handling events etc). If we don't have a valid register cache, fallback >>>> + to using a default mask to remove non-address bits. */ >>>> + if (tdep->has_pauth () && current_regcache_exists ()) >>> >>> Maybe I'm completely off the mark here, but does target_has_registers >>> not help at all here? >> >> It does, and that was the first thing I considered for guarding this block against a situation where we >> have no current thread. >> >> But... >> >>> >>> For a process_stratum_target, this should end up in this function: >>> >>> bool >>> process_stratum_target::has_registers () >>> { >>> /* Can't read registers from no inferior. */ >>> return inferior_ptid != null_ptid; >>> } >>> >>> And given that switch_to_no_thread () includes the line: >>> >>> inferior_ptid = null_ptid; >>> >> >> ... despite the call to switch_to_no_thread in switch_to_inferior_no_thread from do_target_wait_1 >> in the backtrace above, the call to ps_xfer_memory sets inferior_ptid momentarily before reading >> memory: >> >> >> static ps_err_e >> ps_xfer_memory (const struct ps_prochandle *ph, psaddr_t addr, >> gdb_byte *buf, size_t len, int write) >> { >> scoped_restore_current_inferior restore_inferior; >> set_current_inferior (ph->thread->inf); >> >> scoped_restore_current_program_space restore_current_progspace; >> set_current_program_space (ph->thread->inf->pspace); >> >> scoped_restore save_inferior_ptid = make_scoped_restore (&inferior_ptid); >> inferior_ptid = ph->thread->ptid; >> >> CORE_ADDR core_addr = ps_addr_to_core_addr (addr); >> >> int ret; >> if (write) >> ret = target_write_memory (core_addr, buf, len); >> else >> ret = target_read_memory (core_addr, buf, len); >> return (ret == 0 ? PS_OK : PS_ERR); >> } >> >> >> Maybe this shouldn't happen, or maybe it is just an unfortunate state to be in. But this >> prevents the use of target_has_registers to guard against the lack of registers, since, >> although current_thread_ is still nullptr, inferior_ptid is valid and is not null_ptid. >> >> So doing a direct check of current_thread_ through a helper function >> seemed safer. > > Personally, I find this sort of information really valuable to include > in the commit message. > > If someone is digging through this code years from now, all they'll have > is your commit message. They, like me, might think about > target_has_registers, and then they have to change GDB, test everything, > and rediscover why target_has_registers isn't going to work. > You're right. I'll add that to v2. >> >> And just to clarify, this is just one of the issues where we attempt to fetch a current >> register cache with a null current_thread_, leading to an assertion. >> >> The other issue, which is solved with a try/catch block, yields an error message about not >> being able to access registers (because the thread we want to fetch registers from is still >> running). >> >> Maybe in the future we should prevent calls to register fetch/store functions if the thread we >> want to fetch data from is still running. Right now this isn't a problem besides the AArch64 >> target though. > > I think thread_info::executing() should tell you if the thread is > running... At least, I think that's the intention. I don't think that > field is always correct, so keeping the try/catch is probably a good > idea, at least for now. > > I don't object to this patch going in as is then (maybe with an extended > commit message), but I guess you should figure out with Simon if his > idea is better or not... Thanks for the feedback. I'll wait for Simon's take on it. > > Thanks for all the details, > Andrew > > >> >>> My hope would be that when we have no thread selected, >>> target_has_registers will return false. >>> >>> The other thing I worried about before suggesting the above is this >>> depends on the current_inferior in order to identify a target stack. >>> But reading registers already makes that assumption, right.. reading a >>> register might require us to actually go get the registers, so we are >>> already assuming that the current_inferior() is sane at this point. >>> >>> Thanks, >>> Andrew >>> >>> >>> >>> >>>> { >>>> /* Fetch the PAC masks. These masks are per-process, so we can just >>>> - fetch data from whatever thread we have at the moment. >>>> + fetch data from whatever thread we have at the moment, hence why we >>>> + use the current register cache. */ >>>> >>>> - Also, we have both a code mask and a data mask. For now they are the >>>> - same, but this may change in the future. */ >>>> struct regcache *regs = get_current_regcache (); >>>> CORE_ADDR cmask, dmask; >>>> int dmask_regnum = AARCH64_PAUTH_DMASK_REGNUM (tdep->pauth_reg_base); >>>> @@ -3583,13 +3587,29 @@ aarch64_remove_non_address_bits (struct gdbarch *gdbarch, CORE_ADDR pointer) >>>> cmask_regnum = AARCH64_PAUTH_CMASK_HIGH_REGNUM (tdep->pauth_reg_base); >>>> } >>>> >>>> - if (regs->cooked_read (dmask_regnum, &dmask) != REG_VALID) >>>> - dmask = mask; >>>> + /* GDB supports reading memory from threads that are running, but in >>>> + those cases GDB cannot access registers. If reading the pointer >>>> + authentication registers fails, fallback to using a default mask. >>>> + >>>> + We could potentially get it wrong if the masks have been updated >>>> + while the thread is running, but eventually we will see the thread >>>> + stop and will be able to fetch the correct data. */ >>>> + try >>>> + { >>>> + /* We have both a code mask and a data mask. For now they are >>>> + the same, but this may change in the future. */ >>>> + if (regs->cooked_read (dmask_regnum, &dmask) != REG_VALID) >>>> + dmask = mask; >>>> >>>> - if (regs->cooked_read (cmask_regnum, &cmask) != REG_VALID) >>>> - cmask = mask; >>>> + if (regs->cooked_read (cmask_regnum, &cmask) != REG_VALID) >>>> + cmask = mask; >>>> >>>> - mask |= aarch64_mask_from_pac_registers (cmask, dmask); >>>> + mask |= aarch64_mask_from_pac_registers (cmask, dmask); >>>> + } >>>> + catch (const gdb_exception_error &e) >>>> + { >>>> + /* We failed to fetch the contents of the masks from registers. */ >>>> + } >>>> } >>>> >>>> return aarch64_remove_top_bits (pointer, mask); >>>> diff --git a/gdb/gdbthread.h b/gdb/gdbthread.h >>>> index c0f27a8a66e..ebb8f140770 100644 >>>> --- a/gdb/gdbthread.h >>>> +++ b/gdb/gdbthread.h >>>> @@ -875,6 +875,14 @@ class scoped_restore_current_thread >>>> enum language m_lang; >>>> }; >>>> >>>> +/* Return TRUE if an inferior thread exists and is currently selected, >>>> + meaning it is safe to call inferior_thread () without risking an assertion. >>>> + >>>> + Return FALSE otherwise. If so, inferior_thread () will assert if >>>> + called. */ >>>> + >>>> +extern bool inferior_thread_exists (void); >>>> + >>>> /* Returns a pointer into the thread_info corresponding to >>>> INFERIOR_PTID. INFERIOR_PTID *must* be in the thread list. */ >>>> extern struct thread_info* inferior_thread (void); >>>> diff --git a/gdb/regcache.c b/gdb/regcache.c >>>> index af76fab1a34..4685b01e9ab 100644 >>>> --- a/gdb/regcache.c >>>> +++ b/gdb/regcache.c >>>> @@ -422,6 +422,16 @@ get_thread_regcache (thread_info *thread) >>>> thread->ptid); >>>> } >>>> >>>> +/* See regcache.h. */ >>>> + >>>> +bool >>>> +current_regcache_exists (void) >>>> +{ >>>> + /* If we have an inferior thread selected, we should have a register >>>> + cache as well. */ >>>> + return inferior_thread_exists (); >>>> +} >>>> + >>>> struct regcache * >>>> get_current_regcache (void) >>>> { >>>> diff --git a/gdb/regcache.h b/gdb/regcache.h >>>> index b9ffab9950d..adff7e44cb9 100644 >>>> --- a/gdb/regcache.h >>>> +++ b/gdb/regcache.h >>>> @@ -30,6 +30,14 @@ struct address_space; >>>> class thread_info; >>>> struct process_stratum_target; >>>> >>>> +/* Return TRUE if we can access the register cache from the >>>> + currently-selected thread. If so, we can call get_current_regcache () >>>> + safely without risking an assertion due to the lack of an inferior thread. >>>> + >>>> + Return FALSE otherwise. In this case, attempting to call >>>> + get_current_regcache () will run into an assertion. */ >>>> + >>>> +extern bool current_regcache_exists (void); >>>> extern struct regcache *get_current_regcache (void); >>>> extern struct regcache *get_thread_regcache (process_stratum_target *target, >>>> ptid_t ptid); >>>> diff --git a/gdb/thread.c b/gdb/thread.c >>>> index e4d98ce966a..545bbf2a411 100644 >>>> --- a/gdb/thread.c >>>> +++ b/gdb/thread.c >>>> @@ -80,6 +80,14 @@ is_current_thread (const thread_info *thr) >>>> return thr == current_thread_; >>>> } >>>> >>>> +/* See gdbthread.h. */ >>>> + >>>> +bool >>>> +inferior_thread_exists () >>>> +{ >>>> + return current_thread_ != nullptr; >>>> +} >>>> + >>>> struct thread_info* >>>> inferior_thread (void) >>>> { >>>> -- >>>> 2.25.1 >>> >