FTR: it should be llvm r312855 or later

r313222 has been rolled 4 days ago.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c34fa467f79029d9b5291a229569e6c8bde374d1

commit c34fa467f79029d9b5291a229569e6c8bde374d1
Author: Max Moroz <mmoroz@chromium.org>
Date: Fri Sep 22 23:39:27 2017

Add fuzzer runtime to the libraries being downloaded among other clang stuff.

R=dpranke@chromium.org, kcc@chromium.org

Bug:  764514 
Change-Id: I9e410878a53c70c8bd9ee01f0cadd80cb0cf7d77
Reviewed-on: https://chromium-review.googlesource.com/676141
Reviewed-by: Hans Wennborg <hans@chromium.org>
Commit-Queue: Max Moroz <mmoroz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#503904}
[modify] https://crrev.com/c34fa467f79029d9b5291a229569e6c8bde374d1/tools/clang/scripts/package.py
[modify] https://crrev.com/c34fa467f79029d9b5291a229569e6c8bde374d1/tools/clang/scripts/update.py

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/da74915e156fb8dfc833bf30dca790084a55fcab

commit da74915e156fb8dfc833bf30dca790084a55fcab
Author: Max Moroz <mmoroz@chromium.org>
Date: Fri Sep 22 23:44:08 2017

Add libfuzzer_tests to chromium.fyi Clang ToT testers (w/ and w/o ASan) on Linux.

Bug:  764514 
Change-Id: I36e1c70445a984757627da91d88ddb5a1ef5553d
Reviewed-on: https://chromium-review.googlesource.com/679619
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Commit-Queue: Max Moroz <mmoroz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#503905}
[modify] https://crrev.com/da74915e156fb8dfc833bf30dca790084a55fcab/testing/buildbot/chromium.fyi.json
[modify] https://crrev.com/da74915e156fb8dfc833bf30dca790084a55fcab/testing/buildbot/gn_isolate_map.pyl

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c9e738432a1bce852e99ba4deb9f9dcac8450fdd

commit c9e738432a1bce852e99ba4deb9f9dcac8450fdd
Author: Hans Wennborg <hans@chromium.org>
Date: Sun Sep 24 02:47:04 2017

Revert "Add libfuzzer_tests to chromium.fyi Clang ToT testers (w/ and w/o ASan) on Linux."

This reverts commit da74915e156fb8dfc833bf30dca790084a55fcab.

Reason for revert:
It broke the buildbot:
https://build.chromium.org/p/chromium.fyi/builders/ClangToTLinux/builds/9153

/b/c/builder/ClangToTLinux/src/buildtools/linux64/gn gen //out/Release --check --runtime-deps-list-file=/b/c/builder/ClangToTLinux/src/out/Release/runtime_deps
  -> returned 1
ERROR The label "//testing/libfuzzer:libfuzzer_tests(//build/toolchain/linux:clang_x64)" isn't a target.
When reading the line:
  //testing/libfuzzer:libfuzzer_tests
from the --runtime-deps-list-file=/b/c/builder/ClangToTLinux/src/out/Release/runtime_deps
GN gen failed: 1

Original change's description:
> Add libfuzzer_tests to chromium.fyi Clang ToT testers (w/ and w/o ASan) on Linux.
> 
> Bug:  764514 
> Change-Id: I36e1c70445a984757627da91d88ddb5a1ef5553d
> Reviewed-on: https://chromium-review.googlesource.com/679619
> Reviewed-by: Dirk Pranke <dpranke@chromium.org>
> Commit-Queue: Max Moroz <mmoroz@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#503905}

TBR=hans@chromium.org,dpranke@chromium.org,mmoroz@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  764514 
Change-Id: Iff89445360d517751b3e5cf6066d72d66ad251cb
Reviewed-on: https://chromium-review.googlesource.com/680354
Reviewed-by: Hans Wennborg <hans@chromium.org>
Commit-Queue: Hans Wennborg <hans@chromium.org>
Cr-Commit-Position: refs/heads/master@{#503949}
[modify] https://crrev.com/c9e738432a1bce852e99ba4deb9f9dcac8450fdd/testing/buildbot/chromium.fyi.json
[modify] https://crrev.com/c9e738432a1bce852e99ba4deb9f9dcac8450fdd/testing/buildbot/gn_isolate_map.pyl

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7e5cda7a6dd435e245622f34a57f91c01c7e2e5d

commit 7e5cda7a6dd435e245622f34a57f91c01c7e2e5d
Author: Max Moroz <mmoroz@chromium.org>
Date: Thu Sep 28 21:42:38 2017

Reland "Add libfuzzer_tests to chromium.fyi Clang ToT testers (w/ and w/o ASan) on Linux."

Bug:  764514 

Original CL: https://chromium-review.googlesource.com/679619

R=dpranke@chromium.org

Change-Id: I412ac11cbe2c73de9e7b724e2689a97144aad47e
Reviewed-on: https://chromium-review.googlesource.com/688029
Reviewed-by: Oliver Chang <ochang@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Commit-Queue: Max Moroz <mmoroz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#505167}
[modify] https://crrev.com/7e5cda7a6dd435e245622f34a57f91c01c7e2e5d/testing/buildbot/chromium.fyi.json
[modify] https://crrev.com/7e5cda7a6dd435e245622f34a57f91c01c7e2e5d/testing/buildbot/gn_isolate_map.pyl

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b8d32e142c236cd2bcf904a01fb3fc15f45125f3

commit b8d32e142c236cd2bcf904a01fb3fc15f45125f3
Author: Hans Wennborg <hans@chromium.org>
Date: Thu Oct 05 16:36:19 2017

Revert "Reland "Add libfuzzer_tests to chromium.fyi Clang ToT testers (w/ and w/o ASan) on Linux.""

This reverts commit 7e5cda7a6dd435e245622f34a57f91c01c7e2e5d.

Reason for revert:
The test is failing. See e.g.
https://build.chromium.org/p/chromium.fyi/builders/ClangToTLinux%20tester/builds/2976

Before re-enabling on the ToT Clang bots, please have the test enabled
and passing on regular buildbots, including the CQ.

This also removes the test from the Clang bots from the new chromium.clang waterfall.

Original change's description:
> Reland "Add libfuzzer_tests to chromium.fyi Clang ToT testers (w/ and w/o ASan) on Linux."
> 
> Bug:  764514 
> 
> Original CL: https://chromium-review.googlesource.com/679619
> 
> R=​dpranke@chromium.org
> 
> Change-Id: I412ac11cbe2c73de9e7b724e2689a97144aad47e
> Reviewed-on: https://chromium-review.googlesource.com/688029
> Reviewed-by: Oliver Chang <ochang@chromium.org>
> Reviewed-by: Dirk Pranke <dpranke@chromium.org>
> Commit-Queue: Max Moroz <mmoroz@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#505167}

TBR=dpranke@chromium.org,ochang@chromium.org,mmoroz@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  764514 ,765859
Change-Id: I7af22d752dfcc48c580a7a704d02b2b5a065da1b
Reviewed-on: https://chromium-review.googlesource.com/701424
Commit-Queue: Hans Wennborg <hans@chromium.org>
Reviewed-by: Max Moroz <mmoroz@chromium.org>
Reviewed-by: Hans Wennborg <hans@chromium.org>
Cr-Commit-Position: refs/heads/master@{#506759}
[modify] https://crrev.com/b8d32e142c236cd2bcf904a01fb3fc15f45125f3/testing/buildbot/chromium.clang.json
[modify] https://crrev.com/b8d32e142c236cd2bcf904a01fb3fc15f45125f3/testing/buildbot/chromium.fyi.json
[modify] https://crrev.com/b8d32e142c236cd2bcf904a01fb3fc15f45125f3/testing/buildbot/gn_isolate_map.pyl

I got a little concern here... Imagine there is a bad revision of libFuzzer. In our current approach, we always can roll it back or quickly roll forward. After migrating to -fsanitizer=fuzzer it wouldn't be so easy. Chrome Build folks said that they wouldn't revert clang roll if fuzzer is broken. Rolling forward is also not always trivial.

We need switch to -fsanitize=fuzzer though, otherwise some things on Mac don't work :/

Regarding my last concern in c#11, we should be doing the migration using "-fsanitize=fuzzer-no-link", in a way similar to https://github.com/google/oss-fuzz/commit/62048995161677436b1e0326dd9d2c004f3f8124

I'm trying that locally right now

That works, but doesn't solve other issues e.g.  bug 693760  or  bug 787723 . For instance, I have:

1) $ nm -gU out/asan/obj/v8/fuzzer_support/fuzzer-support.o
0000000000000f10 T _LLVMFuzzerInitialize
<...>

2) $ nm -gU out/asan/obj/v8/json_fuzzer/json.o 
0000000000000000 T _LLVMFuzzerTestOneInput

Ninja invokes linker with the following flags:

$ clang++  -Wcrl,dsym,.  -stdlib=libc++ -arch x86_64 -segprot PROTECTED_MEMORY rw r -Werror -Wl,-dead_strip -isysroot ../../build/mac_files/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.12.sdk -mmacosx-version-min=10.9.0 -fsanitize=fuzzer-no-link -fsanitize=address -fno-sanitize-address-use-after-scope -Wl,-ObjC -Wl,-U,_LLVMFuzzerCustomCrossOver -Wl,-U,_LLVMFuzzerCustomMutator -Wl,-U,_LLVMFuzzerInitialize -Wl,-U,_sanitizer_options_link_helper -fsanitize=address -fno-sanitize-address-use-after-scope -o "./v8_json_parser_fuzzer" -Wl,-filelist,"./v8_json_parser_fuzzer.rsp"

where "./v8_json_parser_fuzzer.rsp" contains all necessary object files:
obj/v8/json_fuzzer/json.o
obj/third_party/libFuzzer/libfuzzer/FuzzerClangCounters.o
<...>
obj/third_party/libFuzzer/libfuzzer/FuzzerUtilWindows.o
obj/v8/fuzzer_support/fuzzer-support.o
obj/v8/v8_base/accessors.o
<...>


However, the final binary v8_json_parser_fuzzer doesn't have LLVMFuzzerInitialize, as the linked thinks that it's not used.

I'm again thinking about my previous suggestion to use weak symbols: https://reviews.llvm.org/D37526?id=114055

Alright, it's not dead_stripping actually! At least, not only "-Wl,-dead_strip" option. With that option, LLVMFuzzerInitialize is still linked in and working properly.

I've created two files locally:

mmoroz-macpro:weak_symbols mmoroz$ cat init.cpp 
#include <iostream>

extern "C" int LLVMFuzzerInitialize(int *argc, char ***argv) {
  std::cout << "LLVMFuzzerInitialize: ";
  std::cout << *argc << ' ' << (*argv)[0] << std::endl;
  return 0;
}

mmoroz-macpro:weak_symbols mmoroz$ cat fuzz.cpp 
#include <iostream>

extern "C" int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size) {
  std::cout << "LLVMFuzzerTestOneInput: " << Size;
  if (Size)
  	std::cout << ' ' << Data[0];
  std::cout << std::endl;
  return 0;
}

And tried building them separately, + linking against libFuzzer objects precisely the same way as our GN config does:

clang++ -Wcrl,dsym,. -stdlib=libc++ -arch x86_64 -segprot PROTECTED_MEMORY rw r -Werror -Wl,-dead_strip \
  -isysroot /Users/mmoroz/projects/chromium/src/build/mac_files/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.12.sdk \
  -mmacosx-version-min=10.9.0 -fsanitize=fuzzer-no-link -fsanitize=address -fno-sanitize-address-use-after-scope \
  -Wl,-ObjC -Wl,-U,_LLVMFuzzerCustomCrossOver -Wl,-U,_LLVMFuzzerCustomMutator -Wl,-U,_LLVMFuzzerInitialize \
  -Wl,-U,_sanitizer_options_link_helper -fsanitize=address -fno-sanitize-address-use-after-scope \
  -o fuzzer -Wl,-filelist,"./objects.rsp"


And after that, I got LLVMFuzzerInitialize working properly. I felt completely confused at that point, but it appears that some other compilation (not linking!) flags break the things here.

When I tried compiling init.cpp and fuzz.cpp exactly the same way as our GN config does, I managed to reproduced the problem:

$ ./build.sh 
+ CXX=/Users/mmoroz/projects/chromium/src/third_party/llvm-build/Release+Asserts/bin/clang++
+ rm -rf fuzz.o init.o fuzzer
+ /Users/mmoroz/projects/chromium/src/third_party/llvm-build/Release+Asserts/bin/clang++ -MMD -MF init.o.d -DV8_DEPRECATION_WARNINGS -DNO_TCMALLOC -DMEMORY_TOOL_REPLACES_ALLOCATOR -DMEMORY_SANITIZER_INITIAL_SIZE -DADDRESS_SANITIZER -DFULL_SAFE_BROWSING -DSAFE_BROWSING_CSD -DSAFE_BROWSING_DB_LOCAL -DCHROMIUM_BUILD -DFIELDTRIAL_TESTING_ENABLED -DCR_XCODE_VERSION=0832 '-DCR_CLANG_REVISION="318667-1"' -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__ASSERT_MACROS_DEFINE_VERSIONS_WITHOUT_UNDERSCORE=0 -DFUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION -DNDEBUG -DNVALGRIND -DDYNAMIC_ANNOTATIONS_ENABLED=0 -DV8_TYPED_ARRAY_MAX_SIZE_IN_HEAP=64 -DV8_INTL_SUPPORT -DENABLE_HANDLE_ZAPPING -DV8_USE_SNAPSHOT -DV8_USE_EXTERNAL_STARTUP_DATA -DV8_CONCURRENT_MARKING -DV8_TARGET_ARCH_X64 -DU_USING_ICU_NAMESPACE=0 -DU_ENABLE_DYLOAD=0 -DU_STATIC_IMPLEMENTATION -DICU_UTIL_DATA_IMPL=ICU_UTIL_DATA_FILE -DUCHAR_TYPE=uint16_t -fno-strict-aliasing -fstack-protector -Wno-builtin-macro-redefined -D__DATE__= -D__TIME__= -D__TIMESTAMP__= -fcolor-diagnostics -Xclang -mllvm -Xclang -instcombine-lower-dbg-declare=0 -no-canonical-prefixes -arch x86_64 -Wall -Werror -Wextra -Wunguarded-availability -Wno-missing-field-initializers -Wno-unused-parameter -Wno-c++11-narrowing -Wno-covered-switch-default -Wno-unneeded-internal-declaration -Wno-inconsistent-missing-override -Wno-undefined-var-template -Wno-nonportable-include-path -Wno-address-of-packed-member -Wno-unused-lambda-capture -Wno-user-defined-warnings -Wno-enum-compare-switch -Wno-tautological-unsigned-zero-compare -Wno-null-pointer-arithmetic -Wno-tautological-constant-compare -Wtautological-constant-out-of-range-compare -fno-omit-frame-pointer -gdwarf-2 -fno-standalone-debug -isysroot /Users/mmoroz/projects/chromium/src/build/mac_files/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.12.sdk -mmacosx-version-min=10.9.0 -gcolumn-info -fsanitize=fuzzer-no-link -fsanitize=address -fno-sanitize-address-use-after-scope -mllvm -asan-globals=0 -fsanitize-blacklist=/Users/mmoroz/projects/chromium/src/tools/memory/asan/blacklist.txt -fvisibility=hidden -Xclang -load -Xclang /Users/mmoroz/projects/chromium/src/third_party/llvm-build/Release+Asserts/lib/libFindBadConstructs.dylib -Xclang -add-plugin -Xclang find-bad-constructs -Wheader-hygiene -Wstring-conversion -Wtautological-overlap-compare -Wsign-compare -Winconsistent-missing-override -Wshorten-64-to-32 -O2 -std=c++14 -stdlib=libc++ -fno-exceptions -fno-rtti -fvisibility-inlines-hidden -c init.cpp -o init.o
+ /Users/mmoroz/projects/chromium/src/third_party/llvm-build/Release+Asserts/bin/clang++ -MMD -MF init.o.d -DV8_DEPRECATION_WARNINGS -DNO_TCMALLOC -DMEMORY_TOOL_REPLACES_ALLOCATOR -DMEMORY_SANITIZER_INITIAL_SIZE -DADDRESS_SANITIZER -DFULL_SAFE_BROWSING -DSAFE_BROWSING_CSD -DSAFE_BROWSING_DB_LOCAL -DCHROMIUM_BUILD -DFIELDTRIAL_TESTING_ENABLED -DCR_XCODE_VERSION=0832 '-DCR_CLANG_REVISION="318667-1"' -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__ASSERT_MACROS_DEFINE_VERSIONS_WITHOUT_UNDERSCORE=0 -DFUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION -DNDEBUG -DNVALGRIND -DDYNAMIC_ANNOTATIONS_ENABLED=0 -DV8_TYPED_ARRAY_MAX_SIZE_IN_HEAP=64 -DV8_INTL_SUPPORT -DENABLE_HANDLE_ZAPPING -DV8_USE_SNAPSHOT -DV8_USE_EXTERNAL_STARTUP_DATA -DV8_CONCURRENT_MARKING -DV8_TARGET_ARCH_X64 -DU_USING_ICU_NAMESPACE=0 -DU_ENABLE_DYLOAD=0 -DU_STATIC_IMPLEMENTATION -DICU_UTIL_DATA_IMPL=ICU_UTIL_DATA_FILE -DUCHAR_TYPE=uint16_t -fno-strict-aliasing -fstack-protector -Wno-builtin-macro-redefined -D__DATE__= -D__TIME__= -D__TIMESTAMP__= -fcolor-diagnostics -Xclang -mllvm -Xclang -instcombine-lower-dbg-declare=0 -no-canonical-prefixes -arch x86_64 -Wall -Werror -Wextra -Wunguarded-availability -Wno-missing-field-initializers -Wno-unused-parameter -Wno-c++11-narrowing -Wno-covered-switch-default -Wno-unneeded-internal-declaration -Wno-inconsistent-missing-override -Wno-undefined-var-template -Wno-nonportable-include-path -Wno-address-of-packed-member -Wno-unused-lambda-capture -Wno-user-defined-warnings -Wno-enum-compare-switch -Wno-tautological-unsigned-zero-compare -Wno-null-pointer-arithmetic -Wno-tautological-constant-compare -Wtautological-constant-out-of-range-compare -fno-omit-frame-pointer -gdwarf-2 -fno-standalone-debug -isysroot /Users/mmoroz/projects/chromium/src/build/mac_files/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.12.sdk -mmacosx-version-min=10.9.0 -gcolumn-info -fsanitize=fuzzer-no-link -fsanitize=address -fno-sanitize-address-use-after-scope -mllvm -asan-globals=0 -fsanitize-blacklist=/Users/mmoroz/projects/chromium/src/tools/memory/asan/blacklist.txt -fvisibility=hidden -Xclang -load -Xclang /Users/mmoroz/projects/chromium/src/third_party/llvm-build/Release+Asserts/lib/libFindBadConstructs.dylib -Xclang -add-plugin -Xclang find-bad-constructs -Wheader-hygiene -Wstring-conversion -Wtautological-overlap-compare -Wsign-compare -Winconsistent-missing-override -Wshorten-64-to-32 -O2 -std=c++14 -stdlib=libc++ -fno-exceptions -fno-rtti -fvisibility-inlines-hidden -c fuzz.cpp -o fuzz.o
+ /Users/mmoroz/projects/chromium/src/third_party/llvm-build/Release+Asserts/bin/clang++ -Wcrl,dsym,. -stdlib=libc++ -arch x86_64 -segprot PROTECTED_MEMORY rw r -Werror -Wl,-dead_strip -isysroot /Users/mmoroz/projects/chromium/src/build/mac_files/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.12.sdk -mmacosx-version-min=10.9.0 -fsanitize=fuzzer-no-link -fsanitize=address -fno-sanitize-address-use-after-scope -Wl,-ObjC -Wl,-U,_LLVMFuzzerCustomCrossOver -Wl,-U,_LLVMFuzzerCustomMutator -Wl,-U,_LLVMFuzzerInitialize -Wl,-U,_sanitizer_options_link_helper -fsanitize=address -fno-sanitize-address-use-after-scope -o fuzzer -Wl,-filelist,./objects.rsp
mmoroz-macpro:weak_symbols mmoroz$ 
mmoroz-macpro:weak_symbols mmoroz$ 
mmoroz-macpro:weak_symbols mmoroz$ ./fuzzer -runs=4 2>&1 | egrep LLVMFuzzerInitialize
WARNING: Failed to find function "LLVMFuzzerInitialize". Reason dlsym(RTLD_DEFAULT, LLVMFuzzerInitialize): symbol not found.


So, now I just need to figure out which of nearly hundred compilation flags causes LLVMFuzzerInitialize to be eliminated.

Perhaps -O2 and -fvisibility-inlines-hidden and/or -fvisibiilty=hidden does it?

Thank you Robert! Indeed, is the trouble maker. It looks like there are some projects when developers have to avoid using that for Mac, e.g. https://cs.chromium.org/chromium/src/third_party/widevine/cdm/BUILD.gn?type=cs&q=%22fvisibility%3Dhidden%22&sq=package:chromium&l=113:

    if (is_posix && !is_mac) {
      cflags = [ "-fvisibility=hidden" ]
    }


so I'll probably do something like that for fuzzing builds.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c1406d52464e16da16aae9cb189c4f28e6412358

commit c1406d52464e16da16aae9cb189c4f28e6412358
Author: Max Moroz <mmoroz@chromium.org>
Date: Thu Dec 07 19:25:12 2017

Migrate to -fsanitize=fuzzer-no-link when use_fuzzing_engine=true.

-fsanitize=fuzzer and -fsanitize=fuzzer-no-link are two compilation
flags that enable coverage instrumentation needed for libFuzzer.

The instrumentation has more stuff under the hood compared to
-fsanitize=trace-pc-guard. Also, it can be changing over time
without a need to update GN flags again and again (e.g. move from
edge to trace-pc-guard or something like that).

Bug:  764514 
Change-Id: I48ef328dee49a9620a1b44bd5cd920f116e1bc1b
Reviewed-on: https://chromium-review.googlesource.com/802395
Commit-Queue: Max Moroz <mmoroz@chromium.org>
Reviewed-by: Oliver Chang <ochang@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522505}
[modify] https://crrev.com/c1406d52464e16da16aae9cb189c4f28e6412358/build/config/sanitizers/BUILD.gn
[modify] https://crrev.com/c1406d52464e16da16aae9cb189c4f28e6412358/build/config/sanitizers/sanitizers.gni
[modify] https://crrev.com/c1406d52464e16da16aae9cb189c4f28e6412358/build/toolchain/concurrent_links.gni

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8181cec6f208b34465eaacb64e247a1fcce9bcd6

commit 8181cec6f208b34465eaacb64e247a1fcce9bcd6
Author: Max Moroz <mmoroz@chromium.org>
Date: Thu Dec 07 23:35:41 2017

Revert "Migrate to -fsanitize=fuzzer-no-link when use_fuzzing_engine=true."

This reverts commit c1406d52464e16da16aae9cb189c4f28e6412358.

Reason for revert: The builds are failing on linking of some fuzzers: https://logs.chromium.org/v/?s=chromium%2Fbb%2Fchromium.fyi%2FLibfuzzer_Upload_Linux_ASan%2F7088%2F%2B%2Frecipes%2Fsteps%2Fcompile%2F0%2Fstdout

Original change's description:
> Migrate to -fsanitize=fuzzer-no-link when use_fuzzing_engine=true.
> 
> -fsanitize=fuzzer and -fsanitize=fuzzer-no-link are two compilation
> flags that enable coverage instrumentation needed for libFuzzer.
> 
> The instrumentation has more stuff under the hood compared to
> -fsanitize=trace-pc-guard. Also, it can be changing over time
> without a need to update GN flags again and again (e.g. move from
> edge to trace-pc-guard or something like that).
> 
> Bug:  764514 
> Change-Id: I48ef328dee49a9620a1b44bd5cd920f116e1bc1b
> Reviewed-on: https://chromium-review.googlesource.com/802395
> Commit-Queue: Max Moroz <mmoroz@chromium.org>
> Reviewed-by: Oliver Chang <ochang@chromium.org>
> Reviewed-by: Dirk Pranke <dpranke@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#522505}

TBR=kcc@chromium.org,inferno@chromium.org,dpranke@chromium.org,ochang@chromium.org,mmoroz@chromium.org

Change-Id: I86f316ba7b13c6cb015a3245e3860047ab4859ea
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  764514 
Reviewed-on: https://chromium-review.googlesource.com/815460
Reviewed-by: Max Moroz <mmoroz@chromium.org>
Commit-Queue: Max Moroz <mmoroz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522615}
[modify] https://crrev.com/8181cec6f208b34465eaacb64e247a1fcce9bcd6/build/config/sanitizers/BUILD.gn
[modify] https://crrev.com/8181cec6f208b34465eaacb64e247a1fcce9bcd6/build/config/sanitizers/sanitizers.gni
[modify] https://crrev.com/8181cec6f208b34465eaacb64e247a1fcce9bcd6/build/toolchain/concurrent_links.gni

It's still unclear to me why the linker failure is happening after that change, so I'll have to take a closer look later.

https://logs.chromium.org/v/?s=chromium%2Fbb%2Fchromium.fyi%2FLibfuzzer_Upload_Linux_ASan%2F7088%2F%2B%2Frecipes%2Fsteps%2Fcompile%2F0%2Fstdout :

[22995/31652] LINK ./v4_get_hash_protocol_manager_fuzzer
FAILED: v4_get_hash_protocol_manager_fuzzer 
python "../../build/toolchain/gcc_link_wrapper.py" --output="./v4_get_hash_protocol_manager_fuzzer" -- ../../third_party/llvm-build/Release+Asserts/bin/clang++ -Wl,--fatal-warnings -fPIC -Wl,-z,noexecstack -Wl,-z,now -Wl,-z,relro -fuse-ld=lld -m64 -Werror -Wl,-O1 -Wl,--gc-sections -nostdlib++ --sysroot=../../build/linux/debian_stretch_amd64-sysroot -L../../build/linux/debian_stretch_amd64-sysroot/lib/x86_64-linux-gnu -Wl,-rpath-link=../../build/linux/debian_stretch_amd64-sysroot/lib/x86_64-linux-gnu -L../../build/linux/debian_stretch_amd64-sysroot/usr/lib/x86_64-linux-gnu -Wl,-rpath-link=../../build/linux/debian_stretch_amd64-sysroot/usr/lib/x86_64-linux-gnu -fsanitize=address -fsanitize-address-use-after-scope -fsanitize=leak -fsanitize=fuzzer-no-link -Wl,-rpath-link=. -Wl,--disable-new-dtags -Wl,-rpath=\$ORIGIN/. -Wl,-rpath-link=. -Wl,--export-dynamic -Wl,-u_sanitizer_options_link_helper -fsanitize=address -fsanitize-address-use-after-scope -fsanitize=leak -fsanitize=fuzzer-no-link -o "./v4_get_hash_protocol_manager_fuzzer" -Wl,--start-group @"./v4_get_hash_protocol_manager_fuzzer.rsp" ./libc++.so -Wl,--end-group   -ldl -lpthread -lrt -lgmodule-2.0 -lgobject-2.0 -lgthread-2.0 -lglib-2.0 -lnss3 -lnssutil3 -lsmime3 -lplds4 -lplc4 -lnspr4 -lgconf-2 -lresolv -lgio-2.0 -lfontconfig -lasound -lX11 -lX11-xcb -lxcb -lXcomposite -lXcursor -lXdamage -lXext -lXfixes -lXi -lXrender -lXtst -lXrandr -lm -lz -lpci -lXss -lpangocairo-1.0 -lpango-1.0 -lcairo -ldbus-1 -latk-1.0 -latk-bridge-2.0 -lcups 
/b/build/slave/Libfuzzer_Upload_Linux_ASan/build/src/out/Release/../../third_party/llvm-build/Release+Asserts/bin/ld.lld: error: undefined symbol: blink::WebFontRendering::SetSkiaFontManager(sk_sp<SkFontMgr>)
>>> referenced by zygote_main_linux.cc:466 (../../content/zygote/zygote_main_linux.cc:466)
>>>               obj/content/browser/browser/zygote_main_linux.o:(content::ZygotePreSandboxInit())
clang: error: linker command failed with exit code 1 (use -v to see invocation)

I'm still stuck with the error from the comment above.

I've compared compilation and link flags before and after my change, the only differences are:

# Compilation:

-fsanitize-coverage=trace-pc-guard
-sanitizer-coverage-prune-blocks=1

    vs

-fsanitize=fuzzer-no-link


# Link:

-fsanitize-coverage=trace-pc-guard

    vs

-fsanitize=fuzzer-no-link


Neither of those should affect visibility of https://cs.chromium.org/chromium/src/third_party/WebKit/public/web/linux/WebFontRendering.h?q=blink::WebFontRendering::SetSkiaFontManager&sq=package:chromium&l=45&dr=Ss

IIUC, fuzzer-no-link enables the following coverage options:

-fsanitize-coverage-8bit-counters
-fsanitize-coverage-indirect-calls
-fsanitize-coverage-trace-pc
-fsanitize-coverage-pc-table


Let me try eliminating each of them via -fnosanitize-coverage-...

Yeah, pc-table is the culprit.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ddd48e3299a1adf1e12f4d1fe680fd47eac8700d

commit ddd48e3299a1adf1e12f4d1fe680fd47eac8700d
Author: Max Moroz <mmoroz@chromium.org>
Date: Wed Jan 03 02:31:38 2018

Reland "Migrate to -fsanitize=fuzzer-no-link when use_fuzzing_engine=true".

-fsanitize=fuzzer and -fsanitize=fuzzer-no-link are two compilation
flags that enable coverage instrumentation needed for libFuzzer.

The instrumentation has more stuff under the hood compared to
-fsanitize=trace-pc-guard. Also, it can be changing over time
without a need to update GN flags again and again (e.g. move from
edge to trace-pc-guard or something like that).

Bug:  764514 
Change-Id: I53bf5a3355335f4f627e9024b7ed7fe601c9ecfd

> Revert "Migrate to -fsanitize=fuzzer-no-link when use_fuzzing_engine=true."
>
> This reverts commit c1406d52464e16da16aae9cb189c4f28e6412358.
>
> Reason for revert: The builds are failing on linking of some fuzzers: https://logs.chromium.org/v/?s=chromium%2Fbb%2Fchromium.fyi%2FLibfuzzer_Upload_Linux_ASan%2F7088%2F%2B%2Frecipes%2Fsteps%2Fcompile%2F0%2Fstdout
>
> Original change's description:
> > Migrate to -fsanitize=fuzzer-no-link when use_fuzzing_engine=true.
> >
> > -fsanitize=fuzzer and -fsanitize=fuzzer-no-link are two compilation
> > flags that enable coverage instrumentation needed for libFuzzer.
> >
> > The instrumentation has more stuff under the hood compared to
> > -fsanitize=trace-pc-guard. Also, it can be changing over time
> > without a need to update GN flags again and again (e.g. move from
> > edge to trace-pc-guard or something like that).
> >
> > Bug:  764514 
> > Change-Id: I48ef328dee49a9620a1b44bd5cd920f116e1bc1b
> > Reviewed-on: https://chromium-review.googlesource.com/802395
> > Commit-Queue: Max Moroz <mmoroz@chromium.org>
> > Reviewed-by: Oliver Chang <ochang@chromium.org>
> > Reviewed-by: Dirk Pranke <dpranke@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#522505}

Change-Id: I53bf5a3355335f4f627e9024b7ed7fe601c9ecfd
Reviewed-on: https://chromium-review.googlesource.com/846100
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Commit-Queue: Max Moroz <mmoroz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#526592}
[modify] https://crrev.com/ddd48e3299a1adf1e12f4d1fe680fd47eac8700d/build/config/sanitizers/BUILD.gn
[modify] https://crrev.com/ddd48e3299a1adf1e12f4d1fe680fd47eac8700d/build/config/sanitizers/sanitizers.gni
[modify] https://crrev.com/ddd48e3299a1adf1e12f4d1fe680fd47eac8700d/build/toolchain/concurrent_links.gni

Kostya, the last change seems to work well, but I had to use "-fno-sanitize-coverage=pc-table" to avoid link failures. How bad is that?

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/890a78251380463ed81454104bb91e3700ffff42

commit 890a78251380463ed81454104bb91e3700ffff42
Author: Max Moroz <mmoroz@chromium.org>
Date: Fri Jan 05 21:56:41 2018

Revert migration to -fsanitize=fizzer-no-link because of multiple issues.

- AFL doesn't see coverage instrumentation, as it currently relies on
__sanitizer_cov_trace_pc_guard, i.e. -fsanitize=trace-pc-guard.

- coverage generation for libFuzzer is broken, sancov doesn't see any edges.

R=inferno@chromium.org, metzman@chromium.org

Bug:  764514 , 798928
Change-Id: Ic4775b53d1ff03af4660b5f930a892182c9f021b
Reviewed-on: https://chromium-review.googlesource.com/852826
Reviewed-by: Abhishek Arya <inferno@chromium.org>
Reviewed-by: Jonathan Metzman <metzman@chromium.org>
Commit-Queue: Abhishek Arya <inferno@chromium.org>
Cr-Commit-Position: refs/heads/master@{#527401}
[modify] https://crrev.com/890a78251380463ed81454104bb91e3700ffff42/build/config/sanitizers/BUILD.gn
[modify] https://crrev.com/890a78251380463ed81454104bb91e3700ffff42/build/config/sanitizers/sanitizers.gni

I've just realized that we have one more problem here. Internals of -fsanitize=fuzzer-no-link are implemented in clang. Thus, if something breaks after a clang roll, we'll have to revert it. However, clang rolls are already quite painful (e.g.  issue 787920 ), so it's unlikely that our breakages would be a good enough reason to revert a roll that was successful otherwise.

The only way to deal with that situation safely, is to provide test coverage on the main waterfall (and maybe even CQ?). Pretty much the same reasoning as has been used for code coverage tools: https://chromium-review.googlesource.com/c/chromium/src/+/688221#message-c4f2f69499cabfc45e39b5658228ba592f99d15d

    "The rule we want to use going forward is that if something is being used on the "main bots" in Chromium,
     we want it to be downloaded by default in `gclient runhooks`. "


With that, we have to invest more on Build / Test Infra side, to provide good test coverage (e.g. bad build checks: https://github.com/google/oss-fuzz/blob/7c917865b118b7bd3f7409a2e76bc39c3ea15e3b/infra/base-images/base-runner/bad_build_check) before moving forward with this.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/88e2dc4f17fdf5e35b09e32ef8754acd256d0a35

commit 88e2dc4f17fdf5e35b09e32ef8754acd256d0a35
Author: Max Moroz <mmoroz@chromium.org>
Date: Tue Jun 05 16:31:49 2018

Fuzzing: second attempt to migrate to -fsanitize=fuzzer-no-link.

This instrumentation flag is recommended by libFuzzer authors, as it may enable
various different instrumentation options which are actual and recommended for use.
The flag is implemented in clang, so we won't have to update individual
instrumentation flags anymore.

See  crbug.com/764514  and https://github.com/google/oss-fuzz/issues/832

AFL still supports only trace-pc-guard instrumentation, so we have to use separate
compilation flags for AFL and libFuzzer, the same way as we do in OSS-Fuzz.

Previous revert: https://chromium-review.googlesource.com/c/chromium/src/+/852826

The issue with sancov report generation is not resolved yet. We can either land
this and break coverage on CF for some time, until I resolve crbug.com/818467 OR
we can wait until crbug.com/818467 gets resolved first. I'd prefer landing this now
and fixing coverage on CF after that.

Bug:  764514 
Change-Id: I186a73c4dc0deab5daf2ec2c97baf04c0cb92103
Reviewed-on: https://chromium-review.googlesource.com/1085371
Commit-Queue: Max Moroz <mmoroz@chromium.org>
Reviewed-by: Jonathan Metzman <metzman@chromium.org>
Reviewed-by: Abhishek Arya <inferno@chromium.org>
Cr-Commit-Position: refs/heads/master@{#564532}
[modify] https://crrev.com/88e2dc4f17fdf5e35b09e32ef8754acd256d0a35/build/config/sanitizers/BUILD.gn
[modify] https://crrev.com/88e2dc4f17fdf5e35b09e32ef8754acd256d0a35/build/config/sanitizers/sanitizers.gni

Fingers crossed that nothing breaks...

UBSan builder is red, but doesn't seem to be related from a quick glance: https://build.chromium.org/deprecated/chromium.fyi/builders/Libfuzzer%20Upload%20Linux%20UBSan/builds/14011

Tested locally, that seems to be my change actually :( Reverting for now.

kcc@, does fuzzer-no-link uses "typeinfo" anyhow? Do we need any extra flags? The issue happens with UBSan build only. I wonder if "-fno-rtti" can help.

mmoroz@mmoroz2:~/Projects/new/chromium/src$ ninja -C out/ubsan -j1000 media_es_parser_adts_fuzzer
ninja: Entering directory `out/ubsan'
[5580/5580] LINK ./media_es_parser_adts_fuzzer
FAILED: media_es_parser_adts_fuzzer 
python "../../build/toolchain/gcc_link_wrapper.py" --output="./media_es_parser_adts_fuzzer" -- ../../third_party/llvm-build/Release+Asserts/bin/clang++ -Wl,--fatal-warnings -fPIC -Wl,-z,noexecstack -Wl,-z,now -Wl,-z,relro -fuse-ld=lld -Wl,--color-diagnostics -m64 -Werror -Wl,-O2 -Wl,--gc-sections -nostdlib++ --sysroot=../../build/linux/debian_sid_amd64-sysroot -L../../build/linux/debian_sid_amd64-sysroot/usr/local/lib/x86_64-linux-gnu -Wl,-rpath-link=../../build/linux/debian_sid_amd64-sysroot/usr/local/lib/x86_64-linux-gnu -L../../build/linux/debian_sid_amd64-sysroot/lib/x86_64-linux-gnu -Wl,-rpath-link=../../build/linux/debian_sid_amd64-sysroot/lib/x86_64-linux-gnu -L../../build/linux/debian_sid_amd64-sysroot/usr/lib/x86_64-linux-gnu -Wl,-rpath-link=../../build/linux/debian_sid_amd64-sysroot/usr/lib/x86_64-linux-gnu -fsanitize=undefined -fsanitize=fuzzer-no-link -rdynamic -Wl,-rpath-link=. -Wl,--disable-new-dtags -Wl,-rpath=\$ORIGIN/. -Wl,-rpath-link=. -Wl,-u_sanitizer_options_link_helper -fsanitize=undefined -fsanitize=fuzzer-no-link -o "./media_es_parser_adts_fuzzer" -Wl,--start-group @"./media_es_parser_adts_fuzzer.rsp" ./libc++.so -Wl,--end-group   -ldl -lpthread -lrt -lgmodule-2.0 -lgobject-2.0 -lgthread-2.0 -lglib-2.0 -lnss3 -lnssutil3 -lsmime3 -lplds4 -lplc4 -lnspr4 -lasound -lX11 -lX11-xcb -lxcb -lXcomposite -lXcursor -lXdamage -lXext -lXfixes -lXi -lXrender -lXtst -lXrandr -lm -lz 
/usr/local/google/home/mmoroz/Projects/new/chromium/src/out/ubsan/../../third_party/llvm-build/Release+Asserts/bin/ld.lld: error: undefined symbol: typeinfo for mkvparser::IMkvReader
>>> referenced by mkvmuxer.cc
>>>               libwebm/mkvmuxer.o:(.data..L__unnamed_6+0x18) in archive obj/third_party/libwebm/libwebm.a
clang: error: linker command failed with exit code 1 (use -v to see invocation)
ninja: build stopped: subcommand failed.
mmoroz@mmoroz2:~/Projects/new/chromium/src$ git checkout help
error: pathspec 'help' did not match any file(s) known to git.
mmoroz@mmoroz2:~/Projects/new/chromium/src$ git checkout build/config/sanitizers/BUILD.gn 924c8465fff86ce13f8a4ca4d352807e0616f0b1
error: pathspec '924c8465fff86ce13f8a4ca4d352807e0616f0b1' did not match any file(s) known to git.
mmoroz@mmoroz2:~/Projects/new/chromium/src$ git checkout 924c8465fff86ce13f8a4ca4d352807e0616f0b1 build/config/sanitizers/BUILD.gn
mmoroz@mmoroz2:~/Projects/new/chromium/src$ git checkout 924c8465fff86ce13f8a4ca4d352807e0616f0b1 build/config/sanitizers/sanitizers.gni
mmoroz@mmoroz2:~/Projects/new/chromium/src$ 
mmoroz@mmoroz2:~/Projects/new/chromium/src$ 
mmoroz@mmoroz2:~/Projects/new/chromium/src$ ninja -C out/ubsan -j1000 media_es_parser_adts_fuzzer
ninja: Entering directory `out/ubsan'
[1/1] Regenerating ninja files
[5080/5080] LINK ./media_es_parser_adts_fuzzer
mmoroz@mmoroz2:~/Projects/new/chromium/src$ 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2370d9e8cbc0e8f33ef1b53f8ad6b50aba00a0c7

commit 2370d9e8cbc0e8f33ef1b53f8ad6b50aba00a0c7
Author: Max Moroz <mmoroz@chromium.org>
Date: Tue Jun 05 18:37:04 2018

Revert "Fuzzing: second attempt to migrate to -fsanitize=fuzzer-no-link."

This reverts commit 88e2dc4f17fdf5e35b09e32ef8754acd256d0a35.

Reason for revert: UBSan builder is broken: https://build.chromium.org/deprecated/chromium.fyi/builders/Libfuzzer%20Upload%20Linux%20UBSan/builds/14011

Original change's description:
> Fuzzing: second attempt to migrate to -fsanitize=fuzzer-no-link.
> 
> This instrumentation flag is recommended by libFuzzer authors, as it may enable
> various different instrumentation options which are actual and recommended for use.
> The flag is implemented in clang, so we won't have to update individual
> instrumentation flags anymore.
> 
> See  crbug.com/764514  and https://github.com/google/oss-fuzz/issues/832
> 
> AFL still supports only trace-pc-guard instrumentation, so we have to use separate
> compilation flags for AFL and libFuzzer, the same way as we do in OSS-Fuzz.
> 
> Previous revert: https://chromium-review.googlesource.com/c/chromium/src/+/852826
> 
> The issue with sancov report generation is not resolved yet. We can either land
> this and break coverage on CF for some time, until I resolve crbug.com/818467 OR
> we can wait until crbug.com/818467 gets resolved first. I'd prefer landing this now
> and fixing coverage on CF after that.
> 
> Bug:  764514 
> Change-Id: I186a73c4dc0deab5daf2ec2c97baf04c0cb92103
> Reviewed-on: https://chromium-review.googlesource.com/1085371
> Commit-Queue: Max Moroz <mmoroz@chromium.org>
> Reviewed-by: Jonathan Metzman <metzman@chromium.org>
> Reviewed-by: Abhishek Arya <inferno@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#564532}

TBR=inferno@chromium.org,ochang@chromium.org,mmoroz@chromium.org,metzman@chromium.org

Change-Id: I9e654b2c1e7fbdeb4166a39bff4186d1cdd80e83
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  764514 
Reviewed-on: https://chromium-review.googlesource.com/1087431
Reviewed-by: Max Moroz <mmoroz@chromium.org>
Commit-Queue: Max Moroz <mmoroz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#564591}
[modify] https://crrev.com/2370d9e8cbc0e8f33ef1b53f8ad6b50aba00a0c7/build/config/sanitizers/BUILD.gn
[modify] https://crrev.com/2370d9e8cbc0e8f33ef1b53f8ad6b50aba00a0c7/build/config/sanitizers/sanitizers.gni

> does fuzzer-no-link uses "typeinfo" anyhow? 

No. 

> Do we need any extra flags? The issue happens with UBSan build only. I wonder if "-fno-rtti" can help.


Yes, either try -fno-rtti or -fno-sanitize=vptr 

Thanks for the reply!

"-fno-rtti" doesn't help, as it conflicts with "-fsanitize=vptr".

Removing "-fsanitize=vptr" is not really an option, as we've seen bugs detected with it.

I can add "-fno-sanitize=vptr" to third_party/libwebm (3 source files) and to media/muxers (2 source files), in that case everything works.

However, I'm surprised that the issue arise only when -fsanitize=fuzzer-no-link is used.

If I replace "-fsanitize=fuzzer-no-link" with "-fsanitize-coverage=inline-8bit-counters,indirect-calls,trace-cmp,pc-table", the issue still persists.

If I remove "pc-table", the issue doesn't reproduce.


So, I guess "pc-table" somehow affects that.

I'm not sure, but maybe IRB.CreatePointerCast() somehow uses typeinfo when doing SanitizerCoverageModule::CreatePCArray().

Well, making mkvparser::~IMkvReader() pure virtual resolves the issue (https://cs.chromium.org/chromium/src/third_party/libwebm/source/mkvparser/mkvparser.h?type=cs&sq=package:chromium&g=0&l=25).

I hardly understand why it is so, because C++ is too hard. But I'll ask //media folks whether they're fine with that.

It's not the only issue though. There also is: 

third_party/llvm-build/Release+Asserts/bin/ld.lld: error: undefined symbol: typeinfo for ppapi::proxy::Dispatcher
>>> referenced by serialized_var.cc
>>>               obj/ppapi/proxy/ipc_sources/serialized_var.o:(.data..L__unnamed_11+0x18)

Based on https://stackoverflow.com/questions/307352/g-undefined-reference-to-typeinfo/4184877, my understanding is that if we have a forward declaration of an abstract class, for each of its methods we should either have definitions presented in the same file or those methods need to be marked as pure virtual.

For example, the patch below also works:


mmoroz@mmoroz2:~/Projects/new/chromium/src/third_party/libwebm/source$ git diff
diff --git a/mkvparser/mkvparser.cc b/mkvparser/mkvparser.cc
index e7b76f7..7ca2376 100644
--- a/mkvparser/mkvparser.cc
+++ b/mkvparser/mkvparser.cc
@@ -36,7 +36,7 @@ inline bool isnan(double val) { return std::isnan(val); }
 inline bool isinf(double val) { return std::isinf(val); }
 #endif  // MSC_COMPAT
 
-IMkvReader::~IMkvReader() {}
+// IMkvReader::~IMkvReader() {}
 
 template <typename Type>
 Type* SafeArrayAlloc(unsigned long long num_elements,
diff --git a/mkvparser/mkvparser.h b/mkvparser/mkvparser.h
index 26c2b7e..e3ff85b 100644
--- a/mkvparser/mkvparser.h
+++ b/mkvparser/mkvparser.h
@@ -22,7 +22,7 @@ class IMkvReader {
   virtual int Length(long long* total, long long* available) = 0;
 
  protected:
-  virtual ~IMkvReader();
+  virtual ~IMkvReader() {}
 };

Stuff in ppapi/proxy/ classes a bit more complicated. If I mark some methods pure virtual, that breaks other ancestors which do not define them. The only option I see now is to move some declarations from .cpp to .h, let me see how much that would take.

I've managed to built all 313 fuzz targets after patching libwebm and moving some code in ppapi/proxy. Will upload separate CLs for everything and see what OWNERS think about that.

typo in c#42: move some definitions, not declarations :)

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f41bf12a2eb8d63fc371c6d431e7d7d05835b54d

commit f41bf12a2eb8d63fc371c6d431e7d7d05835b54d
Author: Max Moroz <mmoroz@chromium.org>
Date: Wed Jun 06 22:48:37 2018

Roll src/third_party/libwebm/source/ af81f2602..01c1d1d76 (4 commits)

https://chromium.googlesource.com/webm/libwebm.git/+log/af81f26025b7..01c1d1d76f13

$ git log af81f2602..01c1d1d76 --date=short --no-merges --format='%ad %ae %s'
2018-06-06 mmoroz Move definition of IMkvReader::~IMkvReader() to mkvparser.h.
2018-05-14 johannkoenig replace vp10 with av1
2018-04-24 johannkoenig remove unused PesHeader
2018-04-24 johannkoenig add codereview.settings

Created with:
  roll-dep src/third_party/libwebm/source

Bug: 849812, 764514 
Change-Id: I87e412794820fd1d8a7a320faca2b4220b7bf23b
Reviewed-on: https://chromium-review.googlesource.com/1089571
Reviewed-by: Emircan Uysaler <emircan@chromium.org>
Commit-Queue: Max Moroz <mmoroz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#565072}
[modify] https://crrev.com/f41bf12a2eb8d63fc371c6d431e7d7d05835b54d/DEPS

Filed a bug against sanitizers: https://github.com/google/sanitizers/issues/971

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6d7b69ac40b01d87e8499cb8232631c69bd3269a

commit 6d7b69ac40b01d87e8499cb8232631c69bd3269a
Author: Max Moroz <mmoroz@chromium.org>
Date: Wed Aug 15 17:17:17 2018

Fuzzing: the fourth attempt to migrate to -fsanitize=fuzzer-no-link.

This instrumentation flag is recommended by libFuzzer authors, as it may enable
various different instrumentation options which are actual and recommended for use.
The flag is implemented in clang, so we won't have to update individual
instrumentation flags anymore.

See  crbug.com/764514  and https://github.com/google/oss-fuzz/issues/832

AFL still supports only trace-pc-guard instrumentation, so we have to use separate
compilation flags for AFL and libFuzzer, the same way as we do in OSS-Fuzz.

Previous reverts:

1) https://chromium-review.googlesource.com/c/chromium/src/+/852826

The issue with sancov report generation is not resolved yet. We can either land
this and break coverage on CF for some time, until I resolve crbug.com/818467 OR
we can wait until crbug.com/818467 gets resolved first. I'd prefer landing this now
and fixing coverage on CF after that.

2) https://chromium-review.googlesource.com/c/chromium/src/+/1087431

The issue with linker trying to link in symbols which were not used and thus
were not properly instrumented has been reported to the sanitizers project
and fixed on LLVM side: https://github.com/google/sanitizers/issues/971.

3) https://chromium-review.googlesource.com/c/chromium/src/+/1110348

Dead code stripping led to libFuzzer complaining about a mismatch of the size of
coverage PC tables: https://crbug.com/856239.
Fixed upstream: https://reviews.llvm.org/rL336941.

Bug:  764514 
Change-Id: Ic19a6c27ee37143f3ca126e3bc57c9182e962750
Reviewed-on: https://chromium-review.googlesource.com/1175851
Reviewed-by: Abhishek Arya <inferno@chromium.org>
Reviewed-by: Jonathan Metzman <metzman@chromium.org>
Commit-Queue: Max Moroz <mmoroz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583294}
[modify] https://crrev.com/6d7b69ac40b01d87e8499cb8232631c69bd3269a/build/config/sanitizers/BUILD.gn
[modify] https://crrev.com/6d7b69ac40b01d87e8499cb8232631c69bd3269a/build/config/sanitizers/sanitizers.gni

It's quite hard to evaluate impact of this migration, given that ClusterFuzz is like a creature living its own busy life :) i.e. things are changing, some parts are breaking, and experiments are running on a regular basis, etc.

However, if I look at libfuzzer_chrome_asan job stats by day (https://clusterfuzz.com/v2/fuzzer-stats/by-day/date-start/2018-07-01/date-end/2018-08-22/fuzzer/libFuzzer/job/libfuzzer_chrome_asan)

(please ignore Aug 6th to Aug 14th, CPU allocation was not correct in that period)

I'm seeing the following trends since Aug ~16th:

- average execution speed decreased (expected)
- average edge coverage decreased (expected), but seems to be growing (awesome, but need more time)
- new inputs added (new_tests_added column) increased 2-3x (awesome)
- new_crashes count also increased

I'm not making any claims though, as we've enabled ML RNN generation around the same time, plus I see that now libFuzzer job gets more cycles than it used to get previously (~1.6x more, see runs_count column).


libfuzzer_chrome_ubsan job shows similar trends: https://clusterfuzz.com/v2/fuzzer-stats/by-day/date-start/2018-07-01/date-end/2018-08-22/fuzzer/libFuzzer/job/libfuzzer_chrome_ubsan

which is good, especially given that UBSan jobs has the same amount of cycles as it had in July / early August.


With all this being said, I'm glad that things are improving and not regressing. Closing the issue.