-
Notifications
You must be signed in to change notification settings - Fork 306
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
DAOS-16501 build: Add libsanitize #15105
base: master
Are you sure you want to change the base?
Changes from all commits
2d97760
8d233f5
ce48861
b398459
fbc4faa
7e9f339
e7e8139
430daf6
bf62474
3e7f272
f66748c
4f31826
472d178
35446b7
409fc8d
afc358a
9a4a358
ec60363
343449a
e2a272b
1bbf2b1
9535e1c
df2675b
6b25da9
192f66d
9ef5a08
2566572
0011ba7
d81556c
4fb5e2e
5daf2d6
e5e647f
8453507
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,9 @@ | ||
daos (2.7.101-4) unstable; urgency=medium | ||
[ Cedric Koch-Hofer] | ||
* Add support of the libasan | ||
|
||
-- Cedric Koch-Hofer <[email protected]> Mon, 20 Jan 2025 14:12:00 -0700 | ||
|
||
daos (2.7.101-3) unstable; urgency=medium | ||
[ Jeff Olivier ] | ||
* Switch from libfuse3 to libfused | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -122,6 +122,57 @@ scons reduces the clutter from compiler setup. | |
Additionally, the tool supports options to filter by directory and file names and specify a lower | ||
bound value to report. | ||
|
||
### Address Sanitizer | ||
|
||
To debug memory corruptions, leaks, and other issues, DAOS can be compiled with the | ||
[AddressSanitizer (ASan)](https://github.com/google/sanitizers/wiki/AddressSanitizer) library. | ||
|
||
#### Building with ASan | ||
|
||
To build instrumented libraries, executables, RPMs, etc., use the `SANITIZERS=address` flag with the | ||
`scons` command. This option is also managed by the CI system through a Jenkins configuration | ||
variable. | ||
|
||
ASan is supported with both Clang and GCC compilers. However, some compiler-specific configurations, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should do both. I will add this info int the
|
||
such as maximum function stack size, may lead to unexpected behavior in the instrumented | ||
binaries. | ||
|
||
#### Customizing ASan Behavior | ||
ASan behavior can be configured using the `ASAN_OPTIONS` environment variable. For example, you can | ||
add the following entry to the `env_vars` section of the `daos_server.yml` configuration file: | ||
```yaml | ||
engines: | ||
- .. | ||
env_vars: | ||
... | ||
- ASAN_OPTIONS=atexit=1:print_stats=1:log_path=/tmp/daos_engine0.asan:disable_coredump=1:handle_segv=2:handle_abort=2:handle_sigfpe=2:handle_sigill=2:handle_sigbus=2:use_sigaltstack=1 | ||
``` | ||
For detailed information, see the | ||
[Sanitizer Common Flags](https://github.com/google/sanitizers/wiki/SanitizerCommonFlags) and | ||
[AddressSanitizer Flags](https://github.com/google/sanitizers/wiki/AddressSanitizerFlags) | ||
documentation pages. | ||
|
||
#### Known Issues | ||
|
||
ASan support in DAOS is still experimental and has the following known issues: | ||
1. **Library Path Issues** | ||
ASan's instrumentation of `dlopen()` removes `RPATH` and `RUNPATH` entries passed to the | ||
compiler. As a result, libraries like `librdb` may not be found if DAOS is built with SCons and | ||
installed in a non-standard location. | ||
**Workaround**: Use the `LD_LIBRARY_PATH` environment variable or the `ldconfig` command. | ||
More details are available in [Bug 27790](https://bugs.llvm.org/show_bug.cgi?id=27790). | ||
|
||
2. **Missing Statistics on SIGKILL** | ||
Stopping an application with `kill -s SIGKILL` prevents ASan from printing statistics, such as | ||
detected memory leaks. For instance, this occurs when stopping DAOS engines using: | ||
```bash | ||
dmg system stop --force | ||
``` | ||
|
||
3. **Integration with Test Framework** | ||
ASan is not fully integrated with the DAOS regression testing framework. Some tests, such as | ||
those using Valgrind, may fail due to false positives. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To be honest, I have not dive too much into this but it seems reasonable to think that they will not be well interact. At this time, I am sure that the unit test are miss interpreting the Asan report output. I have planned to do this in the follow-up PR.
|
||
|
||
## Go dependencies | ||
|
||
Developers contributing Go code may need to change the external dependencies | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
# Copyright 2016-2024 Intel Corporation | ||
# Copyright 2025 Hewlett Packard Enterprise Development LP | ||
# | ||
# Permission is hereby granted, free of charge, to any person obtaining a copy | ||
|
@@ -277,7 +277,15 @@ | |
abt_build = ['./configure', | ||
'--prefix=$ARGOBOTS_PREFIX', | ||
'CC=gcc', | ||
'--enable-stack-unwind'] | ||
'--enable-stack-unwind=yes'] | ||
try: | ||
if reqs.get_env('SANITIZERS') != "": | ||
# NOTE the address sanitizer library add some extra info on the stack and thus ULTs | ||
# need a bigger stack | ||
print("Increase argobots default stack size from 16384 to 32768") | ||
abt_build += ['--enable-default-stacksize=32768'] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point, I will try to find a way to increase It when we are using the libasan.
|
||
except KeyError: | ||
pass | ||
|
||
if reqs.target_type == 'debug': | ||
abt_build.append('--enable-debug=most') | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,6 @@ | ||
/** | ||
* (C) Copyright 2018-2024 Intel Corporation. | ||
* (C) Copyright 2025 Hewlett Packard Enterprise Development LP | ||
* | ||
* SPDX-License-Identifier: BSD-2-Clause-Patent | ||
*/ | ||
|
@@ -83,7 +84,7 @@ | |
dattr.da_chunk_size = DFS_DEFAULT_CHUNK_SIZE; | ||
|
||
if (attr->da_hints[0] != 0) { | ||
strncpy(dattr.da_hints, attr->da_hints, DAOS_CONT_HINT_MAX_LEN); | ||
memcpy(dattr.da_hints, attr->da_hints, DAOS_CONT_HINT_MAX_LEN - 1); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. . There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
dattr.da_hints[DAOS_CONT_HINT_MAX_LEN - 1] = '\0'; | ||
} | ||
} else { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,6 @@ | ||
/** | ||
* (C) Copyright 2018-2024 Intel Corporation. | ||
* (C) Copyright 2025 Hewlett Packard Enterprise Development LP | ||
* | ||
* SPDX-License-Identifier: BSD-2-Clause-Patent | ||
*/ | ||
|
@@ -510,10 +511,11 @@ | |
D_GOTO(err, rc = EINVAL); | ||
} | ||
|
||
strncpy(new_obj->name, obj->name, DFS_MAX_NAME + 1); | ||
new_obj->dfs = dfs; | ||
new_obj->mode = obj->mode; | ||
new_obj->flags = flags; | ||
memcpy(new_obj->name, obj->name, DFS_MAX_NAME); | ||
new_obj->name[DFS_MAX_NAME] = '\0'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. was there a compiler warning for this? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I do not fully understand, but I have this kind of new compiler warning that I struggle to solve:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The better solution will be to replace all
Alternatively, we can ignore this warning here and in all similar places with pragmas:
I see many places in DAOS where |
||
new_obj->dfs = dfs; | ||
new_obj->mode = obj->mode; | ||
new_obj->flags = flags; | ||
oid_cp(&new_obj->parent_oid, obj->parent_oid); | ||
oid_cp(&new_obj->oid, obj->oid); | ||
|
||
|
@@ -616,8 +618,8 @@ | |
oid_cp(&obj_glob->parent_oid, obj->parent_oid); | ||
uuid_copy(obj_glob->coh_uuid, coh_uuid); | ||
uuid_copy(obj_glob->cont_uuid, cont_uuid); | ||
strncpy(obj_glob->name, obj->name, DFS_MAX_NAME + 1); | ||
obj_glob->name[DFS_MAX_NAME] = 0; | ||
memcpy(obj_glob->name, obj->name, DFS_MAX_NAME); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. . There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
obj_glob->name[DFS_MAX_NAME] = '\0'; | ||
if (S_ISDIR(obj_glob->mode)) | ||
return 0; | ||
rc = dfs_get_chunk_size(obj, &obj_glob->chunk_size); | ||
|
@@ -674,7 +676,7 @@ | |
|
||
oid_cp(&obj->oid, obj_glob->oid); | ||
oid_cp(&obj->parent_oid, obj_glob->parent_oid); | ||
strncpy(obj->name, obj_glob->name, DFS_MAX_NAME + 1); | ||
memcpy(obj->name, obj_glob->name, DFS_MAX_NAME); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. . There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
obj->name[DFS_MAX_NAME] = '\0'; | ||
obj->mode = obj_glob->mode; | ||
obj->dfs = dfs; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,6 @@ | ||
/** | ||
* (C) Copyright 2016-2024 Intel Corporation. | ||
* (C) Copyright 2025 Hewlett Packard Enterprise Development LP | ||
* | ||
* SPDX-License-Identifier: BSD-2-Clause-Patent | ||
*/ | ||
|
@@ -118,11 +119,12 @@ | |
|
||
/* Save the old name so that we can invalidate it in later */ | ||
wipe_parent = inode->ie_parent; | ||
strncpy(wipe_name, inode->ie_name, NAME_MAX); | ||
memcpy(wipe_name, inode->ie_name, NAME_MAX); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. . There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
wipe_name[NAME_MAX] = '\0'; | ||
|
||
inode->ie_parent = ie->ie_parent; | ||
strncpy(inode->ie_name, ie->ie_name, NAME_MAX + 1); | ||
memcpy(inode->ie_name, ie->ie_name, NAME_MAX); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. . There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
inode->ie_name[NAME_MAX] = '\0'; | ||
} | ||
atomic_fetch_sub_relaxed(&ie->ie_ref, 1); | ||
dfuse_ie_close(dfuse_info, ie); | ||
|
@@ -295,6 +297,7 @@ | |
DFUSE_TRA_DEBUG(ie, "Attr len is %zi", attr_len); | ||
|
||
strncpy(ie->ie_name, name, NAME_MAX); | ||
ie->ie_name[NAME_MAX] = '\0'; | ||
|
||
dfs_obj2id(ie->ie_obj, &ie->ie_oid); | ||
|
||
|
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 is not
libasan
specific argument.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.