This is an archive of the discontinued LLVM Phabricator instance.

Differential D83987

Recommit "[libFuzzer] Disable implicit builtin knowledge about memcmp-like functions when -fsanitize=fuzzer-no-link is given."
ClosedPublic

Authored by dokyungs on Jul 16 2020, 2:31 PM.

Details

Reviewers
morehouse
hctim
Commits
rGb52b2e1c1880: Recommit "[libFuzzer] Disable implicit builtin knowledge about memcmp-like…
rG12d1124c49be: [libFuzzer] Disable implicit builtin knowledge about memcmp-like functions when…
Summary

This patch disables implicit builtin knowledge about memcmp-like functions when compiling the program for fuzzing, i.e., when -fsanitize=fuzzer(-no-link) is given. This allows libFuzzer to always intercept memcmp-like functions as it effectively disables optimizing calls to such functions into different forms. This is done by adding a set of flags (-fno-builtin-memcmp and others) in the clang driver. Individual -fno-builtin-* flags previously used in several libFuzzer tests are now removed, as it is now done automatically in the clang driver.

The patch was once reverted in 8ef9e2bf355d05bc81d8b0fe1e5333eec59a0a91, as this patch was dependent on a reverted commit f78d9fceea736d431e9e3cbca291e3909e3aa46d. This reverted commit was recommitted in 831ae45e3dc609e43ba561af07670a8fe47461ef, so relanding this dependent patch too.

Diff Detail

Event Timeline

dokyungs created this revision.Jul 16 2020, 2:31 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptJul 16 2020, 2:31 PM
Herald added subscribers: Restricted Project, cfe-commits. · View Herald Transcript
dokyungs edited the summary of this revision. (Show Details)Jul 16 2020, 2:34 PM
hctim accepted this revision.Jul 16 2020, 2:51 PM

LGTM w/ nit

clang/lib/Driver/SanitizerArgs.cpp
1091

Nit - add a comment here mentioning the libfuzzer interceptors, and that other sanitizers normally do this by propagating IR attributes.

This revision is now accepted and ready to land.Jul 16 2020, 2:51 PM
dokyungs updated this revision to Diff 278610.Jul 16 2020, 3:00 PM

Addressed comments.

dokyungs marked an inline comment as done.Jul 16 2020, 3:01 PM

Thanks Mitch for the comment! Added a comment that explains that.

hctim added inline comments.Jul 16 2020, 3:07 PM
clang/lib/Driver/SanitizerArgs.cpp
1092

I'd say "the following -fno-builtin-* flags force the compiler to emit interposable libcalls to these functions"

dokyungs updated this revision to Diff 278613.Jul 16 2020, 3:10 PM

Adjusted the comment as suggested.

dokyungs marked an inline comment as done.Jul 16 2020, 3:10 PM
morehouse accepted this revision.Jul 16 2020, 3:16 PM

LGTM

Harbormaster failed remote builds in B64595: Diff 278602!Jul 16 2020, 3:17 PM
Harbormaster failed remote builds in B64596: Diff 278610!Jul 16 2020, 3:33 PM
hctim accepted this revision.Jul 16 2020, 3:39 PM

LGTM

Harbormaster failed remote builds in B64600: Diff 278613!Jul 16 2020, 3:41 PM
Closed by commit rG12d1124c49be: [libFuzzer] Disable implicit builtin knowledge about memcmp-like functions when… (authored by dokyungs). · Explain WhyJul 16 2020, 3:55 PM
This revision was automatically updated to reflect the committed changes.
dokyungs reopened this revision.Jul 24 2020, 9:57 AM
This revision is now accepted and ready to land.Jul 24 2020, 9:57 AM
dokyungs updated this revision to Diff 280507.Jul 24 2020, 9:57 AM

Relanding this reverted commit. (See summary)

dokyungs retitled this revision from [libFuzzer] Disable implicit builtin knowledge about memcmp-like functions when -fsanitize=fuzzer-no-link is given. to Recommit "[libFuzzer] Disable implicit builtin knowledge about memcmp-like functions when -fsanitize=fuzzer-no-link is given.".Jul 24 2020, 9:58 AM
dokyungs edited the summary of this revision. (Show Details)
dokyungs added a reverted change: rGf78d9fceea73: [libFuzzer] Link libFuzzer's own interceptors when other compiler runtimes are….
Harbormaster completed remote builds in B65590: Diff 280507.Jul 24 2020, 11:36 AM
dokyungs updated this revision to Diff 280981.Jul 27 2020, 11:06 AM

Add '-fno-builtin-bcmp' and a corresponding test case.

dokyungs updated this revision to Diff 280982.Jul 27 2020, 11:07 AM

Disable noasan-bcmp.test on darwin.

morehouse accepted this revision.Jul 27 2020, 11:09 AM

LGTM

Closed by commit rGb52b2e1c1880: Recommit "[libFuzzer] Disable implicit builtin knowledge about memcmp-like… (authored by dokyungs). · Explain WhyJul 27 2020, 11:28 AM
This revision was automatically updated to reflect the committed changes.
Harbormaster completed remote builds in B65877: Diff 280981.Jul 27 2020, 11:57 AM
Harbormaster completed remote builds in B65878: Diff 280982.Jul 27 2020, 12:24 PM

Revision Contents

PathSize
clang/
lib/
Driver/
17 lines
compiler-rt/
test/
fuzzer/
4 lines
4 lines
2 lines
4 lines
4 lines
4 lines

Diff 280990

clang/lib/Driver/SanitizerArgs.cpp

Show First 20 Lines • Show All 1,082 Lines • ▼ Show 20 Linesvoid SanitizerArgs::addArgs(const ToolChain &TC, const llvm::opt::ArgList &Args,
// ASan: This is mainly to help LSan with cases such as // ASan: This is mainly to help LSan with cases such as
// https://github.com/google/sanitizers/issues/373 // https://github.com/google/sanitizers/issues/373
// We can't make this conditional on -fsanitize=leak, as that flag shouldn't // We can't make this conditional on -fsanitize=leak, as that flag shouldn't
// affect compilation. // affect compilation.
if (Sanitizers.has(SanitizerKind::Memory) || if (Sanitizers.has(SanitizerKind::Memory) ||
Sanitizers.has(SanitizerKind::Address)) Sanitizers.has(SanitizerKind::Address))
CmdArgs.push_back("-fno-assume-sane-operator-new"); CmdArgs.push_back("-fno-assume-sane-operator-new");
// libFuzzer wants to intercept calls to certain library functions, so the
hctimUnsubmitted
Done
ReplyInline Actions

Nit - add a comment here mentioning the libfuzzer interceptors, and that other sanitizers normally do this by propagating IR attributes.

hctim: Nit - add a comment here mentioning the libfuzzer interceptors, and that other sanitizers…
// following -fno-builtin-* flags force the compiler to emit interposable
hctimUnsubmitted
Done
ReplyInline Actions

I'd say "the following -fno-builtin-* flags force the compiler to emit interposable libcalls to these functions"

hctim: I'd say "the following -fno-builtin-* flags force the compiler to emit interposable libcalls to…
// libcalls to these functions. Other sanitizers effectively do the same thing
// by marking all library call sites with NoBuiltin attribute in their LLVM
// pass. (see llvm::maybeMarkSanitizerLibraryCallNoBuiltin)
if (Sanitizers.has(SanitizerKind::FuzzerNoLink)) {
CmdArgs.push_back("-fno-builtin-bcmp");
CmdArgs.push_back("-fno-builtin-memcmp");
CmdArgs.push_back("-fno-builtin-strncmp");
CmdArgs.push_back("-fno-builtin-strcmp");
CmdArgs.push_back("-fno-builtin-strncasecmp");
CmdArgs.push_back("-fno-builtin-strcasecmp");
CmdArgs.push_back("-fno-builtin-strstr");
CmdArgs.push_back("-fno-builtin-strcasestr");
CmdArgs.push_back("-fno-builtin-memmem");
}
// Require -fvisibility= flag on non-Windows when compiling if vptr CFI is // Require -fvisibility= flag on non-Windows when compiling if vptr CFI is
// enabled. // enabled.
if (Sanitizers.hasOneOf(CFIClasses) && !TC.getTriple().isOSWindows() && if (Sanitizers.hasOneOf(CFIClasses) && !TC.getTriple().isOSWindows() &&
!Args.hasArg(options::OPT_fvisibility_EQ)) { !Args.hasArg(options::OPT_fvisibility_EQ)) {
TC.getDriver().Diag(clang::diag::err_drv_argument_only_allowed_with) TC.getDriver().Diag(clang::diag::err_drv_argument_only_allowed_with)
<< lastArgumentForMask(TC.getDriver(), Args, << lastArgumentForMask(TC.getDriver(), Args,
Sanitizers.Mask & CFIClasses) Sanitizers.Mask & CFIClasses)
<< "-fvisibility="; << "-fvisibility=";
▲ Show 20 Lines • Show All 105 Lines • Show Last 20 Lines

compiler-rt/test/fuzzer/noasan-bcmp.test

  • This file was added.
UNSUPPORTED: darwin, freebsd, windows
RUN: %cpp_compiler -fno-sanitize=address -DMEMCMP=bcmp %S/MemcmpTest.cpp -o %t
RUN: not %run %t -seed=1 -runs=10000000 2>&1 | FileCheck %s
CHECK: BINGO

compiler-rt/test/fuzzer/noasan-memcmp.test

UNSUPPORTED: darwin, freebsd, windows UNSUPPORTED: darwin, freebsd, windows
RUN: %cpp_compiler -fno-sanitize=address -fno-builtin-memcmp %S/MemcmpTest.cpp -o %t-NoAsanMemcmpTest RUN: %cpp_compiler -fno-sanitize=address %S/MemcmpTest.cpp -o %t-NoAsanMemcmpTest
RUN: not %run %t-NoAsanMemcmpTest -seed=1 -runs=10000000 2>&1 | FileCheck %s RUN: not %run %t-NoAsanMemcmpTest -seed=1 -runs=10000000 2>&1 | FileCheck %s
RUN: %cpp_compiler -fno-sanitize=address -fno-builtin-calloc -fno-builtin-memcmp %S/CustomAllocator.cpp %S/MemcmpTest.cpp -o %t-NoAsanCustomAllocatorMemcmpTest RUN: %cpp_compiler -fno-sanitize=address -fno-builtin-calloc %S/CustomAllocator.cpp %S/MemcmpTest.cpp -o %t-NoAsanCustomAllocatorMemcmpTest
RUN: not %run %t-NoAsanCustomAllocatorMemcmpTest -seed=1 -runs=10000000 2>&1 | FileCheck %s RUN: not %run %t-NoAsanCustomAllocatorMemcmpTest -seed=1 -runs=10000000 2>&1 | FileCheck %s
CHECK: BINGO CHECK: BINGO

compiler-rt/test/fuzzer/noasan-memcmp64.test

UNSUPPORTED: darwin, freebsd, windows UNSUPPORTED: darwin, freebsd, windows
RUN: %cpp_compiler -fno-sanitize=address -fno-builtin-memcmp %S/Memcmp64BytesTest.cpp -o %t-NoAsanMemcmp64BytesTest RUN: %cpp_compiler -fno-sanitize=address %S/Memcmp64BytesTest.cpp -o %t-NoAsanMemcmp64BytesTest
RUN: not %run %t-NoAsanMemcmp64BytesTest -seed=1 -runs=1000000 2>&1 | FileCheck %s RUN: not %run %t-NoAsanMemcmp64BytesTest -seed=1 -runs=1000000 2>&1 | FileCheck %s
CHECK: BINGO CHECK: BINGO

compiler-rt/test/fuzzer/noasan-strcmp.test

UNSUPPORTED: darwin, freebsd, windows UNSUPPORTED: darwin, freebsd, windows
RUN: %cpp_compiler -fno-sanitize=address -fno-builtin-strcmp %S/StrcmpTest.cpp -o %t-NoAsanStrcmpTest RUN: %cpp_compiler -fno-sanitize=address %S/StrcmpTest.cpp -o %t-NoAsanStrcmpTest
RUN: not %run %t-NoAsanStrcmpTest -seed=1 -runs=2000000 2>&1 | FileCheck %s RUN: not %run %t-NoAsanStrcmpTest -seed=1 -runs=2000000 2>&1 | FileCheck %s
RUN: %cpp_compiler -fno-sanitize=address -fno-builtin-calloc -fno-builtin-strcmp %S/CustomAllocator.cpp %S/StrcmpTest.cpp -o %t-NoAsanCustomAllocatorStrcmpTest RUN: %cpp_compiler -fno-sanitize=address -fno-builtin-calloc %S/CustomAllocator.cpp %S/StrcmpTest.cpp -o %t-NoAsanCustomAllocatorStrcmpTest
RUN: not %run %t-NoAsanCustomAllocatorStrcmpTest -seed=1 -runs=2000000 2>&1 | FileCheck %s RUN: not %run %t-NoAsanCustomAllocatorStrcmpTest -seed=1 -runs=2000000 2>&1 | FileCheck %s
CHECK: BINGO CHECK: BINGO

compiler-rt/test/fuzzer/noasan-strncmp.test

UNSUPPORTED: darwin, freebsd, windows UNSUPPORTED: darwin, freebsd, windows
RUN: %cpp_compiler -fno-sanitize=address -fno-builtin-strncmp %S/StrncmpTest.cpp -o %t-NoAsanStrncmpTest RUN: %cpp_compiler -fno-sanitize=address %S/StrncmpTest.cpp -o %t-NoAsanStrncmpTest
RUN: not %run %t-NoAsanStrncmpTest -seed=2 -runs=10000000 2>&1 | FileCheck %s RUN: not %run %t-NoAsanStrncmpTest -seed=2 -runs=10000000 2>&1 | FileCheck %s
RUN: %cpp_compiler -fno-sanitize=address -fno-builtin-calloc -fno-builtin-strncmp %S/CustomAllocator.cpp %S/StrncmpTest.cpp -o %t-NoAsanCustomAllocatorStrncmpTest RUN: %cpp_compiler -fno-sanitize=address -fno-builtin-calloc %S/CustomAllocator.cpp %S/StrncmpTest.cpp -o %t-NoAsanCustomAllocatorStrncmpTest
RUN: not %run %t-NoAsanCustomAllocatorStrncmpTest -seed=2 -runs=10000000 2>&1 | FileCheck %s RUN: not %run %t-NoAsanCustomAllocatorStrncmpTest -seed=2 -runs=10000000 2>&1 | FileCheck %s
CHECK: BINGO CHECK: BINGO

compiler-rt/test/fuzzer/noasan-strstr.test

UNSUPPORTED: darwin, freebsd, windows UNSUPPORTED: darwin, freebsd, windows
RUN: %cpp_compiler -fno-sanitize=address -fno-builtin-strstr %S/StrstrTest.cpp -o %t-NoAsanStrstrTest RUN: %cpp_compiler -fno-sanitize=address %S/StrstrTest.cpp -o %t-NoAsanStrstrTest
RUN: not %run %t-NoAsanStrstrTest -seed=1 -runs=2000000 2>&1 | FileCheck %s RUN: not %run %t-NoAsanStrstrTest -seed=1 -runs=2000000 2>&1 | FileCheck %s
RUN: %cpp_compiler -fno-sanitize=address -fno-builtin-calloc -fno-builtin-strstr %S/CustomAllocator.cpp %S/StrstrTest.cpp -o %t-NoAsanCustomAllocatorStrstrTest RUN: %cpp_compiler -fno-sanitize=address -fno-builtin-calloc %S/CustomAllocator.cpp %S/StrstrTest.cpp -o %t-NoAsanCustomAllocatorStrstrTest
RUN: not %run %t-NoAsanCustomAllocatorStrstrTest -seed=1 -runs=2000000 2>&1 | FileCheck %s RUN: not %run %t-NoAsanCustomAllocatorStrstrTest -seed=1 -runs=2000000 2>&1 | FileCheck %s
CHECK: BINGO CHECK: BINGO