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

protoc AddressSanitizer: new-delete-type-mismatch #20171

Open
isanych opened this issue Jan 30, 2025 · 13 comments · May be fixed by #20245
Open

protoc AddressSanitizer: new-delete-type-mismatch #20171

isanych opened this issue Jan 30, 2025 · 13 comments · May be fixed by #20245

Comments

@isanych
Copy link

isanych commented Jan 30, 2025

What version of protobuf and what language are you using?
Version: v5.29.3
Language: C++

What operating system (Linux, Windows, ...) and version?
Windows 11 24H2 and Windows Server 2022

What runtime / compiler are you using (e.g., python version or gcc version)
Visual Studio 2022 v17.12.4 and v17.8.8 with address sanitizer enabled

What did you do?
Steps to reproduce the behavior:

git clone -b 2025 https://github.com/isanych/vcpkg.git
cd vcpkg
bootstrap-vcpkg.bat
vcpkg install --triplet=x64-windows-sanitizer --host-triplet=x64-windows-sanitizer grpc

What did you expect to see

protobuf provides CMake targets:

  # this is heuristically generated, and may not be correct
  find_package(protobuf CONFIG REQUIRED)
  target_link_libraries(main PRIVATE protobuf::libprotoc protobuf::libprotobuf protobuf::libprotobuf-lite)

protobuf provides pkg-config modules:

  # Google's Data Interchange Format
  protobuf-lite

  # Google's Data Interchange Format
  protobuf

grpc provides CMake targets:

  # this is heuristically generated, and may not be correct
  find_package(gRPC CONFIG REQUIRED)
  # note: 15 additional targets are not displayed.
  target_link_libraries(main PRIVATE gRPC::gpr gRPC::grpc gRPC::grpc++ gRPC::grpc++_alts)

What did you see instead?

-- Building x64-windows-sanitizer-dbg
CMake Error at scripts/cmake/vcpkg_execute_build_process.cmake:134 (message):
    Command failed: C:/vcpkg/downloads/tools/cmake-3.30.1-windows/cmake-3.30.1-windows-i386/bin/cmake.exe --build . --config Debug --target install -- -v -j41
    Working Directory: C:/vcpkg/buildtrees/grpc/x64-windows-sanitizer-dbg
    See logs for more information:
      C:\vcpkg\buildtrees\grpc\install-x64-windows-sanitizer-dbg-out.log

Call Stack (most recent call first):
  installed/x64-windows-sanitizer/share/vcpkg-cmake/vcpkg_cmake_build.cmake:74 (vcpkg_execute_build_process)
  installed/x64-windows-sanitizer/share/vcpkg-cmake/vcpkg_cmake_install.cmake:16 (vcpkg_cmake_build)
  ports/grpc/portfile.cmake:77 (vcpkg_cmake_install)
  scripts/ports.cmake:196 (include)


error: building grpc:x64-windows-sanitizer failed with: BUILD_FAILED

and in C:\vcpkg\buildtrees\grpc\install-x64-windows-sanitizer-dbg-out.log multiple sanitizer errors:

C:\Windows\system32\cmd.exe /C "cd /D C:\vcpkg\buildtrees\grpc\x64-windows-sanitizer-dbg\protos && C:\vcpkg\installed\x64-windows-sanitizer\tools\protobuf\protoc.exe --grpc_out=generate_mock_code=true:C:/vcpkg/buildtrees/grpc/x64-windows-sanitizer-dbg/gens --cpp_out=C:/vcpkg/buildtrees/grpc/x64-windows-sanitizer-dbg/gens --plugin=protoc-gen-grpc=C:/vcpkg/buildtrees/grpc/x64-windows-sanitizer-dbg/grpc_cpp_plugin.exe -I . -I C:/vcpkg/installed/x64-windows-sanitizer/include src/proto/grpc/channelz/channelz.proto"
=================================================================
==6592==ERROR: AddressSanitizer: new-delete-type-mismatch on 0x123897db0e80 in thread T0:
  object passed to delete has wrong type:
  size of the allocated type:   49 bytes;
  size of the deallocated type: 32 bytes.
    #0 0x7ff802cc5f23 in operator delete(void *, unsigned __int64) D:\a\_work\1\s\src\vctools\asan\llvm\compiler-rt\lib\asan\asan_win_delete_scalar_size_thunk.cpp:41
    #1 0x7ff8029d8445 in google::protobuf::DynamicMessage::`scalar deleting dtor'(unsigned int) (C:\vcpkg\installed\x64-windows-sanitizer\tools\protobuf\libprotobuf.dll+0x180278445)
    #2 0x7ff8029df198 in google::protobuf::internal::ExtensionSet::{dtor}::__l5::<lambda_f61099c3c65faad063678c06d654e9bf>::operator() C:\vcpkg\buildtrees\protobuf\src\v5.29.3-6c24724110.clean\src\google\protobuf\extension_set.cc:189
    #3 0x7ff8029df198 in google::protobuf::internal::ExtensionSet::ForEachPrefetchImpl<google::protobuf::internal::ExtensionSet::KeyValue *,<lambda_f61099c3c65faad063678c06d654e9bf>,google::protobuf::internal::ExtensionSet::PrefetchNta> C:\vcpkg\buildtrees\protobuf\src\v5.29.3-6c24724110.clean\src\google\protobuf\extension_set.h:830
    #4 0x7ff8029de779 in google::protobuf::internal::ExtensionSet::ForEach<<lambda_f61099c3c65faad063678c06d654e9bf>,google::protobuf::internal::ExtensionSet::PrefetchNta> C:\vcpkg\buildtrees\protobuf\src\v5.29.3-6c24724110.clean\src\google\protobuf\extension_set.h:845
    #5 0x7ff8029ece46 in google::protobuf::internal::ExtensionSet::~ExtensionSet(void) C:\vcpkg\buildtrees\protobuf\src\v5.29.3-6c24724110.clean\src\google\protobuf\extension_set.cc:189
    #6 0x7ff8029d6e2c in google::protobuf::DynamicMessage::~DynamicMessage(void) C:\vcpkg\buildtrees\protobuf\src\v5.29.3-6c24724110.clean\src\google\protobuf\dynamic_message.cc:502
    #7 0x7ff8029d8433 in google::protobuf::DynamicMessage::`scalar deleting dtor'(unsigned int) (C:\vcpkg\installed\x64-windows-sanitizer\tools\protobuf\libprotobuf.dll+0x180278433)
    #8 0x7ff802a1b9af in std::default_delete<google::protobuf::Message>::operator() C:\opt\BuildTools\VC\Tools\MSVC\14.38.33130\include\memory:3170
    #9 0x7ff802a1b9af in std::unique_ptr<google::protobuf::Message,std::default_delete<google::protobuf::Message> >::{dtor} C:\opt\BuildTools\VC\Tools\MSVC\14.38.33130\include\memory:3280
    #10 0x7ff802a1b9af in google::protobuf::FeatureResolver::CompileDefaults(class google::protobuf::Descriptor const *, class absl::lts_20240722::Span<class google::protobuf::FieldDescriptor const *const>, enum google::protobuf::Edition, enum google::protobuf::Edition) C:\vcpkg\buildtrees\protobuf\src\v5.29.3-6c24724110.clean\src\google\protobuf\feature_resolver.cc:485
    #11 0x7ff8037eb46e in google::protobuf::compiler::CommandLineInterface::SetupFeatureResolution(class google::protobuf::DescriptorPool &) C:\vcpkg\buildtrees\protobuf\src\v5.29.3-6c24724110.clean\src\google\protobuf\compiler\command_line_interface.cc:1569
    #12 0x7ff8037e909e in google::protobuf::compiler::CommandLineInterface::Run(int, char const *const *const) C:\vcpkg\buildtrees\protobuf\src\v5.29.3-6c24724110.clean\src\google\protobuf\compiler\command_line_interface.cc:1259
    #13 0x7ff6153041c6 in google::protobuf::compiler::ProtobufMain(int, char **const) C:\vcpkg\buildtrees\protobuf\src\v5.29.3-6c24724110.clean\src\google\protobuf\compiler\main.cc:112
    #14 0x7ff615304a47 in main C:\vcpkg\buildtrees\protobuf\src\v5.29.3-6c24724110.clean\src\google\protobuf\compiler\main.cc:137
    #15 0x7ff615305b7b in invoke_main D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:78
    #16 0x7ff615305b7b in __scrt_common_main_seh D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:288
    #17 0x7ff82ebe4caf  (C:\Windows\System32\KERNEL32.DLL+0x180014caf)
    #18 0x7ff84fe3edca  (C:\Windows\SYSTEM32\ntdll.dll+0x18007edca)

0x123897db0e80 is located 0 bytes inside of 49-byte region [0x123897db0e80,0x123897db0eb1)
allocated by thread T0 here:
    #0 0x7ff802cc5e95 in operator new(unsigned __int64) D:\a\_work\1\s\src\vctools\asan\llvm\compiler-rt\lib\asan\asan_win_new_scalar_thunk.cpp:40
    #1 0x7ff802c05290 in google::protobuf::internal::MessageCreator::New C:\vcpkg\buildtrees\protobuf\src\v5.29.3-6c24724110.clean\src\google\protobuf\message_lite.h:1300
    #2 0x7ff802c05290 in google::protobuf::MessageLite::New(class google::protobuf::Arena *) const C:\vcpkg\buildtrees\protobuf\src\v5.29.3-6c24724110.clean\src\google\protobuf\message_lite.cc:83
    #3 0x7ff802a10d2e in google::protobuf::internal::ExtensionSet::MutableMessage(class google::protobuf::FieldDescriptor const *, class google::protobuf::MessageFactory *) C:\vcpkg\buildtrees\protobuf\src\v5.29.3-6c24724110.clean\src\google\protobuf\extension_set_heavy.cc:144
    #4 0x7ff802a46786 in google::protobuf::Reflection::MutableMessage(class google::protobuf::Message *, class google::protobuf::FieldDescriptor const *, class google::protobuf::MessageFactory *) const C:\vcpkg\buildtrees\protobuf\src\v5.29.3-6c24724110.clean\src\google\protobuf\generated_message_reflection.cc:2368
    #5 0x7ff802a1b6a0 in google::protobuf::FeatureResolver::CompileDefaults(class google::protobuf::Descriptor const *, class absl::lts_20240722::Span<class google::protobuf::FieldDescriptor const *const>, enum google::protobuf::Edition, enum google::protobuf::Edition) C:\vcpkg\buildtrees\protobuf\src\v5.29.3-6c24724110.clean\src\google\protobuf\feature_resolver.cc:472
    #6 0x7ff8037eb46e in google::protobuf::compiler::CommandLineInterface::SetupFeatureResolution(class google::protobuf::DescriptorPool &) C:\vcpkg\buildtrees\protobuf\src\v5.29.3-6c24724110.clean\src\google\protobuf\compiler\command_line_interface.cc:1569
    #7 0x7ff8037e909e in google::protobuf::compiler::CommandLineInterface::Run(int, char const *const *const) C:\vcpkg\buildtrees\protobuf\src\v5.29.3-6c24724110.clean\src\google\protobuf\compiler\command_line_interface.cc:1259
    #8 0x7ff6153041c6 in google::protobuf::compiler::ProtobufMain(int, char **const) C:\vcpkg\buildtrees\protobuf\src\v5.29.3-6c24724110.clean\src\google\protobuf\compiler\main.cc:112
    #9 0x7ff615304a47 in main C:\vcpkg\buildtrees\protobuf\src\v5.29.3-6c24724110.clean\src\google\protobuf\compiler\main.cc:137
    #10 0x7ff615305b7b in invoke_main D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:78
    #11 0x7ff615305b7b in __scrt_common_main_seh D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:288
    #12 0x7ff82ebe4caf  (C:\Windows\System32\KERNEL32.DLL+0x180014caf)
    #13 0x7ff84fe3edca  (C:\Windows\SYSTEM32\ntdll.dll+0x18007edca)

SUMMARY: AddressSanitizer: new-delete-type-mismatch D:\a\_work\1\s\src\vctools\asan\llvm\compiler-rt\lib\asan\asan_win_delete_scalar_size_thunk.cpp:41 in operator delete(void *, unsigned __int64)
==6592==HINT: if you don't care about these errors you may set ASAN_OPTIONS=new_delete_type_mismatch=0
==6592==ABORTING

Make sure you include information that can help us debug (full error message, exception listing, stack trace, logs).

Anything else we should know about your project / environment
I can workaround asan errors with

set ASAN_OPTIONS=new_delete_type_mismatch=0
set VCPKG_KEEP_ENV_VARS=ASAN_OPTIONS

but such protoc would require similar workarounds when used, so better workaround is to replace protoc binary with statically linked protoc without sanitizer.

@isanych isanych added the untriaged auto added to all issues by default when created. label Jan 30, 2025
@acozzette
Copy link
Member

Is it possible that part of your binary is built with ASAN while part of it is not? I suspect that this is probably happening and causing the errors. From what I understand, ASAN changes ABI, and so you can't safely build for ASAN unless you do it for the entire binary.

@isanych
Copy link
Author

isanych commented Jan 30, 2025

I really doubt it - vcpkg building protobuf with cmake, and in cmake cache (config-x64-windows-sanitizer-dbg-CMakeCache.txt.log) I see global flags

CMAKE_CXX_FLAGS:STRING= /nologo /DWIN32 /D_WINDOWS /utf-8 /GR /EHsc /MP -vmg -fsanitize=address
CMAKE_C_FLAGS:STRING= /nologo /DWIN32 /D_WINDOWS /utf-8 /MP -vmg -fsanitize=address

and protoc works properly when I disable this particular check in asan with

ASAN_OPTIONS=new_delete_type_mismatch=0

@isanych
Copy link
Author

isanych commented Jan 30, 2025

Maybe because it deallocates less bytes than allocated it is not leading to anything bad in non asan builds, but it still does not look ok.

@acozzette
Copy link
Member

Actually from looking more closely at the stack trace, I think there is a different problem which is that the binary is mixing code from C:\vcpkg\installed\x64-windows-sanitizer\tools\protobuf\libprotobuf.dll and C:\vcpkg\buildtrees\protobuf\src\v5.29.3-6c24724110.clean/..., but there should only be one libprotobuf.

@isanych
Copy link
Author

isanych commented Jan 30, 2025

that's how vcpkg works. it builds ports in buildtree, then install them in package, then copy package to installed - and tools are used from installed after that - buildtree can be deleted after build - errors happens not in protobuf build, but in grpc build when it trying to use installed protoc for generation

C:\vcpkg\installed\x64-windows-sanitizer\tools\protobuf\protoc.exe --grpc_out=generate_mock_code=true:C:/vcpkg/buildtrees/grpc/x64-windows-sanitizer-dbg/gens --cpp_out=C:/vcpkg/buildtrees/grpc/x64-windows-sanitizer-dbg/gens --plugin=protoc-gen-grpc=C:/vcpkg/buildtrees/grpc/x64-windows-sanitizer-dbg/grpc_cpp_plugin.exe -I . -I C:/vcpkg/installed/x64-windows-sanitizer/include src/proto/grpc/channelz/channelz.proto"

@acozzette
Copy link
Member

I'm not very familiar with vcpkg, but from the stack trace it looks like it's getting some parts of libprotobuf from an installed DLL and other parts from a buildtree. I'm pretty sure that means something is going wrong, because that implies ODR violations from having two separate copies of libprotobuf.

It could be that I'm misinterpreting the stack trace--maybe everything is actually in C:\vcpkg\installed\x64-windows-sanitizer\tools\protobuf\libprotobuf.dll but the stack track trace mentions the original source file and line number. Do you have a way to confirm that protoc is only linked against a single copy of libprotobuf?

However, I remain suspicious that the problem is with the build setup rather than the code per se. Protoc is pretty well battle-tested, so it seems unlikely (though not impossible) that it would have serious memory management errors like the ones in the ASAN errors above. With those kind of errors it would also be surprising that protoc worked correctly with ASAN disabled.

@isanych
Copy link
Author

isanych commented Jan 30, 2025

protoc and all it dependencies are in C:\vcpkg\installed\x64-windows-sanitizer\tools\protobuf, you see C:\vcpkg\buildtrees\protobuf\src\v5.29.3-6c24724110.clean in stack trace because that where source were located during build, the same way as you see D:\a_work\1\s\src\vctools\asan\llvm\compiler-rt\lib\asan\asan_win_new_scalar_thunk.cpp - that's where Visual Studio sources were located when Microsoft was building Visual Studio, there is no mix up of build options or libraries in this case

@isanych
Copy link
Author

isanych commented Jan 30, 2025

I used docker image to test commands reproducing issue and that container is gone, so you can see small differences in logs in comments vs initial description.
Here extract of ninja file config-x64-windows-sanitizer-dbg-ninja.log used to build protoc.exe on my local machine

# =============================================================================
# Link build statements for EXECUTABLE target protoc


#############################################
# Link the executable bin\protoc.exe

build bin\protoc.exe: CXX_EXECUTABLE_LINKER__protoc_Debug CMakeFiles\protoc.dir\src\google\protobuf\compiler\main.cc.obj CMakeFiles\protoc.dir\version.rc.res | bin\libprotocd.lib bin\libprotobufd.lib C$:\opt\vcpkg\installed\x64-windows-sanitizer\debug\lib\abseil_dll.lib || bin\libprotobufd.dll bin\libprotocd.dll
  FLAGS = /nologo /DWIN32 /D_WINDOWS /utf-8 /GR /EHsc /MP -vmg -fsanitize=address /D_DEBUG /MDd /Z7 /Ob0 /Od /RTC1 -Oy- -MDd
  LINK_FLAGS = /machine:x64 /nologo    /debug /INCREMENTAL /subsystem:console
  LINK_LIBRARIES = bin\libprotocd.lib  bin\libprotobufd.lib  C:\opt\vcpkg\installed\x64-windows-sanitizer\debug\lib\abseil_dll.lib  kernel32.lib user32.lib gdi32.lib winspool.lib shell32.lib ole32.lib oleaut32.lib uuid.lib comdlg32.lib advapi32.lib
  OBJECT_DIR = CMakeFiles\protoc.dir
  POST_BUILD = cd .
  PRE_LINK = cd .
  TARGET_COMPILE_PDB = CMakeFiles\protoc.dir\
  TARGET_FILE = bin\protoc.exe
  TARGET_IMPLIB = bin\protoc.lib
  TARGET_PDB = bin\protoc.pdb

here link command from build log install-x64-windows-sanitizer-dbg-out.log

[242/243] C:\WINDOWS\system32\cmd.exe /C "cd . && C:\opt\apps\cmake\3.31.4\bin\cmake.exe -E vs_link_exe --msvc-ver=1942 --intdir=CMakeFiles\protoc.dir --rc=C:\PROGRA~2\WI3CF2~1\10\bin\100226~1.0\x64\rc.exe --mt=C:\PROGRA~2\WI3CF2~1\10\bin\100226~1.0\x64\mt.exe --manifests  -- C:\opt\2022.12\VC\Tools\MSVC\14.42.34433\bin\Hostx64\x64\link.exe  CMakeFiles\protoc.dir\src\google\protobuf\compiler\main.cc.obj CMakeFiles\protoc.dir\version.rc.res  /out:bin\protoc.exe /implib:bin\protoc.lib /pdb:bin\protoc.pdb /version:29.3 /machine:x64 /nologo    /debug /INCREMENTAL /subsystem:console  bin\libprotocd.lib  bin\libprotobufd.lib  C:\opt\vcpkg\installed\x64-windows-sanitizer\debug\lib\abseil_dll.lib  kernel32.lib user32.lib gdi32.lib winspool.lib shell32.lib ole32.lib oleaut32.lib uuid.lib comdlg32.lib advapi32.lib && cd ."

If you want I can repeat docker build and attach all logs or even whole vcpkg directory, but it will be huge

@isanych
Copy link
Author

isanych commented Feb 1, 2025

How to reproduce in windows docker (Visual Studio 2022 v17.12.4 atm)

docker pull amitie10g/visualstudio2022-workload-vctools:ltsc2022
docker run --rm -ti amitie10g/visualstudio2022-workload-vctools:ltsc2022
# in docker:
cd C:\
choco install -y git
Import-Module $env:ChocolateyInstall\helpers\chocolateyProfile.psm1
RefreshEnv
git clone -b 2025 https://github.com/isanych/vcpkg.git
cd vcpkg
.\bootstrap-vcpkg.bat
.\vcpkg install --triplet=x64-windows-sanitizer --host-triplet=x64-windows-sanitizer grpc

and check C:\vcpkg\buildtrees\grpc\install-x64-windows-sanitizer-dbg-out.log

@isanych
Copy link
Author

isanych commented Feb 1, 2025

I've replaced protoc binary with non sanitized version, and errors still there, so there is a good chance that errors are coming from grpc plugin and not from protoc itself, I'll try to confirm that tomorrow

@isanych
Copy link
Author

isanych commented Feb 2, 2025

I see what is happening, in

PROTOBUF_ALWAYS_INLINE inline MessageLite* MessageCreator::New(
    const MessageLite* prototype_for_func,
    const MessageLite* prototype_for_copy, Arena* arena) const {
  return PlacementNew<test_call>(prototype_for_func, prototype_for_copy,
                                 arena != nullptr
                                     ? arena->AllocateAligned(allocation_size_)
                                     : ::operator new(allocation_size_),
                                 arena);

MessageLite allocated as ::operator new(allocation_size_), but then deleted by typed delete in

void ExtensionSet::Extension::Free() {
...
          delete ptr.message_value;

(message_value is MessageLite*). MessageLite has a destructor, so we want to call delete like this.
With this patch

diff --git a/src/google/protobuf/dynamic_message.cc b/src/google/protobuf/dynamic_message.cc
index b7a7482..a7f7194 100644
--- a/src/google/protobuf/dynamic_message.cc
+++ b/src/google/protobuf/dynamic_message.cc
@@ -214,9 +214,7 @@ class DynamicMessage final : public Message {
   // manually call the global operator delete. Calling the destructor is taken
   // care of for us. This makes DynamicMessage compatible with -fsized-delete.
   // It doesn't work for MSVC though.
-#ifndef _MSC_VER
   static void operator delete(void* ptr) { ::operator delete(ptr); }
-#endif  // !_MSC_VER
 #endif
 
  private:
diff --git a/src/google/protobuf/extension_set.cc b/src/google/protobuf/extension_set.cc
index bad7d59..7b7965b 100644
--- a/src/google/protobuf/extension_set.cc
+++ b/src/google/protobuf/extension_set.cc
@@ -1576,7 +1576,8 @@ void ExtensionSet::Extension::Free() {
         if (is_lazy) {
           delete ptr.lazymessage_value;
         } else {
-          delete ptr.message_value;
+          ptr.message_value->~MessageLite();
+          ::operator delete(ptr.message_value);
         }
         break;
       default:

I'm able to build protobuf and grpc with sanitizer, comment says that operator delete does not work for VS, but I'm targeting only VS 2022 and it is working for me there.
Adding static void operator delete into MessageLite should be cleaner solution, but I'm getting sanitizer's failures with it, so I'm using above patch for now - with it I'm able to use sanitizer with VS 2022 and gcc 14

@JasonLunn
Copy link
Contributor

@isanych - can you turn your patch into a PR so that we can run it in CI?

@JasonLunn JasonLunn added c++ windows wait for user action and removed untriaged auto added to all issues by default when created. labels Feb 5, 2025
@isanych isanych linked a pull request Feb 5, 2025 that will close this issue
@isanych
Copy link
Author

isanych commented Feb 5, 2025

@JasonLunn, created #20245

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants