This is an archive of the discontinued LLVM Phabricator instance.

[libFuzzer][MSVC] Use alternatename for ext functions
ClosedPublic

Authored by metzman on Jan 9 2019, 1:52 PM.

Details

Summary

Use alternatename for external functions only when using
MSVC since Clang doesn't support it and MSVC doesn't support
Clang's method (weak aliases).

Event Timeline

metzman created this revision.Jan 9 2019, 1:52 PM
metzman updated this revision to Diff 180925.Jan 9 2019, 2:06 PM

trivial fix

metzman updated this revision to Diff 180928.Jan 9 2019, 2:08 PM
  • close paren
metzman updated this revision to Diff 180931.Jan 9 2019, 2:11 PM

Undo accidental change

metzman added subscribers: thakis, rnk.

Matt could you please take a look?

For context, this patch is part of a series that I am trying to land to get libFuzzer working when compiled by MSVC.

@rnk or @thakis what do you think of this workaround for this bug I came up with?

I tried using things like volatile loads of the function pointers to prevent the optimizer from getting in the way, but I couldn't come up with a reliable solution, so I decided not to use alternatename when using clang.

morehouse added inline comments.Jan 10 2019, 1:12 PM
compiler-rt/lib/fuzzer/FuzzerExtFunctionsAlternatename.cpp
39

Maybe move the above macros into FuzzerDefs.h or create a FuzzerWinDefs.h. Then FuzzerExtFunctionsAlternatename.cpp is unneeded, and we can reuse FuzzerExtFunctionsWeakAlias.cpp.

compiler-rt/lib/fuzzer/FuzzerExtFunctionsWeakAlias.cpp
26

This attribute could also become a macro in FuzzerDefs.h or FuzzerWinDefs.h

metzman marked 2 inline comments as done.Jan 10 2019, 1:19 PM
metzman added inline comments.
compiler-rt/lib/fuzzer/FuzzerExtFunctionsAlternatename.cpp
39

What do you think of moving them both into a single file FuzzerExtFunctionsWin.cpp?
It seems better to define a macro in the only file it will ever be used than to define it in a header where there is less context and it could be leaked.
Your call of course though.

compiler-rt/lib/fuzzer/FuzzerExtFunctionsWeakAlias.cpp
26

If you agree with my above comment to create FuzzerExtFunctionsWin.cpp, then what do you think of just defining this as a macro in that file?

morehouse added inline comments.Jan 10 2019, 1:57 PM
compiler-rt/lib/fuzzer/FuzzerExtFunctionsWeakAlias.cpp
26

SGTM

metzman updated this revision to Diff 181636.Jan 14 2019, 1:25 PM
  • move to one file
metzman updated this revision to Diff 181638.Jan 14 2019, 1:28 PM
  • add comma
morehouse added inline comments.Jan 14 2019, 2:58 PM
compiler-rt/lib/fuzzer/FuzzerExtFunctionsWindows.cpp
37 ↗(On Diff #181640)

Let's move all the macros outside of the extern "C" block except for the EXT_FUNC definition.

56 ↗(On Diff #181640)

You can make the definition of WIN_WEAK_ALIAS depend on LIBFUZZER_MSVC and then use the same EXT_FUNC as above.

metzman updated this revision to Diff 181689.Jan 14 2019, 6:04 PM
  • make suggested changes
This revision is now accepted and ready to land.Jan 14 2019, 6:22 PM
This revision was automatically updated to reflect the committed changes.
Herald added subscribers: Restricted Project, llvm-commits. · View Herald TranscriptJan 14 2019, 6:24 PM