Page MenuHomePhabricator

Avoid leakage of file descriptors in LLDB and LLGS
ClosedPublic

Authored by labath on Feb 3 2015, 8:08 AM.

Details

Summary

Both LLDB and LLGS are leaking file descriptors into the debugged process. This plugs the leak by
closing the unneeded descriptors. In one case I use O_CLOEXEC, which I hope is supported on
relevant platforms. I also added a regression test and plugged a fd leak in dosep.py.

Diff Detail

Repository
rL LLVM

Event Timeline

labath updated this revision to Diff 19226.Feb 3 2015, 8:08 AM
labath retitled this revision from to Avoid leakage of file descriptors in LLDB and LLGS.
labath updated this object.
labath edited the test plan for this revision. (Show Details)
labath added reviewers: vharron, clayborg.
labath added a subscriber: Unknown Object (MLST).
labath updated this revision to Diff 19228.Feb 3 2015, 8:12 AM

Update main.c: remove tabs

zturner added a subscriber: zturner.Feb 3 2015, 8:37 AM

This doesn't seem like the best way for the test to do this. I don't know
much about linux, but windows at least we can just have the debugger
directly figure out how many handles are open in the target process. And
this is a pretty useful thing to be able to do in a debugger, for example
if you're debugging a handle leak.

Is this not possible on linux? If it is, it would be much better to just
have the debugger enumerate the target's handles and verify there are only 3

labath added a comment.Feb 3 2015, 8:47 AM

On linux, you can just enumerate the files in /proc/PID/fd/. However, this approach does not work on mac (afaik there is no proc filesystem there). This was the most general solution I could think of, and it should work on most posix implementations (those that we care about at least). I am aware that it will likely not work on windows. If you have a proposal how to test it, I will be happy to add it.

I agree that file handle/descriptor inspection using the debugger is a very useful feature, but I think implementing it is out of scope of what I am trying to achieve here.

I had hoped there was already a command in lldb that did this, but i guess
not. Can you just mark the test xfail for windows?

labath updated this revision to Diff 19233.Feb 3 2015, 9:03 AM

Mark the test as XFAIL on windows.

vharron accepted this revision.Feb 3 2015, 10:37 AM
vharron edited edge metadata.

Thanks for adding a test to confirm/protect.

LGTM If this builds/runs on OSX and Linux and doesn't cause any test regressions. (other than maybe this test on OSX)

This revision is now accepted and ready to land.Feb 3 2015, 10:37 AM
clayborg accepted this revision.Feb 3 2015, 11:39 AM
clayborg edited edge metadata.

Looks good.

This revision was automatically updated to reflect the committed changes.
ovyalov added a subscriber: ovyalov.Feb 4 2015, 1:54 PM

The CL is causing TestProcessLaunch.py regression.

lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp
1551

This statement makes TestProcesslaunch.test_io_with_dwarf to fail with SIGHUP:

runCmd: process launch -i /usr/local/google/home/ovyalov/projects/lldb/dev/lldb/test/functionalities/process_launch/input-file.txt -o /usr/local/google/home/ovyalov/projects/lldb/dev/lldb/test/functionalities/process_launch/output-test.out -e /usr/local/google/home/ovyalov/projects/lldb/dev/lldb/test/functionalities/process_launch/output-test.err
output: Process 25324 launching
Process 25324 stopped

  • thread #1: tid = 25324, 0x00007ffff7ddb2d0, name = 'a.out', stop reason = signal SIGHUP frame #0: 0x00007ffff7ddb2d0

-> 0x7ffff7ddb2d0: movq %rsp, %rdi

0x7ffff7ddb2d3: callq  0x7ffff7ddea70
0x7ffff7ddb2d8: movq   %rax, %r12
0x7ffff7ddb2db: movl   0x221b17(%rip), %eax
labath added a comment.Feb 5 2015, 1:21 AM

looking into it

labath added a comment.Feb 5 2015, 8:27 AM

Proposed solution in D7440, please verify.