Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve sanitizer support #6513

Closed
LebedevRI opened this issue Dec 23, 2021 · 3 comments
Closed

Improve sanitizer support #6513

LebedevRI opened this issue Dec 23, 2021 · 3 comments

Comments

@LebedevRI
Copy link
Contributor

Hi! I'd like to make use of halide in RawSpeed library (darktable-org/rawspeed#325).

Now that the halide is packaged in debian, it's mostly a no-brainer,
but unfortunately for me RawSpeed is an avid user of oss-fuzz,
and unfortunately i think not loosing that coverage is more important than making use of Halide.

As far as i can tell, there's some support for ASAN and TSAN instrumentation,
which means there are some missing things still: (list)

  • MSAN (should be easy?)
  • Sanitizer coverage
  • UBSan (it's frontend-based, i'm not sure how this would look, only for the actual user-written C/C++ code)
  • UX improvement to just make use of the standard clang -fsanitize= flags?

Thoughts?

Blocks darktable-org/rawspeed#325.

@steven-johnson
Copy link
Contributor

  • MSAN: we have support for MSAN as well, but AOT-only (MSAN is documented as incompatibile with JIT implementations in LLVM)
  • What is "Sanitizer Coverage"?
  • UBSan is basically all clang at this point, and thus irrelevant for Halide-generated code. We could add such a mode to Halide but first we'd need to explicitly enumerate everything that is known to be UB in the Halide language; I'm not certain if that's well-defined at this point. (FWIW, UBSan sounds great but in practice it's almost impossible to use it with any C++ codebase unless it's built that way from the beginning; even inside Google it's basically admitted to be of limited use because you'll hit so much UB in well-behaved C++ programs, which really tells you something about the state of C++, alas)
  • UX improvements I'm all in favor of.

But yeah, PRs for any and all of this very welcome, sanitizers are great tools and we'd love more support.

See also #6436

LebedevRI added a commit to LebedevRI/Halide that referenced this issue Dec 25, 2021
Please refer to https://clang.llvm.org/docs/SanitizerCoverage.html

TLDR: `ModuleSanitizerCoveragePass` instruments the IR by inserting
calls to callbacks at certain constructs. What the callbacks should do
is up to the implementation. They are effectively required for fuzzing
to be effective, and are provided by e.g. libfuzzer.

One huge caveat is `SanitizerCoverageOptions` which controls
which which callbacks should actually be inserted.
I just don't know what to do about it. Right now i have hardcoded
the set that would have been enabled by `-fsanitize=fuzzer-no-link`,
because the alternative, due to halide unflexibility,
would be to introduce ~16 suboptions to control each one.
LebedevRI added a commit to LebedevRI/Halide that referenced this issue Dec 25, 2021
Please refer to https://clang.llvm.org/docs/SanitizerCoverage.html

TLDR: `ModuleSanitizerCoveragePass` instruments the IR by inserting
calls to callbacks at certain constructs. What the callbacks should do
is up to the implementation. They are effectively required for fuzzing
to be effective, and are provided by e.g. libfuzzer.

One huge caveat is `SanitizerCoverageOptions` which controls
which which callbacks should actually be inserted.
I just don't know what to do about it. Right now i have hardcoded
the set that would have been enabled by `-fsanitize=fuzzer-no-link`,
because the alternative, due to halide unflexibility,
would be to introduce ~16 suboptions to control each one.
LebedevRI added a commit to LebedevRI/Halide that referenced this issue Dec 30, 2021
Please refer to https://clang.llvm.org/docs/SanitizerCoverage.html

TLDR: `ModuleSanitizerCoveragePass` instruments the IR by inserting
calls to callbacks at certain constructs. What the callbacks should do
is up to the implementation. They are effectively required for fuzzing
to be effective, and are provided by e.g. libfuzzer.

One huge caveat is `SanitizerCoverageOptions` which controls
which which callbacks should actually be inserted.
I just don't know what to do about it. Right now i have hardcoded
the set that would have been enabled by `-fsanitize=fuzzer-no-link`,
because the alternative, due to halide unflexibility,
would be to introduce ~16 suboptions to control each one.
LebedevRI added a commit to LebedevRI/Halide that referenced this issue Dec 30, 2021
Please refer to https://clang.llvm.org/docs/SanitizerCoverage.html

TLDR: `ModuleSanitizerCoveragePass` instruments the IR by inserting
calls to callbacks at certain constructs. What the callbacks should do
is up to the implementation. They are effectively required for fuzzing
to be effective, and are provided by e.g. libfuzzer.

One huge caveat is `SanitizerCoverageOptions` which controls
which which callbacks should actually be inserted.
I just don't know what to do about it. Right now i have hardcoded
the set that would have been enabled by `-fsanitize=fuzzer-no-link`,
because the alternative, due to halide unflexibility,
would be to introduce ~16 suboptions to control each one.
LebedevRI added a commit to LebedevRI/Halide that referenced this issue Dec 30, 2021
Please refer to https://clang.llvm.org/docs/SanitizerCoverage.html

TLDR: `ModuleSanitizerCoveragePass` instruments the IR by inserting
calls to callbacks at certain constructs. What the callbacks should do
is up to the implementation. They are effectively required for fuzzing
to be effective, and are provided by e.g. libfuzzer.

One huge caveat is `SanitizerCoverageOptions` which controls
which which callbacks should actually be inserted.
I just don't know what to do about it. Right now i have hardcoded
the set that would have been enabled by `-fsanitize=fuzzer-no-link`,
because the alternative, due to halide unflexibility,
would be to introduce ~16 suboptions to control each one.
LebedevRI added a commit to LebedevRI/Halide that referenced this issue Dec 30, 2021
Please refer to https://clang.llvm.org/docs/SanitizerCoverage.html

TLDR: `ModuleSanitizerCoveragePass` instruments the IR by inserting
calls to callbacks at certain constructs. What the callbacks should do
is up to the implementation. They are effectively required for fuzzing
to be effective, and are provided by e.g. libfuzzer.

One huge caveat is `SanitizerCoverageOptions` which controls
which which callbacks should actually be inserted.
I just don't know what to do about it. Right now i have hardcoded
the set that would have been enabled by `-fsanitize=fuzzer-no-link`,
because the alternative, due to halide unflexibility,
would be to introduce ~16 suboptions to control each one.
LebedevRI added a commit to LebedevRI/Halide that referenced this issue Dec 30, 2021
Please refer to https://clang.llvm.org/docs/SanitizerCoverage.html

TLDR: `ModuleSanitizerCoveragePass` instruments the IR by inserting
calls to callbacks at certain constructs. What the callbacks should do
is up to the implementation. They are effectively required for fuzzing
to be effective, and are provided by e.g. libfuzzer.

One huge caveat is `SanitizerCoverageOptions` which controls
which which callbacks should actually be inserted.
I just don't know what to do about it. Right now i have hardcoded
the set that would have been enabled by `-fsanitize=fuzzer-no-link`,
because the alternative, due to halide unflexibility,
would be to introduce ~16 suboptions to control each one.
LebedevRI added a commit to LebedevRI/Halide that referenced this issue Dec 30, 2021
Please refer to https://clang.llvm.org/docs/SanitizerCoverage.html

TLDR: `ModuleSanitizerCoveragePass` instruments the IR by inserting
calls to callbacks at certain constructs. What the callbacks should do
is up to the implementation. They are effectively required for fuzzing
to be effective, and are provided by e.g. libfuzzer.

One huge caveat is `SanitizerCoverageOptions` which controls
which which callbacks should actually be inserted.
I just don't know what to do about it. Right now i have hardcoded
the set that would have been enabled by `-fsanitize=fuzzer-no-link`,
because the alternative, due to halide unflexibility,
would be to introduce ~16 suboptions to control each one.
LebedevRI added a commit to LebedevRI/Halide that referenced this issue Dec 30, 2021
Please refer to https://clang.llvm.org/docs/SanitizerCoverage.html

TLDR: `ModuleSanitizerCoveragePass` instruments the IR by inserting
calls to callbacks at certain constructs. What the callbacks should do
is up to the implementation. They are effectively required for fuzzing
to be effective, and are provided by e.g. libfuzzer.

One huge caveat is `SanitizerCoverageOptions` which controls
which which callbacks should actually be inserted.
I just don't know what to do about it. Right now i have hardcoded
the set that would have been enabled by `-fsanitize=fuzzer-no-link`,
because the alternative, due to halide unflexibility,
would be to introduce ~16 suboptions to control each one.
steven-johnson added a commit that referenced this issue Jan 4, 2022
* Implement SanitizerCoverage support (Refs. #6513)

Please refer to https://clang.llvm.org/docs/SanitizerCoverage.html

TLDR: `ModuleSanitizerCoveragePass` instruments the IR by inserting
calls to callbacks at certain constructs. What the callbacks should do
is up to the implementation. They are effectively required for fuzzing
to be effective, and are provided by e.g. libfuzzer.

One huge caveat is `SanitizerCoverageOptions` which controls
which which callbacks should actually be inserted.
I just don't know what to do about it. Right now i have hardcoded
the set that would have been enabled by `-fsanitize=fuzzer-no-link`,
because the alternative, due to halide unflexibility,
would be to introduce ~16 suboptions to control each one.

* Simplify test

* sancov test: avoid potential signedness warnings.

* Rename all instances of sancov to sanitizecoverage

* Adjust spelling of "SanitizerCoverage" in some places

* Actually adjust the feature name in build system for the test

* Hopefully fix Makefile build

Co-authored-by: Steven Johnson <[email protected]>
@LebedevRI
Copy link
Contributor Author

Ok, sanitizer-wise i think things are actually good now. Thanks!
Minimally, i still miss -march= handling of #6515/#6514, will look into that next.

More generally, i think there are some more things potentially missing,
with not all being feasible, but just to list:

  • XRay (trivial, will likely look into that)
  • (LLVM-specific) LTO (uh)
  • (LLVM-specific) PGO (uh)
  • Source-based coverage (probably quite complicated, aside from most basic form)
  • Big-endian (???)

@zvookin
Copy link
Member

zvookin commented Jan 4, 2022

Per undefined behavior, the general approach in Halide is to not have it. At least not in the default compilation settings. There are a few cases such as aliasing of input and output buffers. We do have the option to compile with no asserts and to allow race conditions, which can result in UB like phenomena, but generally the goal is to define the results, or at least bound them to a reasonable range. (This is somewhat required due to bounds inference for example.)

@LebedevRI LebedevRI closed this as not planned Won't fix, can't repro, duplicate, stale Aug 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants