From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-io1-xd2f.google.com (mail-io1-xd2f.google.com [IPv6:2607:f8b0:4864:20::d2f]) by sourceware.org (Postfix) with ESMTPS id 7AC1B388F05D for ; Tue, 16 Jun 2020 15:38:10 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 7AC1B388F05D Received: by mail-io1-xd2f.google.com with SMTP id o5so22352828iow.8 for ; Tue, 16 Jun 2020 08:38:10 -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:references:in-reply-to:from:date :message-id:subject:to:cc; bh=YSs5B1csyRfOZPOy31Unvufw4yWEBNs+50AiyNtJ/uU=; b=RekUvXMd3XLdTIrQdiVp2r+m3fE/3RLtl7a7GRa8LJ4eCMN3H/6PEJRZ+SHcm6WSY0 Tp17YYiIRhCP7IeAqAIku0z5rUDw/AQa2PXE3cC7qss3Zf0tDKI3DIFihQebmYHXelcP XL09ZXCiBS3hIEAb/mfNcYha5EdXFp4meAO6SpDTb/xTRdTAjtOYEA5p9/iWNbZOXz/1 HAscDFYCb9n+Gwn1XX3jSJ6GTeqdMViOdL9drQ33ywgUSgrKPAt/Vz4KHpSwOXkpNIHG 73iOPF8vb8GUIwhrHUyzISQMs2rMzMF7iRfuJRNgxHTqU01HS+8tSkRMcYQo9zl+wCHc ZAEQ== X-Gm-Message-State: AOAM533veIs8E4fHVeHxU7TM/VYcGvoPXVFrjOXsG6ALgoYSBucnaqdY w2LS71W4NAr5Pn6yycF7f3eqbkP4JgdxYVsx8QJ/GlFE X-Google-Smtp-Source: ABdhPJwCwbyhXjwacBPpWXD28YDOdUh4Dbgy7glH0nFKfqvnQRA3bOEDFfhNJbtQluwOfDu3I2lvXFLuNCWNOXP9WtI= X-Received: by 2002:a6b:c910:: with SMTP id z16mr3208156iof.199.1592321889770; Tue, 16 Jun 2020 08:38:09 -0700 (PDT) MIME-Version: 1.0 References: <87lfl634rs.fsf@tromey.com> <87h7vsqk5h.fsf@tromey.com> In-Reply-To: From: Caroline Tice Date: Tue, 16 Jun 2020 08:37:58 -0700 Message-ID: Subject: Re: [PATCH] Fix issues with reading rnglists, especially from dwo files, for DWARF v5 To: Tom Tromey Cc: Caroline Tice via Gdb-patches , Eric Christopher X-Spam-Status: No, score=-19.6 required=5.0 tests=BAYES_00, DKIMWL_WL_MED, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, ENV_AND_HDR_SPF_MATCH, HTML_MESSAGE, KAM_NUMSUBJECT, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, USER_IN_DEF_DKIM_WL, USER_IN_DEF_SPF_WL autolearn=no autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org Content-Type: text/plain; charset="UTF-8" X-Content-Filtered-By: Mailman/MimeDel 2.1.29 X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 16 Jun 2020 15:38:12 -0000 Ping? Could somebody please review this? -- Caroline cmtice@google.com On Tue, Jun 9, 2020 at 4:32 PM Caroline Tice wrote: > While playing with this a bit more (getting ready to work on my .dwp > file patch), I noticed that there was an important bit I left out of > the original patch: dwarf2_rnglists_process, in the original code, > always reads out of the rnglist section in the main objfile, even when > there is a .rnglist section in a .dwo file that it should be reading > instead. I had some local code that fixes this, but it didn't make it > into my previous patch (I apologize for that). The attached patch has > been updated to contain this fix as well. It also contains my fixes > and testcase from the previous patch. > > I ran the testsuite again (adding the -gdwarf-5 and -gsplit-dwarf > flags in testsuite/lib/gdb.exp), and compiling everything with clang: > > $ make check RUNTESTFLAGS="CC_FOR_TARGET=${CLANG_CC} > CXX_FOR_TARGET=${CLANG_CXX}" > > This time my patched GDB passed quite a few tests that the unpatched > version failed (when compiled with clang and passed the flags): > > Testsuite summary from unpatched GDB: > > === gdb Summary === > # of expected passes 55725 > # of unexpected failures 2956 > # of unexpected successes 83 > # of expected failures 169 > # of known failures 57 > # of unresolved testcases 115 > # of untested testcases 175 > # of unsupported tests 272 > # of paths in test names 1 > # of duplicate test names 269 > > > Testsuite summary from patched GDB: > > === gdb Summary === > > # of expected passes 56173 > # of unexpected failures 2523 > # of unexpected successes 2 > # of expected failures 250 > # of known failures 57 > # of unresolved testcases 111 > # of untested testcases 174 > # of unsupported tests 272 > # of paths in test names 1 > # of duplicate test names 269 > > Some of the tests that have more passes with the patch include: > gdb.base/break-entry.exp, gdb.base/info-fun.exp, > gdb.base/info-shared.exp,gdb.base/return-nodebug.exp, > gdb.dwarf2/fission-base.exp > (I can send you a complete list if you really want it) > > Anyway below is the updated patch for review. > > -- Caroline > cmtice@google.com > > gdb/ChangeLog: > > 2020-06-09 Caroline Tice > > * dwarf2/read.c (struct dwop_section_names): Add rnglists_dwo. > (dwop_section_names): Add .debug_rnglists.dwo, > .zdebug_rnglists.dwo. > (struct dwarf2_cu): Add cu_ranges_from_skeleton field. > (struct dwo_sections): Add rnglists field. > (dwarf2_ranges_read): Add tag parameter. > (cu_debug_rnglist_section): New function (decl & definition). > (cutu_reader::cutu_reader): Before replacing the skeleton unit > comp_unit_die with the dwo comp_unit_die, check to see if the > skeleton > unit die has a DW_AT_ranges, and if so set the > cu_ranges_from_skeleton > field in the cu. > (dwarf2_locate_dwo_sections): Add code to read rnglists_dwo > section. > (dwarf2_rnglists_process): Add a dwarf_tag parameter, for the kind > of > die whose range is being checked; add code to read the rnglist > section > from the dwo file rather than from the main objfile, if > appropriate. > Add cases for DW_RLE_base_addressx, > DW_RLE_startx_length, DW_RLE_startx_endx. Also, update to only add > the base address to DW_RLE_offset_pairs (not to all ranges). > (dwarf2_ranges_process): Add dwarf tag parameter and pass it to > dwarf2_rnglists_process. > (dwarf2_ranges_read): Add dwarf tag parameter and pass it to > dwarf2_ranges_process. > (dwarf2_get_pc_bounds): Check for DW_FORM_rnglistx when setting > need_ranges_base. Also pass die tag to dwarf2_ranges_read. > (dwarf2_record_block_ranges): Check for DW_FORM_rnglistx when > setting > need_ranges_base. Also pass die tag to dwarf2_ranges_read. > (read_full_die_1): Add code to read DW_AT_rnglists_base and assign > to > cu->ranges_base. > (partial_die_info::read): Check for DW_FORM_rnglistx when setting > need_ranges_base. Also pass die tag to dwarf2_ranges_read. > (read_rnglist_index): New function. > (read_attribute_reprocess): Add code for DW_FORM_rnglistx. > (read_attribute_value): Mark DW_FORM_rnglistx with need_reprocess. > > gdb/testsuite/ChangeLog: > > 2020-06-09 Caroline Tice > > * gdb.dwarf2/dw5-rnglist-test.cc: New file. > * gdb.dwarf2/dw5-rnglist-test.exp: New file. > > > On Thu, Jun 4, 2020 at 2:39 PM Caroline Tice wrote: > > > > I've been playing with running the testsuite with -gdwarf-5 > > -gsplit-dwarf and using the clang compiler. I did not find any tests > > that showed the issues I was running into (but I did discover a small > > bug in my patch, which my newer patch fixes). With my fixed patch, > > there are no testsuite differences. I have created a new testsuite > > test case, which does fail with current ToT and passes with my patch, > > but that only works if you compile it with clang -- if you compile it > > with GCC it passes in both cases (because GCC is not generating the > > DW_FORM_rmglistx that my patch is handling/fixing). > > > > I've updated the patch to include the requested format changes, remove > > the pieces that are only used in an upcoming patch, fix the small > > issue I found while testing, and I've added the test to the testsuite. > > To run the test and see the issue: > > > > $ make check RUNTESTFLAGS="CC_FOR_TARGET=${CLANG_CC} > > CXX_FOR_TARGET=${CLANG_CXX} dw5-rnglist-test.exp" > > > > gdb/ChangeLog (updated) > > > > 2020-06-04 Caroline Tice > > > > * dwarf2/read.c (struct dwop_section_names): Add rnglists_dwo. > > (dwop_section_names): Add .debug_rnglists.dwo, > .zdebug_rnglists.dwo. > > (struct dwo_sections): Add rnglists field. > > (cu_debug_rnglist_section): New function (decl & definition). > > (dwarf2_locate_dwo_sections): Add code to read rnglists_dwo > section. > > (dwarf2_rnglists_process): Add cases for DW_RLE_base_addressx, > > DW_RLE_startx_length, DW_RLE_startx_endx. Also, update to only > add > > the base address to DW_RLE_offset_pairs (not to all ranges). > > (dwarf2_get_pc_bounds): Check for DW_FORM_rnglistx when setting > > need_ranges_base. > > (dwarf2_record_block_ranges): Check for DW_FORM_rnglistx when > setting > > need_ranges_base. > > (read_full_die_1): Add code to read DW_AT_rnglists_base and > assign to > > cu->ranges_base. > > (partial_die_info::read): Check for DW_FORM_rnglistx when setting > > need_ranges_base. > > (read_rnglist_index): New function. > > (read_attribute_reprocess): Add code for DW_FORM_rnglistx. > > (read_attribute_value): Mark DW_FORM_rnglistx with > need_reprocess. > > > > gdb/testsuite/ChangeLog: > > > > 2020-06-04 Caroline Tice > > > > * gdb.dwarf2/dw5-rnglist-test.cc: New file. > > * gdb.dwarf2/dw5-rnglist-test.exp: New file. > > > > > > > > > > > > On Wed, Jun 3, 2020 at 7:49 AM Tom Tromey wrote: > > > > > > Caroline> $ clang++ -gdwarf-5 -O0 -gsplit-dwarf pre-order.cpp > > > Caroline> pre-order-common.cpp -o pre-order > > > > > > I wonder if the test suite can be run this way. > > > > > > Caroline> Oh. Those get used in my next upcoming patch (where I > update GDB to > > > Caroline> handle DWARFv5 .dwp files). I can either leave them in this > patch, or > > > Caroline> remove them from here and put them in the next one, > whichever you > > > Caroline> prefer. > > > > > > I think it's better for patches to be reasonably self-contained when > > > possible. > > > > > > Also, there were other patches for DWARFv5 on the list in "recent" > (this > > > year) times. I wonder if those are relevant / related. > > > > > > Tom >