CA-407106: fix various XHA warnings from the Clang static analyzer#22
Merged
GeraldEV merged 9 commits intoxenserver:masterfrom Mar 5, 2025
Merged
Conversation
edwintorok
force-pushed
the
private/edvint/warnings
branch
from
bc5b68a to
8c4fc0b
Compare
Collaborator
|
A description would be nice |
freddy77
reviewed
Feb 24, 2025
freddy77
reviewed
Feb 24, 2025
Collaborator
|
Can you clean the binary files? |
Contributor
Author
Thanks for spotting that, looks like |
freddy77
reviewed
Feb 24, 2025
Comment on lines
8
to
9
| compile_commands.json | ||
| .cache/ |
Collaborator
There was a problem hiding this comment.
I would avoid adding specific tool settings.
edwintorok
force-pushed
the
private/edvint/warnings
branch
2 times, most recently
from
c8347fc to
d7919a1
Compare
rosslagerwall
approved these changes
Mar 5, 2025
Collaborator
rosslagerwall
left a comment
rosslagerwall
left a comment
There was a problem hiding this comment.
A few minor comments, but this generally seems fine.
GeraldEV
reviewed
Mar 5, 2025
GeraldEV
approved these changes
Mar 5, 2025
edwintorok
force-pushed
the
private/edvint/warnings
branch
from
dbbe629 to
0f69670
Compare
Signed-off-by: Edwin Török <edwin.torok@cloud.com>
`sm_fence` didn't declare what arguments it takes, but this is deprecated. ``` ../include/sm.h:368:1: warning: a function declaration without a prototype is deprecated in all versions of C and is treated as a zero-parameter prototype in C23, conflicting with a subsequent definition [-Wdeprecated-non-prototype] sm.c:530:1: note: conflicting prototype is here ``` Signed-off-by: Edwin Török <edwin.torok@cloud.com>
Nested functions are a GCC-only extension and are not supported by clang and clangd at all. Signed-off-by: Edwin Török <edwin.torok@cloud.com>
Use `sigaction` and `sigprocmask` instead. There is also `signal`, but the manpage says to not use it. Indeed `sigaction` has more reliable semantics by default in POSIX (e.g. it blocks the signal while you are handling it, so you don't reenter it while it is executing). Unfortunately the new functions are more verbose, so introduce a loop for setting up the signal handlers. Signed-off-by: Edwin Török <edwin.torok@cloud.com>
These are either set again later to another value, or never read. Signed-off-by: Edwin Török <edwin.torok@cloud.com>
The code always gets 10 return addresses, however we can't know for sure that we actually have 10 addresses on the stack.
We might try to read beyond the stack and crash.
The compiler warns about this:
```
log.c:452:9: warning: calling ‘__builtin_frame_address’ with a nonzero argument is unsafe [-Wframe-address]
452 | if (__builtin_frame_address(x) == 0) \
| ^~~~~~~~~~~~~~~~~~~~~~~~~~
log.c:471:5: note: in expansion of macro ‘FILL_RETURN_ADDRESS’
```
Instead use a GNU libc specific function: backtrace and backtrace_symbols to get a stacktrace safely.
This is a short live program and would close it on exit anyway, but we don't want to ignore this kind of error in the rest of the program. Signed-off-by: Edwin Török <edwin.torok@cloud.com>
If a timeout is reached, then the loop is never entered, and 'index' is uninitialized. All the logging calls would use an uninitialized value, and later `my_sfdomain` would also be uninitialized and various decisions would be taken based on its state. Ensure that these variables are initialized at least once in the loop by moving the timeout condition. It is very unlikely that this'd actually happen, but without this change the static analyzer wouldn't be able to spot other bugs that'd leave these values uninitialized. Signed-off-by: Edwin Török <edwin.torok@cloud.com>
Also disable warnings we can't/don't want to fix now: * clang-diagnostic-reserved-macro-identifier The bug is in Xen, it mandates using `__XEN_TOOLS__` in `sysctl.h` Signed-off-by: Edwin Török <edwin.torok@cloud.com>
edwintorok
force-pushed
the
private/edvint/warnings
branch
from
0f69670 to
8944196
Compare
GeraldEV
approved these changes
Mar 5, 2025
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
4 participants
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.
Fixes for:
log_messagetakes a format string as argument