Skip to content

test/ci: support for clang asan/tsan in build and travis.#904

Merged
htuch merged 3 commits intoenvoyproxy:masterfrom
htuch:clang-san
May 7, 2017
Merged

test/ci: support for clang asan/tsan in build and travis.#904
htuch merged 3 commits intoenvoyproxy:masterfrom
htuch:clang-san

Conversation

@htuch
Copy link
Copy Markdown
Member

@htuch htuch commented May 5, 2017

  • Add --config={clang-asan,clang-tsan} configuration option to Bazel CLI for when building with
    clang-5.0.

  • Add clang-asan, which also checks some static initialization order stuff.

  • Add ubsan to asan/clang-asan configs, the two play nicely together.

  • Switch bazel.asan from gcc-4.9+asan to clang-5.0+clang-asan.

  • Add bazel.tsan CI target, enabled in .travis.yml.

  • Minor fixes to filesystem_impl_test and uuid_util_test that were necessary to make ASAN/UBSAN
    combo on clang-5.0 work.

MSAN is not included due to the issue discussed at #443.

bazel/README.md Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, thanks. I've been meaning to ask you why we didn't have ubsan.

ci/do_ci.sh Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be --config=clang-tsan ?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yeah, snafu. It does actually pass with clang-tsan, will update.

tools/bazel.rc Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason why we don't use -fsanitize=undefined here too?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Too many unaligned pointer issues. We can fix these later if we care (certainly a performance advantage and more interesting as we consider running on more than just x86), I'll add a TODO.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of curiosity where are the unaligned pointer issues?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Needs master merge, also, might be worth it to just put bazel.debug target into this PR but up to you.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: can you fix this to be dir_exists

@htuch
Copy link
Copy Markdown
Member Author

htuch commented May 7, 2017

I'm going to split out the code changes to deal with UBSAN from this PR and do a full rebase, there are now enough code changes that it makes sense to have them considered in distinct PRs.

* Add --config={clang-asan,clang-tsan} configuration option to Bazel CLI for when building with
  clang-5.0.

* Add clang-asan, which also checks some static initialization order stuff.

* Add ubsan to asan/clang-asan configs, the two play nicely together.

* Switch bazel.asan from gcc-4.9+asan to clang-5.0+clang-asan.

* Add bazel.tsan CI target, enabled in .travis.yml.

MSAN is not included due to the issue discussed at envoyproxy#443.
@htuch htuch merged commit 7dc254c into envoyproxy:master May 7, 2017
@htuch htuch deleted the clang-san branch May 7, 2017 23:02
@htuch htuch mentioned this pull request May 7, 2017
rshriram pushed a commit to rshriram/envoy that referenced this pull request Oct 30, 2018
Automatic merge from submit-queue.

[DO NOT MERGE] Auto PR to update dependencies of proxy

This PR will be merged automatically once checks are successful.
```release-note
none
```
nezdolik pushed a commit to nezdolik/envoy that referenced this pull request May 4, 2024
This is what other mallocs do (glibc malloc and jemalloc). The idea is
malloc is usually initialized very eary. So if we register atfork
handler at that time, we're likely to be first. And that makes our
atfork handler a bit safer, since there is much less chance of some
other library installing their "take all locks" handler first and
having fork take malloc lock before library's lock and deadlocking.

This should address issue envoyproxy#904.
mathetake added a commit that referenced this pull request Mar 3, 2026
**Description**

The build tags were present for all integration tests under ./tests
directory such as e2e, crdcel, extproc, and controller testing where
each of them is using the built artifact rather than the normal unit
tests.

---------

Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com>
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

Successfully merging this pull request may close these issues.

3 participants