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

Add support MemorySanitizer for casr-san #249

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

PavlNekrasov
Copy link

Follow up to #248

Reporter: Pavel Nekrasov ([email protected])

@SweetVishnya
Copy link
Collaborator

SweetVishnya commented Feb 25, 2025

@PavlNekrasov, test_casr_san is failing. You may debug it locally via cargo test test_casr_san

@PavlNekrasov
Copy link
Author

@PavlNekrasov, test_casr_san is failing. You may debug it locally via cargo test test_casr_san

The test is passing on my end. Could you please provide more details about what's failing?

Снимок экрана 2025-02-25 в 15 03 54

@SweetVishnya
Copy link
Collaborator

@PavlNekrasov, test_casr_san is failing. You may debug it locally via cargo test test_casr_san

The test is passing on my end. Could you please provide more details about what's failing?

Снимок экрана 2025-02-25 в 15 03 54

https://github.com/ispras/casr/actions/runs/13518558010/job/37773097005#step:5:1406

you may try adding debug printing and debug inside this pr

@SweetVishnya
Copy link
Collaborator

btw, the test, also, fails on my personal machine with Ubuntu 22.04. You may try to reproduce it inside a clean docker with ubuntu 22.04

@PavlNekrasov
Copy link
Author

btw, the test, also, fails on my personal machine with Ubuntu 22.04. You may try to reproduce it inside a clean docker with ubuntu 22.04

I run on ALT Linux, and the stacktrace is as follows:

==521550==WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x499fd8 in main (/root/pavel_dev/casr_msan/msan+0x499fd8)
    #1 0x7fba3ccefefc in __libc_start_main (/lib64/libc.so.6+0x27efc)
    #2 0x41c359 in _start /usr/src/RPM/BUILD/glibc-2.32-alt5.p10.3/csu/../sysdeps/x86_64/start.S:120

SUMMARY: MemorySanitizer: use-of-uninitialized-value (/root/pavel_dev/casr_msan/msan+0x499fd8) in main
Exiting

In Ubuntu 22, it is:

==624838==WARNING: MemorySanitizer: use-of-uninitialized-value
   #0 0x56019e20a588 in main (/home/msan+0xa6588) (BuildId: 8ea89468690f1a76a3f9ae71d7f6622d3c201fda)
   #1 0x7f8f01807d8f  (/lib/x86_64-linux-gnu/libc.so.6+0x29d8f) (BuildId: 490fef8403240c91833978d494d39e537409b92e)
   #2 0x7f8f01807e3f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x29e3f) (BuildId: 490fef8403240c91833978d494d39e537409b92e)
   #3 0x56019e1822a4 in _start (/home/msan+0x1e2a4) (BuildId: 8ea89468690f1a76a3f9ae71d7f6622d3c201fda)

SUMMARY: MemorySanitizer: use-of-uninitialized-value (/home/msan+0xa6588) (BuildId: 8ea89468690f1a76a3f9ae71d7f6622d3c201fda) in main
Exiting

I've corrected the tests to account for this

@SweetVishnya
Copy link
Collaborator

@PavlNekrasov, thanks a lot! Please, give us some time to review the patch

Copy link

codecov bot commented Feb 26, 2025

Codecov Report

Attention: Patch coverage is 58.82353% with 14 lines in your changes missing coverage. Please review.

Project coverage is 66.37%. Comparing base (b260c9f) to head (b8449da).

Files with missing lines Patch % Lines
casr/src/bin/casr-cli.rs 0.00% 12 Missing ⚠️
casr/src/bin/casr-san.rs 86.66% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #249      +/-   ##
==========================================
- Coverage   66.40%   66.37%   -0.03%     
==========================================
  Files          34       34              
  Lines        8213     8245      +32     
==========================================
+ Hits         5454     5473      +19     
- Misses       2759     2772      +13     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@SweetVishnya
Copy link
Collaborator

@PavlNekrasov, can you, also, please, update the README and add an example of collecting report for MemorySanitizer?

report.execution_class = severity;
} else {
eprintln!("Couldn't estimate severity. {}", severity.err().unwrap());
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I suppose we may move the duplicate code here inside a function

@Avgor46
Copy link
Member

Avgor46 commented Feb 26, 2025

Update README.md and docs/classes.md

@PavlNekrasov
Copy link
Author

Update README.md and docs/classes.md

done

@@ -74,6 +74,7 @@ crashes.
It can analyze crashes from different sources:

* AddressSanitizer
* MemorySanitizer
Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Member

@hkctkuy hkctkuy left a comment

Choose a reason for hiding this comment

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

@PavlNekrasov, we also need to support correct work with casr-python, when MemorySanitizer is triggered in C/C++ python extensions. It must be enough to modify this code.

@@ -145,6 +146,11 @@ Create report from AddressSanitizer output:
$ clang++ -fsanitize=address -O0 -g casr/tests/casr_tests/test_asan_df.cpp -o test_asan_df
$ casr-san -o asan.casrep -- ./test_asan_df

Create report from MemorySanitizer output:

$ clang++ -fsanitize=memory -O0 casr/tests/casr_tests/test_msan.cpp -o test_msan
Copy link
Member

@hkctkuy hkctkuy Feb 26, 2025

Choose a reason for hiding this comment

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

Add -g flag. Just for the sake of uniformity.

@@ -3080,6 +3080,68 @@ fn test_casr_san() {
panic!("Couldn't parse json report file.");
}

// Msan test
Copy link
Member

Choose a reason for hiding this comment

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

Imho, this needs to be taken out into a separate test.

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.

4 participants