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

Figure out something for CI so we don't have to build w/ -D_GLIBCXX_USE_CXX11_ABI=0 #81

Closed
arlolra opened this issue Apr 3, 2018 · 13 comments

Comments

@arlolra
Copy link
Collaborator

arlolra commented Apr 3, 2018

See #79

@eighthave
Copy link
Contributor

eighthave commented Nov 22, 2018

FYI, the Android builds are using -D_GLIBCXX_USE_CXX11_ABI=1 #90

@arlolra
Copy link
Collaborator Author

arlolra commented Nov 22, 2018

FYI, the Android builds are using -D_GLIBCXX_USE_CXX11_ABI=1

But that's using gitlab-ci, not travis.

@eighthave
Copy link
Contributor

eighthave commented Nov 22, 2018 via email

@uumaro
Copy link
Collaborator

uumaro commented Nov 26, 2018

The current situation is awkward... @arlolra, do I understand right that the only reason for setting -D_GLIBCXX_USE_CXX11_ABI=0 in .pc files is compatibility with the Travis CI environment?

It seems that setting =0 is working against defaults. The Tor Browser build already uses =1, just by using the defaults in projects/webrtc, which requires externally overriding the .pc file value on the command line.

How does one test changes to the .travis.yml file? It looks like Travis now supports dist: xenial (16.04); I'd like to try that, using prebuilt libraries built with =1.

@eighthave:

The Android builds in general need to use -D_GLIBCXX_USE_CXX11_ABI=1 whether run by gitlab-ci, manually, or otherwise. As far as I can tell, the Android build of Chromium/libwebrtc seems to force it somehow.

Yes indeed: in order to get the linux-amd64 and linux-arm libraries built with with =0, I had to hack the webrtc source :( I didn't find a way to control it externally.

@eighthave
Copy link
Contributor

The Travis-CI enviroment is ancient: Ubuntu 14.04 running Go 1.6. I see no reason to support something that old, especially for a project based in Go, which as a platform seems all about leaving the past behind.

@arlolra
Copy link
Collaborator Author

arlolra commented Nov 26, 2018

The current situation is awkward... @arlolra, do I understand right that the only reason for setting -D_GLIBCXX_USE_CXX11_ABI=0 in .pc files is compatibility with the Travis CI environment?

Yes, looking back at when this was filed, I had said,

I briefly tried setting -D_GLIBCXX_USE_CXX11_ABI=1 in CI but couldn't get it to work. I think the toolchain is built with --disable-libstdcxx-dual-abi so that definition is moot.

#79 (comment)

How does one test changes to the .travis.yml file? It looks like Travis now supports dist: xenial (16.04); I'd like to try that, using prebuilt libraries built with =1.

Seems reasonable.

Since you don't have permissions on this repo, you'll need to fork and enable Travis on a personal repo. It can be set to run on every push to a branch. Alternatively, you can just submit a pull request (and continue force pushing to a branch created for that) and it'll run, since it's enable on pulls here.

@uumaro
Copy link
Collaborator

uumaro commented Nov 29, 2018

I did a test in #92, which changes dist: trusty to dist: xenial and removes -D_GLIBCXX_USE_CXX11_ABI=0 from the .pc files (and rebuilds the magic). It failed, but because of an asan error, which is not what I was expecting.

It seems like the error may be related to the ABI change; it happens under FakeDataChannel::label which only constructs and returns a std::string. Unless there is some weird automatic variable lifetime issue I'm missing. If I patch CGO_Channel_Label to return strdup("fake"), then a similar error occurs in CGO_Channel_Protocol.

I'm dropping the log here until I can look at it more closely.

  DataChannel ✔✔✔✔✔✔✔✔✔✔
    Callbacks should fire correctly 
      OnMessage ✔✔✔✔✔✔✔✔✔✔✔✔
      StateChangeCallbacks ✔✔✔✔✔✔✔=================================================================
==5438==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7fff826448b0 at pc 0x7f0448ada964 bp 0x7fff82644860 sp 0x7fff82644008
WRITE of size 4 at 0x7fff826448b0 thread T0
    #0 0x7f0448ada963 in __asan_memcpy (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x8c963)
    #1 0x57ed3f in std::char_traits<char>::copy(char*, char const*, unsigned long) /usr/include/c++/5/bits/char_traits.h:290
    #2 0x57ed3f in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_S_copy(char*, char const*, unsigned long) /usr/include/c++/5/bits/basic_string.h:299
    #3 0x57ed3f in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_S_copy_chars(char*, char const*, char const*) /usr/include/c++/5/bits/basic_string.h:346
    #4 0x57ed3f in void std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_construct<char const*>(char const*, char const*, std::forward_iterator_tag) /usr/include/c++/5/bits/basic_string.tcc:225
    #5 0x57ee25 in void std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_construct_aux<char const*>(char const*, char const*, std::__false_type) /usr/include/c++/5/bits/basic_string.h:195
    #6 0x57ee25 in void std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_construct<char const*>(char const*, char const*) /usr/include/c++/5/bits/basic_string.h:214
    #7 0x57ee25 in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::basic_string(char const*, std::allocator<char> const&) /usr/include/c++/5/bits/basic_string.h:457
    #8 0x57ee25 in FakeDataChannel::label[abi:cxx11]() const /home/travis/gopath/src/github.com/keroserene/go-webrtc/datachannel.cc:99
    #9 0x57ee25 in CGO_Channel_Label /home/travis/gopath/src/github.com/keroserene/go-webrtc/datachannel.cc:31
    #10 0x57c47f in _cgo_aeba63b75ef5_Cfunc_CGO_Channel_Label (/tmp/go-build842285155/github.com/keroserene/go-webrtc/_test/go-webrtc.test+0x57c47f)
    #11 0x47039f in runtime.asmcgocall (/tmp/go-build842285155/github.com/keroserene/go-webrtc/_test/go-webrtc.test+0x47039f)
Address 0x7fff826448b0 is located in stack of thread T0==5438==AddressSanitizer CHECK failed: ../../../../src/libsanitizer/asan/asan_thread.cc:231 "((ptr[0] == kCurrentStackFrameMagic)) != (0)" (0x0, 0x0)
    #0 0x7f0448aee661  (/usr/lib/x86_64-linux-gnu/libasan.so.2+0xa0661)
    #1 0x7f0448af3613 in __sanitizer::CheckFailed(char const*, int, char const*, unsigned long long, unsigned long long) (/usr/lib/x86_64-linux-gnu/libasan.so.2+0xa5613)
    #2 0x7f0448af1359  (/usr/lib/x86_64-linux-gnu/libasan.so.2+0xa3359)
    #3 0x7f0448aeb769  (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x9d769)
    #4 0x7f0448aebd4a  (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x9dd4a)
    #5 0x7f0448aed626 in __asan_report_error (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x9f626)
    #6 0x7f0448ada981 in __asan_memcpy (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x8c981)
    #7 0x57ed3f in std::char_traits<char>::copy(char*, char const*, unsigned long) /usr/include/c++/5/bits/char_traits.h:290
    #8 0x57ed3f in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_S_copy(char*, char const*, unsigned long) /usr/include/c++/5/bits/basic_string.h:299
    #9 0x57ed3f in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_S_copy_chars(char*, char const*, char const*) /usr/include/c++/5/bits/basic_string.h:346
    #10 0x57ed3f in void std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_construct<char const*>(char const*, char const*, std::forward_iterator_tag) /usr/include/c++/5/bits/basic_string.tcc:225
    #11 0x57ee25 in void std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_construct_aux<char const*>(char const*, char const*, std::__false_type) /usr/include/c++/5/bits/basic_string.h:195
    #12 0x57ee25 in void std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_construct<char const*>(char const*, char const*) /usr/include/c++/5/bits/basic_string.h:214
    #13 0x57ee25 in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::basic_string(char const*, std::allocator<char> const&) /usr/include/c++/5/bits/basic_string.h:457
    #14 0x57ee25 in FakeDataChannel::label[abi:cxx11]() const /home/travis/gopath/src/github.com/keroserene/go-webrtc/datachannel.cc:99
    #15 0x57ee25 in CGO_Channel_Label /home/travis/gopath/src/github.com/keroserene/go-webrtc/datachannel.cc:31
    #16 0x57c47f in _cgo_aeba63b75ef5_Cfunc_CGO_Channel_Label (/tmp/go-build842285155/github.com/keroserene/go-webrtc/_test/go-webrtc.test+0x57c47f)
    #17 0x47039f in runtime.asmcgocall (/tmp/go-build842285155/github.com/keroserene/go-webrtc/_test/go-webrtc.test+0x47039f)
exit status 1
FAIL	github.com/keroserene/go-webrtc	0.034s
The command "if [[ "$OS" == "linux" ]]; then CGO_CFLAGS="-fsanitize=address" CGO_LDFLAGS="-fsanitize=address -fuse-ld=gold" go test -v .; fi" exited with 1.

@uumaro
Copy link
Collaborator

uumaro commented Nov 30, 2018

It seems like the error may be related to the ABI change; it happens under FakeDataChannel::label which only constructs and returns a std::string. Unless there is some weird automatic variable lifetime issue I'm missing. If I patch CGO_Channel_Label to return strdup("fake"), then a similar error occurs in CGO_Channel_Protocol.

arlolra points out that this error is reminiscent of #43 (golang/go#16150). If I read the latter ticket correctly, it says that ASan and cgo are not generally compatible. #43 was about converting a Go string to a C++ string; and the error in this ticket is the other direction. So maybe this is not a real error? Maybe we should get rid of the ASan tests?

uumaro pushed a commit to uumaro/go-webrtc that referenced this issue Nov 30, 2018
This had already been disabled for darwin because of
keroserene#43
golang/go#16150.
Upgrading the Travis dist from trusty to xenial also seems to cause an
error on linux:
keroserene#81 (comment)
@arlolra
Copy link
Collaborator Author

arlolra commented Nov 30, 2018

Maybe we should get rid of the ASan tests?

But then you wouldn't have caught #95

@uumaro
Copy link
Collaborator

uumaro commented Nov 30, 2018

So maybe this is not a real error? Maybe we should get rid of the ASan tests?

Indeed, I removed the -fsanitize=address test in a commit in #92, and the CI build completed successfully. So the ABI situation is sorted out; the error was being caused instead by some interation with AddressSanitizer.

@uumaro
Copy link
Collaborator

uumaro commented Nov 30, 2018

Maybe we should get rid of the ASan tests?

But then you wouldn't have caught #95

Correct, the FakeDataChannel::label in this ticket seems like a false positive; but I'm less sure that #95 is benign.

@arlolra
Copy link
Collaborator Author

arlolra commented Nov 30, 2018

but I'm less sure that #95 is benign.

Yeah, that's why I retracted (deleted) my comment on the other ticket; it looks like something worth investigating.

arlolra pushed a commit that referenced this issue Nov 30, 2018
This had already been disabled for darwin because of
#43
golang/go#16150.
Upgrading the Travis dist from trusty to xenial also seems to cause an
error on linux:
#81 (comment)
@arlolra
Copy link
Collaborator Author

arlolra commented Nov 30, 2018

Resolved by #92

@arlolra arlolra closed this as completed Nov 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants