This is the mail archive of the elfutils-devel@sourceware.org mailing list for the elfutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] libdwfl: Special case linux-gate/vdso.so in core_file_read_eagerly.


On Fri, 2015-03-27 at 15:29 -0700, Roland McGrath wrote:
> Please add some comments so this is all clear when reading the code cold.

Good idea. I added some comments throughout the function.
From d6a573a0ec54a1758837503a2c3211ed66f76e1c Mon Sep 17 00:00:00 2001
From: Mark Wielaard <mjw@redhat.com>
Date: Tue, 31 Mar 2015 11:33:53 +0200
Subject: [PATCH] libdwfl: Special case core_file_read_eagerly for small ELF
 images.

Small ELF images, like linux-gate or linux-vdso, might be available in the
core file, but not on disk, even if we have a build-id. If the whole image
is small enough try to read them in from the core file to make sure symbols
and unwind information are always available for them. We would already map
them in if the core file was opened with ELF_C_READ_MMAP.

https://bugzilla.redhat.com/show_bug.cgi?id=1129756

Signed-off-by: Mark Wielaard <mjw@redhat.com>
---
 libdwfl/ChangeLog   |  4 ++++
 libdwfl/core-file.c | 26 +++++++++++++++++++-------
 2 files changed, 23 insertions(+), 7 deletions(-)

diff --git a/libdwfl/ChangeLog b/libdwfl/ChangeLog
index d40dbae..143d381 100644
--- a/libdwfl/ChangeLog
+++ b/libdwfl/ChangeLog
@@ -1,3 +1,7 @@
+2015-03-31  Mark Wielaard  <mjw@redhat.com>
+
+       * core-file.c (core_file_read_eagerly): Special case small images.
+
 2015-01-26  Mark Wielaard  <mjw@redhat.com>
 
 	* dwfl_module_getdwarf.c (find_symtab): Explicitly clear symdata,
diff --git a/libdwfl/core-file.c b/libdwfl/core-file.c
index 50031ae..324e9d2 100644
--- a/libdwfl/core-file.c
+++ b/libdwfl/core-file.c
@@ -1,5 +1,5 @@
 /* Core file handling.
-   Copyright (C) 2008-2010, 2013 Red Hat, Inc.
+   Copyright (C) 2008-2010, 2013, 2015 Red Hat, Inc.
    This file is part of elfutils.
 
    This file is free software; you can redistribute it and/or modify
@@ -161,6 +161,9 @@ dwfl_report_core_segments (Dwfl *dwfl, Elf *elf, size_t phnum, GElf_Phdr *notes)
 /* Never read more than this much without mmap.  */
 #define MAX_EAGER_COST	8192
 
+/* Dwfl_Module_Callback passed to and called by dwfl_segment_report_module
+   to read in a segment as ELF image directly if possible or indicate an
+   attempt must be made to read in the while segment right now.  */
 static bool
 core_file_read_eagerly (Dwfl_Module *mod,
 			void **userdata __attribute__ ((unused)),
@@ -174,6 +177,10 @@ core_file_read_eagerly (Dwfl_Module *mod,
 {
   Elf *core = arg;
 
+  /* The available buffer is often the whole segment when the core file
+     was mmap'd if used together with the dwfl_elf_phdr_memory_callback.
+     Which means that if it is complete we can just construct the whole
+     ELF image right now without having to read in anything more.  */
   if (whole <= *buffer_available)
     {
       /* All there ever was, we already have on hand.  */
@@ -198,8 +205,9 @@ core_file_read_eagerly (Dwfl_Module *mod,
       return *elfp != NULL;
     }
 
-  /* We don't have the whole file.
-     Figure out if this is better than nothing.  */
+  /* We don't have the whole file.  Which either means the core file
+     wasn't mmap'd, but needs to still be read in, or that the segment
+     is truncated.  Figure out if this is better than nothing.  */
 
   if (worthwhile == 0)
     /* Caller doesn't think so.  */
@@ -212,12 +220,16 @@ core_file_read_eagerly (Dwfl_Module *mod,
     requires find_elf hook re-doing the magic to fall back if no file found
   */
 
-  if (mod->build_id_len > 0)
-    /* There is a build ID that could help us find the whole file,
-       which might be more useful than what we have.
-       We'll just rely on that.  */
+  if (whole > MAX_EAGER_COST && mod->build_id_len > 0)
+    /* We can't cheaply read the whole file here, so we'd
+       be using a partial file.  But there is a build ID that could
+       help us find the whole file, which might be more useful than
+       what we have.  We'll just rely on that.  */
     return false;
 
+  /* The file is either small (most likely the vdso) or big and incomplete,
+     but we don't have a build-id.  */
+
   if (core->map_address != NULL)
     /* It's cheap to get, so get it.  */
     return true;
-- 
1.8.3.1


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]