From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from EUR03-AM7-obe.outbound.protection.outlook.com (mail-am7eur03olkn2036.outbound.protection.outlook.com [40.92.59.36]) by sourceware.org (Postfix) with ESMTPS id 991CC3858D28 for ; Mon, 8 Apr 2024 14:39:37 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 991CC3858D28 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=hotmail.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=hotmail.de ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 991CC3858D28 Authentication-Results: server2.sourceware.org; arc=pass smtp.remote-ip=40.92.59.36 ARC-Seal: i=2; a=rsa-sha256; d=sourceware.org; s=key; t=1712587182; cv=pass; b=WRmfLouIUIEK8drzBE0HeyGfExUnFEKTtBCYQlHuPnkDtv5cvSHpOm8n/SMrOZSP9GTNI/6BKU8mF8gogc8+R5FZX0zwrUUJTNDzsjrxbF62kjekf1hLIDq0unPhqaVlHoi5lwTS5ULin/me+rv7zEVLRdwJhyaCiaP+PaVqPxM= ARC-Message-Signature: i=2; a=rsa-sha256; d=sourceware.org; s=key; t=1712587182; c=relaxed/simple; bh=NFH1yXkiRia1U7XpgXIer7kQfI/B5niwNi577QbQ2OI=; h=DKIM-Signature:Message-ID:Date:Subject:To:From:MIME-Version; b=rhCQxC7YdDpJtlXGZdwQjvGb6PS2nIbv7rHfuPdaj98ErW2GogBiWpE0/5U+FNifNAGYcuW0HqWpPh87PZtZhjqaQlctjwkHrcOzQhhCXL9rieEXqAeolI/J/xV3M/jTPPpwlMaspQtnnXJM6JbiLYkM7PPUXsNVyJG/xV0PikU= ARC-Authentication-Results: i=2; server2.sourceware.org ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=RbvQKTVCwf95+FAqK3K5L6Ek0NJVCMglV+PmL3h3yBNYvFxDvnP5zK6rE4njyXKL7m+zfygBEDEL5LBrHGn83quJX7SydkwYICKCSCw8bYoDPNlBzVMeiwdmQ8qenStqk+zEvXWBNfZ0YIvaKoujri12QVVisiq69RSCxyZWqnmy8v3aFM3LulDCZbvpPfdTurAzLGMsXldtpVMX4/5JnVXN30cOAFHyQTvCP1FAjTFGV0NvLh2weBtcyWRUo8l27XrYDW5yWh+RnjL41wfNeJ6tCaOrwBZ+8S4vz7IagHJfqgqEkUxW15K6tcqkFCiTw+tHVeollUQf6aT3+TjjCw== 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=rgmh9G+lh0mP1mpaF1lz/u02h8pU0vwFs48abrgLBY0=; b=nnFHKuWZNnCqVnEXF4c6xH5cQ2eFT9I8OO4k5LSmSkUNopnHY7+cFtrOfTUgJzTgKVnyUzz1F212BJPD+eyBcjALAzlxitNR2q2Xwz2QYAgtzYuVuoIM2WvG/8sWRYAIHrQqM9hgzS49//i0+1d9SNWxFRbk4F7ZMERCJ+tWikIGrgS98nwRf+Fw/tohffbY+KP4rQZkHTzXXUR6ST47RMKkR4NYuvtqigyv7zYZlWROomMI3tWPas8qCHG6zraQPMNvJDFWRBiZR4qebnVxdv8ZMGx+oQlzhDFF/nj7JL9kP1LuBaGaLSM025Uh2UzRDQzrtTRGKIWlzL31+5Ci2g== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=none; dmarc=none; dkim=none; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=HOTMAIL.DE; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=rgmh9G+lh0mP1mpaF1lz/u02h8pU0vwFs48abrgLBY0=; b=VPT3AnQMzbl0NkSujEz9LbRu3iUwKRY0N2rlJA+ErroJYK0q8R3Y84bnTJsly5431wRDwKXxU8Ya8HA96uBqdLmgrA06sCXDY/0m/0kZMHsCNykji6gujiDF2vRMEtWrNl+0MCHYW14RwBpnXdN7RCz6moTzxdCS9TXYfY4Z42QkQSIGQUaB+17FZfR3n3kSuBQlTUgwJu7RrD5OpcoJwAsSNrWkXuXqLUC36t45MijXAwuBFtSOtAi41iZGOvSbWrsIBw0D63/MAOWIiQC0TWOyoYy4/hB6RZ2DBN0m7LyXHk7p4Sb78OEfnH9jE4+hs4iCjyDqDFT9XWu0kOSBVA== Received: from AS8P193MB1285.EURP193.PROD.OUTLOOK.COM (2603:10a6:20b:333::21) by DU2P193MB2049.EURP193.PROD.OUTLOOK.COM (2603:10a6:10:2fd::10) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.7409.46; Mon, 8 Apr 2024 14:39:35 +0000 Received: from AS8P193MB1285.EURP193.PROD.OUTLOOK.COM ([fe80::5403:f1ad:efaf:1f71]) by AS8P193MB1285.EURP193.PROD.OUTLOOK.COM ([fe80::5403:f1ad:efaf:1f71%4]) with mapi id 15.20.7409.042; Mon, 8 Apr 2024 14:39:35 +0000 Message-ID: Date: Mon, 8 Apr 2024 16:41:22 +0200 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v3 1/2] [gdb/symtab] Fix an out of bounds array access in find_epilogue_using_linetable To: Tom de Vries , "gdb-patches@sourceware.org" References: <20240405151012.14763-1-tdevries@suse.de> <544ecc72-916f-436e-b4f2-093bea6882a9@suse.de> <1c28bfe4-8841-4363-8460-d2d3bd5e529d@suse.de> Content-Language: en-US From: Bernd Edlinger In-Reply-To: <1c28bfe4-8841-4363-8460-d2d3bd5e529d@suse.de> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-TMN: [Mt+49OuaQ0r3t+fLT6uVu2Kjl/X9XKc0b4JI6+OJQkm8Mid0nrZv6mlMyczUmY19] X-ClientProxiedBy: FR4P281CA0074.DEUP281.PROD.OUTLOOK.COM (2603:10a6:d10:ce::11) To AS8P193MB1285.EURP193.PROD.OUTLOOK.COM (2603:10a6:20b:333::21) X-Microsoft-Original-Message-ID: MIME-Version: 1.0 X-MS-Exchange-MessageSentRepresentingType: 1 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: AS8P193MB1285:EE_|DU2P193MB2049:EE_ X-MS-Office365-Filtering-Correlation-Id: 3e2844f1-09e4-47a6-bb2e-08dc57d9b5b5 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: 51KvCd6MZQyh2hxBwhg3l25rxWwk3I2IwtobezNLhiC7JTvejobpylNuyCKCtEWBf3SHUpyKnr5mM2ckcFatgSKP/HSqH2jjcOShhEtsUVMCxj/w63aCu7EnqohEcHYRGlT1sNqrQB+75qimAo0GKcShOY8lLuyw9c6c2MNcVOwYO9gyLFRaIiTLB84byYlOM4OWZ4ORMLnCe1hvbsf6wfxY+GjO1V5IfAV3tJnOnugJA+4I1yai90w4cn02VHCmesG3ZVS6440mlg+lmWDHUkx1Tu6O7/ulxDUwfwbKwYUKb4ucZQLG7qWXNlGHrQ10k8ZDvo9GKbANJPJaDIEKAJLceIHggYSGrcH79jZocYRKLOmmCaODzXx6mykgjp1FsKP6sR/XDzXWIZD4cMQ1qii9JHxy1UIogsaH7VsUulerEW9Y6qEiH09YnPgH8s1JknkC9tbsgA48bOt+MijWAiRVTsiYEmYhLO811arRr1UdIoT2Y0zUQOiLUNgFOhri1NrVjNMtoPsrjsIPa3KgTBebhNI34sMi9W6PALZ0pjutJLsW382trBpx1nFDiHUQ+JVv1pp1rpgKrTZGMGhnHLnG2XzOhKrzisc19HmHwqlk71Zo7NYya7TnEdAKwb9rZQLEOjnWlRbngDnwNKyqMFCVYHOT8Jb4OUXuTJ/k9oY= X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?SzdBQ25SbG5DM2VFM3I5dngvQmRLTHQ0Q0pFZkJNM3doVEcxbEhJckl5QzhN?= =?utf-8?B?WDFDckhaYTFicHVvcldrWEtwMm56cGNMOGY3ckMwVHRtNkF1VHREWFlFb0xa?= =?utf-8?B?MFR5U0pwV2tPeDFGSnpXYnBUbzdFQm8yczVBM0EvSDNreFROQndPeUJFSVJn?= =?utf-8?B?WGJwZ0tGbUFOYnBTb090MTFJbk5lSVpZako3RnRDMW9pZ29uSFVpY0RUNm94?= =?utf-8?B?TDFIOFZLTDZYaDVGVmh0QjJKQWRETVZwQlNIcmtHOHVFcDZVeUUxdk1LZ3pv?= =?utf-8?B?MVpKWnBPNkhBcW1DSW1YZU53Q0JJdGdPM0hldklSNzV1ZW4rNFdSMWszcE0x?= =?utf-8?B?SzUyNXlkcjV2MzZOUUZORzB4YitBb0pzNDhRRDFxWDh5NkViWWYrSGJRa0xa?= =?utf-8?B?Z2lZTzk2NThwcUlJdEx2NFJPVzUyMjN5MWtOZ3NsbVE0S3U3YXJjL0Y5akQ1?= =?utf-8?B?U0hCMGdlY3AyU29pMktwRC9md2JZQjFaUi8zRCtURU94a1FXWjR2YVB2WVFv?= =?utf-8?B?eVErVDBXMXRGT1hxZ0pkNkJSMllwemRXSm41amcwbmwrbWNhQnlxNHFkekNN?= =?utf-8?B?YU42YXZhU1BvcU95ZDJyVDVrS2Z1YUE4SGRVNll1eEVIVGtuR0ZoS2srbmsy?= =?utf-8?B?WXdOMkkrRmcrWngyOTZ0QjgxdmFrZEpMVjR3UGF0aGlpZVRkb0w5UjFTMDUv?= =?utf-8?B?Z3BRc3pQZGc4TFlJWlFJakk4d3RLTFRuZ1RRQ2xETE5RRXhBZjc1YzJsU1RD?= =?utf-8?B?dVZLR2hoTmFPQkliUXFkYTlkRHVmQkJBUzZqRmhyQW1jS1hYZW1wZ3FWRzh1?= =?utf-8?B?L0Q4d2xzY29uMnJ0c0hZeEdyYXU5NllmcFBZZGtuZWNFMUsrVDZ4VkViTzdp?= =?utf-8?B?YXRkdkNsV3VJbTNicE1QVzdsdlZ4TXl4K0hHRmxKL1I5cEtxOGpOWEpkcWg1?= =?utf-8?B?QjlOa0RiVGloQXdNeTRCLy9RKzEvY0h0cGhJYjhWbERVRTRXRGQxTTdBc29x?= =?utf-8?B?TjFQYmdmY2Fqak95eWJiYUV2VzlrUnFYZ2RqVTNXclgzMzgrQ3dBMmFQVkEy?= =?utf-8?B?OHUrNDNIR1cyUkJSUFZScjdZVmhQaXJ3clY3Yzk3cVdseG5wR29sb3V3OHFp?= =?utf-8?B?TW9PNWZ1cC95Nlo3ODdGRGNGY1hUZnBrOEVmTkJmWGs4RGR3RHJ0Q29Wa1lB?= =?utf-8?B?MUJJbjVvSTg5ek5LYnV6S0wrS3JiYTBXM1YyeXJzc1hLbFZ3Nm0vUUxuQk9v?= =?utf-8?B?S2hHRmFtWlpXbWJmQzhscWZsVzZ0UWc2cEw0YUs3bzY5dmxMd2JlbnBMenJF?= =?utf-8?B?V2FGR0IrK282S1lsWkgyN3hiazVnVDkzTnVGTUgvQ3J1TnZMTWRiODBQZzN2?= =?utf-8?B?QU1xU0JCTHZJSm9WbjNRMmk2b0VhdTVveUpQSG5NZ3p6R0wzdldwV0I3aWVO?= =?utf-8?B?dmc5WCtvYnk1N2pvZ0YxczV3Nm1YOFhSejdwMTJjckFXOXFYeEZSTE9yWDgr?= =?utf-8?B?a2NOcVlFN3Z2K3Q2eDcvOTFqRk5sRzQ0cGJQWDBqWWNOcFV4MVRyU0xjV1Vr?= =?utf-8?B?Yld0YXAvZnRlaHQwZW9yL0lXK1VQc1VKWkY4T1ovOFc1TWZGd2hhbk1pSVdW?= =?utf-8?B?WnduVkdGUzViRDRGd0VpR3cwa2NRUjl4UkhqV3hZTFo2TE1tTXZjc3dVMmFJ?= =?utf-8?B?N3ZqNzNUTHRtVHEvZ3pqRnRWY3Y5eGhXdHovdGpFOUk2bVA2d2VTVEh2RkdF?= =?utf-8?Q?+pNiE+8ziIeiwi+Z1yxVp9ehuAjCFN2pYW/vDqi?= X-OriginatorOrg: sct-15-20-4755-11-msonline-outlook-80ceb.templateTenant X-MS-Exchange-CrossTenant-Network-Message-Id: 3e2844f1-09e4-47a6-bb2e-08dc57d9b5b5 X-MS-Exchange-CrossTenant-AuthSource: AS8P193MB1285.EURP193.PROD.OUTLOOK.COM X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 08 Apr 2024 14:39:35.1144 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 84df9e7f-e9f6-40af-b435-aaaaaaaaaaaa X-MS-Exchange-CrossTenant-RMS-PersistedConsumerOrg: 00000000-0000-0000-0000-000000000000 X-MS-Exchange-Transport-CrossTenantHeadersStamped: DU2P193MB2049 X-Spam-Status: No, score=-11.8 required=5.0 tests=BAYES_00,BODY_8BITS,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,GIT_PATCH_0,KAM_SHORT,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,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: Hi Tom, we are making good progress. On 4/8/24 14:58, Tom de Vries wrote: > On 4/7/24 10:17, Bernd Edlinger wrote: >> >> Hmm, this can be an assertion, because >> the line table was found by find_pc_line (start_pc, 0); >> so the linetable is guaranteed to be non-empty. >> BTW: empty linetables are usually NULL pointers, >> so that probably the assertion should >> also assert like >> gdb_assert (linetable != NULL && linetable->nitems > 0); >> > > I've updated the if condition to include the nullptr check, but didn't turn it into an assert.  By doing so we gain nothing, and add a potential user inconvenience. > ok. >>> >>> We could also just simply handle the cornercase by returning {}, I went forth and back a bit on that, and decided to support it on the basis that at least currently we have dwarf assembly test-cases in the testsuite that trigger this path, though I've submitted a series to clean that up. >>> >>> But I'm still on the fence about this, if you prefer a "return {}" I'm fine with that. >>> > > I've thought about this a bit more over the weekend, and I managed to convince myself that a simple "return {}" is the proper solution.  The rationale is that an incorrectly written dwarf assembly test-case is not a good reason to support a corner-case.  If we do stumble one day into a compiler that generates the incorrect debug info, we can always opt to add a workaround for this (which we then can test extensively).  But adding a workaround without such an incentive is pointless. > >>>>> +    } >>>>> +      else >>>>> +    gdb_assert (unrel_end <= it->unrelocated_pc ()); >>>> >>>> Why do you not check that 'it' points to an end_sequence >>>> at exactly unrel_end? >>>> It could be anything at an address much higher PC than unrel_end. >>> >>> This assert spells out the post-condition of the call to std::lower_bound, in case it found an entry. >>> >>> If there's debug info where one line entry straddles two functions, the call returns the entry after it, which doesn't have address unrel_end. >>> >>> Having said that, we can unsupport such a scenario by doing: >>> ... >>>        else >>>          { >>>            if (unrel_end < it->unrelocated_pc ()) >>>              return {}; >>>            gdb_assert (unrel_end == it->unrelocated_pc ()); >>> ... >>> >> >> I think we should not look at the `it` in any case. >> If there is an inconsistency in the debug info, a >> debug message that can be enabled in maintainer mode >> would be good enough. >> But even in this case, I would prefer a best effort, >> and continue whenever possible. >> > > IMO, a best effort only makes sense in case there's a compiler release generating the incorrect debug info. > Consider this counter example: The debug info is correct, but the internal representation is not what you probably expect. $ cat hello.c #include int main() { printf("hello "); #include "world.inc" /*** End of hello.c ***/ $ cat world.inc printf("world\n"); return 0; } /*** End of world.inc ***/ gcc -g -o hello hello.c The line table starting at main ends between the two printf statements. I demonstrate the effect this little mod over your v4 patch version: diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c index 0bb7a24cbd0..35c1fb85409 100644 --- a/gdb/amd64-tdep.c +++ b/gdb/amd64-tdep.c @@ -2908,6 +2908,7 @@ amd64_epilogue_frame_sniffer_1 (const struct frame_unwind *self, /* We're not in the inner frame, so assume we're not in an epilogue. */ return 0; +#if 0 bool unwind_valid_p = compunit_epilogue_unwind_valid (find_pc_compunit_symtab (pc)); if (override_p) @@ -2924,6 +2925,7 @@ amd64_epilogue_frame_sniffer_1 (const struct frame_unwind *self, "amd64 epilogue". */ return 0; } +#endif /* Check whether we're in an epilogue. */ return amd64_stack_frame_destroyed_p_1 (gdbarch, pc); diff --git a/gdb/symtab.c b/gdb/symtab.c index 427d7b9f8b2..bb3c1bba70b 100644 --- a/gdb/symtab.c +++ b/gdb/symtab.c @@ -4192,6 +4192,7 @@ find_epilogue_using_linetable (CORE_ADDR func_addr) extent of the function, which shouldn't happen with compiler-generated debug info. Handle the corner case conservatively. */ + printf("at linetable end nitems=%d\n", linetable->nitems); return {}; } else @@ -4202,6 +4203,7 @@ find_epilogue_using_linetable (CORE_ADDR func_addr) function. This can happen if the previous entry straddles two functions, which shouldn't happen with compiler-generated debug info. Handle the corner case conservatively. */ + printf("unrel_end = %lx < it->unrelocated_pc = %lx\n", (long)unrel_end, (long)it->unrelocated_pc()); return {}; } gdb_assert (unrel_end == it->unrelocated_pc ()); So this debug info which is not invalid at all, triggers your error conditions: $ ..../gdb-build/gdb/gdb hello GNU gdb (GDB) 15.0.50.20240406-git Copyright (C) 2024 Free Software Foundation, Inc. License GPLv3+: GNU GPL version 3 or later This is free software: you are free to change and redistribute it. There is NO WARRANTY, to the extent permitted by law. Type "show copying" and "show warranty" for details. This GDB was configured as "x86_64-pc-linux-gnu". Type "show configuration" for configuration details. For bug reporting instructions, please see: . Find the GDB manual and other documentation resources online at: . For help, type "help". Type "apropos word" to search for commands related to "word"... Reading symbols from hello... (gdb) b main Breakpoint 1 at 0x114d: file hello.c, line 4. (gdb) r Starting program: /home/.../hello [Thread debugging using libthread_db enabled] Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1". at linetable end nitems=3 Breakpoint 1, main () at hello.c:4 4 printf("hello "); (gdb) n at linetable end nitems=3 at linetable end nitems=3 1 printf("world\n"); (gdb) n hello world at linetable end nitems=3 at linetable end nitems=3 2 return 0; (gdb) maint info line-table objfile: .../hello ((struct objfile *) 0x5641508bf6b0) compunit_symtab: hello.c ((struct compunit_symtab *) 0x5641508bfbc0) symtab: .../hello.c ((struct symtab *) 0x5641508bfc40) linetable: ((struct linetable *) 0x564150920ce0): INDEX LINE REL-ADDRESS UNREL-ADDRESS IS-STMT PROLOGUE-END EPILOGUE-BEGIN 0 3 0x0000555555555149 0x0000000000001149 Y 1 4 0x000055555555514d 0x000000000000114d Y 2 END 0x0000555555555161 0x0000000000001161 Y objfile: .../hello ((struct objfile *) 0x5641508bf6b0) compunit_symtab: hello.c ((struct compunit_symtab *) 0x5641508bfbc0) symtab: .../world.inc ((struct symtab *) 0x5641508bfc80) linetable: ((struct linetable *) 0x564150920c80): INDEX LINE REL-ADDRESS UNREL-ADDRESS IS-STMT PROLOGUE-END EPILOGUE-BEGIN 0 1 0x0000555555555161 0x0000000000001161 Y 1 2 0x0000555555555170 0x0000000000001170 Y 2 3 0x0000555555555175 0x0000000000001175 Y 3 END 0x0000555555555177 0x0000000000001177 Y objfile: .../hello ((struct objfile *) 0x5641508bf6b0) compunit_symtab: hello.c ((struct compunit_symtab *) 0x5641508bfbc0) symtab: /usr/include/stdio.h ((struct symtab *) 0x5641508bfcc0) linetable: ((struct linetable *) 0x0): No line table. And this is also why I would suggest to look into using the line-table that has addresses near the end of the range like this: --- a/gdb/symtab.c +++ b/gdb/symtab.c @@ -4156,7 +4156,7 @@ find_epilogue_using_linetable (CORE_ADDR func_addr) if (!find_pc_partial_function (func_addr, nullptr, &start_pc, &end_pc)) return {}; - const struct symtab_and_line sal = find_pc_line (start_pc, 0); + const struct symtab_and_line sal = find_pc_line (end_pc - 1, 0); if (sal.symtab != nullptr && sal.symtab->language () != language_asm) { struct objfile *objfile = sal.symtab->compunit ()->objfile (); Thanks Bernd.