-
Notifications
You must be signed in to change notification settings - Fork 85
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
Introduce Valgrind to the CI pipeline #369
base: master
Are you sure you want to change the base?
Conversation
f6a5926
to
85b9001
Compare
To merge this pull request, all the memory-related leaks need to be addressed first. Currently, based on my initial testing, when disabling SDL and JIT, no memory leaks are found. When enabling JIT, there will be plenty of leaks. |
85b9001
to
7069c80
Compare
The static analyzer alone can't catch all the runtime issues. In this commit, the dynamic analysis tool Valgrind is added to the CI pipeline. ENABLE_SDL is set to 0, in order to reduce noise caused by the external libraries. Reference: - https://valgrind.org/docs/manual/quick-start.html - https://linux.die.net/man/1/gcc
7069c80
to
485d987
Compare
@@ -11,6 +11,14 @@ CFLAGS = -std=gnu99 -O2 -Wall -Wextra | |||
CFLAGS += -Wno-unused-label | |||
CFLAGS += -include src/common.h | |||
|
|||
ENABLE_VALGRIND ?= 0 | |||
ifeq ("$(ENABLE_UBSAN)", "1") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like a typo to me. Did you mean 'ENABLE_VALGRIND' instead of 'ENABLE_UBSAN'?
# according to gcc's man page: "If you use multiple -O options, with or without level numbers, the last such option is the one that is effective." | ||
# In order to use Valgrind, we need to compile with -g | ||
CFLAGS += -g | ||
LDFLAGS += -g |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The '-g' flag is not specifically designed for valgrind. Have you considered using a different name such as ENABLE_DEBUG instead of ENABLE_VALGRIND?
The static analyzer alone can't catch all the runtime issues. In this commit, the dynamic analysis tool Valgrind is added to the CI pipeline.
ENABLE_SDL is set to 0, in order to reduce noise caused by the external
libraries.
Reference: