Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Simon Marchi <simon.marchi@polymtl.ca>
To: Ali Tamur <tamur@google.com>, gdb-patches@sourceware.org
Subject: Re: [PATCH] Dwarf 5: Handle debug_str_offsets and indexed attributes that have base offsets.
Date: Sat, 11 Jan 2020 13:35:00 -0000	[thread overview]
Message-ID: <487d0942-b589-6007-8ff0-ad38cdab08e8@polymtl.ca> (raw)
In-Reply-To: <20200111034542.57319-1-tamur@google.com>

On 2020-01-10 10:45 p.m., Ali Tamur via gdb-patches wrote:
>> Could this be
>> cu->addr_base = lookup_addr_base (die);
> gdb.dwarf2/fission-base.exp consistently failed when I tried your suggestion,
> and now I am scared whether it is a safe change. (I don't understand the
> control flow well, but if cu->addr_base was assigned previously and
> lookup_addr_base fails, it would unassign the previous value.

Yeah, I'm not sure either.  I'm fairly confident that if, for some reason,
the field is already set, then it would set it to the same value.  After all,
stub_comp_unit_die is the stub DIE that corresponds to the DWO file we
are reading.  If for some reason, there  is already a value in cu->addr_base,
then it must come from this DIE.

If your tests pass, I think we can leave it like this, but of course that can
be changed if that proves to be wrong.

>> what is the reason we need to have two separate loops here again?
> Converted to a single loop.
> 
>> What happens if we encounter one these indirect string forms (strx and
>> company) here, but we don't enter any of the two "if" clauses above?
> I think that means an incorrect DWARF; I removed the if condition, so the
> complaint in read_stub_str_index will trigger.

Ok, that's fine with me, it will error out immediatly in that case.

The patch LGTM, but I think we are just missing a ChangeLog entry?

Simon


  reply	other threads:[~2020-01-11 13:35 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <f8620619-d480-6e9c-39f0-f3d540bc4d0d@polymtl.ca>
2020-01-09  0:57 ` Ali Tamur via gdb-patches
2020-01-10  3:46   ` Simon Marchi
2020-01-11  3:45     ` Ali Tamur via gdb-patches
2020-01-11 13:35       ` Simon Marchi [this message]
2020-01-14  3:46         ` Ali Tamur via gdb-patches
2019-11-01 23:44 Ali Tamur via gdb-patches
2019-11-12 20:25 ` Ali Tamur via gdb-patches
2019-11-15  3:05 ` Simon Marchi
2019-11-19  4:46   ` Ali Tamur via gdb-patches
2019-11-19  5:27     ` Simon Marchi
2019-11-20  4:29     ` Simon Marchi
2019-12-27  0:41       ` Ali Tamur via gdb-patches
2019-12-27 23:20         ` Simon Marchi
2020-01-08  5:19           ` Ali Tamur via gdb-patches

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=487d0942-b589-6007-8ff0-ad38cdab08e8@polymtl.ca \
    --to=simon.marchi@polymtl.ca \
    --cc=gdb-patches@sourceware.org \
    --cc=tamur@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox