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.

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

Enable testing on newer GCC releases and clean up warnings on them #107

Closed
wants to merge 5 commits into from

Conversation

penguin359
Copy link
Contributor

@penguin359 penguin359 commented Aug 11, 2024

In my earlier PR for enabling build testing automation, I stopped at GCC 9.x because newer releases were triggering warnings and would therefore fail the build since -Werror was in effect. This PR adds in those versions and fixes all warnings up through GCC 14.x.

The main issue is the call to strncpy() in the STRNCPY_TERMINATED macro triggers GCC's concerns over a possible non-terminated string even though a quick analysis will show that it is indeed terminated. This warning is present up through at least GCC 14.x. I initially tried to replace the macro with a call to strlcpy() which is a safe version of strncpy(), but that is too new for many systems so I have the warning silenced inside the macro on newer GCC versions. There are no more warnings on any version of GCC tested now.

  • Run all matrix combinations even if some fail
  • Move to a non-deprecated checkout version
  • Added newer GCC versions for test
  • Added missing build dependencies
  • Use the safe strlcpy() when present
  • Make macro safer when strlcpy() is not available
  • Disable invalid warning about use of strncpy()

Also, ignore an auto-generated file made by newer automake releases.

Enable all warnings for build.

Move to non-deprecated checkout version.
Added missing dependencies
The current macro, which uses strncpy() and then forces a valid
termination, triggers warnings under GCC 10. It can also cause an
out-of-bounds write if size is 0.

strlcpy() was added in glibc 2.38 (July 2023).
Macro would previously trigger an out-of-bound write if passed a size of
zero. Also, the size was not protected against a possible operator
precedence issue.
GCC 10+ is producing a warning that strncpy() may truncate without valid
nul termination, however, the code is easily checked that nul
termination is added immediately afterwards.
@penguin359 penguin359 changed the title test newer gcc Enable testing on newer GCC releases and clean up warnings on them Aug 11, 2024
@penguin359
Copy link
Contributor Author

Hmm, I guess since I am a contributor outside the project modifying the GitHub workflow, it won't automatically run the workflow without approval. I suppose that makes sense, but I was expecting it would just run. In any case, is pretty quick to run the checks once approved.

@penguin359
Copy link
Contributor Author

penguin359 commented Aug 12, 2024

After reviewing the build logs from the -Wstringop-truncation warning I tried to silence in my commit ede7ee3, I see that it was actually trying to warn me about the issue with the fixed-sized string string arrays where the source was IFNAMSIZ+1 and the destination was only IFNAMSIZ. That commit should probably be backed out and the warning should go away once issue #108 is fixed. Interestingly, while GCC 10.x seems to detect this with just a -Wall, it looks like GCC 9.x can trigger the warning, but only with ASAN enabled and using the fortified strncpy(). The build logs showing the warning are here:

https://github.com/penguin359/openlldp/actions/runs/10337712563/job/28614883097#step:8:153

@apconole
Copy link
Contributor

Applied, thanks!

@apconole apconole closed this Aug 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants