From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 21843 invoked by alias); 18 Sep 2014 16:42:09 -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 21830 invoked by uid 89); 18 Sep 2014 16:42:08 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.7 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_LOW,RP_MATCHES_RCVD,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-pd0-f202.google.com Received: from mail-pd0-f202.google.com (HELO mail-pd0-f202.google.com) (209.85.192.202) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Thu, 18 Sep 2014 16:42:06 +0000 Received: by mail-pd0-f202.google.com with SMTP id ft15so334529pdb.5 for ; Thu, 18 Sep 2014 09:42:04 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:mime-version:content-type :content-transfer-encoding:message-id:date:to:cc:subject:in-reply-to :references; bh=E8fHJ3hw/FbseubeOQ9//+Hie2jAFzEowGPXScaosk0=; b=LSWsBwTLjlAdEpkR6oe6LQHHEiv8qzemcnyw/pSprx4XxxnIDyVVP7yvJR5qm6n6NT lZ/QL+9R27rU88+n171czC4173qKaSD2Jfw2j5DXm97VVR/H+uxIoDdVjJwXsLWH/I7U JyoPNK9LqLCPuviU78ouaO1d1VRv2xYyKIfd9lQclwcTjro7W5OuuJTPHEzmsdHAcP6n U3tDhJeNsvXYi0jCmgU0JsMy/9N9ywv6VaMTUVU+sIAwnModn1ke34t3r11T5NpuTGKG xBPGtZ6g1/LYF022/gnp5Vg0zwg7OJe+278wtIM4lkMhhv0GbnU7qUW/rewDG8wGeuc8 2cRA== X-Gm-Message-State: ALoCoQlNqa8cKaCDclxr2R6FHcS9PMki6oCvB+GsCUkQh7yMS2LKxxM9om6vpPO+ep4S1Nz8tVEP/k/SvbGOueKHei5Xv5Nc63qpEopyb+Z8hImiowg9cuDQvC6wkOxihWQ0aTTmUGKOCdeQNoGz/maEliG3uCLjeJSciuakdXT4h+4QXD1L6YE= X-Received: by 10.70.37.72 with SMTP id w8mr4545900pdj.0.1411058524877; Thu, 18 Sep 2014 09:42:04 -0700 (PDT) Received: from corpmail-nozzle1-1.hot.corp.google.com ([100.108.1.104]) by gmr-mx.google.com with ESMTPS id t28si982640yhb.4.2014.09.18.09.42.04 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 18 Sep 2014 09:42:04 -0700 (PDT) Received: from ruffy2.mtv.corp.google.com ([172.17.128.107]) by corpmail-nozzle1-1.hot.corp.google.com with ESMTP id fWMhbLyv.1; Thu, 18 Sep 2014 09:42:04 -0700 From: Doug Evans MIME-Version: 1.0 Content-Type: text/plain; charset=unknown Content-Transfer-Encoding: 7bit Message-ID: <21531.2907.799313.742622@ruffy2.mtv.corp.google.com> Date: Thu, 18 Sep 2014 16:42:00 -0000 To: Yao Qi Cc: Subject: Re: [PATCH 1/3] Check function is GC'ed In-Reply-To: <87ioko1ec0.fsf@codesourcery.com> References: <53D8A264.1050103@codesourcery.com> <1408609338-17561-1-git-send-email-yao@codesourcery.com> <21495.32892.341399.802579@ruffy2.mtv.corp.google.com> <87sikggadr.fsf@codesourcery.com> <21527.13753.980949.662368@ruffy2.mtv.corp.google.com> <87ioko1ec0.fsf@codesourcery.com> X-IsSubscribed: yes X-SW-Source: 2014-09/txt/msg00620.txt.bz2 Yao Qi writes: > Doug Evans writes: > > Thanks for the review, Doug. > > > > /* NOTE: pst->dirname is DW_AT_comp_dir (if present). */ > > > - dwarf_decode_lines (lh, pst->dirname, cu, pst); > > > + dwarf_decode_lines (lh, pst->dirname, cu, pst, lowpc); > > > > We don't really need a "lowpc" argument to dwarf2_build_include_psymtabs. > > Replace that with: > > > > dwarf_decode_lines (lh, pst->dirname, cu, pst, pst->textlow); > > > > and remove the lowpc argument to dwarf2_build_include_psymtabs. > > > > That is fine to me. The reason I add "lowpc" argument to > dwarf2_build_include_psymtabs is that I'd like to keep the similarity > between dwarf2_build_include_psymtabs and handle_DW_AT_stmt_list. > > > > @@ -17252,7 +17255,8 @@ dwarf_finish_line (struct gdbarch *gdbarch, struct subfile *subfile, > > > > > > static void > > > dwarf_decode_lines_1 (struct line_header *lh, const char *comp_dir, > > > - struct dwarf2_cu *cu, const int decode_for_pst_p) > > > + struct dwarf2_cu *cu, const int decode_for_pst_p, > > > + CORE_ADDR lowpc) > > > { > > > const gdb_byte *line_ptr, *extended_end; > > > const gdb_byte *line_end; > > > @@ -17375,7 +17379,9 @@ dwarf_decode_lines_1 (struct line_header *lh, const char *comp_dir, > > > case DW_LNE_set_address: > > > address = read_address (abfd, line_ptr, cu, &bytes_read); > > > > > > - if (address == 0 && !dwarf2_per_objfile->has_section_at_zero) > > > + if (address == 0 > > > + && (!dwarf2_per_objfile->has_section_at_zero > > > + || lowpc > address)) > > > > I'm trying to decide whether to keep the > > "!dwarf2_per_objfile->has_section_at_zero" > > test here. It feels wrong to keep it: What does it matter > > whether any section has address zero? It was only used as a proxy > > for a better test. Here we know we have an address > > that is outside the CU in question, which is a far more specific test > > than just checking whether any section is at address zero. > > But maybe I'm missing something. > > I thought about this when I wrote the patch. I keep it in order to > preserve GDB's behaviour. It should be OK to remove it. > > > > > One could even argue that the "address == 0" test is also > > now superfluous. > > > > IOW, I'm wondering if we could just write the following here: > > > > if (lowpc > address) > > > > But if we write it that way then the code no longer readily expresses > > what we're trying to do here which is handle the specific case expressed by > > the comment that follows: "This line table is for a function which has been > > GCd by the linker." Instead we'd be now testing for a more general > > case which would include bad debug info. > > > > What do you think? > > > > I'm leaning towards not changing things too much in this patch > > and only handling GC'd functions. Therefore I'm leaning towards > > something like the following: > > > > /* If address < lowpc then it's not a usable value, it's > > outside the pc range of the CU. However, we restrict > > the test to only address values of zero to preserve > > GDB's previous behaviour which is to handle the specific > > case of a function being GC'd by the linker. */ > > if (address == 0 && address < lowpc) > > I agree with you on this. This patch is to fix a GDB bug when a > function is GC'ed by linker, we can concentrate on this at first. We > can remove "address == 0" if it fixes some bugs we find in the future. > > Patch below is updated to address your comments above. > > -- > Yao ( ) > > From: Yao Qi > Subject: [PATCH] Check function is GC'ed > > I see the following fail on arm-none-eabi target, > > (gdb) b 24^M > Breakpoint 1 at 0x4: file > ../../../../git/gdb/testsuite/gdb.base/break-on-linker-gcd-function.cc, > line 24.^M > (gdb) FAIL: gdb.base/break-on-linker-gcd-function.exp: b 24 > > Currently, we are using flag has_section_at_zero to determine whether > address zero in debug info means the corresponding code has been > GC'ed, like this: > > case DW_LNE_set_address: > address = read_address (abfd, line_ptr, cu, &bytes_read); > > if (address == 0 && !dwarf2_per_objfile->has_section_at_zero) > { > /* This line table is for a function which has been > GCd by the linker. Ignore it. PR gdb/12528 */ > > However, this is incorrect on some bare metal targets, as .text > section is located at 0x0, so dwarf2_per_objfile->has_section_at_zero > is true. If a function is GC'ed by linker, the address is zero. GDB > thinks address zero is a function's address rather than this function > is GC'ed. > > In this patch, we choose 'lowpc' got in read_file_scope to check > whether 'lowpc' is greater than zero. If it isn't, address zero really > means the function is GC'ed. In this patch, we pass 'lowpc' in > read_file_scope through handle_DW_AT_stmt_list and dwarf_decode_lines, > and to dwarf_decode_lines_1 finally. > > This patch fixes the fail above. This patch also covers the path that > partial symbol isn't used, which is tested by starting gdb with > --readnow option. > > It is regression tested on x86-linux with > target_board=dwarf4-gdb-index, and arm-none-eabi. OK to apply? > > gdb: > > 2014-09-16 Yao Qi > > * dwarf2read.c (dwarf_decode_lines): Update declaration. > (handle_DW_AT_stmt_list): Add argument 'lowpc'. Update > comments. Callers update. > (dwarf_decode_lines): Likewise. > (dwarf_decode_lines_1): Add argument 'lowpc'. Update > comments. Skip the line table if 'lowpc' is greater than > 'address'. Don't check > dwarf2_per_objfile->has_section_at_zero. > > gdb/testsuite: > > 2014-09-16 Yao Qi > > * gdb.base/break-on-linker-gcd-function.exp: Move test into new > proc set_breakpoint_on_gcd_function. Invoke > set_breakpoint_on_gcd_function. Restart GDB with --readnow and > invoke set_breakpoint_on_gcd_function again. LGTM.