[LU-16518] Fix Clang build errors Created: 01/Feb/23 Updated: 29/Nov/23 Resolved: 29/Nov/23 |
|
| Status: | Resolved |
| Project: | Lustre |
| Component/s: | None |
| Affects Version/s: | None |
| Fix Version/s: | Lustre 2.16.0 |
| Type: | Bug | Priority: | Minor |
| Reporter: | Tim Day | Assignee: | Tim Day |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | None | ||
| Environment: |
Clang 15.0.6 |
||
| Attachments: |
sanity-flr.test_42.debug_log.tmp.1679582991.log.bz2
serial.txt
stdout.txt
|
||||||||
| Issue Links: |
|
||||||||
| Severity: | 3 | ||||||||
| Rank (Obsolete): | 9223372036854775807 | ||||||||
| Description |
|
I've been experimenting with using Clang 15.0.6 to compile Lustre. The compiler runs into a number of build issues. Clang seems to be stricter than GCC in a few cases. Lustre can still be compiled by disabling the errors [1], but a first glance the errors seem legitimate. I'm making this ticket because I have a patch to fix some of the failures. It seems like some Clang-based static analyzers were used in the past [2], although I'm not sure if compiling Lustre with Clang is super common. I think it could be useful to have Clang as an option for development purposes, along the lines of ClangBuiltLinux [3]. It's seems like some kind of plugin [4] was developed at some point. It would be for an old (3.0) Clang, but it could be interesting to look at.
[1] Errors that needed to be disabled:
-Wno-error=enum-conversion -Wno-error=unused-but-set-variable -Wno-error=uninitialized -Wno-error=format -Wno-error=strncat-size -Wno-error=switch -Wno-error=strlcpy-strlcat-size -Wno-error=unneeded-internal-declaration -Wno-error=self-assign -Wno-error=tautological-constant-out-of-range-compare -Wno-error=constant-logical-operand -Wno-error=pointer-bool-conversion -Wno-error=unused-function -Wno-error=parentheses-equality -Wno-error=deprecated-non-prototype [2] Old Clang tickets: https://jira.whamcloud.com/browse/LU-871 https://jira.whamcloud.com/browse/LU-2675 [3] Clang/Linux: https://clangbuiltlinux.github.io/ [4] Plugin: https://wiki.whamcloud.com/pages/viewpage.action?pageId=18645101
|
| Comments |
| Comment by Gerrit Updater [ 01/Feb/23 ] |
|
"Timothy Day <timday@amazon.com>" uploaded a new patch: https://review.whamcloud.com/c/fs/lustre-release/+/49859 |
| Comment by Gerrit Updater [ 03/Feb/23 ] |
|
"Timothy Day <timday@amazon.com>" uploaded a new patch: https://review.whamcloud.com/c/fs/lustre-release/+/49901 |
| Comment by Gerrit Updater [ 06/Feb/23 ] |
|
"Timothy Day <timday@amazon.com>" uploaded a new patch: https://review.whamcloud.com/c/fs/lustre-release/+/49916 |
| Comment by Gerrit Updater [ 18/Feb/23 ] |
|
"Timothy Day <timday@amazon.com>" uploaded a new patch: https://review.whamcloud.com/c/fs/lustre-release/+/50063 |
| Comment by Gerrit Updater [ 01/Mar/23 ] |
|
"Timothy Day <timday@amazon.com>" uploaded a new patch: https://review.whamcloud.com/c/fs/lustre-release/+/50161 |
| Comment by Gerrit Updater [ 01/Mar/23 ] |
|
"Timothy Day <timday@amazon.com>" uploaded a new patch: https://review.whamcloud.com/c/fs/lustre-release/+/50162 |
| Comment by Gerrit Updater [ 01/Mar/23 ] |
|
"Oleg Drokin <green@whamcloud.com>" merged in patch https://review.whamcloud.com/c/fs/lustre-release/+/49916/ |
| Comment by Gerrit Updater [ 08/Mar/23 ] |
|
"Oleg Drokin <green@whamcloud.com>" merged in patch https://review.whamcloud.com/c/fs/lustre-release/+/49859/ |
| Comment by Gerrit Updater [ 16/Mar/23 ] |
|
"Timothy Day <timday@amazon.com>" uploaded a new patch: https://review.whamcloud.com/c/fs/lustre-release/+/50318 |
| Comment by Gerrit Updater [ 19/Mar/23 ] |
|
"Timothy Day <timday@amazon.com>" uploaded a new patch: https://review.whamcloud.com/c/fs/lustre-release/+/50332 |
| Comment by Gerrit Updater [ 21/Mar/23 ] |
|
"Oleg Drokin <green@whamcloud.com>" merged in patch https://review.whamcloud.com/c/fs/lustre-release/+/50161/ |
| Comment by Gerrit Updater [ 21/Mar/23 ] |
|
"Oleg Drokin <green@whamcloud.com>" merged in patch https://review.whamcloud.com/c/fs/lustre-release/+/50162/ |
| Comment by Alex Zhuravlev [ 22/Mar/23 ] |
|
this is very strange but have a look at sanity-flr/42 bisection: COMMIT TESTED PASSED FAILED COMMIT DESCRIPTION 25a53cdc80 1 0 1 BAD LU-16491 lfs: test getdirstripe YAML 85b76aa91a 1 0 1 BAD LU-16610 ldiskfs: fix directory corruption on openeuler 22.03 5772650cc5 1 0 1 BAD LU-16509 lnet: memcpy false positive in brw_test b5365d5e00 1 0 1 BAD LU-16518 libcfs: fix clang build errors 632dc6729a 1 0 1 BAD LU-16518 utils: fix clang build errors 764e19186c 10 10 0 GOOD LU-16603 protocol: add OBD_BRW_COMPRESSED 25c6b7ad28 10 10 0 GOOD LU-16589 tests: fix hard-link failure in sanityn/55d 23224e03dc 10 10 0 GOOD LU-16587 utils: give lfs migrate a larger buffer 2bdff20f97 10 10 0 GOOD LU-16586 build: Remove old check for linux_selinux_enabled 404a1e827b 10 10 0 GOOD LU-16626 build: remove python2 dependencies |
| Comment by Alex Zhuravlev [ 22/Mar/23 ] |
|
if I revert |
| Comment by Tim Day [ 22/Mar/23 ] |
|
I can't seem to find a failure in Maloo, and I can't reproduce the error locally: ... lt-lfs mirror verify: '/mnt/lustre/d42.sanity-flr/f42.sanity-flr-1' chunk [0x800000, 0xa00000) is only valid in mirror 2: skipped lt-lfs mirror verify: '/mnt/lustre/d42.sanity-flr/f42.sanity-flr-1' chunk [0xa00000, 0x1000000) is only valid in mirror 2: skipped lt-lfs mirror verify: '/mnt/lustre/d42.sanity-flr/f42.sanity-flr-1' chunk [0x1000000, 0xffffffffffffffff) exceeds file size 0xa00002: skipped PASS 42 (1s) == sanity-flr test complete, duration 2 sec ============== 19:07:38 (1679512058) sanity-flr returned 0 Finished at Wed Mar 22 19:07:38 UTC 2023 in 3s ./auster: completed with rc 0 Do you have some logs of the test failing that I could look at? Or some advice for reproducing the failure? |
| Comment by Tim Day [ 23/Mar/23 ] |
|
Thanks for the logs. I suspect it's related to the change in liblustreapi_layout.c, specifically "uint64_t mirror_end = LUSTRE_EOF;" Initializing that variable to "0" caused some failures for sanity-flr (https://testing.whamcloud.com/test_sets/09898346-75f3-4364-807b-dfe4c2d84fef). The tests passed in Maloo using "LUSTRE_EOF", but could be causing a failure for your setup. Could you try reverting just that line? If that works, then that code might need to be refactored a bit. I'm still trying to reproduce on my end. |
| Comment by Alex Zhuravlev [ 25/Mar/23 ] |
you're right, that helped and sanity-flr/42 passes with that specific change reverted. |
| Comment by Tim Day [ 27/Mar/23 ] |
|
It's strange that sanity-flr/42 is unhappy when `mirror_end` get initialized. I'll try to put together a patch/small refactor to fix this. |
| Comment by Gerrit Updater [ 04/Apr/23 ] |
|
"Oleg Drokin <green@whamcloud.com>" merged in patch https://review.whamcloud.com/c/fs/lustre-release/+/49901/ |
| Comment by Gerrit Updater [ 11/Apr/23 ] |
|
"Oleg Drokin <green@whamcloud.com>" merged in patch https://review.whamcloud.com/c/fs/lustre-release/+/50318/ |
| Comment by Gerrit Updater [ 14/Jun/23 ] |
|
"Timothy Day <timday@amazon.com>" uploaded a new patch: https://review.whamcloud.com/c/fs/lustre-release/+/51311 |
| Comment by Gerrit Updater [ 28/Jun/23 ] |
|
"Oleg Drokin <green@whamcloud.com>" merged in patch https://review.whamcloud.com/c/fs/lustre-release/+/50332/ |
| Comment by Gerrit Updater [ 08/Jul/23 ] |
|
"Oleg Drokin <green@whamcloud.com>" merged in patch https://review.whamcloud.com/c/fs/lustre-release/+/50063/ |
| Comment by Gerrit Updater [ 08/Jul/23 ] |
|
"Oleg Drokin <green@whamcloud.com>" merged in patch https://review.whamcloud.com/c/fs/lustre-release/+/51311/ |
| Comment by Gerrit Updater [ 03/Oct/23 ] |
|
"Timothy Day <timday@amazon.com>" uploaded a new patch: https://review.whamcloud.com/c/fs/lustre-release/+/52565 |
| Comment by Gerrit Updater [ 03/Oct/23 ] |
|
"Timothy Day <timday@amazon.com>" uploaded a new patch: https://review.whamcloud.com/c/fs/lustre-release/+/52566 |
| Comment by Gerrit Updater [ 08/Nov/23 ] |
|
"Oleg Drokin <green@whamcloud.com>" merged in patch https://review.whamcloud.com/c/fs/lustre-release/+/52565/ |
| Comment by Gerrit Updater [ 17/Nov/23 ] |
|
"Timothy Day <timday@amazon.com>" uploaded a new patch: https://review.whamcloud.com/c/fs/lustre-release/+/53174 |
| Comment by Gerrit Updater [ 18/Nov/23 ] |
|
"Oleg Drokin <green@whamcloud.com>" merged in patch https://review.whamcloud.com/c/fs/lustre-release/+/52566/ |
| Comment by Gerrit Updater [ 29/Nov/23 ] |
|
"Oleg Drokin <green@whamcloud.com>" merged in patch https://review.whamcloud.com/c/fs/lustre-release/+/53174/ |
| Comment by Peter Jones [ 29/Nov/23 ] |
|
Landed for 2.16 |