Re: [LTO]: Handle DW_form_ref_addr

[Mark Mitchell <mark at codesourcery dot com>]

Daniel Berlin wrote:
> So, first, I just wanted to point out that the libelf 0.8.5 I found
> included in my OpenSUSE 10.1 distro (and the web), and Ubuntu, doesn't
> include "elf_getshstrndx" (all other functions are there).

I happened to be using a RHEL 4 machine, so that explains the
difference.  Yes, I agree that we may have to add this somewhere, and/or
contribute it to the other libelf.

> This patch enables us to handle DW_FORM_ref_addr, including both forward
> and backward references to compilation units.

Way cool!

> Along the way to supporting DW_FORM_ref_addr, I fixed a bug in the
> compilation unit length calculation. 

Thanks!

> I had to make a copy of the DWARF2_Internal_CompUnit structure, because
> it does not include the offset from the beginning of debug_info.

+struct DWARF2_CompUnit
+{
+  /* Offset from start of .debug_info for this CU.  */
+  unsigned long cu_start_offset;

Given that we're creating this structure anyhow, we may as well use
uint64_t for these fields, so as to accommodate 64-bit DWARF.

> 2. lto_read_form has an extra output pointer to the context necessary to
> read the form.

> The only ugly part of this patch is that we have to have a pointer to
> the info_fd in the context.
> 
> 1. Change every single place that takes an lto_fd to take an
> lto_info_fd, and change all of them to access the cur pointers through base.

I'd prefer this option.

> What are we doing about testing, BTW?

Heh.  I've got "Write DejaGNU bits" on my to-do list after "Build symbol
table infrastructure" and "Build merge infrastructure".  So, right now,
nothing. :-)

Here are a few other nits on the patch.

+   Set FIELD_SIZE to how long the initial length field was.  */

"Set *FIELD_SIZE to the size (in bytes) of the initial length field."

+static DWARF2_CompUnit *
+find_cu_for_offset (const lto_info_fd *fd,
+		    unsigned long offset)

Shouldn't that be a uint64_t?  Also, shouldn't this function should
check for an out-of-range offset, and call lto_file_corrupt_error?

 static void
 lto_read_form (lto_fd *fd,
 	       const DWARF2_attr *attr,
-	       const lto_context *context,
+	       lto_context *context,
+	       lto_context **form_context,

Need documentation for the new FORM_CONTEXT parameter.

+  /* We should only read the abbreviations once.  */

You cut-and-paste the comment, but now it should say "compilation units"
instead of "abbreviations".

+static void
+lto_set_cu_context (lto_context *context, lto_fd *fd,
+		    DWARF2_CompUnit *unit)
+{
+  context->cu_start = fd->start;
+  context->cu_end = fd->start + unit->cu_length;
+  context->current_cu = unit;
+}

Why isn't context->cu_start set to fd->start + unit->cu_start_offset?
And, then context->cu_end set to context->cu_start + context->cu_length?

OK with changes above, thanks!

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713