[PATCH 0/2] nolibc: fix two build issues at -O0

[PATCH 0/2] nolibc: fix two build issues at -O0

[Willy Tarreau]

Hello Paul,

please find in this series two build fixes to apply on top of the series
you've already queued:
 - gcc errors at -O0 because it doesn't eliminate non-inline code and
   keeps the reference to the external "environ" from getenv()
 - clang fails at -O0 because __builtin_strlen() calls strlen()

I don't care about optimizing at -O0 by definition but at least it should
build and work correctly! I've verified that rcutorture still runs.

Thanks!
Willy

---
Willy Tarreau (2):
  tools/nolibc/string: do not use __builtin_strlen() at -O0
  tools/nolibc/stdlib: only reference the external environ when inlined

 tools/include/nolibc/stdlib.h | 22 +++++++++++++++-------
 tools/include/nolibc/string.h | 11 ++++++++++-
 2 files changed, 25 insertions(+), 8 deletions(-)

-- 
2.35.1

[PATCH 1/2] tools/nolibc/string: do not use __builtin_strlen() at -O0

[Willy Tarreau]

clang wants to use strlen() for __builtin_strlen() at -O0. We don't
really care about -O0 but it at least ought to build, so let's make
sure we don't choke on this, by dropping the optimizationn for
constant strings in this case.

Signed-off-by: Willy Tarreau <w@1wt.eu>
---
 tools/include/nolibc/string.h | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/tools/include/nolibc/string.h b/tools/include/nolibc/string.h
index 0d5e870c7c0b..75a453870498 100644
--- a/tools/include/nolibc/string.h
+++ b/tools/include/nolibc/string.h
@@ -122,7 +122,9 @@ char *strcpy(char *dst, const char *src)
 	return ret;
 }
 
-/* this function is only used with arguments that are not constants */
+/* this function is only used with arguments that are not constants or when
+ * it's not known because optimizations are disabled.
+ */
 static __attribute__((unused))
 size_t nolibc_strlen(const char *str)
 {
@@ -132,11 +134,18 @@ size_t nolibc_strlen(const char *str)
 	return len;
 }
 
+/* do not trust __builtin_constant_p() at -O0, as clang will emit a test and
+ * the two branches, then will rely on an external definition of strlen().
+ */
+#if defined(__OPTIMIZE__)
 #define strlen(str) ({                          \
 	__builtin_constant_p((str)) ?           \
 		__builtin_strlen((str)) :       \
 		nolibc_strlen((str));           \
 })
+#else
+#define strlen(str) nolibc_strlen((str))
+#endif
 
 static __attribute__((unused))
 size_t strlcat(char *dst, const char *src, size_t size)
-- 
2.35.1

[PATCH 2/2] tools/nolibc/stdlib: only reference the external environ when inlined

[Willy Tarreau]

When building with gcc at -O0 we're seeing link errors due to the
"environ" variable being referenced by getenv(). The problem is that
at -O0 gcc will not inline getenv() and will not drop the external
reference. One solution would be to locally declare the variable as
weak, but then it would appear in all programs even those not using
it, and would be confusing to users of getenv() who would forget to
set environ to envp.

An alternate approach used in this patch consists in always inlining
the outer part of getenv() that references this extern so that it's
always dropped when not used. The biggest part of the function was
now moved to a new function called _getenv() that's still not inlined
by default.

Reported-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>
Signed-off-by: Willy Tarreau <w@1wt.eu>
---
 tools/include/nolibc/stdlib.h | 22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/tools/include/nolibc/stdlib.h b/tools/include/nolibc/stdlib.h
index aca8616335e3..8a07e263f0d0 100644
--- a/tools/include/nolibc/stdlib.h
+++ b/tools/include/nolibc/stdlib.h
@@ -60,16 +60,17 @@ int atoi(const char *s)
 	return atol(s);
 }
 
-/* Tries to find the environment variable named <name> in the environment array
- * pointed to by global variable "environ" which must be declared as a char **,
- * and must be terminated by a NULL (it is recommended to set this variable to
- * the "envp" argument of main()). If the requested environment variable exists
- * its value is returned otherwise NULL is returned.
+/* getenv() tries to find the environment variable named <name> in the
+ * environment array pointed to by global variable "environ" which must be
+ * declared as a char **, and must be terminated by a NULL (it is recommended
+ * to set this variable to the "envp" argument of main()). If the requested
+ * environment variable exists its value is returned otherwise NULL is
+ * returned. getenv() is forcefully inlined so that the reference to "environ"
+ * will be dropped if unused, even at -O0.
  */
 static __attribute__((unused))
-char *getenv(const char *name)
+char *_getenv(const char *name, char **environ)
 {
-	extern char **environ;
 	int idx, i;
 
 	if (environ) {
@@ -83,6 +84,13 @@ char *getenv(const char *name)
 	return NULL;
 }
 
+static inline __attribute__((unused,always_inline))
+char *getenv(const char *name)
+{
+	extern char **environ;
+	return _getenv(name, environ);
+}
+
 /* Converts the unsigned long integer <in> to its hex representation into
  * buffer <buffer>, which must be long enough to store the number and the
  * trailing zero (17 bytes for "ffffffffffffffff" or 9 for "ffffffff"). The
-- 
2.35.1

Re: [PATCH 2/2] tools/nolibc/stdlib: only reference the external environ when inlined

[Ammar Faizi]

On 3/23/22 2:18 PM, Willy Tarreau wrote:
> When building with gcc at -O0 we're seeing link errors due to the
> "environ" variable being referenced by getenv(). The problem is that
> at -O0 gcc will not inline getenv() and will not drop the external
> reference. One solution would be to locally declare the variable as
> weak, but then it would appear in all programs even those not using
> it, and would be confusing to users of getenv() who would forget to
> set environ to envp.
> 
> An alternate approach used in this patch consists in always inlining
> the outer part of getenv() that references this extern so that it's
> always dropped when not used. The biggest part of the function was
> now moved to a new function called _getenv() that's still not inlined
> by default.
> 
> Reported-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>
> Signed-off-by: Willy Tarreau <w@1wt.eu>
> ---

This one works nicely. I will resend my previous RFC after this one
lands in Paul's tree to avoid conflict.

Tested-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>

Thanks!

-- 
Ammar Faizi

Re: [PATCH 0/2] nolibc: fix two build issues at -O0

[Paul E. McKenney]

On Wed, Mar 23, 2022 at 08:18:05AM +0100, Willy Tarreau wrote:
> Hello Paul,
> 
> please find in this series two build fixes to apply on top of the series
> you've already queued:
>  - gcc errors at -O0 because it doesn't eliminate non-inline code and
>    keeps the reference to the external "environ" from getenv()
>  - clang fails at -O0 because __builtin_strlen() calls strlen()
> 
> I don't care about optimizing at -O0 by definition but at least it should
> build and work correctly! I've verified that rcutorture still runs.

Queued with Ammar's Tested-by, thank you both!

							Thanx, Paul

> Thanks!
> Willy
> 
> ---
> Willy Tarreau (2):
>   tools/nolibc/string: do not use __builtin_strlen() at -O0
>   tools/nolibc/stdlib: only reference the external environ when inlined
> 
>  tools/include/nolibc/stdlib.h | 22 +++++++++++++++-------
>  tools/include/nolibc/string.h | 11 ++++++++++-
>  2 files changed, 25 insertions(+), 8 deletions(-)
> 
> -- 
> 2.35.1
>