This is an archive of the discontinued LLVM Phabricator instance.

Simplify test a bit
ClosedPublic

Authored by rafael on Apr 3 2017, 12:08 PM.

Details

Reviewers
filcab
rnk
Summary

There are two cases to consider:

We are using the internal shell. This will still fail because of ulimit.
We are using an external shell. In this case the difference is that we now also constrain FileCheck to use less than 4 MB of of stack, which it should :-)

Diff Detail

Event Timeline

rafael created this revision.Apr 3 2017, 12:08 PM
rnk edited edge metadata.Apr 3 2017, 6:37 PM

I thought ulimit was a bash builtin that affects the limits of the shell process. Even if ulimit exists as a separate command, I don't think shelling out to it will affect the parent process. Maybe this is a good place to run bash -c?

filcab edited edge metadata.Apr 5 2017, 8:26 AM

I think Rafael's reasoning makes sense and am ok with this.
In the internal shell it wasn't working, so there's no change there. In the external shell, the change looks benign.
I don't expect FileCheck, even compiled with -O0, to have a problem with a 4MiB stack.

rnk accepted this revision.Apr 5 2017, 11:44 AM

lgtm

Sure, this is simpler, and I'm not concerned about FileCheck blowing out a 4MB stack.

Do you want to add REQUIRES: shell in this change as well, since this test probably still fails with "ulimit: command not found" when setting LIT_USE_INTERNAL_SHELL=1?

This revision is now accepted and ready to land.Apr 5 2017, 11:44 AM
rafael closed this revision.Apr 5 2017, 1:40 PM