This is an archive of the discontinued LLVM Phabricator instance.

Paths

Table of Contentst

-
compiler-rt/lib/fuzzer/
-
lib/
-
fuzzer/
1/2
FuzzerInterface.h

Differential D57305

[libFuzzer][Windows] Use dllexport for all declarations in FuzzerInterface.h
ClosedPublic

Authored by metzman on Jan 27 2019, 12:37 PM.

Download Raw Diff

Details

Reviewers

vitalybuka
morehouse

Commits

rG8f7fc95ab7aa: [libFuzzer][Windows] Use dllexport for all declarations in FuzzerInterface.h
rCRT352395: [libFuzzer][Windows] Use dllexport for all declarations in FuzzerInterface.h
rL352395: [libFuzzer][Windows] Use dllexport for all declarations in FuzzerInterface.h

Summary

Use dllexport for all declarations in FuzzerInterface.h Use it for clang
even though clang supports default visibility attribute to prevent a
warning from being thrown when LLVMFuzzerMutate is defined with dllexport.
This makes FUZZER_INTERFACE_VISIBILITY (FuzzerInterface.h) consistent with
ATTRIBUTE_INTERFACE (FuzzerDefs.h) when using clang on Windows.

Diff Detail

Build Status

Buildable 27396
Build 27395: arc lint + arc unit

Event Timeline

metzman created this revision.Jan 27 2019, 12:37 PM

Harbormaster completed remote builds in B27367: Diff 183765.Jan 27 2019, 12:37 PM

Don't use default visibility with clang on Windows

Harbormaster completed remote builds in B27368: Diff 183766.Jan 27 2019, 12:38 PM

Please take a look.
This change silences a warning when compiling libFuzzer for Windows with clang.
It shouldn't affect anything other than compiling libFuzzer with clang on Windows.

morehouse added inline comments.Jan 28 2019, 8:50 AM

compiler-rt/lib/fuzzer/FuzzerInterface.h
30	Why only 32-bit? Also, shouldn't this be `#if defined(_WIN32)`?

check if _WIN32 is defined

Harbormaster completed remote builds in B27396: Diff 183889.Jan 28 2019, 9:20 AM

metzman marked an inline comment as done.Jan 28 2019, 9:30 AM

metzman added inline comments.

compiler-rt/lib/fuzzer/FuzzerInterface.h
30	Why only 32-bit? `_WIN32` is defined for 64-bit and 32-bit builds. We use it in `FuzzerDefs.h` to set `LIBFUZZER_WINDOWS`. Also, shouldn't this be `#if defined(_WIN32)`? I've changed it to be this. Funny that you noticed this though, since I came to the opposite conclusion when bikeshedding before uploading the initial patch. `#if defined(_WIN32)` is more correct but `#if _WIN32` works because undefined macros evaluate to false. I did this to be consistent with `FuzzerDefs.h` which uses `#if _WIN32` but should be using `#if defined(_WIN32)`, (same for other platform macros).

LGTM

This revision is now accepted and ready to land.Jan 28 2019, 9:48 AM

Closed by commit rL352395: [libFuzzer][Windows] Use dllexport for all declarations in FuzzerInterface.h (authored by metzman). · Explain WhyJan 28 2019, 9:51 AM

This revision was automatically updated to reflect the committed changes.

Herald added subscribers: llvm-commits, delcypher. · View Herald TranscriptJan 28 2019, 9:51 AM

Revision Contents

Path

Size

compiler-rt/

lib/

fuzzer/

FuzzerInterface.h

2 lines

Diff 183889

compiler-rt/lib/fuzzer/FuzzerInterface.h

	Show All 21 Lines
	#include <stdint.h>			#include <stdint.h>

	#ifdef __cplusplus			#ifdef __cplusplus
	extern "C" {			extern "C" {
	#endif // __cplusplus			#endif // __cplusplus

	// Define FUZZER_INTERFACE_VISIBILITY to set default visibility in a way that			// Define FUZZER_INTERFACE_VISIBILITY to set default visibility in a way that
	// doesn't break MSVC.			// doesn't break MSVC.
	#if defined(_MSC_VER) && !defined(__clang__)			#if defined(_WIN32)
				morehouseUnsubmitted Not Done Reply Inline Actions Why only 32-bit? Also, shouldn't this be `#if defined(_WIN32)`? morehouse: Why only 32-bit? Also, shouldn't this be `#if defined(_WIN32)`?
				metzmanAuthorUnsubmitted Done Reply Inline Actions Why only 32-bit? `_WIN32` is defined for 64-bit and 32-bit builds. We use it in `FuzzerDefs.h` to set `LIBFUZZER_WINDOWS`. Also, shouldn't this be `#if defined(_WIN32)`? I've changed it to be this. Funny that you noticed this though, since I came to the opposite conclusion when bikeshedding before uploading the initial patch. `#if defined(_WIN32)` is more correct but `#if _WIN32` works because undefined macros evaluate to false. I did this to be consistent with `FuzzerDefs.h` which uses `#if _WIN32` but should be using `#if defined(_WIN32)`, (same for other platform macros). metzman: >Why only 32-bit? `_WIN32` is defined for 64-bit and 32-bit builds. We use it in `FuzzerDefs.
	#define FUZZER_INTERFACE_VISIBILITY __declspec(dllexport)			#define FUZZER_INTERFACE_VISIBILITY __declspec(dllexport)
	#else			#else
	#define FUZZER_INTERFACE_VISIBILITY __attribute__((visibility("default")))			#define FUZZER_INTERFACE_VISIBILITY __attribute__((visibility("default")))
	#endif			#endif

	// Mandatory user-provided target function.			// Mandatory user-provided target function.
	// Executes the code under test with [Data, Data+Size) as the input.			// Executes the code under test with [Data, Data+Size) as the input.
	// libFuzzer will invoke this function many times with different inputs.			// libFuzzer will invoke this function many times with different inputs.
	▲ Show 20 Lines • Show All 41 Lines • Show Last 20 Lines