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

Animation: Silence false positive -Wstringop-overflow warning #58755

Conversation

akien-mga
Copy link
Member

@akien-mga akien-mga commented Mar 4, 2022

And disable debug code which was wrongly left enabled.

@marxin I don't know if that's an expected false positive from GCC, or a situation which is still risky enough (while being correct in this case) to warrant warning just in case. I reduced it to this:

#include <cstdint>
#include <cstdio>

uint32_t some_func(const uint32_t components) {
	uint8_t header[8];
	uint32_t header_bytes = 0;
	for (uint32_t i = 0; i < components; i++) {
		header_bytes += 2;
	}
	header_bytes += 2;
	// This works it around, but shouldn't be needed AFAICT.
	//while (header_bytes != 8 && header_bytes % 4 != 0) {
	while (header_bytes % 4 != 0) {
		header[header_bytes++] = 0;
	}
	for (uint32_t i = 0; i < header_bytes; i++) {
		printf("%d\n", header[i]);
	}
	return header_bytes;
}

int main() {
	some_func(1);
	some_func(3);
	return 0;
}
$ g++ main.cpp -O3 -Wstringop-overflow=2
main.cpp: In function ‘uint32_t some_func(uint32_t)’:
main.cpp:14:40: warning: writing 1 byte into a region of size 0 [-Wstringop-overflow=]
   14 |                 header[header_bytes++] = 0;
      |                 ~~~~~~~~~~~~~~~~~~~~~~~^~~
main.cpp:5:17: note: at offset 8 into destination object ‘header’ of size 8
    5 |         uint8_t header[8];
      |                 ^~~~~~

It doesn't happen with lower levels of optimization (and happens with any level of -Wstringop-overflow=).

And disable debug code which was wrongly left enabled.
@marxin
Copy link
Contributor

marxin commented Mar 4, 2022

I've just reported the false positive here:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104789

Note the warning has non-zero false positive rate :P

@akien-mga akien-mga merged commit f356c8a into godotengine:master Mar 4, 2022
@akien-mga akien-mga deleted the gcc-silence-Wstringop-overflow-false-positive branch March 4, 2022 17:09
@marxin
Copy link
Contributor

marxin commented Mar 7, 2022

@akien-mga: Can you please reply to https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104789#c3 ?

@akien-mga
Copy link
Member Author

akien-mga commented Mar 7, 2022

@akien-mga: Can you please reply to gcc.gnu.org/bugzilla/show_bug.cgi?id=104789#c3 ?

I'm not familiar with the term "bounded roo". I reduced the test case as much as I could to focus on the minimal code that would trigger the warning which seems to me like a false positive - is it not one?

I can try to include all the relevant code to do marshalling but it seems to me that it will just make the test case more complex - while the minimal one does reproduce an issue with the same symptoms.

@marxin
Copy link
Contributor

marxin commented Mar 7, 2022

All right, so please reply it to the GCC bug.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants