[compiler-rt] r264773 - [profile] Make a test work if run by the super-user

Sean Silva via llvm-commits llvm-commits at lists.llvm.org
Tue Mar 29 13:23:27 PDT 2016


On Tue, Mar 29, 2016 at 1:11 PM, Vedant Kumar <vsk at apple.com> wrote:

>
> > On Mar 29, 2016, at 1:10 PM, Sean Silva <chisophugis at gmail.com> wrote:
> >
> >
> >
> > On Tue, Mar 29, 2016 at 1:03 PM, Vedant Kumar <vsk at apple.com> wrote:
> >
> > > On Mar 29, 2016, at 1:00 PM, Sean Silva <chisophugis at gmail.com> wrote:
> > >
> > >
> > >
> > > On Tue, Mar 29, 2016 at 12:55 PM, Vedant Kumar <vsk at apple.com> wrote:
> > > Darwin is funny. I tried an empty string also, but the test failed :(.
> > >
> > > Interesting. Are the quotes necessary? For the record, for PS4, both
> `"/"` (your patch), `/`, and `` (i.e. empty) work.
> >
> > It actually fails (i.e, proceeds with the write) with and without the
> quotes.
> >
> > Sorry, maybe I should clarify. For my testing on PS4, this test works
> fine with either `"/"` (your patch), `/`, and `` (i.e. empty). Did you
> intend to say that `"/"` works (i.e. the test case passes) on Darwin? I'm
> not sure what to make of your statement "with and without the quotes".
>
> Yes, this test case passes on Darwin (i.e, with `"/"`). However, it does
> not pass with `` or `""` on Darwin.
>

I'm a bit curious about this. Wouldn't this test always fail if we pass a
directory name instead of a file name? Then we would call
`fopen("some-directory-name/", ...)` which should fail even as root. It
seems a bit less magical for the test to use `LLVM_PROFILE_FILE=%t/` or
something. Does that work for you?


-- Sean Silva


>
>
> > > That said, it's important the test is portable. I'll revert and try
> again if the Windows bots have issues.
> > >
> > > I haven't tested a windows-targeting configuration (and don't know if
> we have a bot for that). The closest I have is windows-hosted targeting PS4.
> >
> > Hm, it could be that the public Windows bots have never run
> check-profile. The old version of the code used chmod etc. without a
> `REQUIRES: shell` line.
> >
> > Should we add a require line to avoid breaking future Windows bots?
> >
> > Let's see if anything breaks first.
>
> Ok, fair enough.
>
> vedant
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160329/fa406a94/attachment-0001.html>


More information about the llvm-commits mailing list