This is an archive of the discontinued LLVM Phabricator instance.

[libfuzzer] Fix fuzzer-oom.test on windows and reenable it.
ClosedPublic

Authored by metzman on Sep 6 2018, 8:54 AM.

Diff Detail

Event Timeline

metzman created this revision.Sep 6 2018, 8:54 AM

Please take a look.
Another issue with the file extension.
These issues aren't trivial to figure out, I wonder if all tests should compile binaries with exe extensions so problems aren't accidentally introduced.

Dor1s accepted this revision.Sep 6 2018, 9:33 AM

I wonder if there is any trick in lit to resolve this .exe issue. How do other tests work on windows? I see that /compiler-rt/test/asan/TestCases/Windows/ has a bunch of tests that don't use .exe, but some of them have .exe too.

compiler-rt/test/fuzzer/fuzzer-oom.test
4 ↗(On Diff #164226)

what is "exp"?

This revision is now accepted and ready to land.Sep 6 2018, 9:33 AM
metzman marked an inline comment as done.Sep 6 2018, 9:43 AM

I wonder if there is any trick in lit to resolve this .exe issue.

Not sure. Maybe cpp_compiler can be some kind of script that wraps clang? Probably the easiest solution for me would be for test binaries to be built with cmake (as they used to be) rather than by lit.

How do other tests work on windows? I see that /compiler-rt/test/asan/TestCases/Windows/ has a bunch of tests that don't use .exe, but some of them have .exe too.

For most windows tests it doesn't matter if exe is used or not. This is the second one where I've seen it make a difference.

compiler-rt/test/fuzzer/fuzzer-oom.test
4 ↗(On Diff #164226)

https://msdn.microsoft.com/en-us/library/se8y7dcs.aspx
I'm not 100% if it is exp, lib or both being overwritten that causes symbolization to be subtly wrong. But it is true that both get overwritten.
But it is true that if neither is overwritten then things work. My first fix here was actually to reverse the binary names (eg: OutOfMemoryTest-%t )

Dor1s edited the summary of this revision. (Show Details)Sep 6 2018, 9:46 AM
Dor1s updated this revision to Diff 164230.Sep 6 2018, 9:48 AM
Dor1s edited the summary of this revision. (Show Details)

getting ready to land.

Herald added subscribers: Restricted Project, llvm-commits, delcypher. · View Herald TranscriptSep 6 2018, 9:48 AM
This revision was automatically updated to reflect the committed changes.