On Wed, Jun 12, 2024 at 03:11:30PM -0700, Junio C Hamano wrote:
> By the way, I do not know if any compiler gives us such a feature,
> but if the trick to squelch a false positive were finer grained, I
> would have been much more receptive to the idea of building with
> different optimization level, allowing a bit more false positives.
>
> The workaround everybody jumps at is to initialize the variable to a
> meaningless value (like 0) and I have explained why it is suboptimal
> already. But if we can tell less intelligent compilers "we know our
> use of this variable AT THIS POINT is safe", e.g. by annotating the
> above snippet of the code, perhaps like this:
>
> if (ret) {
> if (data)
> /* -Wno-uninitialized (mtimes_size) */
> munmap(data, mtimes_size);
> printf("debug %d\n", (int)mtimes_size);
>
> then it would be clear to the compiler that understand the
> annotation that inside that "if (data)" block, we know that
> the use of mtimes_size is not using an uninitialized variable.
>
> For the use of the same variable on the next "debug" line, because
> it is outside of that "if (data)" block, the annotation should have
> no effect, and the compiler is free to do its own analysis and we
> will accept if it finds mtimes_size can be used uninitialized there.
> Any new use added for the same variable will not be masked by a
> meaningless initialization if we can use such a "workaround" to
> squelch false positives.
I agree that such an annotation is much more focused. It's still not
foolproof, though (e.g., we might chance earlier code so that the
data/mtimes_size correlation is no longer true).
I think you could do it with:
if (data)
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wuninitialized"
munmap(data, mtimes_size);
#pragma GCC diagnostic pop
which is...ugly. There's a _Pragma() operator, too, which I think would
let you make a macro like:
if (data)
SUPPRESS("-Wuninitialized", munmap(data, mtimes_size));
which is maybe slightly less horrific? Still pretty magical though.
But if the alternative is to do none of that, and just continue to avoid
looking for warnings with -Os, I prefer that.
-Peff
