[Bug tools/21522] eu-strip generates empty output if there is nothing to do

2017-06-14 Thread mark at klomp dot org
https://sourceware.org/bugzilla/show_bug.cgi?id=21522

Mark Wielaard  changed:

   What|Removed |Added

 Status|ASSIGNED|RESOLVED
 Resolution|--- |FIXED

--- Comment #3 from Mark Wielaard  ---
commit b58aebe71e0b4863db1b7fd3e942e36303257f3a
Author: Mark Wielaard 
Date:   Wed Jun 7 20:32:38 2017 +0200

strip: Don't generate empty output file when nothing to do.

If there was nothing to do strip would skip generating a separate
debug file if one was requested, but it would also not finish the
creation of a new output file (with the non-stripped sections).
Also if there was an error any partially created output would be kept.

Make sure that when the -o output file option is given we always generate
a complete output file (except on error). Also make sure that when the -f
debug file option is given it is only generated when it is not empty.

Add testcase run-strip-nothing.sh that tests the various combinations.

https://sourceware.org/bugzilla/show_bug.cgi?id=21522

Signed-off-by: Mark Wielaard 

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug tools/21525] Multiple .shstrtab sections since eu-readelf 0.166

2017-06-14 Thread mark at klomp dot org
https://sourceware.org/bugzilla/show_bug.cgi?id=21525

Mark Wielaard  changed:

   What|Removed |Added

 Status|ASSIGNED|RESOLVED
 Resolution|--- |FIXED

--- Comment #3 from Mark Wielaard  ---
commit e0b3232e9a0bc8215ca1be29b1ec6df43811eb87
Author: Mark Wielaard 
Date:   Wed Jun 7 14:05:36 2017 +0200

strip: Make sure old .shstrab is removed when eu-strip recreates it.

Although we always recreate the .shstrtab section for the new output
file we never explicitly assumed it could be removed. It might not be
possible to remove it when the section string table is shared with
a symbol table. But if it is removable we should (and recreate it for
the new section list).

Regression introduced in commit elfutils-0.163-33-gdf7dfab.
"Handle merged strtab/shstrtab string tables in strip and unstrip."
Add extra testcase to explicitly check for this case.

https://sourceware.org/bugzilla/show_bug.cgi?id=21525

Signed-off-by: Mark Wielaard 

-- 
You are receiving this mail because:
You are on the CC list for the bug.

Re: How to debug broken unwinding?

2017-06-14 Thread Mark Wielaard
On Tue, 2017-06-13 at 18:17 +0200, Mark Wielaard wrote:
> The tricky case is that last case. If the previous frame is a signal
> frame then this frame is also an activation (so you don't need to
> subtract 1 to get the actual address that was executing). I am not sure
> I fully understand anymore why that is. This must be a frame that is
> itself not an initial or signal frame, but that is called (activated)
> from a signal frame.
> 
> But assuming that is the correct semantics then your original
> observation seems to explain what is going on. The code doesn't account
> for the fact that during an unwind other modules can be reported that
> might make it able to unwind the current frame. And it caches the
> failure state of the unwind of the current frame, so it doesn't retry
> when asked.
> 
> I think we could reset the state of the current frame if any more
> modules are reported. But is it possible for your parser or perf to
> report modules as soon as it knows about them, not lazily during an
> unwind? Is reporting lazily at the last possible moment an optimization
> that really helps? I would assume that you end up reporting all modules
> anyway. So it actually seems easier to report everything upfront.

The following should reset the state of the current thread frame if a
module is added during unwinding. I haven't replicated your issue yet to
verify. And adding modules outside of dwfl_report_begin/end during
unwind wasn't really intended to work. But since there is code out there
doing it anyway maybe we should try to support it. Does the following
help your case?

diff --git a/libdwfl/dwfl_module.c b/libdwfl/dwfl_module.c
index 510bd69..dc81538 100644
--- a/libdwfl/dwfl_module.c
+++ b/libdwfl/dwfl_module.c
@@ -156,6 +156,9 @@ use (Dwfl_Module *mod, Dwfl_Module **tailp, Dwfl *dwfl)
   dwfl->lookup_module = NULL;
 }
 
+  /* Note that the module list changed.  */
+  dwfl->dwfl_module_state++;
+
   return mod;
 }
 
diff --git a/libdwfl/frame_unwind.c b/libdwfl/frame_unwind.c
index 4dc9c43..9b2c2d4 100644
--- a/libdwfl/frame_unwind.c
+++ b/libdwfl/frame_unwind.c
@@ -520,6 +520,7 @@ new_unwound (Dwfl_Frame *state)
   unwound = malloc (sizeof (*unwound) + sizeof (*unwound->regs) * nregs);
   if (unlikely (unwound == NULL))
 return NULL;
+  thread->dwfl_module_state = process->dwfl->dwfl_module_state;
   state->unwound = unwound;
   unwound->thread = thread;
   unwound->unwound = NULL;
@@ -700,7 +701,22 @@ internal_function
 __libdwfl_frame_unwind (Dwfl_Frame *state)
 {
   if (state->unwound)
-return;
+{
+  /* If it wasn't an error, then go for it.  */
+  if (state->unwound->pc_state != DWFL_FRAME_STATE_ERROR)
+   return;
+
+  /* If it was an error and the dwfl module list is the same, too bad.  */
+  uint64_t t_state, d_state;
+  t_state = state->thread->dwfl_module_state;
+  d_state = state->thread->process->dwfl->dwfl_module_state;
+  if (t_state == d_state)
+   return;
+
+  /* OK, lets try again. */
+  free (state->unwound);
+  state->unwound = NULL;
+}
   /* Do not ask dwfl_frame_pc for ISACTIVATION, it would try to unwind STATE
  which would deadlock us.  */
   Dwarf_Addr pc;
diff --git a/libdwfl/libdwflP.h b/libdwfl/libdwflP.h
index 7d5f795..9d38373 100644
--- a/libdwfl/libdwflP.h
+++ b/libdwfl/libdwflP.h
@@ -131,6 +131,11 @@ struct Dwfl
   Dwfl_Module **lookup_module; /* Module associated with segment, or null.  */
   int *lookup_segndx;  /* User segment index, or -1.  */
 
+  /* State (counter) of Dwfl module list.  Used to check by Dwfl_Thread
+ when updating Dwfl_Frame state if there was a previous error that
+ might be resolved when using an updated Dwfl module list.  */
+  uint64_t dwfl_module_state;
+
   /* Cache from last dwfl_report_segment call.  */
   const void *lookup_tail_ident;
   GElf_Off lookup_tail_vaddr;
@@ -244,6 +249,10 @@ struct Dwfl_Thread
  Later the processed frames get freed and this pointer is updated.  */
   Dwfl_Frame *unwound;
   void *callbacks_arg;
+  /* State of the Dwfl Modules last time an unwind wof the Dwfl_Frame
+ was attempted that resulted in an error.  Indicates it can be
+ retried if it is different from the state of the Dwfl.  */
+  uint64_t dwfl_module_state;
 };
 
 /* See its typedef in libdwfl.h.  */




Re: overflows in Dwfl_Thread_Callbacks::memory_read callback

2017-06-14 Thread Mark Wielaard
On Tue, 2017-06-13 at 18:15 +0200, Milian Wolff wrote:
> > I am not following the above trace completely, but what is going on
> > seems to be that we have CFI and want to get a register value. So we
> > call dwarf_frame_register to determine the DWARF expression operations
> > that we need to execute to get the register value. dwarf_frame_register
> > determines that that the register is described by a register offset
> > value rule, so it generates operations saying see an the CFA
> > (DW_OP_call_frame_cfa) plus some offset (DW_OP_plus_uconst) as a value
> > (so read the value from cfa + offset, which is somewhere on the stack).
> > But then the cfa comes out as 15? That is obviously bogus. But I don't
> > really understand how that happened. It should be a value somewhere near
> > the current stack. Then reading 15 - offset (16) clearly fails.
> 
> Right, so that cfa value - is that in the DWARF info? So is the dso "broken"? 
> Or is this some runtime value that we returned earlier in our register or 
> memory callback implementation?

This is how DWARF defines the CFA:

An area of memory that is allocated on a stack called a “call
frame.” The call frame is identified by an address on the stack.
We refer to this address as the Canonical Frame Address or CFA.
Typically, the CFA is defined to be the value of the stack
pointer at the call site in the previous frame (which may be
different from its value on entry to the current frame).

So in practice it points inside the stack where the previous stack
pointer was when this function was called. Various things can often be
found as offset of the CFA, the return address, saved register values,
arguments to the current function etc. You start unwinding by first
calculating the CFA. I cannot immediately see why that went wrong in
this case. But either we picked the wrong CFI (Call Frame Information)
for the current pc and are just calculating something bogus. Or one of
the needed values to calculate the CFA was garbage (either a bad
register value or a bad stack value).

Cheers,

Makr