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

[PATCH 1/2] make error()'s constant return value more visible

From
Jeff King <peff@peff.net>
Date
Dec 15, 2012, 17:37 UTC
Message-ID
<20121215173736.GA22069@sigill.intra.peff.net>
In-Reply-To
<20121215173621.GA21011@sigill.intra.peff.net>

When git is compiled with "gcc -Wuninitialized -O3", some inlined calls provide an additional opportunity for the compiler to do static analysis on variable initialization. For example, with two functions like this:

  int get_foo(int *foo)
  {
	if (something_that_might_fail() < 0)
		return error("unable to get foo");
	*foo = 0;
	return 0;
  }
  void some_fun(void)
  {
	  int foo;
	  if (get_foo(&foo) < 0)
		  return -1;
	  printf("foo is %d\n", foo);
  }

If get_foo() is not inlined, then when compiling some_fun, gcc sees only that a pointer to the local variable is passed, and must assume that it is an out parameter that is initialized after get_foo returns.

However, when get_foo() is inlined, the compiler may look at all of the code together and see that some code paths in get_foo() do not initialize the variable. As a result, it prints a warning. But what the compiler can't see is that error() always returns -1, and therefore we know that either we return early from some_fun, or foo ends up initialized, and the code is safe. The warning is a false positive.

If we can make the compiler aware that error() will always return -1, it can do a better job of analysis. The simplest method would be to inline the error() function. However, this doesn't work, because gcc will not inline a variadc function. We can work around this by defining a macro. This relies on two gcc extensions:

  1. Variadic macros (these are present in C99, but we do
     not rely on that).
  2. Gcc treats the "##" paste operator specially between a
     comma and __VA_ARGS__, which lets our variadic macro
     work even if no format parameters are passed to
     error().

Since we are using these extra features, we hide the macro behind an #ifdef. This is OK, though, because our goal was just to help gcc.

Signed-off-by: Jeff King <peff@peff.net>
---
 git-compat-util.h | 11 +++++++++++
 usage.c           |  1 +
 2 files changed, 12 insertions(+)
diff --git a/git-compat-util.h b/git-compat-util.h
index 2e79b8a..9002bca 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -288,6 +288,17 @@ extern void warning(const char *err, ...) __attribute__((format (printf, 1, 2)))
 extern int error(const char *err, ...) __attribute__((format (printf, 1, 2)));
 extern void warning(const char *err, ...) __attribute__((format (printf, 1, 2)));
 
+/*
+ * Let callers be aware of the constant return value; this can help
+ * gcc with -Wuninitialized analysis. We have to restrict this trick to
+ * gcc, though, because of the variadic macro and the magic ## comma pasting
+ * behavior. But since we're only trying to help gcc, anyway, it's OK; other
+ * compilers will fall back to using the function as usual.
+ */
+#ifdef __GNUC__
+#define error(fmt, ...) (error((fmt), ##__VA_ARGS__), -1)
+#endif
+
 extern void set_die_routine(NORETURN_PTR void (*routine)(const char *err, va_list params));
 extern void set_error_routine(void (*routine)(const char *err, va_list params));
 
diff --git a/usage.c b/usage.c
index 8eab281..40b3de5 100644
--- a/usage.c
+++ b/usage.c
@@ -130,6 +130,7 @@ void NORETURN die_errno(const char *fmt, ...)
 	va_end(params);
 }
 
+#undef error
 int error(const char *err, ...)
 {
 	va_list params;
-- 
1.8.0.2.4.g59402aa
Previous: Jeff KingNext: Jeff King
Message 11 of 12 in “compiling git with gcc -O3 -Wuninitialized”
  1. 0/3 compiling git with gcc -O3 -WuninitializedJeff King, Dec 14, 2012
  2. 1/3 remote-testsvn: fix unitialized variableJeff King, Dec 14, 2012
  3. Florian AchleitnerDec 15, 2012
  4. 2/3 inline error functions with constant returnsJeff King, Dec 14, 2012
  5. 3/3 silence some -Wuninitialized warnings around errorsJeff King, Dec 14, 2012
  6. Nguyen Thai Ngoc DuyDec 15, 2012
  7. Jeff KingDec 15, 2012
  8. Johannes SixtDec 15, 2012
  9. Jeff KingDec 15, 2012
  10. 0/2 compiling git with gcc -O3 -WuninitializedJeff King, Dec 15, 2012
  11. 1/2 make error()'s constant return value more visibleJeff King, Dec 15, 2012
  12. 2/2 silence some -Wuninitialized false positivesJeff King, Dec 15, 2012

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.