HomePhabricator

Move the "run" alias from process launch --shell to process launch --shell…
AuditedrL248338

Description

Move the "run" alias from process launch --shell to process launch --shell-expand-args when building on OS X
The argdumper-based launching is more friendly to System Integrity Protection, and will work on older releases of OS X as well

Leave non-Apple builds alone

Details

Auditors
dawn
Committed
enricoSep 22 2015, 3:57 PM
Parents
rL248337: test runner: remove print from prior commit; fixup listner socket backlog
Branches
Unknown
Tags
Unknown

Event Timeline

dawn raised a concern with this commit.Sep 23 2015, 12:42 PM
dawn added a subscriber: lldb-commits.
dawn added a subscriber: dawn.

Hi Enrico,

This commit has caused catasrophic test failures on OSX. The failure count went
up by 512.

lldb is built with cmake/ninja, and tests are run in the test dir as follows:

cd ~/llvm
mkdir -p build_ninja && cd build_ninja
cmake -G Ninja .. "-DLLVM_TARGETS_TO_BUILD=ARM;X86;AArch64" -DCMAKE_CXX_FLAGS="-std=c++11 -stdlib=libc++" -DCMAKE_BUILD_TYPE=Release
security unlock-keychain -p <psw>  /Users/testuser/Library/Keychains/login.keychain
ninja
cd ../tools/lldb/test
./dotest.py --output-on-success -v

Can you have a look please?

Thanks in advance,
-Dawn

brucem added a subscriber: brucem.Sep 23 2015, 10:09 PM

I ran into this myself and looked into it.

What is happening is that it is looking for argdumper and not finding it. This happens because it is looking in the lib directory within the build directory, but argdumper is in the bin directory (where it should be).

The reason that it looks in the wrong place comes down to logic within
HostInfoMacOSX::ComputeSupportExeDirectory which expects everything to always be built as a typical Mac OS X bundle.

This does prevent lldb from launching any inferior currently on Mac OS X when built with cmake.

tfiala added a subscriber: tfiala.Sep 23 2015, 11:34 PM

Are you all still seeing this now? I am heading to sleep but can have a
look early in the morning...

Yes. With a change that I'm going to submit for review:

batavia:build-llvm bruce$ bin/lldb bin/clang
(lldb) target create "bin/clang"
Current executable set to 'bin/clang' (x86_64).
(lldb) b main
Breakpoint 1: where = clang`main, address = 0x0000000100002890
(lldb) run
error: could not find argdumper tool: /Users/bruce/Development/build-llvm/lib/argdumper
(lldb)

I filed this:
https://llvm.org/bugs/show_bug.cgi?id=24926

And I'm starting to look at it now.

What is happening is that it is looking for argdumper and not finding it. This happens because it is looking in the lib directory within the build directory, but argdumper is in the bin directory (where it should be).

I'm not sure I would say that is accurate. argdumper is an internal implementation detail binary,, and isn't meant to be a stand-alone utility. In Linux, it would most properly be located in libexec. (Older pre-layout-standardization would put it in lib). So I'd expect it in lib as well on OS X since OS X doesn't use a libexec.

Sounds like this is probably just a cmake configuration issue.

dawn added a comment.EditedSep 24 2015, 1:14 PM

Is there a workaround for this? Ok to revert this commit until it is fixed? It's blocker for me. Thanks.

I read Todd's comments in the bug report - sounds like he may be close to a fix? If it can be fixed in the next day let's wait, else it would be nice to revert please.

Hi Dawn,

See https://llvm.org/bugs/show_bug.cgi?id=24926 for a workaround. I'm developing a fix for it. Still testing but what is there in that patch (in the bug) works for your build scenario as long as you add '--executable /path/to/your/just/build/lldb' when calling dotest.py.

dawn added a comment.Sep 24 2015, 1:43 PM

Hi Todd, yes, I read the bug report and updated my comment while you were writing yours :)

Hi Todd, yes, I read the bug report and updated my comment while you were writing yours :)

Great! Once I validate that the standard OS X build isn't busted (in progress), I'll get it checked in.

The cmake OS X fix went in here:

Sending        source/Host/macosx/HostInfoMacOSX.mm
Transmitting file data .
Committed revision 248545.

Let me know if you still have trouble after that.

dawn accepted this commit.Sep 24 2015, 6:19 PM

Fixed by Todd in r248545. Todd rocks!!! :)

Fixed by Todd in r248545.

Oh good!

Todd rocks!!! :)

You are far too kind :-)