From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from EUR04-VI1-obe.outbound.protection.outlook.com (mail-vi1eur04olkn2094.outbound.protection.outlook.com [40.92.75.94]) by sourceware.org (Postfix) with ESMTPS id DC9B03858D20 for ; Sun, 7 Apr 2024 08:15:31 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org DC9B03858D20 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 DC9B03858D20 Authentication-Results: server2.sourceware.org; arc=pass smtp.remote-ip=40.92.75.94 ARC-Seal: i=2; a=rsa-sha256; d=sourceware.org; s=key; t=1712477741; cv=pass; b=bLtIyGkPHNCFAMYFJP/ouCjlCOV3yAr8AMYq+X3RImN/crSYeTzqgeNZbWD9CuU3oyTI5UT2fafYpSDrMVr42U3jGi2H57u7qV2B9H44jAYyXKQEV+W0WRNwWsZVBhcv1fsQYgfMyvCQ4ioN/VxAQiy6buqhL17yYRzIan0fgwM= ARC-Message-Signature: i=2; a=rsa-sha256; d=sourceware.org; s=key; t=1712477741; c=relaxed/simple; bh=hP9dRfkSN4y4LTvXphi/1bn+AO+M/Ldgcqyf+yYjHSY=; h=DKIM-Signature:Message-ID:Date:Subject:To:From:MIME-Version; b=Qc/sAWoShj/mdfEZzWCHFIuXFDdxhHv984p1CPm3bpzH5jdOhESZ8fItoC0+ZOaepHXuK09C+e91FH33HTDe97c3zYYAuVbZjrjUH9c0Xg9m/OW3TLakQoMQUvzXCFTJyElcRs/lHMTTaZxyh7jerdNuicbsVr1mVQmR7058K8g= ARC-Authentication-Results: i=2; server2.sourceware.org ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=fA4cGc54jwp8MnJ325cyCgX4DnfiVZNgJFO5hGgpf5Kx/SUknABJm6Zua7tIu4ClwyjApB0hF60eU2CcIFoqOevffGzPn7Ic/nN8R2HmNuuktSMhQiGJEDL4DjuvD1/VGuw0Wxkdo+hcVe3MBPKLkiyLdjB6iNvHufsRD5ThaaOG1OeV6uw8pHiu/4t+NalRS6YyhKTksj1rxP9qscsu9Tmd6rnefFPNdnbd1hzmtxYCb+4M3KA4Ood6nWpFbvq/DeyxJ8Q3Fa8gelKTv+M6km60YCyjhE4yAw33rHP5C74BP/9dw1ez+We/J2S1wVUaaOmXZIL9fGV8y5gn3GBQGw== 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=cOkAyuFRFLsyaCeE8zdu8KLfL4SvjiAug7ijWd56XcY=; b=FjDr3x50z4/+IZHqAuJge4UYb1PUIuJ5mccn1VzYOSju79XhdPY42frYcRdO4TqrlhD27/aKcH7tRhQZtV6y5jbLNjeQILEmwt3hR8qmxQdT0mzkqOTQuAMcIaKka5bvb4i+3KuY8w/6Qm5PvcB0pMeMymu00BsWEjYCbYGZInzgRzkkDvEOILLb1AKX0c8ZmQJhVH/WIZBgjlpxrpFEXTty812kdnTcL9Z4lvP3kci3ofVe7TgSAdZjZFjHB7QcfB3+7S0Q8MoAX1qGVtdOljQ1mAtyaeQq/04gMazJOQG/KUnXQt+J6WR+iTkxae38Drh2ieeUiNk41bGNwiW51w== 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=cOkAyuFRFLsyaCeE8zdu8KLfL4SvjiAug7ijWd56XcY=; b=loAnSSZD2rt9Yxxh0ajjiSYgwJfnH3SmoB9q5es0sOgLHEdQy8Ax/gdxA/2hHte6/9ohPQrG5fvjfmhZG/KRZqd8Whw2kksAflk+6FIIwHIaTbL4E0VNQcxLLwtrElAENS8Xza97sJV+JGzWvCt/TpC0noPnctD0hWhW5p+4WjyEhdAGvRvThBfp56NsezGOHL/SHxkiqwzgUcIk59zaJM8nVe4AL+qvTRSl/z/o6bSfXQ29WTu0uVGO447FiOeqViIOUrxPbFEFOPR/xz04CzFFOAHS46KtfJIji2+bKwtbNjtOvbpZAVDKSwGb+PRosrPxHGiTtk4+pOvFAg/hSg== Received: from PAXP193MB1296.EURP193.PROD.OUTLOOK.COM (2603:10a6:102:15a::24) by VI1P193MB0624.EURP193.PROD.OUTLOOK.COM (2603:10a6:800:147::15) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.7409.55; Sun, 7 Apr 2024 08:15:29 +0000 Received: from PAXP193MB1296.EURP193.PROD.OUTLOOK.COM ([fe80::9cd3:f742:8663:6ee]) by PAXP193MB1296.EURP193.PROD.OUTLOOK.COM ([fe80::9cd3:f742:8663:6ee%7]) with mapi id 15.20.7409.042; Sun, 7 Apr 2024 08:15:29 +0000 Message-ID: Date: Sun, 7 Apr 2024 10:17:16 +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> Content-Language: en-US From: Bernd Edlinger In-Reply-To: <544ecc72-916f-436e-b4f2-093bea6882a9@suse.de> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-TMN: [XzoZZFsbZ9MDFMVatqdjpr1ivwXIftM6nmYOCR7e+YtBtpeiYzOGyXzYQO0ZZnH2] X-ClientProxiedBy: FR5P281CA0002.DEUP281.PROD.OUTLOOK.COM (2603:10a6:d10:f2::20) To PAXP193MB1296.EURP193.PROD.OUTLOOK.COM (2603:10a6:102:15a::24) X-Microsoft-Original-Message-ID: MIME-Version: 1.0 X-MS-Exchange-MessageSentRepresentingType: 1 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: PAXP193MB1296:EE_|VI1P193MB0624:EE_ X-MS-Office365-Filtering-Correlation-Id: a22573ee-ffb6-429a-a779-08dc56dae2d4 X-MS-Exchange-SLBlob-MailProps: Cq7lScuPrnpOxDCcKCE3ez6uzgD/gesXfN9s4XHwF7CL/FsMgVv+Qxe++4SG//w2BV2r+5nNco6X3HLP5OZKLrJ3JDjlTIOWNbl2KaaPajeR2Tc2TL1YBe+kkQ79MPxUO3NJlS04dK9WkevSy452Dhn84wYeP1uLu0WbdvBOdKykE+b1AgFNBvDpx56VQwzHtIOMRhsSbPN4ZlUEb2O91p/dOQTtmhicV2P9sPtA+1hCKyxvnK4JvOyqJ1uC7OprYjD6rWY9805u7r1wKiOsYcR2I9cmMxAx7LA0/uYQd9iRfDIgtSQqVRWb+4ObiwXoXzh+hvQxtD+rW+zg18wgMpFDHfJqJ81KPN5kPw2q+EO0wz42LQf3BOKjlEaEqrLyNPGfq9rKSCkdw4ZoodOqPl2Cc7nMaYO0h/8e/BMb9+kttAmCC0VJM37c96fDbZqdhcS4045+3A8QW5kntuPwi12mP0B7uJDlfZcIQ6dEDwwv9FNhHVUs/YdDsbbMNE+snqbMThnRFzs5pYRKQbW+Mu1OeE4h1UGWEnZAj29Aq5si27oXg3CDRLlnGIGvDRQc4f4OR4N6ooxTs1h8QeUDKHG2CJR/DiOaA9BkqHDD3NUjAbhOmXDOdstiRmzt6x8cQpImhYjTde+c8hdaEJeJ/7zE09QIKiYVsYKSPWK2IfomcEE+iO/ANtp3LkcEX/w75gSGUynDcgtVKmb5l5R54IqgOXHT76B1J/d+9KEwhErz8tkre9baOhrVNXBdVUmPHona0TpWYQQ= X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: 3XgvoBm0HVvUJyGI0de/3G7dLWMQe0o3KulOukrZBu5uwMElgR3KihoAtMY/wCNDynt7Gakm1kTY243lUmr99Wna5l5rI+o+YkK18V+uvfMCpsQHGN8Udnv2kWoc+l+mmho09GcKD+9LQeUAcez5VvkWRSxgCS/LSOTIN4+31M6GDJaDRdm2ionP1m/cHcAc3TqmJbNvwaJJsQC3nID5wA0dPpDLpxjhvEknewWUcNVljEATW70BKkxfcfPnwyWR3Q3asZUlNf7HWlrW4I7Z40f3+4I3AFWsdecfpez1lcoalb4h7vKGHhEc3sGAZRjCbvAkuGMMlHcgN+T3GwwaU3xD3L9SaQGcUiZYJDyibEIbBW85komnwm3d/WpXDR30DzPs6xC13lJ+VPOzFKGN40D8dun5cNO18EAl9VHZUekiYSZbImL0sen6Dp+pQEZTFDmMGj9BJqYhFu+mhYMX3a1qrce2yArQShJT4q8Zm33sQ84as9IK7rRnQ2Vk1q7L8szr3ZxlBhBa4JLwTbtFpJO5LVuj1Oa1Ge6nMuebnT7Fbh8c3EW7Tsz5Qby66Gi8 X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?cHlhU2J2emoxRUtoZ2tKNC96T1dNRm8wQlQxdkVoZzBmQmJWUkxHVlFoMzdY?= =?utf-8?B?c3J4LzkxTWVDMzVFaGEzcGtXZnJ5WDQ5cUNJck9TMzkyc0pJcFRiak1haXp5?= =?utf-8?B?YndLeERqVFhEcWdPRnlNZEdneWl2Z2tleWp4SnNOcjFhdGxkSy9JNjZtZWNO?= =?utf-8?B?cUd0YkUxVFNwVmVBVkpLdGRrQ1F3ZTZSbWVLcHJWTGtyVVErZnBUOXdvZTF5?= =?utf-8?B?dCs0YUllOU5NblNla0d3bHBad1JEajV3QmRkQzZQdnFyZmtmbW1hTjd4RjZG?= =?utf-8?B?c2Z6d2hSOWhSYk9ZR2FkNUpudDA4cEdlZzY2ajN3YWJMSk4ySWp4bm5GeHVH?= =?utf-8?B?RW5uaTV2WENMMnF0RnVBUVc4cFZQQ01WSFRvZVFpdE9JWnVzZVo1QTkvYzdK?= =?utf-8?B?cUhaVVlwNmZxZHQ4OHoxc2VuRklMNmN1ZmJsVFBTSFFsaXpNcnovVVNLd2pR?= =?utf-8?B?LytlTE9Zemx6T2hiMDZBd1JrTWp5T3hTdjNad2s4a1M4ckFJUlA0TFlMeE0v?= =?utf-8?B?alo4azd5NGN0UDFmaStTL0pEL3pYMFU4bStmNmhOTHpUNU9lMzdSK2p6dzl6?= =?utf-8?B?WXQ2MjdrY0VqSndXSjhXZTVtNStZVlZjWUZ6Tm50YU9rUXozRm5JNklyS3BU?= =?utf-8?B?K241S1pEZlJQVXUvRzhna0R4ZmZTN0N2Tnhxa1U3eHdHck9aUDM4NWw4OFJG?= =?utf-8?B?V3QybHpJb2ZsTFFVcEt0dzN3WHEvTXVqZC9NcGxIenFMK2VmblBnSHF0b3kr?= =?utf-8?B?Zm9yOXc2by9FczRHV1ZWd0lFTm93bmRpT2VCZ2oyYmtEVk5DTlUzeTBXcWk1?= =?utf-8?B?czVXM1VCQ0E1RTZhWVhEekZCRkN6cXRlYW5IZ1RlT215U0M4L3N2SkJkU3lL?= =?utf-8?B?eURYSUpaWkIwMUZ6NTlDcjl0MDUySWtZdWIvY1VEaG9ZWk0yV0ZNTzZjWGJ0?= =?utf-8?B?ZzlneUdLcTZWbVNiUVFkWWVuYVgvSElHN1ZxcERiZXloWGg3OTVjc1RvWDVy?= =?utf-8?B?M3ZEajJUdWxZeFMxRzhuU1c3WXdyS1B3N0dtMFRmL1RFc2F6cncwOGdlMkpr?= =?utf-8?B?dXl4RnVmM2cyVHZLTmw4SVNMMGdEYUsxS053RlZVQXJaclVvMGhHTXVpVEhV?= =?utf-8?B?NDZiaWtEQmh4Nm1jLzd2WjlXTDRYNlZoWHpoNFVCb1ZtWjZXcGQxcDFTVDJW?= =?utf-8?B?OGFYaUdTR1ZjekpnTGk5SzdJYlpqZmx0SUJwNlVXSHZEeWN6anhlblBuZExH?= =?utf-8?B?R1RvWkNHNnhPQk1wdlNYYjNldFByeHF2amxLazFaQXB5Z3ovVTJxNWhIQVdu?= =?utf-8?B?L0dPQlpBZGpieEhaa2xORGFQblBHOG5BcTNuSmJ5QWQ1SjhLTkxnbjhrK0o2?= =?utf-8?B?YU1UaExCN0FKaURoNStUZi9SRVpRRXphMTF1Yy9OMjRtdjhhT0Q3aC9FM3Zl?= =?utf-8?B?VWk2V1lJRUlXTXBXQ1FmV1Y5dWx3Y1c3NlBtQ1ZNS3JOa0hkSk9yVzZzOHNk?= =?utf-8?B?dmZHMXh2T2h4ZUlXUzdKMFpHSjlBbDJrMnd5Rk5vYklqdW1Lakk0K0lZaHhV?= =?utf-8?B?czFmVkoyeGhnMHc2ZWRBQTN5VmVGR0tBV3g5MURoRlp3dEU2aE1SemI4SUdt?= =?utf-8?B?NmVvQ3ZmSUs1MnRSVzZLbURqMXZmZ2N5SitQbVZiMkNmd2t2YlpNZ2dXandQ?= =?utf-8?B?Um0wd1dKRDVUazRBVTJBbHBEUGRxSGZXUitHTTU5NTNGVjFwUUh5ajBhM045?= =?utf-8?Q?ytpCZB3QcbE2rVgZwgeqMat8T30ruPhuyw0evmY?= X-OriginatorOrg: sct-15-20-4755-11-msonline-outlook-80ceb.templateTenant X-MS-Exchange-CrossTenant-Network-Message-Id: a22573ee-ffb6-429a-a779-08dc56dae2d4 X-MS-Exchange-CrossTenant-AuthSource: PAXP193MB1296.EURP193.PROD.OUTLOOK.COM X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 07 Apr 2024 08:15:29.2227 (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: VI1P193MB0624 X-Spam-Status: No, score=-12.0 required=5.0 tests=BAYES_00,BODY_8BITS,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,GIT_PATCH_0,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: On 4/6/24 10:29, Tom de Vries wrote: > On 4/6/24 07:03, Bernd Edlinger wrote: >> On 4/5/24 17:10, Tom de Vries wrote: >>> >>> diff --git a/gdb/symtab.c b/gdb/symtab.c >>> index 86603dfebc3..0c126d99cd4 100644 >>> --- a/gdb/symtab.c >>> +++ b/gdb/symtab.c >>> @@ -4166,10 +4166,14 @@ find_epilogue_using_linetable (CORE_ADDR func_addr) >>>       = unrelocated_addr (end_pc - objfile->text_section_offset ()); >>>           const linetable *linetable = sal.symtab->linetable (); >>> -      /* This should find the last linetable entry of the current function. >>> -     It is probably where the epilogue begins, but since the DWARF 5 >>> -     spec doesn't guarantee it, we iterate backwards through the function >>> -     until we either find it or are sure that it doesn't exist.  */ >>> +      if (linetable->nitems == 0) >>> +    { >>> +      /* Empty line table.  */ >>> +      return {}; >>> +    } >>> + 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); >>> +      /* Find the first linetable entry after the current function.  Note that >>> +     this also may be an end_sequence entry.  */ >>>         auto it = std::lower_bound >>>       (linetable->item, linetable->item + linetable->nitems, unrel_end, >>>        [] (const linetable_entry <e, unrelocated_addr pc) >>> @@ -4177,13 +4181,74 @@ find_epilogue_using_linetable (CORE_ADDR func_addr) >>>          return lte.unrelocated_pc () < pc; >>>        }); >>>   -      while (it->unrelocated_pc () >= unrel_start) >>> -      { >>> -    if (it->epilogue_begin) >>> -      return {it->pc (objfile)}; >>> -    it --; >>> -      } >>> +      if (it == linetable->item + linetable->nitems) >>> +    { >>> +      /* We couldn't find either: >>> +         - a linetable entry starting the function after the current >>> +           function, or >>> +         - an end_sequence entry that terminates the current function >>> +           at unrel_end. >>> +         This can happen when the linetable doesn't describe the full >>> +         extent of the function.  Even though this is a corner case, which >>> +         may not happen other than in dwarf assembly test-cases, let's >>> +         handle this. >>> + >>> +         Move to the last entry in the linetable, and check that it's an >>> +         end_sequence terminating the current function.  */ >>> +      gdb_assert (it != &linetable->item[0]); >>> +      it--; >>> +      if (!(it->line == 0 >>> +        && unrel_start <= it->unrelocated_pc () >>> +        && it->unrelocated_pc () < unrel_end)) >>> +        return {}; >> >> Why is this check necessary here, and not also when >> this is not the last function in the line-table? >> >> And why is this check necessary at all? >> > > It spells out as much as possible the specific conditions of the corner-case we're handling. > > 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. > >>> +    } >>> +      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. If you look at skip_prologue_using_linetable you see that it does stop the search immediately, when it->unrelocated_pc() reaches unrel_end, or when the end of the linetable is reached: for (; (it < linetable->item + linetable->nitems && it->unrelocated_pc () < unrel_end); it++) if (it->prologue_end) return {it->pc (objfile)}; Therefore I would like find_epilogue_using_linetable to use the same algorithm just in reverse direction. Which means always do `it--` first before using `it`. After all this is just a partial function range, it can end with a jump or a return, and in both cases the linetable entry at unrel_end can belong to a completely different function, and it is not guaranteed to be an end_sequence entry. BTW: I am not sure what happens if the function has multiple line tables, e.g. because of inline functions, or #include to pull in parts of the function body. In that case I would expect that the line table found by find_pc_line (start_pc, 0); may be covering the prologue area, while the epilogue may be missing. Maybe find_pc_line (end_pc - 1, 0); would be better candidate for a line table covering the epilogue area? Thanks. Bernd. > Thanks, > - Tom