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 ubsan checks to WAMR #2147

Merged
merged 1 commit into from
Apr 26, 2023
Merged

Add ubsan checks to WAMR #2147

merged 1 commit into from
Apr 26, 2023

Conversation

Zzzabiyaka
Copy link
Contributor

this PR adds ubsan checks with no aligment enabled to basic CI tests.

@lum1n0us
Copy link
Collaborator

LGTM

endif()

if (SANITIZER STREQUAL "ubsan")
set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -g -O2 -fno-omit-frame-pointer -fsanitize=undefined -fno-sanitize-recover=all -fno-sanitize=alignment" )
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we also need to check for a compiler here, or are we ok with the compiler (sliently) failing if those flags are not supported?

Copy link
Contributor Author

@Zzzabiyaka Zzzabiyaka Apr 25, 2023

Choose a reason for hiding this comment

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

I believe that you won't set these flags for the wrong compiler on the CI side.

But if you think it's better to add here I'll add

@@ -87,6 +87,15 @@ endif()
set(WAMR_ROOT_DIR ${CMAKE_CURRENT_LIST_DIR}/../..)
include (${WAMR_ROOT_DIR}/build-scripts/runtime_lib.cmake)

if (NOT DEFINED SANITIZER)
set(SANITIZER "")
Copy link
Collaborator

Choose a reason for hiding this comment

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

is that variable going to be used outside of this if..else block that we set it here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no for now

Copy link
Collaborator

@loganek loganek left a comment

Choose a reason for hiding this comment

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

looks good, thanks.

@wenyongh wenyongh merged commit 15139d2 into bytecodealliance:main Apr 26, 2023
victoryang00 pushed a commit to victoryang00/wamr-aot-gc-checkpoint-restore that referenced this pull request May 27, 2024
This PR adds ubsan checks with no alignment enabled to basic CI tests for
samples/wasm-c-api.

Co-authored-by: Maksim Litskevich <makslit@amazon.co.uk>
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