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

Venki testing trunk #4

Conversation

VarunNagaraju
Copy link

No description provided.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Clang-Tidy found issue(s) with the introduced code (1/1)

#define NAMED_THD_STAGE_GUARD(name, thd, new_stage) \
raii::Thread_stage_guard name { \
(thd), (new_stage), __func__, __FILE__, __LINE__ \
#define NAMED_THD_STAGE_GUARD(name, thd, new_stage) \

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ cppcoreguidelines-macro-usage ⚠️
function-like macro NAMED_THD_STAGE_GUARD used; consider a constexpr template function

NAMED_THD_STAGE_GUARD(_thread_stage_guard_##new_stage, (thd), (new_stage))

// NOLINTEND(cppcoreguidelines-macro-usage)
#define THD_STAGE_GUARD(thd,new_stage) \

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ cppcoreguidelines-macro-usage ⚠️
function-like macro THD_STAGE_GUARD used; consider a constexpr template function

#define ASSERTION_TAIL \
<< debug_output(fileline) << (_shall_stop_after_assertion = true,""), \
assert(!_shall_stop_after_assertion )
#define AEQ(v1,v2) \

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ cppcoreguidelines-macro-usage ⚠️
function-like macro AEQ used; consider a constexpr template function

ASSERT_EQ(v1,v2) ASSERTION_TAIL; \
++n_assertions; \
} while(0)
#define ANE(v1,v2) \

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ cppcoreguidelines-macro-usage ⚠️
function-like macro ANE used; consider a constexpr template function

#define CHECK_SIZES(POSITION, CAPACITY) \
check_sizes(FILELINE(), debug_output, mbs, buffer_size, POSITION, CAPACITY)
// NOLINTEND(cppcoreguidelines-macro-usage)
#define CHECK_SIZES(POSITION,CAPACITY) \

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ cppcoreguidelines-macro-usage ⚠️
function-like macro CHECK_SIZES used; consider a constexpr template function

@@ -362,8 +362,7 @@ class PayloadEventBufferStreamTest {
// "nolint": as a general rule, malloc should not be used, so
// clang-tidy warns about it. But this is an allocator so it is
// appropriate to use malloc and therefore we suppress the check.
// NOLINTNEXTLINE(cppcoreguidelines-no-malloc)
return std::malloc(n);
return std::malloc(n);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ cppcoreguidelines-no-malloc ⚠️
do not manage memory manually; consider a container or a smart pointer

venkatesh-prasad-v pushed a commit that referenced this pull request Jun 14, 2024
When built with ASAN, a use-after-free is reported for the TcpPortPool.

AddressSanitizer: heap-use-after-free on address 0x60200019f190 at pc
0x00000076a18d bp 0x7fff51e7d1d0 sp 0x7fff51e7d1c0

    #4 0x770b73 in UniqueId::ProcessUniqueIds::erase(unsigned int)
       ../router/tests/helpers/tcp_port_pool.h:112
    percona#5 0x770c48 in UniqueId::~UniqueId()
       ../router/tests/helpers/tcp_port_pool.cc:234
    ...
    percona#12 0x82faa3 in testing::UnitTest::~UnitTest()
	../extra/googletest/googletest-release-1.12.0/googletest/src/gtest.cc:5496
    percona#13 0x7f5fe085ace8 in __run_exit_handlers (/lib64/libc.so.6+0x39ce8)

0x60200019f190 is located 0 bytes inside of 16-byte region
[0x60200019f190,0x60200019f1a0)
freed by thread T0 here:
    #0 0x7f5fe3cbd10f in operator delete(void*, unsigned long)
       (/lib64/libasan.so.6+0xb710f)
    #1 0x7f5fe085ace8 in __run_exit_handlers (/lib64/libc.so.6+0x39ce8)

Background
==========

__run_exit_handlers destroys "static" and "global" variables in reverse
order of their creation.

googletest's unit-tests are a static, and the TcpPortPool also has
ProcessUniqueId's which contains the process-wide unique-ids.

At construct: unittest -> tcp-port-pool -> proces-unique-ids
At destruct : process-unique-ids -> tcp-port-pool -> 💥

The use-after-free happens as the process-unique-ids static is
destructed before the tcp-port-pool which tries to its Ids from the
process-unique-ids.

Change
======

- extend the lifetime of the process-unique-ids to after the last use of
  the tcp-port-pool via a std::shared_ptr<>

Change-Id: I75b8b781e1d240f18ca72f2c86182639a7699f06
venkatesh-prasad-v pushed a commit that referenced this pull request Jun 14, 2024
…nt on Windows and posix [#4]

Introduce quoting functions suitable for POSIX shell (sh) and running
C/C++ programs on Windows via CMD.EXE.

Use them when running a program via ssh. A simple heuristic to guess the
kind of quoting needed on remote host is. If a \ appears in any argument
use the quoting function for Windows. If / appears in any argument use
the quoting function for POSIX.

Change-Id: I851eb3da22d716d181319e825e888631cd16aeb7
venkatesh-prasad-v pushed a commit that referenced this pull request Jun 14, 2024
Problem:
Starting ´ndb_mgmd --bind-address´ may potentially cause abnormal
program termination in MgmtSrvr destructor when ndb_mgmd restart itself.

  Core was generated by `ndb_mgmd --defa'.
  Program terminated with signal SIGABRT,   Aborted.
  #0  0x00007f8ce4066b8f in raise () from /lib64/libc.so.6
  #1  0x00007f8ce4039ea5 in abort () from /lib64/libc.so.6
  #2  0x00007f8ce40a7d97 in __libc_message () from /lib64/libc.so.6
  #3  0x00007f8ce40af08c in malloc_printerr () from /lib64/libc.so.6
  #4  0x00007f8ce40b132d in _int_free () from /lib64/libc.so.6
  percona#5  0x00000000006e9ffe in MgmtSrvr::~MgmtSrvr (this=0x28de4b0) at
mysql/8.0/storage/ndb/src/mgmsrv/MgmtSrvr.cpp:
890
  percona#6  0x00000000006ea09e in MgmtSrvr::~MgmtSrvr (this=0x2) at mysql/8.0/
storage/ndb/src/mgmsrv/MgmtSrvr.cpp:849
  percona#7  0x0000000000700d94 in mgmd_run () at
mysql/8.0/storage/ndb/src/mgmsrv/main.cpp:260
  percona#8  0x0000000000700775 in mgmd_main (argc=<optimized out>,
argv=0x28041d0) at mysql/8.0/storage/ndb/src/
mgmsrv/main.cpp:479

Analysis:
While starting up, the ndb_mgmd will allocate memory for bind_address in
order to potentially rewrite the parameter. When ndb_mgmd restart itself
the memory will be released and dangling pointer causing double free.

Fix:
Drop support for bind_address=[::], it is not documented anywhere, is
not useful and doesn't work.
This means the need to rewrite bind_address is gone and bind_address
argument need neither alloc or free.

Change-Id: I7797109b9d8391394587188d64d4b1f398887e94
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.

1 participant