| Message ID | 557893A6.1050000@ericsson.com |
|---|---|
| State | New, archived |
| Headers |
Received: (qmail 98420 invoked by alias); 10 Jun 2015 19:44:43 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <gdb-patches.sourceware.org> List-Unsubscribe: <mailto:gdb-patches-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/gdb-patches/> List-Post: <mailto:gdb-patches@sourceware.org> List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 98410 invoked by uid 89); 10 Jun 2015 19:44:42 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=AWL, BAYES_00, SPF_PASS autolearn=ham version=3.3.2 X-HELO: usevmg21.ericsson.net Received: from usevmg21.ericsson.net (HELO usevmg21.ericsson.net) (198.24.6.65) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA encrypted) ESMTPS; Wed, 10 Jun 2015 19:44:41 +0000 Received: from EUSAAHC008.ericsson.se (Unknown_Domain [147.117.188.96]) by usevmg21.ericsson.net (Symantec Mail Security) with SMTP id 20.15.06409.FBD28755; Wed, 10 Jun 2015 14:29:51 +0200 (CEST) Received: from [142.133.110.144] (147.117.188.8) by smtp-am.internal.ericsson.com (147.117.188.98) with Microsoft SMTP Server id 14.3.210.2; Wed, 10 Jun 2015 15:44:38 -0400 Message-ID: <557893A6.1050000@ericsson.com> Date: Wed, 10 Jun 2015 15:44:38 -0400 From: Simon Marchi <simon.marchi@ericsson.com> User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 MIME-Version: 1.0 To: Doug Evans <dje@google.com>, <gdb-patches@sourceware.org> Subject: Re: [PATCH 4/6] DWARF Two Level Line Tables: lnp_state_machine, lnp_reader_state References: <001a11376bdcb0959e051717abd5@google.com> In-Reply-To: <001a11376bdcb0959e051717abd5@google.com> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit X-IsSubscribed: yes |
Commit Message
Simon Marchi
June 10, 2015, 7:44 p.m. UTC
On 15-05-27 06:21 PM, Doug Evans wrote: > Doug Evans writes: > > Hi. > > > > This patch puts the line number state machine into a struct > > to make it clear exactly what is part of the state machine > > and what is not. Previously, gdb just had a bunch of local variables. > > > > 2015-03-12 Doug Evans <dje@google.com> > > > > * dwarf2read.c (lnp_state_machine): New typedef. > > (lnp_reader_state): New typedef. > > (dwarf_record_line_1): Renamed from dwarf_record_line. > > All callers updated. > > (dwarf_record_line): New function. > > (init_lnp_state_machine): New function. > > (check_line_address): Replace p_record_line parameter with state. > > All callers updated. > > (dwarf_decode_lines_1): Call dwarf_record_line, init_lnp_state_machine. > > Update to record state in lnp_state_machine. > > Here is what I committed. > Just a few comment changes to remove references to two level > line tables, which are in a later patch. Hi Doug, I have a little question about something this patch. One behaviour changed, but I don't know if it was intentional or not. I assume it is not, since the goal of this patch was to refactor/cleanup. When reading full symbols, after an end_sequence, dwarf_finish_line was called unconditionally. Now, the call to dwarf_finish_line is guarded by: if (state->last_subfile != current_subfile) { Before this patch, the two other calls to dwarf_finish_line were guarded by this if. However, the third one wasn't. Is this change intentional? I am asking this because that call is apparently important for gdb to properly understand DWARF generated by one of our internal compiler. To restore the previous behaviour, I did the following. Would it make sense to have the same change in FSF's gdb? ---8<--- --->8---
Comments
On Wed, Jun 10, 2015 at 12:44 PM, Simon Marchi <simon.marchi@ericsson.com> wrote: > On 15-05-27 06:21 PM, Doug Evans wrote: >> Doug Evans writes: >> > Hi. >> > >> > This patch puts the line number state machine into a struct >> > to make it clear exactly what is part of the state machine >> > and what is not. Previously, gdb just had a bunch of local variables. >> > >> > 2015-03-12 Doug Evans <dje@google.com> >> > >> > * dwarf2read.c (lnp_state_machine): New typedef. >> > (lnp_reader_state): New typedef. >> > (dwarf_record_line_1): Renamed from dwarf_record_line. >> > All callers updated. >> > (dwarf_record_line): New function. >> > (init_lnp_state_machine): New function. >> > (check_line_address): Replace p_record_line parameter with state. >> > All callers updated. >> > (dwarf_decode_lines_1): Call dwarf_record_line, init_lnp_state_machine. >> > Update to record state in lnp_state_machine. >> >> Here is what I committed. >> Just a few comment changes to remove references to two level >> line tables, which are in a later patch. > > Hi Doug, > > I have a little question about something this patch. One behaviour changed, > but I don't know if it was intentional or not. I assume it is not, since the goal > of this patch was to refactor/cleanup. > > When reading full symbols, after an end_sequence, dwarf_finish_line was called > unconditionally. Now, the call to dwarf_finish_line is guarded by: > > if (state->last_subfile != current_subfile) { > > Before this patch, the two other calls to dwarf_finish_line were guarded by this if. > However, the third one wasn't. Is this change intentional? > > I am asking this because that call is apparently important for gdb to properly > understand DWARF generated by one of our internal compiler. To restore the previous > behaviour, I did the following. Would it make sense to have the same change in FSF's gdb? > > ---8<--- > > diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c > index 1e290c3..d79b2e3 100644 > --- a/gdb/dwarf2read.c > +++ b/gdb/dwarf2read.c > @@ -17658,7 +17658,7 @@ dwarf_record_line (lnp_reader_state *reader, lnp_state_machine *state, > lh->file_names[file - 1].included_p = 1; > if (reader->record_lines_p && is_stmt) > { > - if (state->last_subfile != current_subfile) > + if (state->last_subfile != current_subfile || end_sequence) > { > dwarf_finish_line (reader->gdbarch, state->last_subfile, > state->address, state->record_line); > > --->8--- Yikes. My bad, thanks for catching this. LGTM
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c index 1e290c3..d79b2e3 100644 --- a/gdb/dwarf2read.c +++ b/gdb/dwarf2read.c @@ -17658,7 +17658,7 @@ dwarf_record_line (lnp_reader_state *reader, lnp_state_machine *state, lh->file_names[file - 1].included_p = 1; if (reader->record_lines_p && is_stmt) { - if (state->last_subfile != current_subfile) + if (state->last_subfile != current_subfile || end_sequence) { dwarf_finish_line (reader->gdbarch, state->last_subfile, state->address, state->record_line);