git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH/RFC] reflog: silence -O3 -Wuninitialized warning

From
Junio C Hamano <gitster@pobox.com>
Date
Mar 16, 2011, 05:22 UTC
Message-ID
<7vfwqnabbi.fsf@alter.siamese.dyndns.org>
In-Reply-To
<20110316024959.GA24932@elie>
Jonathan Nieder <jrnieder@gmail.com> writes:
Show 14 quoted lines
> Date: Fri, 04 Mar 2011 00:54:53 -0600
>
> Starting with gcc 4.5 (r147852, Pretty-ipa merge: Inliner heruistics
> reorg, 2009-05-25), gcc -O3 -Wall warns when building
> reflog_expire_config:
>
>     warning: 'expire' may be used uninitialized in this function [-Wuninitialized]
>
> The cause: starting with that version, gcc realizes it can inline the
> call to parse_expire_cfg_value.  In the error case, 'expire' is not
> initialized and the function returns early, but gcc does not have
> enough information to figure out that this is an error return.
> ...
> Anyway, maybe this can provide some amusement.

It actually provides only puzzlement. That is obviously a buggy compiler whose warning cannot be trusted.

I am not suggesting to work this around on the caller side, but I wonder what effect the usual workaround to tell the compiler that we know it doesn't understand the validity of this variable by saying:

	unsigned long expire = expire;
have on this?

We obviously do not want to work the compiler bug around in the callee side by assigning to the variable when we didn't parse anything, but the change to inline config_error_nonbool() to force this particular version of the compiler to see the callchain through is _too_ subtle for my taste. The call-chain horizon the next version of the compiler may stop looking may be different, potentially shifting the compiler bug around.

I see two solutions (1) do not use -Wunitialized if the compiler is known to be broken, or (2) initialize the variable to 0 (not the fake "x = x") on the caller side at the place of the definition (yes, we would waste one real assignment) if we really need to work this around, or (3) take this patch.

We could obviously do (2) or (3), but the thing is, I don't think we can have much confidence on -Wuninitialized warnings from this compiler once we go down that route. Is it _guaranteed_ that the compiler bug _always_ err on the false-positive side?

IOW, I'd very much prefer (1) for this particular case and if somebody really cares (2).

Well, it indeed turned out to be amusing, at least for some definition of the word ;-). I was starting to feel somewhat depressed watching news programs from Japan on ustream.

Previous: Nguyen Thai Ngoc DuyNext: Jonathan Nieder
Message 5 of 34 in “reflog: silence -O3 -Wuninitialized warning”
  1. reflog: silence -O3 -Wuninitialized warningJonathan Nieder, Mar 16, 2011
  2. declare 1-bit bitfields to be unsignedJonathan Nieder, Mar 16, 2011
  3. Junio C HamanoMar 16, 2011
  4. Nguyen Thai Ngoc DuyMar 16, 2011
  5. Junio C HamanoMar 16, 2011
  6. Jonathan NiederMar 16, 2011
  7. Johannes SixtMar 16, 2011
  8. Jonathan NiederMar 16, 2011
  9. Johannes SixtMar 16, 2011
  10. Jonathan NiederMar 16, 2011
  11. 0/6 silence -Wuninitialized warnings that previously used the a = a trickJonathan Nieder, Mar 16, 2011
  12. 1/6 match-trees: kill off remaining -Wuninitialized warningJonathan Nieder, Mar 16, 2011
  13. 2/6 run-command: initialize failed_errno to 0Jonathan Nieder, Mar 16, 2011
  14. 3/6 diff --submodule: suppress -Wuninitialized warning by initializing to NULLJonathan Nieder, Mar 16, 2011
  15. 4/6 rsync transport: clarify insert_packed_refsJonathan Nieder, Mar 16, 2011
  16. 5/6 wt-status: protect against invalid change_typeJonathan Nieder, Mar 16, 2011
  17. 6/6 fast-import: suppress -Wuninitialized warning by initializing to NULLJonathan Nieder, Mar 16, 2011
  18. 0/8 more warnings and cleanupsJonathan Nieder, Mar 16, 2011
  19. 1/8 enums: omit trailing comma for portabilityJonathan Nieder, Mar 16, 2011
  20. 2/8 compat: make gcc bswap an inline functionJonathan Nieder, Mar 16, 2011
  21. Johannes SixtMar 16, 2011
  22. Jonathan NiederMar 16, 2011
  23. Junio C HamanoMar 16, 2011
  24. 3/8 svn-fe: do not use "return" for tail call returning voidJonathan Nieder, Mar 16, 2011
  25. 4/8 vcs-svn: remove spurious semicolonsJonathan Nieder, Mar 16, 2011
  26. Junio C HamanoMar 16, 2011
  27. Jonathan NiederMar 16, 2011
  28. 5/8 standardize brace placement in struct definitionsJonathan Nieder, Mar 16, 2011
  29. Junio C HamanoMar 18, 2011
  30. 6/8 branch: split off function that writes tracking info and commit subjectJonathan Nieder, Mar 16, 2011
  31. 7/8 cherry: split off function to print output linesJonathan Nieder, Mar 16, 2011
  32. 8/8 diff --submodule: split into bite-sized piecesJonathan Nieder, Mar 16, 2011
  33. Jens LehmannMar 16, 2011
  34. Jonathan NiederMar 16, 2011

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.