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.

Sign up for GitHub

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

Jump to bottom

Add checks for Asan build type #1562

Merged
merged 1 commit into from
Dec 12, 2024
Merged

Add checks for Asan build type #1562

merged 1 commit into from
Dec 12, 2024

Conversation

mmanzoorTT
Copy link
Contributor

@mmanzoorTT mmanzoorTT commented Dec 10, 2024 •
edited
Loading

  • Address sanitization build causes errors for python bindings and shared libs. Other MLIR based projects (IREE, Stablehlo) seem to have similar issues and just skip building these components for sanitization builds.
  • Address sanitization can be enabled by setting '-DCMAKE_BUILD_TYPE=Asan -DTTMLIR_ENABLE_SHARED_LIB=OFF -DTTMLIR_ENABLE_BINDINGS_PYTHON=OFF' options with cmake command.

closes #1561

nsmithtt
nsmithtt reviewed Dec 10, 2024
View reviewed changes
Copy link
Contributor

@nsmithtt nsmithtt left a comment

Choose a reason for hiding this comment

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

@mmanzoorTT the boilerplate for this already exists (untested recently, though I did have it working at one point).

See file cmake/modules/TTMLIRBuildTypes.cmake.

Usage:

cmake -G Ninja -B build -DCMAKE_BUILD_TYPE=Asan ...

@mmanzoorTT mmanzoorTT mentioned this pull request Dec 10, 2024
Closed
@mmanzoorTT mmanzoorTT force-pushed the mmanzoor/sanitize-build branch from 5a3adab to 97badcd Compare December 11, 2024 01:20
@mmanzoorTT
Copy link
Contributor Author

Thanks @nsmithtt; I have updated my PR and only added checks for Asan build.

@mmanzoorTT mmanzoorTT changed the title Add address sanitization build option Add checks for Asan build type Dec 11, 2024
@mmanzoorTT mmanzoorTT mentioned this pull request Dec 11, 2024
Open
azecevicTT added a commit that referenced this pull request Dec 11, 2024
@azecevicTT
ASAN build fixes failing tests
50106d4 
This PR introduces some changes
#1566, but it seems that it
doesn't fix the majority of failing tests. Both bugs are result of
dangling reference of `ArrayRef` objects, because they receive temporary
`SmallVector` objects.

I've rebased this branch on #1562, for the ability to build with ASAN flag.

Fixes #1565
@azecevicTT azecevicTT force-pushed the mmanzoor/sanitize-build branch from 50106d4 to 97badcd Compare December 11, 2024 11:28
azecevicTT added a commit that referenced this pull request Dec 11, 2024
@azecevicTT
ASAN fix of failing tests
e7fadbf 
This PR introduces some changes #1566, but it seems that it
doesn't fix the majority of failing tests. Both bugs are result of
dangling reference of `ArrayRef` objects, because they receive temporary
`SmallVector` objects.

I've rebased this branch on #1562, for the ability to build with ASAN flag.

Fixes #1565
@azecevicTT
Copy link
Contributor

Sorry for accidentally pushing to this branch, I've reverted it to the previous state. I wanted to start a discussion regarding some changes, don't want to block this PR, since I think this should land on the main as soon as possible.

@azecevicTT azecevicTT mentioned this pull request Dec 11, 2024
Merged
@mmanzoorTT
Add checks for address sanitizer build
58b706e 
* Address sanitization build causes errors for python bindings and shared libs.
Other MLIR based projects (IREE, Stablehlo) seem to have similar issues and just
skip building these components for sanitization builds.
* Address sanitizer build can be enabled by setting '-DCMAKE_BUILD_TYPE=Asan
-DTTMLIR_ENABLE_SHARED_LIB=OFF -DTTMLIR_ENABLE_BINDINGS_PYTHON=OFF' options with
cmake command.
@mmanzoorTT mmanzoorTT force-pushed the mmanzoor/sanitize-build branch from 97badcd to 58b706e Compare December 11, 2024 14:46
@mmanzoorTT mmanzoorTT merged commit be179fc into main Dec 12, 2024
21 checks passed
azecevicTT added a commit that referenced this pull request Dec 12, 2024
@azecevicTT
ASAN fix of failing tests
b42e1dd 
This PR introduces some changes #1566, but it seems that it
doesn't fix the majority of failing tests. Both bugs are result of
dangling reference of `ArrayRef` objects, because they receive temporary
`SmallVector` objects.

I've rebased this branch on #1562, for the ability to build with ASAN flag.

Fixes #1565
azecevicTT added a commit that referenced this pull request Dec 12, 2024
@azecevicTT
ASAN fix of failing tests
abf39e2 
This PR introduces some changes #1566, but it seems that it
doesn't fix the majority of failing tests. Both bugs are result of
dangling reference of `ArrayRef` objects, because they receive temporary
`SmallVector` objects.

I've rebased this branch on #1562, for the ability to build with ASAN flag.

Fixes #1565
azecevicTT added a commit that referenced this pull request Dec 13, 2024
@azecevicTT
ASAN fix of failing tests
5324eee 
This PR introduces some changes #1566, but it seems that it
doesn't fix the majority of failing tests. Both bugs are result of
dangling reference of `ArrayRef` objects, because they receive temporary
`SmallVector` objects.

I've rebased this branch on #1562, for the ability to build with ASAN flag.

Fixes #1565
azecevicTT pushed a commit that referenced this pull request Dec 17, 2024
@mmanzoorTT @azecevicTT
Add checks for address sanitizer build (#1562)
c02ffbe 
* Address sanitization build causes errors for python bindings and shared libs.
Other MLIR based projects (IREE, Stablehlo) seem to have similar issues and just
skip building these components for sanitization builds.
* Address sanitizer build can be enabled by setting '-DCMAKE_BUILD_TYPE=Asan
-DTTMLIR_ENABLE_SHARED_LIB=OFF -DTTMLIR_ENABLE_BINDINGS_PYTHON=OFF' options with
cmake command.
@mmanzoorTT mmanzoorTT deleted the mmanzoor/sanitize-build branch January 21, 2025 15:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nsmithtt nsmithtt nsmithtt approved these changes

@AleksKnezevic AleksKnezevic Awaiting requested review from AleksKnezevic

@sdjordjevicTT sdjordjevicTT Awaiting requested review from sdjordjevicTT

@mtopalovicTT mtopalovicTT Awaiting requested review from mtopalovicTT mtopalovicTT is a code owner

@svuckovicTT svuckovicTT Awaiting requested review from svuckovicTT

@nobradovictt nobradovictt Awaiting requested review from nobradovictt

@jserbedzijaTT jserbedzijaTT Awaiting requested review from jserbedzijaTT

@jnie-TT jnie-TT Awaiting requested review from jnie-TT

@azecevicTT azecevicTT Awaiting requested review from azecevicTT

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Address sanitizer build option
3 participants
@mmanzoorTT @azecevicTT @nsmithtt