Fix Warnings #170

insertinterestingnamehere · 2023-11-02T16:22:58Z

No description provided.

insertinterestingnamehere · 2023-11-09T23:41:22Z

Okay, this one's ready to go. It resolves all warnings enabled in the "picky" build except for those involved in #173. Once that's resolved too we can enable -Werror to avoid regressions here. I haven't looked at anything from -Wextra yet.

src/fastcontext/context.c

insertinterestingnamehere · 2023-11-09T23:53:16Z

Worth noting: this doesn't fix all the warnings in the config scripts since a lot of those warnings are inherited from stuff happening upstream with autotools. I just fixed the warnings internal to the configure scripts that have obvious fixes and come from files in qthreads that aren't vendored in from somewhere else.

Some of the warnings are either inherited from autotoools, inherited from files vendored from upstream, or unavoidable given the nature of the test. This takes care of warnings that don't fall into those categories though.

… versions of gcc.

…stead of pointer arithmetic.

janciesko

Regarding replacing the INTERNAL macro - is the function attribute not valid for all compilers? If we would like to restrict the visibility to hidden for dynamic linkage via static, we might consider changing the macro implementation instead.

janciesko · 2023-11-28T22:10:53Z

src/envariables.c


-        snprintf(dflt_str, 10, "%lu", dflt);
+        snprintf(dflt_str, 21, "%lu", dflt);


The max length of an unsigned long is 20 digits. snprintf needs one more character than the max size, so 21.

janciesko · 2023-11-28T22:17:09Z

src/fastcontext/context.c

@@ -71,7 +65,7 @@ void INTERNAL qt_makectxt(uctxt_t *ucp,
    /* HOWEVER, the function may not be expecting to pull from the stack,
     * several 64-bit architectures expect that args will be in the correct
     * registers! */
-    ucp->mc.mc_edi = va_arg(argp, uintptr_t);
+    ucp->mc.mc_edi = *(uintptr_t*)sp;


This makes more sense to me. Thanks for cleaning it up!

insertinterestingnamehere · 2023-11-29T17:04:20Z

Regarding replacing the INTERNAL macro - is the function attribute not valid for all compilers? If we would like to restrict the visibility to hidden for dynamic linkage via static, we might consider changing the macro implementation instead.

I'm not actually changing the INTERNAL macro here. What I did do is move some functions that were declared as INTERNAL to being static instead since they weren't used anywhere else. In a lot of the cases this was the simplest way to clear up the missing prototypes warnings.

insertinterestingnamehere · 2023-11-29T17:09:34Z

To clarify a bit more: there are a few places the INTERNAL macro is still needed and used. I don't see the need to change it. It's what's needed for specifying functions that we want to be internal to our .so but that need to be linked across translation units. The functions I changed to static aren't used across translation units though so making them static made more sense to get rid of the function prototype warnings (as opposed to doing some kind of forward declaration that allows them to be used elsewhere).

janciesko · 2023-11-29T18:40:27Z

Ok, thanks, in that case, would it make sense to add a new macro, maybe FUNCTION_IS_UNUSED that expands to the static qualifier and using that in those functions?

insertinterestingnamehere · 2023-11-30T16:27:52Z

I think static really is the correct thing here. These functions are used, but only within their translation unit. If they were completely unused I'd just remove them entirely. In theory we could do a macro for translation-unit local linkage, but the extra layer of indirection doesn't really give us anything in this case. Across C implementations that's consistently what static does for functions anyway.

janciesko · 2023-11-30T19:19:34Z

Agreed

janciesko

LGTM

insertinterestingnamehere · 2023-11-30T19:48:57Z

Thank you!

insertinterestingnamehere marked this pull request as draft November 2, 2023 17:18

insertinterestingnamehere force-pushed the warnings branch 5 times, most recently from 3777949 to 39b6ae6 Compare November 7, 2023 22:31

insertinterestingnamehere mentioned this pull request Nov 7, 2023

Stackleft Debugging #176

Merged

insertinterestingnamehere force-pushed the warnings branch 4 times, most recently from e89ac9d to a9d1dc0 Compare November 9, 2023 23:38

insertinterestingnamehere marked this pull request as ready for review November 9, 2023 23:39

insertinterestingnamehere commented Nov 9, 2023

View reviewed changes

src/fastcontext/context.c Outdated Show resolved Hide resolved

insertinterestingnamehere force-pushed the warnings branch from a9d1dc0 to 69a51f4 Compare November 9, 2023 23:55

insertinterestingnamehere requested a review from olivier-snl November 9, 2023 23:55

insertinterestingnamehere mentioned this pull request Nov 10, 2023

Debugging MUSL Libc #179

Merged

insertinterestingnamehere added 6 commits November 10, 2023 11:29

Clear warnings.

8a550ff

Fix warnings about bad function prototypes.

ec6a25d

Fix some warnings and errors inside the configure checks.

bdfbd17

Some of the warnings are either inherited from autotoools, inherited from files vendored from upstream, or unavoidable given the nature of the test. This takes care of warnings that don't fall into those categories though.

Suppress erroneous warning about array bounds that shows up with some…

2ef222f

… versions of gcc.

Suppress warning about icc getting deprecated.

1cfa2ff

Resolve gcc out of bounds warning by using va_arg to acces varargs in…

8b9d3e3

…stead of pointer arithmetic.

insertinterestingnamehere force-pushed the warnings branch from c2db460 to 8b9d3e3 Compare November 10, 2023 19:29

insertinterestingnamehere mentioned this pull request Nov 14, 2023

Fix Undefined Behavior Sanitizer Errors #182

Merged

janciesko requested changes Nov 28, 2023

View reviewed changes

janciesko reviewed Nov 28, 2023

View reviewed changes

janciesko approved these changes Nov 30, 2023

View reviewed changes

insertinterestingnamehere merged commit d6ce514 into sandialabs:main Nov 30, 2023
128 of 297 checks passed

insertinterestingnamehere deleted the warnings branch February 2, 2024 15:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix Warnings #170

Fix Warnings #170

insertinterestingnamehere commented Nov 2, 2023

insertinterestingnamehere commented Nov 9, 2023

insertinterestingnamehere commented Nov 9, 2023

janciesko left a comment

janciesko Nov 28, 2023

insertinterestingnamehere Nov 28, 2023

janciesko Nov 28, 2023 •

edited

Loading

insertinterestingnamehere commented Nov 29, 2023

insertinterestingnamehere commented Nov 29, 2023

janciesko commented Nov 29, 2023

insertinterestingnamehere commented Nov 30, 2023

janciesko commented Nov 30, 2023

janciesko left a comment

insertinterestingnamehere commented Nov 30, 2023


		snprintf(dflt_str, 10, "%lu", dflt);
		snprintf(dflt_str, 21, "%lu", dflt);

Fix Warnings #170

Fix Warnings #170

Conversation

insertinterestingnamehere commented Nov 2, 2023

insertinterestingnamehere commented Nov 9, 2023

insertinterestingnamehere commented Nov 9, 2023

janciesko left a comment

Choose a reason for hiding this comment

janciesko Nov 28, 2023

Choose a reason for hiding this comment

insertinterestingnamehere Nov 28, 2023

Choose a reason for hiding this comment

janciesko Nov 28, 2023 • edited Loading

Choose a reason for hiding this comment

insertinterestingnamehere commented Nov 29, 2023

insertinterestingnamehere commented Nov 29, 2023

janciesko commented Nov 29, 2023

insertinterestingnamehere commented Nov 30, 2023

janciesko commented Nov 30, 2023

janciesko left a comment

Choose a reason for hiding this comment

insertinterestingnamehere commented Nov 30, 2023

janciesko Nov 28, 2023 •

edited

Loading