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

fix multi construct of g_gmock_implicit_sequence #4536

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

CastleOnTheHill
Copy link

When libgmock.a is linked to a multitude of shared libraries, g_gmock_implicit_sequence will be constructed multiple times. For instance:

libgmock.a is linked to libA.so
libgmock.a is linked to libB.so
libA.so and libB.so are linked to BinaryC

When BinaryC executes, g_gmock_implicit_sequence is constructed and destroyed twice, and the private member const pthread_key_t key_ in ThreadLocal will be called in pthread_key_delete twice, resulting in the failure of GTEST_CHECK_POSIX_SUCCESS_(pthread_key_delete(key_)).

Furthermore, ThreadLocal<Sequence*> g_gmock_implicit_sequence is not trivially destructible, which violates the google code style.

…linked to many shared library

Signed-off-by: arvin <icemanpeng@foxmail.com>
Copy link

google-cla bot commented May 13, 2024

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@higher-performance
Copy link
Collaborator

Hi! I'm a little bit confused, how does moving the global into a function avoid the problem of multiple constructions? Wouldn't each shared library still get its own copy of that global? Perhaps the constructions would be avoided if the function isn't called from both libraries, but if/when it is, you should still get two constructions, no?

It seems to me that what you may really want is for libgmock to be shared rather than static?

@WillAyd
Copy link

WillAyd commented Jun 4, 2024

FWIW I am seeing this same issue when compiling against the googletest source, so I don't think the shared / static conversation has anything to do with it. Having a hard time reproducing an MRE but hope to be able to craft one soon.

Here's some backtraces from gdb to illustrate - the first and the last breakpoints are from an object at the same memory address, trying to delete the same pthread key twice and causing UB

Breakpoint 1, testing::internal::ThreadLocal<testing::Sequence*>::~ThreadLocal (this=0x555555790900 <testing::internal::g_gmock_implicit_sequence>, __in_chrg=<optimized out>) at ../subprojects/googletest-1.14.0/googletest/include/gtest/internal/gtest-port.h:1764
1764	  ~ThreadLocal() {
(gdb) bt
#0  testing::internal::ThreadLocal<testing::Sequence*>::~ThreadLocal (this=0x555555790900 <testing::internal::g_gmock_implicit_sequence>, 
    __in_chrg=<optimized out>) at ../subprojects/googletest-1.14.0/googletest/include/gtest/internal/gtest-port.h:1764
#1  0x00007ffff7045495 in __run_exit_handlers (status=0, listp=0x7ffff721a838 <__exit_funcs>, run_list_atexit=run_list_atexit@entry=true, 
    run_dtors=run_dtors@entry=true) at ./stdlib/exit.c:113
#2  0x00007ffff7045610 in __GI_exit (status=<optimized out>) at ./stdlib/exit.c:143
#3  0x00007ffff7029d97 in __libc_start_call_main (main=main@entry=0x55555570216f <main(int, char**)>, argc=argc@entry=1, argv=argv@entry=0x7fffffffda28)
    at ../sysdeps/nptl/libc_start_call_main.h:74
#4  0x00007ffff7029e40 in __libc_start_main_impl (main=0x55555570216f <main(int, char**)>, argc=1, argv=0x7fffffffda28, init=<optimized out>, 
    fini=<optimized out>, rtld_fini=<optimized out>, stack_end=0x7fffffffda18) at ../csu/libc-start.c:392
#5  0x0000555555643725 in _start ()
(gdb) c
Continuing.

Breakpoint 1, testing::internal::ThreadLocal<std::vector<testing::internal::TraceInfo, std::allocator<testing::internal::TraceInfo> > >::~ThreadLocal (this=0x5555557a3d18, __in_chrg=<optimized out>) at ../subprojects/googletest-1.14.0/googletest/include/gtest/internal/gtest-port.h:1764
1764	  ~ThreadLocal() {
(gdb) bt
#0  testing::internal::ThreadLocal<std::vector<testing::internal::TraceInfo, std::allocator<testing::internal::TraceInfo> > >::~ThreadLocal (
    this=0x5555557a3d18, __in_chrg=<optimized out>) at ../subprojects/googletest-1.14.0/googletest/include/gtest/internal/gtest-port.h:1764
#1  0x00005555556cf74b in testing::internal::UnitTestImpl::~UnitTestImpl (this=0x5555557a3ad0, __in_chrg=<optimized out>)
    at ../subprojects/googletest-1.14.0/googletest/src/gtest.cc:5593
#2  0x00005555556cf870 in testing::internal::UnitTestImpl::~UnitTestImpl (this=0x5555557a3ad0, __in_chrg=<optimized out>)
    at ../subprojects/googletest-1.14.0/googletest/src/gtest.cc:5593
#3  0x00005555556cf12e in testing::UnitTest::~UnitTest (this=0x555555790860 <testing::UnitTest::GetInstance()::instance>, __in_chrg=<optimized out>)
    at ../subprojects/googletest-1.14.0/googletest/src/gtest.cc:5538
#4  0x00007ffff7045495 in __run_exit_handlers (status=0, listp=0x7ffff721a838 <__exit_funcs>, run_list_atexit=run_list_atexit@entry=true, 
    run_dtors=run_dtors@entry=true) at ./stdlib/exit.c:113
#5  0x00007ffff7045610 in __GI_exit (status=<optimized out>) at ./stdlib/exit.c:143
#6  0x00007ffff7029d97 in __libc_start_call_main (main=main@entry=0x55555570216f <main(int, char**)>, argc=argc@entry=1, argv=argv@entry=0x7fffffffda28)
    at ../sysdeps/nptl/libc_start_call_main.h:74
#7  0x00007ffff7029e40 in __libc_start_main_impl (main=0x55555570216f <main(int, char**)>, argc=1, argv=0x7fffffffda28, init=<optimized out>, 
    fini=<optimized out>, rtld_fini=<optimized out>, stack_end=0x7fffffffda18) at ../csu/libc-start.c:392
#8  0x0000555555643725 in _start ()
(gdb) c
Continuing.

Breakpoint 1, testing::internal::ThreadLocal<testing::TestPartResultReporterInterface*>::~ThreadLocal (this=0x5555557a3b60, __in_chrg=<optimized out>) at ../subprojects/googletest-1.14.0/googletest/include/gtest/internal/gtest-port.h:1764
1764	  ~ThreadLocal() {
(gdb) bt
#0  testing::internal::ThreadLocal<testing::TestPartResultReporterInterface*>::~ThreadLocal (this=0x5555557a3b60, __in_chrg=<optimized out>)
    at ../subprojects/googletest-1.14.0/googletest/include/gtest/internal/gtest-port.h:1764
#1  0x00005555556cf811 in testing::internal::UnitTestImpl::~UnitTestImpl (this=0x5555557a3ad0, __in_chrg=<optimized out>)
    at ../subprojects/googletest-1.14.0/googletest/src/gtest.cc:5593
#2  0x00005555556cf870 in testing::internal::UnitTestImpl::~UnitTestImpl (this=0x5555557a3ad0, __in_chrg=<optimized out>)
    at ../subprojects/googletest-1.14.0/googletest/src/gtest.cc:5593
#3  0x00005555556cf12e in testing::UnitTest::~UnitTest (this=0x555555790860 <testing::UnitTest::GetInstance()::instance>, __in_chrg=<optimized out>)
    at ../subprojects/googletest-1.14.0/googletest/src/gtest.cc:5538
#4  0x00007ffff7045495 in __run_exit_handlers (status=0, listp=0x7ffff721a838 <__exit_funcs>, run_list_atexit=run_list_atexit@entry=true, 
    run_dtors=run_dtors@entry=true) at ./stdlib/exit.c:113
#5  0x00007ffff7045610 in __GI_exit (status=<optimized out>) at ./stdlib/exit.c:143
#6  0x00007ffff7029d97 in __libc_start_call_main (main=main@entry=0x55555570216f <main(int, char**)>, argc=argc@entry=1, argv=argv@entry=0x7fffffffda28)
    at ../sysdeps/nptl/libc_start_call_main.h:74
#7  0x00007ffff7029e40 in __libc_start_main_impl (main=0x55555570216f <main(int, char**)>, argc=1, argv=0x7fffffffda28, init=<optimized out>, 
    fini=<optimized out>, rtld_fini=<optimized out>, stack_end=0x7fffffffda18) at ../csu/libc-start.c:392
#8  0x0000555555643725 in _start ()
(gdb) c
Continuing.

Breakpoint 1, testing::internal::ThreadLocal<testing::Sequence*>::~ThreadLocal (this=0x555555790900 <testing::internal::g_gmock_implicit_sequence>, __in_chrg=<optimized out>) at ../subprojects/googletest-1.14.0/googletest/include/gtest/internal/gtest-port.h:1764
1764	  ~ThreadLocal() {
(gdb) bt
#0  testing::internal::ThreadLocal<testing::Sequence*>::~ThreadLocal (this=0x555555790900 <testing::internal::g_gmock_implicit_sequence>, 
    __in_chrg=<optimized out>) at ../subprojects/googletest-1.14.0/googletest/include/gtest/internal/gtest-port.h:1764
#1  0x00007ffff7045a56 in __cxa_finalize (d=0x7ffff7de6350) at ./stdlib/cxa_finalize.c:83
#2  0x00007ffff7bb9e77 in __do_global_dtors_aux () from /home/willayd/clones/arrow-adbc/c/builddir/driver/sqlite/../../validation/libadbc_validation.so
#3  0x00007fffffffd8a0 in ?? ()
#4  0x00007ffff7fc924e in _dl_fini () at ./elf/dl-fini.c:142

@higher-performance
Copy link
Collaborator

FWIW I am seeing this same issue when compiling against the googletest source, so I don't think the shared / static conversation has anything to do with it.

Not sure if I understand what you're doing correctly, but compiling each of 2 libraries against the source of the same 3rd is equivalent to static linking as far as this problem is concerned, and hence (as expected in my last comment) you'd expect this to come up there too. But yeah, an MRE would be helpful whenever you have one.

@WillAyd
Copy link

WillAyd commented Jun 4, 2024

Ah OK - thanks for that info.

I am using the Meson wrap for googletest which just provides the source file to other compilation targets. I'll see if I can work around that and get a shared library compiled instead, or try to get this down to something more debuggable. Thanks for the quick reply

@WillAyd
Copy link

WillAyd commented Jun 4, 2024

To be clear though - I don't have the same linkage situation as the OP, just am getting the same error. My linker command looks something like:

c++ -o driver/sqlite/adbc-driver-sqlite-test
subprojects_googletest-1.14.0_googletest_src_gtest-all.cc.o
subprojects_googletest-1.14.0_googlemock_src_gmock-all.cc.o
subprojects_googletest-1.14.0_googlemock_src_gmock_main.cc.o
...

@WillAyd
Copy link

WillAyd commented Jun 4, 2024

While not quite an MRE you can reasonably see this if you work with this project:

https://github.com/WillAyd/arrow-adbc/tree/aeccd962d530a61f31909c083a05412110e77f2b

With the following compilation steps:

cd c
meson setup builddir -Dtests=true -Dsqlite=true && cd builddir
meson compile

Then either meson test or ./driver/sqlite/adbc-driver-sqlite-test for an even more minimal interaction

@higher-performance
Copy link
Collaborator

Thank you, but I suspect we won't have the bandwidth to to track down such an issue in a massive project like this. I'd really need something minimal that I can understand by eyeballing.

@WillAyd
Copy link

WillAyd commented Jun 4, 2024

Totally understood - no expectations for you to go through those steps. Just providing as reference for now

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

Successfully merging this pull request may close these issues.

None yet

4 participants