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

make asan work as much as possible #8148

Merged
merged 2 commits into from
Feb 22, 2022
Merged

make asan work as much as possible #8148

merged 2 commits into from
Feb 22, 2022

Conversation

dataroaring
Copy link
Contributor

Proposed changes

make asan work as much as possible

Issue Number: close #xxx

Problem Summary:

Manually ASAN poisoning is complicated and it is hard to make it work right. There are illustrated examples in http://blog.hostilefork.com/poison-memory-without-asan/.

Stacks of use after poison do not provide enough information to resolve bug, while stacks of use afer free provide. google/sanitizers#191

We'd better implement a mempool using malloc/free directly, thus asan works natively. However we cannot do it in a short time, so we make manual poisoning work as much as possible.

I refers to https://github.com/mcgov/asan_alignment_example.

Checklist(Required)

  1. Does it affect the original behavior: (No)
  2. Has unit tests been added: (No)
  3. Has document been added or modified: (No)
  4. Does it need to update dependencies: (No)
  5. Are there any changes that cannot be rolled back: (No)

Further comments

We need much more work to make asan work, for example we need fix manual poison in ./be/src/olap/rowset/segment_v2/bitshuffle_page.h and I am not sure if there are any places.

This is the 1st patch to make asan work.
Before this patch a use after poison is reported like below
==19305==ERROR: AddressSanitizer: unknown-crash on address
0x625000137013 at pc 0x561c44bcf6b8 bp 0x7ffb75a00910 sp 0x7ffb75a000b8

After this patch the use after poison is reported like below
==17782==ERROR: AddressSanitizer: use-after-poison on address
0x625000137033 at pc 0x55633c8f56b8 bp 0x7ff3dc437930 sp 0x7ff3dc43

Before this patch, a false memory usage is reported like below
==33080==AddressSanitizer CHECK failed: ../../../../src/libsanitizer/
asan/asan_allocator.cpp:189 "((old)) == ((kAllocBegMagic))"

This is the 2nd patch to make ASAN work.
Copy link
Contributor

@morningman morningman left a comment

Choose a reason for hiding this comment

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

LGTM

@github-actions
Copy link
Contributor

PR approved by at least one committer and no changes requested.

@github-actions github-actions bot added the approved Indicates a PR has been approved by one committer. label Feb 21, 2022
@github-actions
Copy link
Contributor

PR approved by anyone and no changes requested.

@yangzhg yangzhg merged commit d6aebc0 into apache:master Feb 22, 2022
@yangzhg
Copy link
Member

yangzhg commented Feb 23, 2022

Hi @dataroaring this pr will cause be/test/runtime/mem_pool_test.cpp failed, can you fix this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by one committer. reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants