[RFC ABRT PATCH v2] Add gdb python plugin which analyzes vulnerability

Jakub Filak jfilak at redhat.com
Mon Jun 17 14:08:30 UTC 2013


On Mon, 2013-06-17 at 15:56 +0200, Denys Vlasenko wrote:
> On 06/13/2013 04:55 PM, Martin Milata wrote:
> > Some remarks regarding the code below:
> > 
> >> +    if self.operands.startswith("0x") or self.operands.startswith("-0x") or self.operands.startswith("("):
> >> +        mem_op_pos = 0
> >> +    else:
> >> +        mem_op_pos = self.operands.find(",0x")
> >> +        if mem_op_pos < 0:
> >> +            mem_op_pos = self.operands.find(",-0x")
> >> +        if mem_op_pos < 0:
> >> +            mem_op_pos = self.operands.find(":0x")
> >> +        if mem_op_pos < 0:
> >> +            mem_op_pos = self.operands.find(":-0x")
> >> +        if mem_op_pos < 0:
> >> +            mem_op_pos = self.operands.find(",(")
> >> +        if mem_op_pos < 0:
> >> +            mem_op_pos = self.operands.find(":(")
> >> +        if mem_op_pos < 0:
> >> +            return False # no memory operands
> >> +        mem_op_pos += 1
> > Can you use a regular expression for this, possibly with splitting the
> > operands into list first? http://docs.python.org/2.7/library/re.html
> 
> I think "import re", compiling of the regexp and matching
> will be slower than searching for a string.
> It is also less understandable:

regex is more understandable for me

> 
> regex = re.compile('^\s+(0x[0-9a-f]+)\s+(\<\+\d+\>):\s+(.*)$')
> 
> I can't easily tell what this regexp is doing.


I can tell what this regexp is doing but I can't easily tell what the
lines above are doing.

> The only benefit is that using regex may take a few lines less
> than existing code.
> 
> >> +        # HACK_ALERT: kernels before 3.?.? do not record siginfo in coredumps,
> >> +        # so $_siginfo isn't present.
> > By the way, do you have any reference for this? The prstatus structure
> > which contains a signal number seems to be present in coredumps at least
> > on 2.6.32. Or am I misunderstanding the problem?
> 
> The point is, gdb needs _complete_ siginfo to be able to show $_siginfo.
> A few bits of signal info in prstatus is a complete siginfo.
> (Of course, $_siginfo.si_signo doesn't need complete siginfo,
> but gdb doesn't know that...)
> 
> >> +    for line in raw_instructions.split("\n"):
> >> +        # line can be:
> >> +        # "Dump of assembler code from 0xAAAA to 0xBBBB:"
> >> +        # "[=>] 0x00000000004004dc[ <+0>]:  push   %rbp"
> >> +        #   (" <+0>" part is present when we run on a live process,
> >> +        #   on coredump it is absent)
> >> +        # "End of assembler dump."
> >> +        # "" (empty line)
> > What about using regular expressions here?
> 
> re.compile('^\s+(0x[0-9a-f]+)\s+(\<\+\d+\>):\s+(.*)$')
> 
> The rest of your comments make sense, I'll update the branch.
> 




More information about the Crash-catcher mailing list