From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 94708 invoked by alias); 26 Sep 2017 17:51:50 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 94696 invoked by uid 89); 26 Sep 2017 17:51:49 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-23.9 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE,RCVD_IN_SORBS_SPAM,RP_MATCHES_RCVD,SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mail-it0-f48.google.com Received: from mail-it0-f48.google.com (HELO mail-it0-f48.google.com) (209.85.214.48) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 26 Sep 2017 17:51:48 +0000 Received: by mail-it0-f48.google.com with SMTP id o200so9020376itg.0 for ; Tue, 26 Sep 2017 10:51:47 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=mtEvKFPP9P4/So4/2ftMfy6FpSdxV1Gd/bhoHBr3amg=; b=MlZY79dkWJJ6pjzVuu3z/ApBqfEugzDcfi26yOgdP8HAGkwAVEO3lq7xpq1p3fFw4Y 1fT8t1LGMwwM4TmKGPvhmuhRRL8eraGo3mikXTOKIzj8RF5n1nsUCSxYOjAdiQK5tbIc EeSUuMrjXbU4q5Etj9ptmCg6+JnPN5jyEnnD2jmLDjEBy4JXvd3YrtzLrlhg4nTIeBo0 DfInrbYwwXJFPmjwXWOdL9U8cLp3DaDmwZyRgmswKh/bmVhTGRuBN/HvA+GcuLOaaBhp lOxMkMOD6QYjqGY5rgCaAaxooO7uzTEOquSRWMyIZVbyuD8ZUGBBnEuSjsgspPS+y9fD Z+SA== X-Gm-Message-State: AHPjjUjbI2qVO/PyqmY+Yi5cYF53dw2O96P/CnIzRQh9Gasr/WXApAVn ThkKTvn7o9RSpL7PU8AjY9I/nS2LbDY+K+XNN/6j1A== X-Google-Smtp-Source: AOwi7QCwlGYsJb0qbJvW3JsRMp81IhEylCqonletjI9UZ+VwIPNif2EOimKU7C2X59vxgD165JUDOUpYwsBfhnmrq1c= X-Received: by 10.36.228.5 with SMTP id o5mr7490358ith.116.1506448305874; Tue, 26 Sep 2017 10:51:45 -0700 (PDT) MIME-Version: 1.0 Received: by 10.107.34.131 with HTTP; Tue, 26 Sep 2017 10:51:05 -0700 (PDT) In-Reply-To: References: From: "Doug Evans via gdb-patches" Reply-To: Doug Evans Date: Tue, 26 Sep 2017 17:51:00 -0000 Message-ID: Subject: Re: [PATCH] Fix reading .dwp files without .debug_tu_index To: Alexander Shaposhnikov Cc: gdb-patches Content-Type: text/plain; charset="UTF-8" X-IsSubscribed: yes X-SW-Source: 2017-09/txt/msg00813.txt.bz2 On Mon, Sep 25, 2017 at 9:13 PM, Alexander Shaposhnikov wrote: > > >I'd go with something like (untested and likely improperly formatted): > >if (dwp_file->cus) > > dwp_file->version = dwp_file-cus->version; > > else if (dwp_file->tus) > > dwp_file->version = dwp_file->tus->version > > else > > dwp_file->version = 2; > > i'm ok with that change, many thanks, > do i need to resend the patch ? (and yeah, i can double check / test it when > i get home) No need, if you're confident all the nits are taken care of [ChangeLog syntax and contents, patch whitespace, testing, and so on ... :-)] > regarding the dependence on the order of sections - (if i am not mistaken) > yeah, it's a separate issue - i can provide more details > when i get to my laptop. Thanks. > > Best regards, > Alexander Shaposhnikov > > On Mon, Sep 25, 2017 at 8:14 PM, Doug Evans wrote: >> >> On Tue, Sep 19, 2017 at 12:58 PM, Alexander Shaposhnikov >> wrote: >> > This patch fixes segmentation fault (due to dereferencing of a null >> > pointer) >> > in dwarf2read.c in the code dwp_file->cus->version != >> > dwp_file->tus->version >> > by adding defensive checks similarly to how it's already done >> > at the lines 11208 - 11215 (in the same file dwarf2read.c). >> > The issue can be reproduced with dwp packages built by llvm-dwp utility >> > (from the current upstream) since at the moment llvm-dwp doesn't create >> > .debug_tu_index section, thus dwp_file->tus will be null. >> > >> > Test plan: >> > >> > BUILD: >> > main.cpp: >> > int f() { >> > int x = 0; >> > int y = 1; >> > return x + y; >> > } >> > g++ -fPIC -gsplit-dwarf -g -O0 main.cpp -o main.exe >> > llvm-dwp main.dwo -o main.exe.dwp >> > # This step is a workaround to a separate issue (unrelated to this >> > patch). >> > # At the moment llvm tools & clang put .strtab section first (its index >> > is >> > 1), >> > # while gcc/gold/binutils put it at the end. >> > # Unfortunately gdb (in the code reading dwps) appears to depend on the >> > order >> > # of sections, >> >> That's odd. Depends on the order of sections how? >> >> > to workaround this (to reproduce the issue which this patch >> > # aims to address) we use objcopy to do the following trick: >> > # if one asks to remove .strtab objcopy will do that but at the same >> > time >> > # it will create a new .shstrtab section at the end. >> > objcopy --remove-section .strtab main.exe.dwp >> > RUN: >> > gdb main.exe >> > One can observe that now there is no crash and debugging functionality >> > works as expected (setting breakpoints, printing local variable, >> > exploring >> > the stack). >> > >> > gdb/ChangeLog: >> > yyyy-mm-dd Alexander Shaposhnikov >> > >> > * dwarf2read.c: Fix segmentation fault on reading >> > >> > .dwp files without .debug_tu_index section. >> > >> > Patch: >> > >> > diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c >> > >> > index b1914cf876..547e3f034e 100644 >> > --- a/gdb/dwarf2read.c >> > +++ b/gdb/dwarf2read.c >> > @@ -11185,7 +11185,8 @@ open_and_init_dwp_file (void) >> > dwp_file->tus = create_dwp_hash_table (dwp_file, 1); >> > >> > /* The DWP file version is stored in the hash table. Oh well. */ >> > - if (dwp_file->cus->version != dwp_file->tus->version) >> > + if (dwp_file->cus && dwp_file->tus >> > + && dwp_file->cus->version != dwp_file->tus->version) >> > { >> > /* Technically speaking, we should try to limp along, but this is >> > pretty bizarre. We use pulongest here because that's the >> > established >> > @@ -11195,7 +11196,7 @@ open_and_init_dwp_file (void) >> > pulongest (dwp_file->cus->version), >> > pulongest (dwp_file->tus->version), dwp_name.c_str ()); >> > } >> > - dwp_file->version = dwp_file->cus->version; >> > + dwp_file->version = dwp_file->cus ? dwp_file->cus->version : 0; >> >> A version of 0 doesn't seem right. >> I'd go with something like (untested and likely improperly formatted): >> >> if (dwp_file->cus) >> dwp_file->version = dwp_file-cus->version; >> else if (dwp_file->tus) >> dwp_file->version = dwp_file->tus->version >> else >> dwp_file->version = 2; >> >> ok with that change. >> >> > >> > if (dwp_file->version == 2) >> > bfd_map_over_sections (dwp_file->dbfd, >> > dwarf2_locate_v2_dwp_sections, >> > >> > >> > Kind regards, >> > Alexander Shaposhnikov > >