Skip to content
This repository was archived by the owner on Sep 17, 2024. It is now read-only.

Avoid string_as_array #9

Merged
merged 1 commit into from
Jan 7, 2016
Merged

Avoid string_as_array #9

merged 1 commit into from
Jan 7, 2016

Conversation

tamird
Copy link
Contributor

@tamird tamird commented Jan 6, 2016

This function is part of the msan-unclean chain described in
protocolbuffers/protobuf#1099.

We see this on the go1.6 branch in the main repo:

==1953==WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x1a8be89 in std::string::size() const /usr/lib/gcc/x86_64-linux-gnu/4.9/../../../../include/c++/4.9/bits/basic_string.h:725:26
    #1 0x1a8be89 in std::string::empty() const /usr/lib/gcc/x86_64-linux-gnu/4.9/../../../../include/c++/4.9/bits/basic_string.h:822
    #2 0x1a8be89 in google::protobuf::string_as_array(std::string*) /go/src/github.com/cockroachdb/c-protobuf/internal/src/google/protobuf/stubs/stl_util.h:85
    #3 0x1a8be89 in google::protobuf::io::mutable_string_data(std::string*) /go/src/github.com/cockroachdb/c-protobuf/internal/src/google/protobuf/io/zero_copy_stream_impl_lite.h:361

Defining LANG_CXX11 morphs this into:

==12370==WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x1533d61 in std::string::_Rep::_M_is_leaked() const /usr/lib/gcc/x86_64-linux-gnu/4.9/../../../../include/c++/4.9/bits/basic_string.h:192:24
    #1 0x1533d61 in std::string::_M_leak() /usr/lib/gcc/x86_64-linux-gnu/4.9/../../../../include/c++/4.9/bits/basic_string.h:316
    #2 0x1533d61 in std::string::operator[](unsigned long) /usr/lib/gcc/x86_64-linux-gnu/4.9/../../../../include/c++/4.9/bits/basic_string.h:860
    #3 0x1533d61 in google::protobuf::io::mutable_string_data(std::string*) /go/src/github.com/cockroachdb/c-protobuf/internal/src/google/protobuf/io/zero_copy_stream_impl_lite.h:363

But this code path is also hit by other msan-unclean chains, so we
end up with fewer blacklisted functions overall.

The string_as_array callsite affected by this:
https://github.com/google/protobuf/blob/v3.0.0-beta-2/src/google/protobuf/io/zero_copy_stream_impl_lite.h#L359:L367

@tamird
Copy link
Contributor Author

tamird commented Jan 7, 2016

Hm, turns out I have to suppress std::string::size() anyway, so this isn't as compelling as I thought.

@petermattis
Copy link
Contributor

Probably good to indicate we have a C++11 compiler, though.

LGTM

This function is part of the msan-unclean chain described in
protocolbuffers/protobuf#1099.

We see this on the go1.6 branch in the main repo:
```
==1953==WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x1a8be89 in std::string::size() const /usr/lib/gcc/x86_64-linux-gnu/4.9/../../../../include/c++/4.9/bits/basic_string.h:725:26
    #1 0x1a8be89 in std::string::empty() const /usr/lib/gcc/x86_64-linux-gnu/4.9/../../../../include/c++/4.9/bits/basic_string.h:822
    #2 0x1a8be89 in google::protobuf::string_as_array(std::string*) /go/src/github.com/cockroachdb/c-protobuf/internal/src/google/protobuf/stubs/stl_util.h:85
    #3 0x1a8be89 in google::protobuf::io::mutable_string_data(std::string*) /go/src/github.com/cockroachdb/c-protobuf/internal/src/google/protobuf/io/zero_copy_stream_impl_lite.h:361
```

Defining LANG_CXX11 morphs this into:
```
==12370==WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x1533d61 in std::string::_Rep::_M_is_leaked() const /usr/lib/gcc/x86_64-linux-gnu/4.9/../../../../include/c++/4.9/bits/basic_string.h:192:24
    #1 0x1533d61 in std::string::_M_leak() /usr/lib/gcc/x86_64-linux-gnu/4.9/../../../../include/c++/4.9/bits/basic_string.h:316
    #2 0x1533d61 in std::string::operator[](unsigned long) /usr/lib/gcc/x86_64-linux-gnu/4.9/../../../../include/c++/4.9/bits/basic_string.h:860
    #3 0x1533d61 in google::protobuf::io::mutable_string_data(std::string*) /go/src/github.com/cockroachdb/c-protobuf/internal/src/google/protobuf/io/zero_copy_stream_impl_lite.h:363
```

But this code path is also hit by other msan-unclean chains, so we
end up with fewer blacklisted functions overall.

The `string_as_array` callsite affected by this:
https://github.com/google/protobuf/blob/v3.0.0-beta-2/src/google/protobuf/io/zero_copy_stream_impl_lite.h#L359:L367
tamird added a commit that referenced this pull request Jan 7, 2016
@tamird tamird merged commit 2c10a39 into cockroachdb:master Jan 7, 2016
@tamird tamird deleted the workaround-msan branch January 7, 2016 14:53
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants