D11236: [KCrash] Establish socket to allow change of ptracer

This revision was automatically updated to reflect the committed changes. Closed by commit R285:2af59ef51f8c: [KCrash] Establish socket to allow change of ptracer (authored by croick). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D11236?vs=49711=50214#toc REPOSITORY R285 KCrash

D11236: [KCrash] Establish socket to allow change of ptracer

croick edited the summary of this revision. REPOSITORY R285 KCrash BRANCH ptracer REVISION DETAIL https://phabricator.kde.org/D11236 To: croick, #frameworks, ossi Cc: dfaure, lepagevalleeemmanuel, kde-frameworks-devel, sitter, michaelh, ngraham, bruns

D11236: [KCrash] Establish socket to allow change of ptracer

ossi accepted this revision. ossi added a comment. This revision is now accepted and ready to land. here too, it should be "ancestor" not "parent" for extra correctness. amending the code comment is optional, too. INLINE COMMENTS > kcrash.cpp:662 > +// For now that will be DrKonqi,

D11236: [KCrash] Establish socket to allow change of ptracer

croick added a comment. Ok, now I got it. After reading the YAMA doc again (more carefully) I realize that it really just is the parent that can be attached to the child by default and not the other way around. I still wonder why I seem to recall that there was a working backtrace for the

D11236: [KCrash] Establish socket to allow change of ptracer

croick edited the summary of this revision. REPOSITORY R285 KCrash REVISION DETAIL https://phabricator.kde.org/D11236 To: croick, #frameworks, ossi Cc: dfaure, lepagevalleeemmanuel, kde-frameworks-devel, sitter, michaelh, ngraham, bruns

D11236: [KCrash] Establish socket to allow change of ptracer

ossi added a comment. i said "3rd line", not "3rd case". it's about the ancestry; see the other change. REPOSITORY R285 KCrash REVISION DETAIL https://phabricator.kde.org/D11236 To: croick, #frameworks, ossi Cc: dfaure, lepagevalleeemmanuel, kde-frameworks-devel, sitter, michaelh,

D11236: [KCrash] Establish socket to allow change of ptracer

croick added a comment. > the 3rd line is still wrong ... So I used the word "tracer" now. Otherwise I don't know what you mean. REPOSITORY R285 KCrash REVISION DETAIL https://phabricator.kde.org/D11236 To: croick, #frameworks, ossi Cc: dfaure, lepagevalleeemmanuel,

D11236: [KCrash] Establish socket to allow change of ptracer

croick edited the summary of this revision. REPOSITORY R285 KCrash REVISION DETAIL https://phabricator.kde.org/D11236 To: croick, #frameworks, ossi Cc: dfaure, lepagevalleeemmanuel, kde-frameworks-devel, sitter, michaelh, ngraham, bruns

D11236: [KCrash] Establish socket to allow change of ptracer

ossi added a comment. the 3rd line is still wrong ... REPOSITORY R285 KCrash REVISION DETAIL https://phabricator.kde.org/D11236 To: croick, #frameworks, ossi Cc: dfaure, lepagevalleeemmanuel, kde-frameworks-devel, sitter, michaelh, ngraham, bruns

D11236: [KCrash] Establish socket to allow change of ptracer

ossi added a comment. right, threads, i didn't think of that. i suppose it might make sense to pthread_kill() from within kcrash to freeze the other threads, but that will be non-atomic, and i wonder whether it wouldn't have undesirable interactions with the attached debugger and/or exiting

D11236: [KCrash] Establish socket to allow change of ptracer

croick edited the summary of this revision. croick edited the test plan for this revision. REPOSITORY R285 KCrash REVISION DETAIL https://phabricator.kde.org/D11236 To: croick, #frameworks, ossi Cc: dfaure, lepagevalleeemmanuel, kde-frameworks-devel, sitter, michaelh, ngraham, bruns

D11236: [KCrash] Establish socket to allow change of ptracer

croick added a comment. I think it's quite useful (see BUG 175362). Remaining threads remain quite busy otherwise, especially since KCrash is closing all file descriptors by default, which leads to a lot of polling of non-existant FDs in the background. Maybe you would like to join the

D11236: [KCrash] Establish socket to allow change of ptracer

ossi added inline comments. INLINE COMMENTS > kcrash.cpp:658 > +// when launching drkonqi. Note that DrKonqi will SIGSTOP this > process in the meantime > +// and only send SIGCONT when it is about to attach a debugger. > #ifdef Q_OS_LINUX ok. but come to think of it, maybe

D11236: [KCrash] Establish socket to allow change of ptracer

croick marked an inline comment as done. REPOSITORY R285 KCrash REVISION DETAIL https://phabricator.kde.org/D11236 To: croick, #frameworks, ossi Cc: dfaure, lepagevalleeemmanuel, kde-frameworks-devel, sitter, michaelh, ngraham, bruns

D11236: [KCrash] Establish socket to allow change of ptracer

croick updated this revision to Diff 49711. croick marked an inline comment as done. croick added a comment. - invert debugger-debuggee hierarchy in comment REPOSITORY R285 KCrash CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D11236?vs=49650=49711 BRANCH ptracer REVISION

D11236: [KCrash] Establish socket to allow change of ptracer

ossi added a comment. commit message inverted here as well. INLINE COMMENTS > kcrash.cpp:658 > +// when launching drkonqi. Note that DrKonqi will SIGSTOP this > process in the meantime > +// and only send SIGCONT when it is to be attached by a debugger. > #ifdef Q_OS_LINUX

D11236: [KCrash] Establish socket to allow change of ptracer

croick edited the summary of this revision. REPOSITORY R285 KCrash REVISION DETAIL https://phabricator.kde.org/D11236 To: croick, #frameworks, ossi Cc: dfaure, lepagevalleeemmanuel, kde-frameworks-devel, sitter, michaelh, ngraham, bruns

D11236: [KCrash] Establish socket to allow change of ptracer

croick marked 2 inline comments as done. croick added a comment. > that sounds suspicious. i don't think the kernel's behavior did changed, and the process hierarchy presumably didn't, either. the right is handed down the ancestry, and that's irrespective of whether the tracer is a "natural"

D11236: [KCrash] Establish socket to allow change of ptracer

croick updated this revision to Diff 49650. croick marked 2 inline comments as done. croick added a comment. - modify comment and remove check for EAGAIN REPOSITORY R285 KCrash CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D11236?vs=49617=49650 BRANCH ptracer REVISION DETAIL

D11236: [KCrash] Establish socket to allow change of ptracer

ossi added a comment. In D11236#394363 , @croick wrote: > Actually the point I removed does not seem to be true any longer. I'm almost certain, that the `prctrl(PR_SET_PTRACER, ...)` was not required until some time ago when starting DrKonqi as

D11236: [KCrash] Establish socket to allow change of ptracer

croick marked 10 inline comments as done. croick added a comment. > please explicitly mark all handled issues as done - you'll notice on the way that you didn't address some of them. Sorry, you were just responding too quickly. > you adjusted the summary kinda the wrong way around

D11236: [KCrash] Establish socket to allow change of ptracer

croick updated this revision to Diff 49617. croick marked 2 inline comments as done. croick added a comment. - unlink old sockets before binding - remove superfluous EAGAIN check - use debuggee pid instead of DrKonqi pid for socket name - fix comments REPOSITORY R285 KCrash CHANGES

D11236: [KCrash] Establish socket to allow change of ptracer

ossi requested changes to this revision. ossi added a comment. This revision now requires changes to proceed. mostly minor issues left. please explicitly mark all handled issues as done - you'll notice on the way that you didn't address some of them. you adjusted the summary kinda

D11236: [KCrash] Establish socket to allow change of ptracer

croick updated this revision to Diff 49609. croick retitled this revision from "[KCrash] Establish socket to allow change of ptrace scope" to "[KCrash] Establish socket to allow change of ptracer". croick edited the summary of this revision. croick added a comment. - use poll instead of