-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Clang address sanitizer detects container-overflow on simple web::json::value::parse #989
Comments
Hi there, I just tried this and asan didn't complain. Perhaps there's something different about my setup than yours, here's what I did: Created a new vcpkg triplets file in
From a clean vcpkg, bootstrapped and then ran this:
Created a directory
Added a test in
Ran:
which reported no asan failures. To make sure asan was working I added a
and asan indeed exploded. Have I missed something? |
@BillyONeal unfortunately, you'll need to use the We also don't have any triplet variables to control the compiler; you'll either need to replace the toolchain file (more reliable) or set the [1] https://github.com/Microsoft/vcpkg/blob/master/docs/users/triplets.md#vcpkg_cxx_flags |
@ras0219-msft In this case we're supposing that the bug lies actually in cpprestsdk which would mean that shouldn't actually make a difference. Last time I did this cpprestsdk exploded horribly; I think one of the recent PRs fixed it... |
Thanks for the responses. I'm not able to look at this in detail right at the moment. But in case it's relevant... I'm not using cmake -GNinja -B${build_dir} -H${release_dir} -DBOOST_ROOT=${boost_root} -DWERROR:BOOL=OFF -DCMAKE_INSTALL_RPATH_USE_LINK_PATH:BOOL=ON -DCMAKE_INSTALL_PREFIX=${install_dir} ${cmake_flags}
ninja -j 2 -C ${build_dir}
ninja -j 2 -C ${build_dir} install ...where, for Clang, the Then in the |
I suspect this may be the difference given the "container overflow" message. Will keep you posted. |
Hmm when I try this I get link failures because the boost libraries were built targeting libstdc++ rather than libc++. If I try to get libc++ versions with vcpkg that fails because openssl's build doesn't like clang. Are you saying you built cpprestsdk without any asan support at all targeting libstdc++ and then tried to link it with some project you built against libc++? That would certainly cause things to explode because different sides of the .so boundary wouldn't agree on the layout of vector and friends. |
Apologies for the very slow response. It turned out that some other bits of code in my project were necessary to trigger the problem; it took quite a bit of effort to boil it down to this: #include <vector>
#include <cpprest/json.h>
void f() {
std::vector<::web::json::value> x;
x.push_back( ::web::json::value{} );
}
int main() {
::web::json::value::parse( R"([ { "a" : "a" }, { "b" : "b" }, { "c" : "c" }, { "d" : "d" } ])" );
} I think you're right that libc++ is relevant. I'm using a boost and cpprestsdk built against libc++. Here are commands to reproduce on Ubuntu 18.04... # Install libssl dev files
sudo apt-get install libssl-dev
# Choose a temporary location and make the directories
export ISSUE_898_ROOTDIR=/tmp/cpprestsdk-issue-989
mkdir -p ${ISSUE_898_ROOTDIR}/{cpprestsdk-clang-build,cpprestsdk-clang-install,boost_1_68_0.clang-install,repro,repro-build}
# Build clang/libc++ Boost
wget "https://dl.bintray.com/boostorg/release/1.68.0/source/boost_1_68_0.tar.gz" -O ${ISSUE_898_ROOTDIR}/boost_1_68_0.tar.gz
tar -zxvf ${ISSUE_898_ROOTDIR}/boost_1_68_0.tar.gz --directory ${ISSUE_898_ROOTDIR}
cd ${ISSUE_898_ROOTDIR}/boost_1_68_0/
./bootstrap.sh --prefix=${ISSUE_898_ROOTDIR}/boost_1_68_0.clang-install
./b2 -j8 --with-atomic --with-chrono --with-date_time --with-filesystem --with-random --with-regex --with-system --with-thread toolset=clang cxxflags="-stdlib=libc++ -std=c++14" linkflags="-stdlib=libc++ -std=c++14" --layout=tagged variant=release dll-path=${ISSUE_898_ROOTDIR}/boost_1_68_0.clang-install/lib
./b2 -j8 --with-atomic --with-chrono --with-date_time --with-filesystem --with-random --with-regex --with-system --with-thread toolset=clang cxxflags="-stdlib=libc++ -std=c++14" linkflags="-stdlib=libc++ -std=c++14" --layout=tagged variant=release dll-path=${ISSUE_898_ROOTDIR}/boost_1_68_0.clang-install/lib install
# Build clang/libc++ cpprest
git clone --recurse-submodules https://github.com/microsoft/cpprestsdk.git ${ISSUE_898_ROOTDIR}/cpprestsdk-master
cmake -GNinja -H${ISSUE_898_ROOTDIR}/cpprestsdk-master -B${ISSUE_898_ROOTDIR}/cpprestsdk-clang-build -DBOOST_ROOT=${ISSUE_898_ROOTDIR}/boost_1_68_0.clang-install -DCMAKE_INSTALL_RPATH_USE_LINK_PATH:BOOL=ON -DCMAKE_INSTALL_PREFIX=${ISSUE_898_ROOTDIR}/cpprestsdk-clang-install -DCMAKE_C_COMPILER=/usr/bin/clang -DCMAKE_CXX_COMPILER=/usr/bin/clang++ -DCMAKE_CXX_FLAGS=" -stdlib=libc++ ${CMAKE_CXX_FLAGS} "
ninja -C ${ISSUE_898_ROOTDIR}/cpprestsdk-clang-build
ninja -C ${ISSUE_898_ROOTDIR}/cpprestsdk-clang-build install
# Write cpp file, compile it with address sanitizer and execute the result
echo '#include <vector>\n#include <cpprest/json.h>\nvoid f() {\n std::vector<::web::json::value> x;\n x.push_back( ::web::json::value{} );\n}\nint main() {\n ::web::json::value::parse( R"([ { "a" : "a" }, { "b" : "b" }, { "c" : "c" }, { "d" : "d" } ])" );\n}\n' > ${ISSUE_898_ROOTDIR}/repro-build/cpprest-issue898.cpp
clang++ -std=c++14 -isystem ${ISSUE_898_ROOTDIR}/cpprestsdk-clang-install/include -stdlib=libc++ -fsanitize=address -fsanitize=undefined -fno-omit-frame-pointer -g ${ISSUE_898_ROOTDIR}/repro-build/cpprest-issue898.cpp -g -o ${ISSUE_898_ROOTDIR}/repro-build/cpprest-issue898.clang_bin -Wl,-rpath,${ISSUE_898_ROOTDIR}/cpprestsdk-clang-install/lib ${ISSUE_898_ROOTDIR}/cpprestsdk-clang-install/lib/libcpprest.so.2.10 -lpthread /usr/lib/x86_64-linux-gnu/libssl.so /usr/lib/x86_64-linux-gnu/libcrypto.so
${ISSUE_898_ROOTDIR}/repro-build/cpprest-issue898.clang_bin |
@tonyelewis No need to apologize -- thanks for reducing it to a repro we can test! I can confirm that it repros for me now. Will keep you posted -- I trying to get cmake to generate debug info now :) |
Now if I could only get this thing to give me a stack trace lol...
|
I think this is a false positive -- the sanitizer is reporting an invalid container read, but we're in the container's destructor. All that code comes from the standard library. Your repro does not build cpprestsdk with sanitizers enabled and https://github.com/google/sanitizers/wiki/AddressSanitizerContainerOverflow indicates that such a configuration can create these kinds of false positives because code in the unsanitized libraries doesn't update asan's view of the end of the container appropriately. I'm trying to build at least cpprestsdk with sanitizers on and see if that works... |
I confirm that if I add |
Ah. Thanks very much for your very quick response and for figuring it out. Apologies that it was ultimately all just a false positive and that I didn't read the AddressSanitizerContainerOverflow doc sufficiently. |
No problem! I didn't read it either -- the "WTF" didn't happen until I finally got the symbolizer working and saw all the code was owned by the standard library. |
Running a test that just contains this line:
...through Clang's address sanitizer, I get:
I've confirmed that this problem still occurs with a recent commit (7368961).
I'm using:
The text was updated successfully, but these errors were encountered: