Conversation
|
Member
Author
|
If possible, try and add coverage reporting to this PR as well. |
ericonr
force-pushed
the
dynamic-analysis
branch
3 times, most recently
from
d7d112f to
d9a1df1
Compare
Member
Author
|
Some sanitizer specific options listed in https://gcc.gnu.org/onlinedocs/gcc/Instrumentation-Options.html and (random example) https://kristerw.blogspot.com/2018/06/useful-gcc-address-sanitizer-checks-not.html might be useful to add as well, via compiler switches and/or environment variables. |
dummy_dev_open was using malloc, so, when the test was finished, AddressSanitizer detected a leak from the pcie_bars struct. In order to allow us to take advantage of the existing dev_close implementation for this test, we switched dummy_dev_open to use mmap instead of malloc. Using mmap means the mappings are no longer tracked by malloc, so avoiding memory leak reports by calling dev_close is actually no longer necessary (yes, the situation is a bit circular). But it's good practice to close all handles, since it can make the program state easier to analyse. Reported-by: LeakSanitizer
Reported-by: LeakSanitizer
ericonr
force-pushed
the
dynamic-analysis
branch
from
d9a1df1 to
edf0abb
Compare
For negative values inside the valid range, float2fixed simply multiplies that value by the fixed point factor, resulting in another negative floating point value. Assigning that value to an unsigned integer is undefined behavior, since it's a value outside of its range. In order to fix this, we can simply add an intermediate cast through int32_t, which can represent negative values, because that's the underlying type for this value; the function only returns uint32_t to simplify any further register field manipulation. Reported-by: UndefinedBehaviorSanitizer
ericonr
force-pushed
the
dynamic-analysis
branch
from
edf0abb to
ceceb52
Compare
Using sanitizers on Alpine doesn't seem to be supported properly, so we are sticking with Debian (glibc) only. Enabling sanitizers can change some compiler passes, possibly generating false-positive warnings [1]. Warnings are already checked by other workflows, so we omit -Werror from this one. Note that sanitizers tend to increase the rate of false positive warnings, most notably those around -Wmaybe-uninitialized. We recommend against combining -Werror and [the use of] sanitizers. In order to improve error messages, we build with debug information. We didn't (explicitly) use all of the available sanitizers, such as: - MemorySanitizer isn't supported by GCC and requires that all code be instrumented, including dependencies [2] - LeakSanitizer is already run as a last phase of AddressSanitizer [3] ThreadSanitizer is used even though it isn't relevant for now, since we don't have any multi-threaded tests yet. However, we can add them in the future and make use of it. [1] https://gcc.gnu.org/onlinedocs/gcc/Instrumentation-Options.html#index-fsanitize_003dbuiltin [2] https://github.com/google/sanitizers/wiki/MemorySanitizer#using-instrumented-libraries [3] https://github.com/google/sanitizers/wiki/AddressSanitizerLeakSanitizer
ericonr
force-pushed
the
dynamic-analysis
branch
3 times, most recently
from
273d5dc to
44e3220
Compare
|
Member
Author
include:
- compiler: ["gcc", "g++"]
sanitizer: "address,undefined"
extraflags: >
-fsanitize=pointer-compare -fsanitize=pointer-subtract
-fsanitize=shift -fsanitize=unreachable -fsanitize=null -fsanitize=return
-fsanitize=signed-integer-overflow -fsanitize=bounds-strict -fsanitize=alignment
-fsanitize=object-size -fsanitize=bool -fsanitize=enum -fsanitize=vptr
-fsanitize=pointer-overflow -fsanitize=builtin
- compiler: ["clang", "clang++"]
sanitizer: "address,undefined"
extraflags: >
-fsanitize-address-use-after-scopeThis didn't work correctly, am giving up for now. Dealing with lists and maps in actions is annoying: https://github.com/orgs/community/discussions/7835 https://github.com/orgs/community/discussions/25227 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.