Skip to content
This repository has been archived by the owner on Dec 3, 2019. It is now read-only.

Fix crash on PtraceCleanup #61

Merged
merged 1 commit into from
Apr 7, 2017
Merged

Conversation

jamespic
Copy link
Contributor

@jamespic jamespic commented Mar 29, 2017

I found I was getting a crash, due to an unhandled SIGTRAP, the second time I profiled a process with --threads. My initial thought was that this was because the munmap stuff was not thread safe (it doesn't pause threads while poking instructions into code), so fixed this.

However, I was wrong. The real issue is that for reasons I don't fully understand, the PtraceCleanup isn't successful unless we explicitly detach from the process - implicitly detaching by exiting does not seem to be sufficient. I've added an explicit PtraceDetach, and I don't see the issue any more.

I've also modified PtraceAttach to use waitpid with __WALL. This was mostly done as a debugging measure (one theory was that maybe it was stopping on the wrong thing, so I thought tightening up the wait logic might force it to fail earlier), although it may have the side effect of allowing --threads on pre-4.9 kernels, so I've left it in.

@jamespic
Copy link
Contributor Author

jamespic commented Mar 29, 2017

I think this may also fix #60. The SYSCALL-ADD-ADD-ADD in the disassembly (just after another SYSCALL, where it couldn't possibly work) in that issue is similar to what I've seen - add %al,(%rax) is 0x0000, so this is the remnant of poking 0x0000000000000f05.

@eklitzke
Copy link
Collaborator

eklitzke commented Apr 7, 2017

Interesting. There are a lot of weird idiosyncrasies around ptrace, so yeah, this may be necessary.

@eklitzke eklitzke merged commit 5aa1799 into uber-archive:master Apr 7, 2017
akatrevorjay added a commit to akatrevorjay/pyflame that referenced this pull request Apr 10, 2017
* origin/master:
  Fix crash on PtraceCleanup (uber-archive#61)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants