AArch64 bot unstable

Hi Gabor,

I noticed that one particular test fails intermittently on the AArch64 bot:

http://lab.llvm.org:8011/builders/clang-native-aarch64-full

FAIL: Profile:: instrprof-set-filename-then-reset-default.c

Some times on stage1, others on stage2, others no fail at all.

All the commits during these builds are not related to profiling or
AArch64, so I believe this has something to do with the setup.

Maybe try to reproduce it locally? It could be something to do with
the linker, too.

cheers,
--renato

I added this test yesterday, so will take a look.

The test is confirming that the invoking
__llvm_profile_set_filename(0) properly resets the profile output to
"default.profraw" (the default). When it fails I see the following
warning:

warning: no profile data available for file
"instrprof-set-filename-then-reset-default.c"
[-Wprofile-instr-unprofiled]

Two of the compiler-rt/profile tests I added are checking for similar
behavior with resetting the filename to the default. I wonder if they
are running in parallel and clobbering each other since the profile
output names are not unique after the reset.

Teresa

Hi Teresa,

This would explain the intermittent failures. Maybe making the names
unique would fix the issues, would that be an easy change?

cheers,
--renato

Two of the compiler-rt/profile tests I added are checking for similar
behavior with resetting the filename to the default. I wonder if they
are running in parallel and clobbering each other since the profile
output names are not unique after the reset.

Hi Teresa,

This would explain the intermittent failures. Maybe making the names
unique would fix the issues, would that be an easy change?

After thinking about it I believe this is what is most likely
happening, and that these two new tests I added will need to be
reverted:

    compiler-rt/trunk/test/profile/instrprof-override-filename-then-reset-default.c
    compiler-rt/trunk/test/profile/instrprof-set-filename-then-reset-default.c

since they aren't writing to unique output names. These are 2 of the 4
tests added in r236056 (the other two are fine).

They can't be changed to write to a unique name since those tests were
specifically testing that the profile output name gets reset to the
default when null is passed to the profile filename setting
interfaces. The other profile tests use unique names.

Since I don't have write access yet, can someone revert those two files for me?

Thanks,
Teresa

Hi Renato,

Thanks for the warning but I am a little busy this week.
If this is still an issue next week I will try to reproduce it
locally.

Thanks,
Gabor

Don't worry, Teresa has found the problem, we'll fix on our end, hopefully. :slight_smile:

cheers!
--renato

Done, r236101.

Thanks!
--renato

Great, thanks!
Teresa

Teresa Johnson <tejohnson@google.com> writes:

Two of the compiler-rt/profile tests I added are checking for similar
behavior with resetting the filename to the default. I wonder if they
are running in parallel and clobbering each other since the profile
output names are not unique after the reset.

Hi Teresa,

This would explain the intermittent failures. Maybe making the names
unique would fix the issues, would that be an easy change?

After thinking about it I believe this is what is most likely
happening, and that these two new tests I added will need to be
reverted:

    compiler-rt/trunk/test/profile/instrprof-override-filename-then-reset-default.c
    compiler-rt/trunk/test/profile/instrprof-set-filename-then-reset-default.c

since they aren't writing to unique output names. These are 2 of the 4
tests added in r236056 (the other two are fine).

They can't be changed to write to a unique name since those tests were
specifically testing that the profile output name gets reset to the
default when null is passed to the profile filename setting
interfaces. The other profile tests use unique names.

Could we have these tests `cd` into a uniquely named directory or
something? Maybe that's more complicated than its worth.

Teresa Johnson <tejohnson@google.com> writes:

Two of the compiler-rt/profile tests I added are checking for similar
behavior with resetting the filename to the default. I wonder if they
are running in parallel and clobbering each other since the profile
output names are not unique after the reset.

Hi Teresa,

This would explain the intermittent failures. Maybe making the names
unique would fix the issues, would that be an easy change?

After thinking about it I believe this is what is most likely
happening, and that these two new tests I added will need to be
reverted:

compiler-rt/trunk/test/profile/instrprof-override-filename-then-reset-default.c
compiler-rt/trunk/test/profile/instrprof-set-filename-then-reset-default.c

since they aren’t writing to unique output names. These are 2 of the 4
tests added in r236056 (the other two are fine).

They can’t be changed to write to a unique name since those tests were
specifically testing that the profile output name gets reset to the
default when null is passed to the profile filename setting
interfaces. The other profile tests use unique names.

Could we have these tests cd into a uniquely named directory or
something? Maybe that’s more complicated than its worth.

That’s what we’ve done in the past and seems a reasonable idea.

-eric

Ok, thanks for the suggestion. I will rework the tests to do that.
Teresa

Ok, thanks for the suggestion. I will rework the tests to do that.

In case you haven't found it already, %T in the lit syntax gives you a
uniquely named directory for the test

Ok, thanks for the suggestion. I will rework the tests to do that.

In case you haven't found it already, %T in the lit syntax gives you a
uniquely named directory for the test

Actually %T is just the base directory where all the
compiler-rt/profile tests are run, not unique per .c test. I fixed the
tests by creating a "%t.d" directory and cd'ing into it and generating
all output locally in that directory.

Patch uploaded here, PTAL:
http://reviews.llvm.org/D9349

Thanks,
Teresa

>
>
>>
>> Ok, thanks for the suggestion. I will rework the tests to do that.
>
>
> In case you haven't found it already, %T in the lit syntax gives you a
> uniquely named directory for the test

Actually %T is just the base directory where all the
compiler-rt/profile tests are run, not unique per .c test.

Ah, sorry for the misleading advice - thanks for the correction.

I fixed the
tests by creating a "%t.d" directory and cd'ing into it and generating
all output locally in that directory.

A quick "grep -r "RUN.*mkdir" certainly shows a few tests doing quite
similar things (most use -p, which I see you've done too).

You'll probably also need REQUIRES: shell in this test to ensure it doesn't
run in environments that don't have a full bash-like shell available (such
as Windows). Check the other tests that use mkdir to see that sort of thing.

>
>
>>
>> Ok, thanks for the suggestion. I will rework the tests to do that.
>
>
> In case you haven't found it already, %T in the lit syntax gives you a
> uniquely named directory for the test

Actually %T is just the base directory where all the
compiler-rt/profile tests are run, not unique per .c test.

Ah, sorry for the misleading advice - thanks for the correction.

I fixed the
tests by creating a "%t.d" directory and cd'ing into it and generating
all output locally in that directory.

A quick "grep -r "RUN.*mkdir" certainly shows a few tests doing quite
similar things (most use -p, which I see you've done too).

You'll probably also need REQUIRES: shell in this test to ensure it doesn't
run in environments that don't have a full bash-like shell available (such
as Windows). Check the other tests that use mkdir to see that sort of thing.

Some have the "REQUIRES: shell" (e.g.
tools/clang/test/VFS/include-virtual-from-real.c) and some don't (e.g.
./tools/clang/test/Analysis/html-diags.c). I looked around for
documentation on when "REQUIRES: shell" is needed but couldn't find
anything specific. The latter test (html-diags.c), which doesn't have
the REQUIRES also does mkdir -p, cd, rm -rf", which are the same
shell-like operations my tests are doing. The former
(include-virtual-from-real.c) is doing some additional shell-like
operations such as sed and echo, which I am not doing. Is there a list
of operations safe to do without adding the "REQUIRES: shell"?

Thanks,
Teresa

>
>
>>
>> >
>> >
>> >>
>> >> Ok, thanks for the suggestion. I will rework the tests to do that.
>> >
>> >
>> > In case you haven't found it already, %T in the lit syntax gives you a
>> > uniquely named directory for the test
>>
>> Actually %T is just the base directory where all the
>> compiler-rt/profile tests are run, not unique per .c test.
>
>
> Ah, sorry for the misleading advice - thanks for the correction.
>
>>
>> I fixed the
>> tests by creating a "%t.d" directory and cd'ing into it and generating
>> all output locally in that directory.
>
>
> A quick "grep -r "RUN.*mkdir" certainly shows a few tests doing quite
> similar things (most use -p, which I see you've done too).
>
> You'll probably also need REQUIRES: shell in this test to ensure it
doesn't
> run in environments that don't have a full bash-like shell available
(such
> as Windows). Check the other tests that use mkdir to see that sort of
thing.

Some have the "REQUIRES: shell" (e.g.
tools/clang/test/VFS/include-virtual-from-real.c) and some don't (e.g.
./tools/clang/test/Analysis/html-diags.c). I looked around for
documentation on when "REQUIRES: shell" is needed but couldn't find
anything specific. The latter test (html-diags.c), which doesn't have
the REQUIRES also does mkdir -p, cd, rm -rf", which are the same
shell-like operations my tests are doing. The former
(include-virtual-from-real.c) is doing some additional shell-like
operations such as sed and echo, which I am not doing.

Sounds plausible then.

Is there a list
of operations safe to do without adding the "REQUIRES: shell"?

Not that I know of. Usually a bit of "oh, this fails on a windows buildbot
-> slap REQUIRES: shell on it" (maybe other people know more). But as
you've seen tests that don't seem to require it and do the same things your
test does - just go with it & if buildbots fail we'll figure it out then.

- David

There isn't current documentation on that, it's basically implementation
defined by the lit shell emulator. I recently added support for 'cd', so
your example will probably work. Usually people commit first and add the
requirement if the test fails, and that's acceptable.

A simple ‘mkdir’ certainly works on Windows, but probably if you add any extra options that would be a problem.

–paulr