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: [tests patchv2] FYI unwinder: tests/ update


On Sun, 2013-12-01 at 18:09 +0100, Jan Kratochvil wrote:
> > And why are they in the test source code?
> 
> Added there:
>   /* These dummy* functions are there so that each of their surrounding
>      functions has some unrelated code around.  The purpose of some of the
>      tests is verify unwinding the very first / after the very last instruction 
>      does not inappropriately slip into the unrelated code around.  */

Thanks, that wasn't clear to me before. Makes sense.

> > I am not really clear on why the changing the PC is needed though.
> 
> It is part of the test how to test unwinding when asynchronous signal happened
> during execution of the very first instruction of a function.  There is the
> risk of accidentally using unwind info for the previous (PC-located before,
> with lower PC addresses) function instead.
>
> Easier method how to test it is welcome.

Aha. OK. Maybe you could try to force a SIGSEGV in a function with as
first instruction something like "*(int *) 0x1234 = 0x12345;" though I
assume that might still move the pc just past this instruction. So just
keep it as is.

You did now document this in backtrace-child. But it would not be a bad
idea to add a little comment to backtrace.c (prepare_thread) to explain
the intent. It is kind of tricky.

> > > I have removed the .plt testing code now.
> > 
> > I don't really understand why it was there in the first place, so cannot
> > tell whether that is good or bad.
> 
> I have already explained that it was a way how to test non-trivial DWARF
> expressions.
> 
> $ readelf -wf /bin/bash|grep DW_CFA_def_cfa_expression
>   DW_CFA_def_cfa_expression (DW_OP_breg7 (rsp): 8; DW_OP_breg16 (rip): 0; DW_OP_lit15; DW_OP_and; DW_OP_lit11; DW_OP_ge; DW_OP_lit3; DW_OP_shl; DW_OP_plus)
> 
> This line is from .plt CFI.  In normal executables DWARF expression is never
> used anywhere else.
> 
> But as tests/cleanup-13.c is now much more thorough test I could drop the .plt
> DWARF expression exploit.

Aha. OK, good.

> > > > This really could use some documentation or better argument parsing IMHO.
> > > > Why not have simpler arguments:
> > > > 
> > > > --backtrace-child
> > > > --backtrace-pid <pid>
> > > > --backtrace-core <corefile>
> > > > --backtrace-exe <exefile>
> > > > --backtrace-exe-core <exefile> <corefile>
> [...]
> > Not really, because I don't see the logic of the current argument
> > parsing. But at least the command line syntax is now documented. It is
> > hard to see this as some general backtracing tool since it has so many
> > x86_64 specific hacks in it.
> 
> So there is now argp option:
>       --backtrace-exec=EXEC  Run executable
> 
> but the other combinations are accessed via standard -e and --core.
> Is it OK this way?

Sure.

> > > Without 'true' the script will fail due to 'set -e'.
> > > 
> > > After program dumps a core its exit code is not zero.  'true' overrides it
> > > there.
> > 
> > Aha. How inconvenient. So you cannot see the difference between whether
> > the core file was actually generated or not because of some failure?
> 
> In such case the backtrace frames will not match the expectation.

Yeah, OK. But if the generation of the core file fails you might want to
note that separately (or even just SKIP the test). Not a big issue. One
day we might have some other way to generate core files that is easier
to see whether it worked or not. For now it has to do.

Thanks,

Mark


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