[boost] Pass sanitizer settings to b2 #6513

mmatrosov · 2021-07-26T15:30:04Z

Specify library name and version: boost/all

Thread sanitizer requires all linked code to be instrumented in order to work properly (https://github.com/google/sanitizers/wiki/ThreadSanitizerCppManual#non-instrumented-code). Thus, it makes sense to support packages built with thread sanitizer.

I assume that compiler.sanitizer setting is defined as in the docs: https://docs.conan.io/en/latest/howtos/sanitizers.html

Specifying CXXFLAGS=-fsanitize=thread env in the profile indeed works for all other libraries that I tried, but not for boost. If I specify this env, boost not only does not build with the sanitizer, but also does not link with zlib. It is not clear to me why this happens, but the proper way to build boost with sanitizers is to pass this option directly to b2: https://www.boost.org/doc/libs/1_76_0/tools/build/doc/html/index.html#_sanitizers

That's exactly what I do in this PR. If compiler.sanitizer setting is not None, I pass it as an appropriate flag to b2.

I've read the guidelines for contributing.
I've followed the PEP8 style guides for Python code in the recipes.
I've used the latest Conan client version.
I've tried at least one configuration locally with the
conan-center hook activated.

ghost · 2021-07-26T15:38:00Z

I detected other pull requests that are modifying boost/all recipe:

[boost] fix library name for custom namespace and custom buildid cases #6510
[boost] Apply patch to fix https://github.com/boostorg/graph/issues/268 #6132
[boost] For Visual Studio, pass some values of settings.compiler.toolset to B2 #5119
boost: use clang-win toolset when compiler=Visual Studio and compiler.toolset=ClangCL #4749

This message is automatically generated by https://github.com/ericLemanissier/conan-center-conflicting-prs so don't hesitate to report issues/improvements there.

madebr · 2021-07-26T20:21:06Z

recipes/boost/all/conanfile.py

@@ -958,6 +958,16 @@ def add_defines(library):
        if self.settings.get_safe("compiler.cppstd"):
            flags.append("cxxflags=%s" % cppstd_flag(self.settings))

+        if self.settings.get_safe("compiler.sanitizer"):


settings.compiler is not an "official" setting.
It's documented, but I think more as an example.
They should have added it to the default settings.yml, if they wanted it to have more support.

Also, it would be weird if only boost would need to listen to this flag. The zlib recipe will ignore compiler.sanitizer.

Can you be more specific about what does not work when you add the following to your profile?

[env] CXXFLAGS=-fsanitize=thread

Boost has the option -o boost:debug_level=X to give more verbose build logs.
Perhaps there you can see what is not working.

maybe it's time to refresh old issue: conan-io/conan#1346
as sanitizers are getting more and more popularity in C++ community, I feel like they should get more love from conan

Won't conan just add -fsanitize=xxx to CFLAGS/CXXFLAGS/LDFLAGS when the sanitizers are enabled?
Then we end up in the same situation.
Of course, then we are able to handle this specific case here in the boost recipe, but I still feel that this is a case of some flags not passing through correctly.

I didn't test this locally yet, so I don't know what the problem is yet.

I am not sure if it's that simple. some sanitizers may require more flags (e.g. see https://clang.llvm.org/docs/ControlFlowIntegrity.html). or might be incompatible with some flags.
but in 99% of cases, I think you're right, it should just imply -fsanitize=xxx.

madebr · 2021-07-26T20:22:25Z

Specify library name and version: boost/all

Thread sanitizer requires all linked code to be instrumented in order to work properly (https://github.com/google/sanitizers/wiki/ThreadSanitizerCppManual#non-instrumented-code). Thus, it makes sense to support packages built with thread sanitizer.

I assume that compiler.sanitizer setting is defined as in the docs: https://docs.conan.io/en/latest/howtos/sanitizers.html

Specifying CXXFLAGS=-fsanitize=thread env in the profile indeed works for all other libraries that I tried, but not for boost. If I specify this env, boost not only does not build with the sanitizer, but also does not link with zlib. It is not clear to me why this happens, but the proper way to build boost with sanitizers is to pass this option directly to b2: https://www.boost.org/doc/libs/1_76_0/tools/build/doc/html/index.html#_sanitizers

If boost does not listen to your CXXFLAGS, then this is a bug.
Can you please open an issue with a reproducer?

mmatrosov · 2021-07-27T17:11:13Z

@SSE4 could you please comment on how do I approach this "An unexpected error happened and has been reported" thing? I how no idea what happened. And where it has been reported :)

SSE4 · 2021-07-27T17:23:43Z

@SSE4 could you please comment on how do I approach this "An unexpected error happened and has been reported" thing? I how no idea what happened. And where it has been reported :)

that I see in logs is just: Unexpected error: Error running command conan config install https://github.com/conan-io/hooks.git -sf hooks -tf hooks
that might have possibly happened:

network errors, like connection issues
resource issues, like outage of disk space, system memory, etc. (we run several jobs in parallel on the same slave, if there are resource leaks of any kind, such issues may happen)

unfortunately these errors happen from time to time. we even have re-tries for these code paths, but they don't always help.

you can't do anything better than just restarting the build. one possible way to do it:

close the PR
wait 10+ seconds
re-open the PR

conan-center-bot · 2021-07-28T19:34:46Z

All green in build 5 (37a7bbc2fbcc2916477976f90bf3fe72bbfd8d00):

boost/1.69.0@:
All packages built successfully! (All logs)
boost/1.72.0@:
All packages built successfully! (All logs)
boost/1.70.0@:
All packages built successfully! (All logs)
boost/1.73.0@:
All packages built successfully! (All logs)
boost/1.75.0@:
All packages built successfully! (All logs)
boost/1.71.0@:
All packages built successfully! (All logs)
boost/1.76.0@:
All packages built successfully! (All logs)
boost/1.74.0@:
All packages built successfully! (All logs)

mathbunnyru

Please, prefer 4 spaces instead of 2, because this file uses 4 spaces everywhere.

madebr · 2021-08-02T10:50:10Z

When using the following profile, named default_sanitize_thread

[settings]
os=Linux
os_build=Linux
arch=x86_64
arch_build=x86_64
compiler=gcc
compiler.version=11
compiler.libcxx=libstdc++11
build_type=Release
[options]
[build_requires]
[env]
CFLAGS=-fsanitize=thread
CXXFLAGS=-fsanitize=thread
LDFLAGS=-fsanitize=thread

with the current boost recipe:

conan create . boost/1.76.0@ -o boost:debug_level=2 -pr default_sanitize_thread

All calls to /usr/bin/g++ have the option -fsanitize=thread added.
See my build log: build.log

Even more, it looks like the thread sanitizer is not happy with the fiber test:

----Running------
> bin/fiber_exe
-----------------
main thread started 140379053040512
thread started 140379011671616
thread started 140379003278912
thread started 140378992801344
fiber a started on thread 140378992801344
fiber b started on thread 140379053040512
fiber c started on thread 140378992801344
fiber d started on thread 140379003278912
fiber e started on thread 140379011671616
fiber f started on thread 140378992801344
fiber i started on thread 140379011671616
fiber h started on thread 140379003278912
fiber g started on thread 140379053040512
fiber k started on thread 140379011671616
fiber j started on thread 140378992801344
fiber l started on thread 140379003278912
fiber m started on thread 140379011671616
fiber o started on thread 140379003278912
fiber n started on thread 140379053040512
fiber q started on thread 140379011671616
fiber p started on thread 140378992801344
fiber r started on thread 140379011671616
fiber t started on thread 140378992801344
fiber s started on thread 140379003278912
fiber v started on thread 140379011671616
fiber u started on thread 140379053040512
fiber w started on thread 140378992801344
fiber x started on thread 140379003278912
fiber y started on thread 140379011671616
fiber z started on thread 140379053040512
ThreadSanitizer:DEADLYSIGNAL
==49423==ERROR: ThreadSanitizer: SEGV on unknown address 0x60000212fff8 (pc 0x7fac8bf91104 bp 0x7fac891be180 sp 0x7fac891be158 T49425)
==49423==The signal is caused by a WRITE memory access.
ThreadSanitizer:DEADLYSIGNAL
ThreadSanitizer:DEADLYSIGNAL
ThreadSanitizer: nested bug in the same thread, aborting.

So I don't really see the need for this pr.

madebr · 2021-08-02T11:38:09Z

This pr has the same result with the fiber test:

----Running------
> bin/fiber_exe
-----------------
main thread started 140038093972352
thread started 140038052505152
thread started 140038044112448
thread started 140038033634880
fiber a started on thread 140038033634880
fiber b started on thread 140038093972352
fiber c started on thread 140038052505152
fiber e started on thread 140038093972352
fiber d started on thread 140038033634880
fiber g started on thread 140038093972352
fiber h started on thread 140038052505152
fiber f started on thread 140038044112448
fiber j started on thread 140038093972352
fiber k started on thread 140038052505152
fiber i started on thread 140038033634880
fiber l started on thread 140038093972352
fiber o started on thread 140038033634880
fiber n started on thread 140038052505152
fiber p started on thread 140038093972352
fiber m started on thread 140038044112448
fiber q started on thread 140038033634880
fiber r started on thread 140038093972352
fiber s started on thread 140038052505152
fiber t started on thread 140038033634880
fiber u started on thread 140038093972352
fiber w started on thread 140038033634880
fiber v started on thread 140038044112448
fiber y started on thread 140038093972352
fiber x started on thread 140038052505152
fiber z started on thread 140038033634880
ThreadSanitizer:DEADLYSIGNAL
==97499==ERROR: ThreadSanitizer: SEGV on unknown address 0x60000325ffe0 (pc 0x7f5d293a7b50 bp 0x7bd00039fcc0 sp 0x7bd00039fc58 T97502)
==97499==The signal is caused by a WRITE memory access.
ThreadSanitizer:DEADLYSIGNAL
ThreadSanitizer:DEADLYSIGNAL
ThreadSanitizer: nested bug in the same thread, aborting.

Also, the test package does not work out of the box because conan does not add -fsanitize=XXX compile/link flags.
I got a lot of undefined refernce to '__tsan_read8' errors.

I added this to the test package as a quick fix:

--- a/recipes/boost/all/test_package/conanfile.py
+++ b/recipes/boost/all/test_package/conanfile.py
@@ -17,6 +17,8 @@
         # FIXME: tools.vcvars added for clang-cl. Remove once conan supports clang-cl properly. (https://github.com/conan-io/conan-center-index/pull/1453)
         with tools.vcvars(self.settings) if (self.settings.os == "Windows" and self.settings.compiler == "clang") else tools.no_op():
             cmake = CMake(self)
+            cmake.definitions["CMAKE_CXX_FLAGS"] = "-fsanitize=thread"
+            cmake.definitions["CMAKE_EXE_LINKER_FLAGS"] = "-fsanitize=thread"
             cmake.definitions["HEADER_ONLY"] = self.options["boost"].header_only
             if not self.options["boost"].header_only:
                 cmake.definitions["Boost_USE_STATIC_LIBS"] = not self.options["boost"].shared

SSE4 · 2021-08-02T12:12:37Z

@madebr it seems like sanitizer seg-faults in your case?

madebr · 2021-08-02T12:15:02Z

Yup, in both cases the thread sanitizer failed in the fiber example.

mmatrosov · 2021-08-02T12:19:23Z

@madebr thanks for the info!

thread sanitizer is not happy with the fiber test

boost has A LOT of things, so I believe there is still point in having sanitized package, even though something does not work. And it does not work in a very loud way, which is a good thing.

madebr · 2021-08-02T12:44:42Z

See mmatrosov#1

mmatrosov · 2021-08-02T14:57:23Z

I tried adding LDFLAGS=-fsanitize=thread and everything worked! With both gcc and clang. It's awesome!

So it seems there is indeed no need in this PR. I am going to close it soon. Let me know if you see any value in it.

madebr · 2021-08-02T15:38:20Z

Does the test package work for you?
In mmatrosov#1, I add some defines.

mmatrosov · 2021-08-02T16:54:07Z

Hm, I am currently having troubles building the test package at all:

boost/1.75.0 (test package): Calling build()
ERROR: boost/1.75.0 (test package): Error in build() method, line 38
	cmake.definitions["WITH_STACKTRACE_ADDR2LINE"] = self.deps_user_info["boost"].stacktrace_addr2line_available
	AttributeError:

UPD: I get it, I have an older recipe revision in the cache.

madebr · 2021-08-02T17:00:06Z

Can you post the complete output of conan test test_package boost/1.75.0@?

mmatrosov · 2021-08-02T17:25:31Z

I was able to build and run the test package without any modifications. It stops with ThreadSanitizer:DEADLYSIGNAL during fiber test as you posted above (I assume you omitted the detailed info from the sanitier starting with WARNING: ThreadSanitizer: data race (pid=1277219) ). The behavior is the same if I apply your changes from mmatrosov#1

madebr · 2021-08-02T17:29:26Z

Yup, you observe the same error as I do.
The defines don't do a thing, although they are documented.
Perhaps the fiber test is wrong?

See conan-io/conan-center-index#6513 (comment)

mmatrosov · 2021-08-02T18:01:11Z

Perhaps the fiber test is wrong?

It's hard to tell, I do not have any experience with boost::fibers

madebr · 2021-08-02T19:35:46Z

So let's close this pr?
If conan ever has conf settings for sanitizers, we can come back to this.

SSE4 · 2021-08-03T04:57:21Z

So let's close this pr?
If conan ever has conf settings for sanitizers, we can come back to this.

if this PR addresses segfault, let's merge it as an improvement

madebr · 2021-08-03T07:22:34Z

@mmatisko
Can you check and merge my PR?
It is a bit correcter, cleaner and had the same behavior.

mmatrosov · 2021-08-03T08:06:17Z

Not sure which segfault are you talking about. If your mean fibers test, then I observe exactly the same errors with and without the PR (including @madebr's changes).

SSE4 · 2021-08-03T08:18:39Z

Not sure which segfault are you talking about. If your mean fibers test, then I observe exactly the same errors with and without the PR (including @madebr's changes).

yes, fibers one:

==97499==ERROR: ThreadSanitizer: SEGV on unknown address 0x60000325ffe0 (pc 0x7f5d293a7b50 bp 0x7bd00039fcc0 sp 0x7bd00039fc58 T97502)

mmatrosov · 2021-08-03T09:19:15Z

Ok, then as I've said, this problem is not affected by the PR, and thus I believe it should be closed.

Thanks a lot for your feedback!

Mikhail Matrosov added 2 commits July 26, 2021 18:20

Pass sanitizer settings to b2

bfa85a9

use settings.get_safe()

2249a9f

ghost mentioned this pull request Jul 26, 2021

[service] Potentially conflicting recipes #3571

Closed

This was referenced Jul 26, 2021

[boost] fix library name for custom namespace and custom buildid cases #6510

Merged

[boost] Apply patch to fix https://github.com/boostorg/graph/issues/268 #6132

Closed

This comment has been minimized.

Sign in to view

conan-center-bot added Failed Unexpected Error labels Jul 26, 2021

Trigger CI

37a7bbc

madebr reviewed Jul 26, 2021

View reviewed changes

SSE4 closed this Jul 27, 2021

SSE4 reopened this Jul 27, 2021

This was referenced Jul 27, 2021

[boost] For Visual Studio, pass some values of settings.compiler.toolset to B2 #5119

Merged

boost: use clang-win toolset when compiler=Visual Studio and compiler.toolset=ClangCL #4749

Merged

This comment has been minimized.

Sign in to view

conan-center-bot added Unexpected Error and removed Unexpected Error labels Jul 27, 2021

mmatrosov closed this Jul 27, 2021

mmatrosov reopened this Jul 27, 2021

conan-center-bot removed Unexpected Error Failed labels Jul 28, 2021

SSE4 approved these changes Jul 29, 2021

View reviewed changes

github-actions bot mentioned this pull request Jul 29, 2021

Pull Requests Ready to be Reviewed prince-chrismc/conan-center-index-pending-review#1

Open

mathbunnyru suggested changes Aug 2, 2021

View reviewed changes

conan-center-bot requested a review from uilianries August 2, 2021 12:01

mmatrosov added a commit to mmatrosov/docs that referenced this pull request Aug 2, 2021

Also add LDFLAGS env var

93d8d3d

See conan-io/conan-center-index#6513 (comment)

mmatrosov mentioned this pull request Aug 2, 2021

Add LDFLAGS env var to the profile for sanitizers conan-io/docs#2181

Open

mmatrosov closed this Aug 3, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[boost] Pass sanitizer settings to b2 #6513

[boost] Pass sanitizer settings to b2 #6513

mmatrosov commented Jul 26, 2021

ghost commented Jul 26, 2021

This comment has been minimized.

madebr Jul 26, 2021 •

edited

Loading

SSE4 Jul 27, 2021

madebr Jul 27, 2021 •

edited

Loading

SSE4 Jul 27, 2021

madebr commented Jul 26, 2021

This comment has been minimized.

mmatrosov commented Jul 27, 2021

SSE4 commented Jul 27, 2021

conan-center-bot commented Jul 28, 2021

mathbunnyru left a comment

madebr commented Aug 2, 2021

madebr commented Aug 2, 2021

SSE4 commented Aug 2, 2021

madebr commented Aug 2, 2021

mmatrosov commented Aug 2, 2021

madebr commented Aug 2, 2021

mmatrosov commented Aug 2, 2021

madebr commented Aug 2, 2021 •

edited

Loading

mmatrosov commented Aug 2, 2021 •

edited

Loading

madebr commented Aug 2, 2021

mmatrosov commented Aug 2, 2021 •

edited

Loading

madebr commented Aug 2, 2021 •

edited

Loading

mmatrosov commented Aug 2, 2021 •

edited

Loading

madebr commented Aug 2, 2021

SSE4 commented Aug 3, 2021

madebr commented Aug 3, 2021

mmatrosov commented Aug 3, 2021

SSE4 commented Aug 3, 2021

mmatrosov commented Aug 3, 2021

[boost] Pass sanitizer settings to b2 #6513

[boost] Pass sanitizer settings to b2 #6513

Conversation

mmatrosov commented Jul 26, 2021

ghost commented Jul 26, 2021

This comment has been minimized.

madebr Jul 26, 2021 • edited Loading

Choose a reason for hiding this comment

SSE4 Jul 27, 2021

Choose a reason for hiding this comment

madebr Jul 27, 2021 • edited Loading

Choose a reason for hiding this comment

SSE4 Jul 27, 2021

Choose a reason for hiding this comment

madebr commented Jul 26, 2021

This comment has been minimized.

mmatrosov commented Jul 27, 2021

SSE4 commented Jul 27, 2021

conan-center-bot commented Jul 28, 2021

mathbunnyru left a comment

Choose a reason for hiding this comment

madebr commented Aug 2, 2021

madebr commented Aug 2, 2021

SSE4 commented Aug 2, 2021

madebr commented Aug 2, 2021

mmatrosov commented Aug 2, 2021

madebr commented Aug 2, 2021

mmatrosov commented Aug 2, 2021

madebr commented Aug 2, 2021 • edited Loading

mmatrosov commented Aug 2, 2021 • edited Loading

madebr commented Aug 2, 2021

mmatrosov commented Aug 2, 2021 • edited Loading

madebr commented Aug 2, 2021 • edited Loading

mmatrosov commented Aug 2, 2021 • edited Loading

madebr commented Aug 2, 2021

SSE4 commented Aug 3, 2021

madebr commented Aug 3, 2021

mmatrosov commented Aug 3, 2021

SSE4 commented Aug 3, 2021

mmatrosov commented Aug 3, 2021

madebr Jul 26, 2021 •

edited

Loading

madebr Jul 27, 2021 •

edited

Loading

madebr commented Aug 2, 2021 •

edited

Loading

mmatrosov commented Aug 2, 2021 •

edited

Loading

mmatrosov commented Aug 2, 2021 •

edited

Loading

madebr commented Aug 2, 2021 •

edited

Loading

mmatrosov commented Aug 2, 2021 •

edited

Loading