This is the mail archive of the
elfutils-devel@sourceware.org
mailing list for the elfutils project.
Re: [PATCH v3 4/6] tests/allfcts.c: Install alternate debug information
- From: Florian Weimer <fweimer at redhat dot com>
- To: elfutils-devel at lists dot fedorahosted dot org
- Date: Tue, 22 Apr 2014 09:37:40 +0200
- Subject: Re: [PATCH v3 4/6] tests/allfcts.c: Install alternate debug information
On 04/18/2014 01:58 PM, Mark Wielaard wrote:
> On Tue, 2014-04-15 at 16:58 +0200, Florian Weimer wrote:
>> This change also adds more error checking and reporting.
>>
>> + * allfcts.c (setup_alt): New function.
>> + (main): Call it. Implementation additional error checking and
>> + reporting.
>
>> +static Dwarf *
>> +setup_alt (Dwarf *main)
>> +{
>> + const void *build_id;
>> + size_t build_id_len;
>> + const char *alt_name = dwelf_dwarf_gnu_debugaltlink
>> + (main, &build_id, &build_id_len);
>> + if (alt_name == NULL)
>> + return NULL;
>> + int fd = open (alt_name, O_RDONLY);
>> + if (fd < 0)
>> + err (1, "open (%s)", alt_name);
>> + Dwarf *dbg_alt = dwarf_begin (fd, DWARF_C_READ);
>> + close (fd);
>
> I am slightly surprised this actually works. Normally you cannot just
> close the fd of the underlying Dwarf (or Elf). For an Elf you can if you
> force it to have been read completely into memory first by doing
> elf_cntl (elf, ELF_C_FDREAD) as is currently done in try_debugaltlink.
Yes, now I'm surprised as well. :-)
> Which you could do here (on the Elf and then call dwarf_begin_elf) if
> you want to make sure to not leak the fd.
I went this route.
>> doff = dwarf_getfuncs (die, cb, NULL, doff);
>> + if (dwarf_errno () != 0)
>> + errx (1, "dwarf_getfuncs (%s): %s",
>> + argv[i], dwarf_errmsg (-1));
>
> I think this works fine, and the original code did this, but in a less
> convenient way (you wouldn't get any error reported). So keep it as is.
>
> But the way dwarf_getfuncs returns an error state is by returning -1.
> Which isn't really documented and somewhat awkward, so what you do is
> nicer. It is just that theoretically a callback could trigger
> dwarf_errno being set and ignore that as being fine, which might be
> silly, but "legal". You would then pick up a non-fatal dwarf_errno.
I added this check because when I still had an actual error code (not
zero) for the missing .gnu_debugaltlink section in
dwelf_dwarf_gnu_debugaltlink, the test case broke in a slightly
non-obvious way.
--
Florian Weimer / Red Hat Product Security Team