*: fix asan false positive case #5742

solotzg · 2022-08-31T04:24:17Z

What problem does this PR solve?

Issue Number: ref #5294

Problem Summary:

https://ci.pingcap.net/blue/rest/organizations/jenkins/pipelines/tiflash-sanitizer-daily/runs/486/nodes/83/steps/87/log/?start=0

[2022-08-30T19:21:23.463Z] ==10469==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7ffe30dca61f at pc 0x000000490b4f bp 0x7ffe30dca550 sp 0x7ffe30dca548
[2022-08-30T19:21:23.463Z] READ of size 32 at 0x7ffe30dca61f thread T0
[2022-08-30T19:21:23.463Z]     #0 0x490b4e in mem_utils::details::load_block32(void const*) /home/jenkins/agent/workspace/tiflash-sanitizer-daily/tiflash/libs/libcommon/include/common/avx2_mem_utils.h:66:12
[2022-08-30T19:21:23.463Z]     #1 0x490b4e in mem_utils::details::get_block32_cmp_eq_mask(void const*, long long vector[4] const&) /home/jenkins/agent/workspace/tiflash-sanitizer-daily/tiflash/libs/libcommon/include/common/avx2_strstr.h:34:24
[2022-08-30T19:21:23.463Z]     #2 0x490b4e in char const* mem_utils::details::avx2_strstr_impl<mem_utils::details::avx2_memchr(char const*, unsigned long, char)::'lambda'(char const*)>(char const*, char, long, mem_utils::details::avx2_memchr(char const*, unsigned long, char)::'lambda'(char const*)&&) /home/jenkins/agent/workspace/tiflash-sanitizer-daily/tiflash/libs/libcommon/include/common/avx2_strstr.h:139:21
[2022-08-30T19:21:23.463Z]     #3 0x490b4e in mem_utils::details::avx2_memchr(char const*, unsigned long, char) /home/jenkins/agent/workspace/tiflash-sanitizer-daily/tiflash/libs/libcommon/include/common/avx2_strstr.h:267:12
[2022-08-30T19:21:23.463Z]     #4 0x490b4e in mem_utils::avx2_memchr(char const*, unsigned long, char) /home/jenkins/agent/workspace/tiflash-sanitizer-daily/tiflash/libs/libcommon/src/avx2_mem_utils_impl.cpp:46:12
[2022-08-30T19:21:23.463Z]     #5 0x42ab3b in TestFunc(unsigned long) /home/jenkins/agent/workspace/tiflash-sanitizer-daily/tiflash/libs/libcommon/src/tests/gtest_mem_utils_opt.cpp:40:5
[2022-08-30T19:21:23.463Z]     #6 0x42ea67 in MemUtilsTestOPT_CompareNormal_Test::TestBody() /home/jenkins/agent/workspace/tiflash-sanitizer-daily/tiflash/libs/libcommon/src/tests/gtest_mem_utils_opt.cpp:85:9
[2022-08-30T19:21:23.464Z]     #7 0x523750 in void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) /home/jenkins/agent/workspace/tiflash-sanitizer-daily/tiflash/contrib/googletest/googletest/src/gtest.cc:2443:10
[2022-08-30T19:21:23.464Z]     #8 0x523750 in void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) /home/jenkins/agent/workspace/tiflash-sanitizer-daily/tiflash/contrib/googletest/googletest/src/gtest.cc:2479:14
[2022-08-30T19:21:23.464Z]     #9 0x4c1158 in testing::Test::Run() /home/jenkins/agent/workspace/tiflash-sanitizer-daily/tiflash/contrib/googletest/googletest/src/gtest.cc:2517:5
[2022-08-30T19:21:23.464Z]     #10 0x4c42e7 in testing::TestInfo::Run() /home/jenkins/agent/workspace/tiflash-sanitizer-daily/tiflash/contrib/googletest/googletest/src/gtest.cc:2693:11
[2022-08-30T19:21:23.464Z]     #11 0x4c585f in testing::TestCase::Run() /home/jenkins/agent/workspace/tiflash-sanitizer-daily/tiflash/contrib/googletest/googletest/src/gtest.cc:2811:28
[2022-08-30T19:21:23.464Z]     #12 0x4f0278 in testing::internal::UnitTestImpl::RunAllTests() /home/jenkins/agent/workspace/tiflash-sanitizer-daily/tiflash/contrib/googletest/googletest/src/gtest.cc:5177:43
[2022-08-30T19:21:23.464Z]     #13 0x525e20 in bool testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) /home/jenkins/agent/workspace/tiflash-sanitizer-daily/tiflash/contrib/googletest/googletest/src/gtest.cc:2443:10
[2022-08-30T19:21:23.464Z]     #14 0x525e20 in bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) /home/jenkins/agent/workspace/tiflash-sanitizer-daily/tiflash/contrib/googletest/googletest/src/gtest.cc:2479:14
[2022-08-30T19:21:23.464Z]     #15 0x4eeaf1 in testing::UnitTest::Run() /home/jenkins/agent/workspace/tiflash-sanitizer-daily/tiflash/contrib/googletest/googletest/src/gtest.cc:4786:10
[2022-08-30T19:21:23.464Z]     #16 0x468f0f in RUN_ALL_TESTS() /home/jenkins/agent/workspace/tiflash-sanitizer-daily/tiflash/contrib/googletest/googletest/include/gtest/gtest.h:2341:46
[2022-08-30T19:21:23.464Z]     #17 0x468f0f in main /home/jenkins/agent/workspace/tiflash-sanitizer-daily/tiflash/contrib/googletest/googletest/src/gtest_main.cc:36:10
[2022-08-30T19:21:23.464Z]     #18 0x7f315cadd554 in __libc_start_main (/lib64/libc.so.6+0x22554)
[2022-08-30T19:21:23.464Z]     #19 0x317ce8 in _start (/tiflash/gtests_libcommon+0x317ce8)

caused by #5658

tiflash/libs/libcommon/include/common/avx2_strstr.h

Lines 130 to 141 in a8c8cb1

    
           // align address to 32 for better performance 
        
           // memory allocator will always alloc memory aligned to `Page Size`(usually 4K, one Block `512B` at least) from system 
        
           // if there is valid data at address S, then it is safe to visit address [ALIGN_TO_PAGE_SIZE(S), ALIGN_TO_PAGE_SIZE(S)+PAGE_SIZE). 
        
           if (uint8_t offset = OFFSET_FROM_ALIGNED(size_t(src), BLOCK32_SIZE); offset != 0) 
        
           { 
        
               // align to 32 
        
               src = reinterpret_cast<decltype(src)>(ALIGNED_ADDR(size_t(src), BLOCK32_SIZE)); 
        
               // right shift offset to remove useless mask bit 
        
               auto mask = get_block32_cmp_eq_mask(src, check_block32) >> offset; 
        
               for (; mask;)

load block 32 from new aligned address may cause false positives when using AddressSanitizer because asan will provide a malloc()/free() alternative and detect memory visitation.
generally it's safe to visit address which won't cross page boundary.

What is changed and how it works?

use stl function if ADDRESS_SANITIZER is defined

verified in https://ci.pingcap.net/blue/organizations/jenkins/tiflash-sanitizer-daily/detail/tiflash-sanitizer-daily/487/pipeline

Check List

Tests

Unit test
Integration test
Manual test (add detailed scripts or steps below)
No code

Side effects

Performance regression: Consumes more CPU
Performance regression: Consumes more Memory
Breaking backward compatibility

Documentation

Release note

None

ti-chi-bot · 2022-08-31T04:24:18Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

windtalker
zanmato1984

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found here.

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

solotzg · 2022-08-31T04:25:48Z

/run-sanitizer-test asan

solotzg · 2022-08-31T04:30:01Z

/run-all-tests

sre-bot · 2022-08-31T04:47:18Z

Coverage for changed files

Filename                      Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
avx2_strstr.h                     149                17    88.59%          14                 0   100.00%         236                38    83.90%          82                 3    96.34%
mem_utils_opt.h                     6                 1    83.33%           3                 1    66.67%          45                33    26.67%           2                 0   100.00%
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                             155                18    88.39%          17                 1    94.12%         281                71    74.73%          84                 3    96.43%

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
18475      8321             54.96%    213736  85890        59.81%

full coverage report (for internal network access only)

windtalker

LGTM

zanmato1984

LGTM

solotzg · 2022-08-31T05:40:38Z

/merge

ti-chi-bot · 2022-08-31T05:40:39Z

@solotzg: It seems you want to merge this PR, I will help you trigger all the tests:

/run-all-tests

You only need to trigger /merge once, and if the CI test fails, you just re-trigger the test that failed and the bot will merge the PR for you after the CI passes.

If you have any questions about the PR merge process, please refer to pr process.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository.

ti-chi-bot · 2022-08-31T05:40:41Z

This pull request has been accepted and is ready to merge.

Commit hash: 3f52c37

sre-bot · 2022-08-31T05:52:14Z

Coverage for changed files

Filename                      Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
avx2_strstr.h                     149                17    88.59%          14                 0   100.00%         236                38    83.90%          82                 3    96.34%
mem_utils_opt.h                     6                 1    83.33%           3                 1    66.67%          45                33    26.67%           2                 0   100.00%
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                             155                18    88.39%          17                 1    94.12%         281                71    74.73%          84                 3    96.43%

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
18475      8322             54.96%    213736  85901        59.81%

full coverage report (for internal network access only)

solotzg · 2022-08-31T07:58:23Z

/run-all-tests

sre-bot · 2022-08-31T08:19:48Z

Coverage for changed files

Filename                      Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
avx2_strstr.h                     149                17    88.59%          14                 0   100.00%         236                38    83.90%          82                 3    96.34%
mem_utils_opt.h                     6                 1    83.33%           3                 1    66.67%          45                33    26.67%           2                 0   100.00%
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                             155                18    88.39%          17                 1    94.12%         281                71    74.73%          84                 3    96.43%

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
18475      8321             54.96%    213736  85932        59.80%

full coverage report (for internal network access only)

solotzg · 2022-08-31T08:44:51Z

/run-integration-test

tests/fullstack-test/expr/from_unixtime.test

solotzg · 2022-08-31T13:16:47Z

/merge

ti-chi-bot · 2022-08-31T13:16:48Z

@solotzg: It seems you want to merge this PR, I will help you trigger all the tests:

/run-all-tests

You only need to trigger /merge once, and if the CI test fails, you just re-trigger the test that failed and the bot will merge the PR for you after the CI passes.

If you have any questions about the PR merge process, please refer to pr process.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository.

sre-bot · 2022-08-31T13:39:47Z

Coverage for changed files

Filename                      Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
avx2_strstr.h                     149                17    88.59%          14                 0   100.00%         236                38    83.90%          82                 3    96.34%
mem_utils_opt.h                     6                 1    83.33%           3                 1    66.67%          45                33    26.67%           2                 0   100.00%
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                             155                18    88.39%          17                 1    94.12%         281                71    74.73%          84                 3    96.43%

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
18475      8322             54.96%    213736  85899        59.81%

full coverage report (for internal network access only)

solotzg · 2022-08-31T14:14:44Z

/rebuild

solotzg · 2022-08-31T14:15:22Z

/merge

ti-chi-bot · 2022-08-31T14:15:25Z

@solotzg: It seems you want to merge this PR, I will help you trigger all the tests:

/run-all-tests

You only need to trigger /merge once, and if the CI test fails, you just re-trigger the test that failed and the bot will merge the PR for you after the CI passes.

If you have any questions about the PR merge process, please refer to pr process.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository.

solotzg · 2022-08-31T14:38:43Z

/rebuild

solotzg · 2022-08-31T14:38:58Z

/merge

ti-chi-bot · 2022-08-31T14:38:59Z

@solotzg: It seems you want to merge this PR, I will help you trigger all the tests:

/run-all-tests

You only need to trigger /merge once, and if the CI test fails, you just re-trigger the test that failed and the bot will merge the PR for you after the CI passes.

If you have any questions about the PR merge process, please refer to pr process.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository.

solotzg · 2022-08-31T14:51:51Z

/rebuild

solotzg · 2022-08-31T14:52:24Z

/run-integration-test

sre-bot · 2022-08-31T15:04:25Z

Coverage for changed files

Filename                      Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
avx2_strstr.h                     149                17    88.59%          14                 0   100.00%         236                38    83.90%          82                 3    96.34%
mem_utils_opt.h                     6                 1    83.33%           3                 1    66.67%          45                33    26.67%           2                 0   100.00%
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                             155                18    88.39%          17                 1    94.12%         281                71    74.73%          84                 3    96.43%

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
18488      8323             54.98%    213856  85922        59.82%

full coverage report (for internal network access only)

ti-chi-bot · 2022-08-31T15:04:42Z

@solotzg: Your PR was out of date, I have automatically updated it for you.

At the same time I will also trigger all tests for you:

/run-all-tests

If the CI test fails, you just re-trigger the test that failed and the bot will merge the PR for you after the CI passes.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository.

JaySon-Huang · 2022-08-31T17:08:00Z

/run-unit-test

sre-bot · 2022-08-31T17:28:24Z

Coverage for changed files

Filename                      Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
avx2_strstr.h                     149                17    88.59%          14                 0   100.00%         236                38    83.90%          82                 3    96.34%
mem_utils_opt.h                     6                 1    83.33%           3                 1    66.67%          45                33    26.67%           2                 0   100.00%
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                             155                18    88.39%          17                 1    94.12%         281                71    74.73%          84                 3    96.43%

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
18488      8322             54.99%    213856  85870        59.85%

full coverage report (for internal network access only)

This reverts commit 0410d3f.

Fix asan false positive case

3f52c37

ti-chi-bot added release-note-none Denotes a PR that doesn't merit a release note. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Aug 31, 2022

solotzg changed the title ~~Fix asan false positive case~~ *: fix asan false positive case Aug 31, 2022

windtalker approved these changes Aug 31, 2022

View reviewed changes

ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Aug 31, 2022

zanmato1984 approved these changes Aug 31, 2022

View reviewed changes

ti-chi-bot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Aug 31, 2022

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Aug 31, 2022

solotzg added 2 commits August 31, 2022 15:56

Update from_unixtime.test

3effb8b

Update from_unixtime.test

592ffdb

solotzg commented Aug 31, 2022

View reviewed changes

tests/fullstack-test/expr/from_unixtime.test Outdated Show resolved Hide resolved

solotzg added 2 commits August 31, 2022 20:54

Update tests/fullstack-test/expr/from_unixtime.test

fb3616e

Merge branch 'master' into fix-asan

fdda139

Merge branch 'master' into fix-asan

b6fafa7

Merge branch 'master' into fix-asan

5e98c64

ti-chi-bot merged commit 0410d3f into pingcap:master Aug 31, 2022

solotzg deleted the fix-asan branch September 1, 2022 00:21

solotzg added a commit to solotzg/tiflash that referenced this pull request Sep 19, 2022

Revert "*: fix asan false positive case (pingcap#5742)"

614c2d5

This reverts commit 0410d3f.

solotzg mentioned this pull request Nov 2, 2022

tsan: disable sanitizer for func StringHashTable::dispatch #6234

Merged

12 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

*: fix asan false positive case #5742

*: fix asan false positive case #5742

solotzg commented Aug 31, 2022 •

edited

Loading

ti-chi-bot commented Aug 31, 2022 •

edited

Loading

solotzg commented Aug 31, 2022

solotzg commented Aug 31, 2022

sre-bot commented Aug 31, 2022

windtalker left a comment

zanmato1984 left a comment

solotzg commented Aug 31, 2022

ti-chi-bot commented Aug 31, 2022

ti-chi-bot commented Aug 31, 2022

sre-bot commented Aug 31, 2022

solotzg commented Aug 31, 2022

sre-bot commented Aug 31, 2022

solotzg commented Aug 31, 2022

solotzg commented Aug 31, 2022

ti-chi-bot commented Aug 31, 2022

sre-bot commented Aug 31, 2022

solotzg commented Aug 31, 2022

solotzg commented Aug 31, 2022

ti-chi-bot commented Aug 31, 2022

solotzg commented Aug 31, 2022

solotzg commented Aug 31, 2022

ti-chi-bot commented Aug 31, 2022

solotzg commented Aug 31, 2022

solotzg commented Aug 31, 2022

sre-bot commented Aug 31, 2022

ti-chi-bot commented Aug 31, 2022

JaySon-Huang commented Aug 31, 2022

sre-bot commented Aug 31, 2022

	// align address to 32 for better performance
	// memory allocator will always alloc memory aligned to `Page Size`(usually 4K, one Block `512B` at least) from system
	// if there is valid data at address S, then it is safe to visit address [ALIGN_TO_PAGE_SIZE(S), ALIGN_TO_PAGE_SIZE(S)+PAGE_SIZE).
	if (uint8_t offset = OFFSET_FROM_ALIGNED(size_t(src), BLOCK32_SIZE); offset != 0)
	{
	// align to 32
	src = reinterpret_cast<decltype(src)>(ALIGNED_ADDR(size_t(src), BLOCK32_SIZE));

	// right shift offset to remove useless mask bit
	auto mask = get_block32_cmp_eq_mask(src, check_block32) >> offset;

	for (; mask;)

*: fix asan false positive case #5742

*: fix asan false positive case #5742

Conversation

solotzg commented Aug 31, 2022 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

ti-chi-bot commented Aug 31, 2022 • edited Loading

solotzg commented Aug 31, 2022

solotzg commented Aug 31, 2022

sre-bot commented Aug 31, 2022

Coverage for changed files

Coverage summary

windtalker left a comment

Choose a reason for hiding this comment

zanmato1984 left a comment

Choose a reason for hiding this comment

solotzg commented Aug 31, 2022

ti-chi-bot commented Aug 31, 2022

ti-chi-bot commented Aug 31, 2022

sre-bot commented Aug 31, 2022

Coverage for changed files

Coverage summary

solotzg commented Aug 31, 2022

sre-bot commented Aug 31, 2022

Coverage for changed files

Coverage summary

solotzg commented Aug 31, 2022

solotzg commented Aug 31, 2022

ti-chi-bot commented Aug 31, 2022

sre-bot commented Aug 31, 2022

Coverage for changed files

Coverage summary

solotzg commented Aug 31, 2022

solotzg commented Aug 31, 2022

ti-chi-bot commented Aug 31, 2022

solotzg commented Aug 31, 2022

solotzg commented Aug 31, 2022

ti-chi-bot commented Aug 31, 2022

solotzg commented Aug 31, 2022

solotzg commented Aug 31, 2022

sre-bot commented Aug 31, 2022

Coverage for changed files

Coverage summary

ti-chi-bot commented Aug 31, 2022

JaySon-Huang commented Aug 31, 2022

sre-bot commented Aug 31, 2022

Coverage for changed files

Coverage summary

solotzg commented Aug 31, 2022 •

edited

Loading

ti-chi-bot commented Aug 31, 2022 •

edited

Loading