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.

Sign up for GitHub

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

Jump to bottom

static analyzer fixes #352

Merged
merged 11 commits into from
Apr 11, 2024
Merged

static analyzer fixes #352

merged 11 commits into from
Apr 11, 2024

Conversation

ghost
Copy link

@ghost ghost commented Feb 22, 2024 •
edited by ghost
Loading

Description of changes:

a15e2c7 nit: silence clangsa warnings on pointer mismatch
ff22381 nit: remove extraneous assignment to self
7ae42c7 nit: test: unit: fix early-return leaks
2eacffd fix: net: strlen(NULL) is UB
9e79a7f fix: net: get_provider: always set return count
1cc9939 fix: topo: dont ignore errno
fe829de fix: api: initialize ofi_accept return value

Fixes various reports found in clang-static-analyzer/CodeChecker reporting. Others remain.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@ghost ghost force-pushed the clangsa-fixes branch from a15e2c7 to e413ee4 Compare February 22, 2024 23:11
@ghost ghost marked this pull request as ready for review February 22, 2024 23:11
@ghost ghost requested review from rauteric, bwbarrett and a team as code owners February 22, 2024 23:11
bwbarrett
bwbarrett requested changes Feb 22, 2024
View reviewed changes
@AvivBenchorin AvivBenchorin added BuildTriggerRequest CI build will be triggered when this label is set and removed BuildTriggerRequest CI build will be triggered when this label is set labels Mar 3, 2024
@ghost ghost force-pushed the clangsa-fixes branch from e413ee4 to f18784d Compare March 11, 2024 21:19
@ghost ghost requested a review from rajachan as a code owner March 11, 2024 21:19
@ghost ghost force-pushed the clangsa-fixes branch 2 times, most recently from 0f501a1 to 9b5f632 Compare March 11, 2024 23:25
@AvivBenchorin AvivBenchorin added BuildTriggerRequest CI build will be triggered when this label is set and removed BuildTriggerRequest CI build will be triggered when this label is set labels Mar 12, 2024
rauteric
rauteric reviewed Mar 21, 2024
View reviewed changes
@ghost ghost force-pushed the clangsa-fixes branch from 9b5f632 to bd25ee9 Compare March 21, 2024 19:19
@rajachan rajachan added BuildTriggerRequest CI build will be triggered when this label is set and removed BuildTriggerRequest CI build will be triggered when this label is set labels Mar 21, 2024
@ghost ghost force-pushed the clangsa-fixes branch from bd25ee9 to fb671d1 Compare April 3, 2024 20:24
@rajachan
Copy link
Member

rajachan commented Apr 9, 2024

CC nccl_ofi_rdma.lo
nccl_ofi_net.c:389:3: error: use of undeclared identifier 'dev_props'
dev_props.mr_scope = NCCL_OFI_MR_SCOPE_ENDPOINT;
^
nccl_ofi_net.c:392:3: error: use of undeclared identifier 'dev_props'
dev_props.mr_scope = NCCL_OFI_MR_SCOPE_DOMAIN;
^
2 errors generated.

Build failures look real.

@ghost ghost force-pushed the clangsa-fixes branch from fb671d1 to 3437c72 Compare April 9, 2024 22:43
Nicholas Sielicki added 3 commits April 9, 2024 15:47
fix(api): initialize ofi_accept return value
50a56a3 
when *recvComm is NULL here, the compiler was under no obligation to
initialize this to any particular value, we must initialize it
explicitly.
fix(topo): dont ignore errno
2a24ca8 
if errno is indicated here, the iterator referenced further down
contains garbage. Exit early when errno < 0.
fix(net): strlen(NULL) is UB
f9039a3 
strlen(NULL) is undefined and may not necessarily return 0. Check
whether the pointer is set before calling strlen here.
@ghost ghost force-pushed the clangsa-fixes branch 2 times, most recently from aad0bb5 to b6d4e88 Compare April 9, 2024 22:57
bwbarrett
bwbarrett previously approved these changes Apr 10, 2024
View reviewed changes
@rajachan rajachan added BuildTriggerRequest CI build will be triggered when this label is set and removed BuildTriggerRequest CI build will be triggered when this label is set labels Apr 10, 2024
Nicholas Sielicki added 5 commits April 10, 2024 14:16
fix: sendrecv: validate user mr_key_size
5425586 
The cast to size_t here was useless because it was performed on the
shift product, rather than the inputs, so this did not widen it.

Fix this casting issue and also check that we would not shift beyond a
size_t (error-out instead).
fix: make nccl_net_ofi_query_provider_capabilities const-correct
69053f0
fix: ofiutils: always write num_prov_infos
cf67fb9
fix(topo): use correct size_t format specifier
205bbdb
build: .gitignore: add clion folder
2652878
@ghost ghost force-pushed the clangsa-fixes branch from b6d4e88 to 2652878 Compare April 10, 2024 21:18
rajachan
rajachan approved these changes Apr 11, 2024
View reviewed changes
@rajachan rajachan merged commit 883d7c7 into aws:master Apr 11, 2024
7 checks passed
@ghost ghost deleted the clangsa-fixes branch May 15, 2024 05:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
BuildTriggerRequest CI build will be triggered when this label is set
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@rajachan @rauteric @bwbarrett @AvivBenchorin