[PATCH 4/4] User manual documentation for tunables

[PATCH 4/4] User manual documentation for tunables

[Siddhesh Poyarekar]

Create a new node for tunables documentation and add notes for the
malloc tunables.

	* manual/tunables.texi: New chapter.
	* manual/Makefile (chapters): Add it.
	* manual/probes.texi (@node): Point to the Tunables chapter.
---
 manual/Makefile      |   3 +-
 manual/probes.texi   |   2 +-
 manual/tunables.texi | 192 +++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 195 insertions(+), 2 deletions(-)
 create mode 100644 manual/tunables.texi

diff --git a/manual/Makefile b/manual/Makefile
index f2f694f..ecc2bf6 100644
--- a/manual/Makefile
+++ b/manual/Makefile
@@ -38,7 +38,8 @@ chapters = $(addsuffix .texi, \
 		       message search pattern io stdio llio filesys	\
 		       pipe socket terminal syslog math arith time	\
 		       resource setjmp signal startup process ipc job	\
-		       nss users sysinfo conf crypt debug threads probes)
+		       nss users sysinfo conf crypt debug threads	\
+		       probes tunables)
 add-chapters = $(wildcard $(foreach d, $(add-ons), ../$d/$d.texi))
 appendices = lang.texi header.texi install.texi maint.texi platform.texi \
 	     contrib.texi
diff --git a/manual/probes.texi b/manual/probes.texi
index 237a918..eb91c62 100644
--- a/manual/probes.texi
+++ b/manual/probes.texi
@@ -1,5 +1,5 @@
 @node Internal Probes
-@c @node Internal Probes, , POSIX Threads, Top
+@c @node Internal Probes, Tunables, POSIX Threads, Top
 @c %MENU% Probes to monitor libc internal behavior
 @chapter Internal probes
 
diff --git a/manual/tunables.texi b/manual/tunables.texi
new file mode 100644
index 0000000..ac8c38f
--- /dev/null
+++ b/manual/tunables.texi
@@ -0,0 +1,192 @@
+@node Tunables
+@c @node Tunables, , Internal Probes, Top
+@c %MENU% Tunable switches to alter libc internal behavior
+@chapter Tunables
+@cindex tunables
+
+@dfn{Tunables} are a feature in @theglibc{} that allows application authors and
+distribution maintainers to alter the runtime library behavior to match
+their workload. These are implemented as a set of switches that may be
+modified in different ways. The current default method to do this is via
+the @env{GLIBC_TUNABLES} environment variable by setting it to a string
+of colon-separated @var{name}=@var{value} pairs.  For example, the following
+example enables malloc checking and sets the malloc trim threshold to 128
+bytes:
+
+@example
+GLIBC_TUNABLES=glibc.malloc.trim_threshold=128:glibc.malloc.check=3
+export GLIBC_TUNABLES
+@end example
+
+Tunables are not part of the @glibcadj{} stable ABI, and they are
+subject to change or removal across releases.  Additionally, the method to
+modify tunable values may change between releases and across distributions.
+It is possible to implement multiple `frontends' for the tunables allowing
+distributions to choose their preferred method at build time.
+
+Finally, the set of tunables available may vary between distributions as
+the tunables feature allows distributions to add their own tunables under
+their own namespace.
+
+@menu
+* Tunable names::  The structure of a tunable name
+* Memory Allocation Tunables::  Tunables in the memory allocation subsystem
+@end menu
+
+@node Tunable names
+@section Tunable names
+@cindex Tunable names
+@cindex Tunable namespaces
+
+A tunable name is split into three components, a top namespace, a tunable
+namespace and the tunable name. The top namespace for tunables implemented in
+@theglibc{} is @code{glibc}. Distributions that choose to add custom tunables
+in their maintained versions of @theglibc{} may choose to do so under their own
+top namespace.
+
+The tunable namespace is a logical grouping of tunables in a single
+module. This currently holds no special significance, although that may
+change in the future.
+
+The tunable name is the actual name of the tunable. It is possible that
+different tunable namespaces may have tunables within them that have the
+same name, likewise for top namespaces. Hence, we only support
+identification of tunables by their full name, i.e. with the top
+namespace, tunable namespace and tunable name, separated by periods.
+
+@node Memory Allocation Tunables
+@section Memory Allocation Tunables
+@cindex memory allocation tunables
+@cindex malloc tunables
+@cindex tunables, malloc
+
+@deftp {Tunable namespace} glibc.malloc
+Memory allocation behavior can be modified by setting any of the
+following tunables in the @code{malloc} namespace:
+@end deftp
+
+@deftp Tunable glibc.malloc.check
+This tunable supersedes the @env{MALLOC_CHECK_} environment variable and is
+identical in features.
+
+Setting this tunable enables a special (less efficient) memory allocator for
+the malloc family of functions that is designed to be tolerant against simple
+errors such as double calls of free with the same argument, or overruns of a
+single byte (off-by-one bugs). Not all such errors can be protected against,
+however, and memory leaks can result.  The following list describes the values
+that this tunable can take and the effect they have on malloc functionality:
+
+@itemize @bullet
+@item @code{0} Ignore all errors.  The default allocator continues to be in
+use, but all errors are silently ignored.
+@item @code{1} Report errors.  The alternate allocator is selected and heap
+corruption, if detected, is reported as diagnostic messages to @code{stderr}
+and the program continues execution.
+@item @code{2} Abort on errors.  The alternate allocator is selected and if
+heap corruption is detected, the program is ended immediately by calling
+@code{abort}.
+@item @code{3} Fully enabled.  The alternate allocator is selected and is fully
+functional.  That is, if heap corruption is detected, a verbose diagnostic
+message is printed to @code{stderr} and the program is ended by calling
+@code{abort}.
+@end itemize
+
+Like @env{MALLOC_CHECK_}, @code{glibc.malloc.check} has a problem in that it
+diverges from normal program behavior by writing to @code{stderr}, which could
+by exploited in SUID and SGID binaries.  Therefore, @code{glibc.malloc.check}
+is disabled by default for SUID and SGID binaries.  This can be enabled again
+by the system administrator by adding a file @file{/etc/suid-debug}; the
+content of the file could be anything or even empty.
+@end deftp
+
+@deftp Tunable glibc.malloc.top_pad
+This tunable supersedes the @env{MALLOC_TOP_PAD_} environment variable and is
+identical in features.
+
+This tunable determines the amount of extra memory in bytes to obtain from the
+system when any of the arenas need to be extended.  It also specifies the
+number of bytes to retain when shrinking any of the arenas.  This provides the
+necessary hysteresis in heap size such that excessive amounts of system calls
+can be avoided.
+
+The default value of this tunable is @samp{0}.
+@end deftp
+
+@deftp Tunable glibc.malloc.perturb
+This tunable supersedes the @env{MALLOC_PERTURB_} environment variable and is
+identical in features.
+
+If set to a non-zero value, memory blocks are initialized with values depending
+on some low order bits of this tunable when they are allocated (except when
+allocated by calloc) and freed.  This can be used to debug the use of
+uninitialized or freed heap memory. Note that this option does not guarantee
+that the freed block will have any specific values. It only guarantees that the
+content the block had before it was freed will be overwritten.
+
+The default value of this tunable is @samp{0}.
+@end deftp
+
+@deftp Tunable glibc.malloc.mmap_threshold
+This tunable supersedes the @env{MALLOC_MMAP_THRESHOLD_} environment variable
+and is identical in features.
+
+When this tunable is set, all chunks larger than this value in bytes are
+allocated outside the normal heap, using the @code{mmap} system call. This way
+it is guaranteed that the memory for these chunks can be returned to the system
+on @code{free}. Note that requests smaller than this threshold might still be
+allocated via @code{mmap}.
+
+If this tunable is not set, the default value is set to @samp{131072} bytes and
+the threshold is adjusted dynamically to suit the allocation patterns of the
+program.  If the tunable is set, the dynamic adjustment is disabled and the
+value is set as static.
+@end deftp
+
+@deftp Tunable glibc.malloc.trim_threshold
+This tunable supersedes the @env{MALLOC_TRIM_THRESHOLD_} environment variable
+and is identical in features.
+
+The value of this tunable is the minimum size (in bytes) of the top-most,
+releasable chunk in an arena that will trigger a system call in order to return
+memory to the system from that arena.
+
+If this tunable is not set, the default value is set as 128 KB and the
+threshold is adjusted dynamically to suit the allocation patterns of the
+program.  If the tunable is set, the dynamic adjustment is disabled and the
+value is set as static.
+@end deftp
+
+@deftp Tunable glibc.malloc.mmap_max
+This tunable supersedes the @env{MALLOC_MMAP_MAX_} environment variable and is
+identical in features.
+
+The value of this tunable is maximum number of chunks to allocate with
+@code{mmap}.  Setting this to zero disables all use of @code{mmap}.
+
+The default value of this tunable is @samp{65536}.
+@end deftp
+
+@deftp Tunable glibc.malloc.arena_test
+This tunable supersedes the @env{MALLOC_ARENA_TEST} environment variable and is
+identical in features.
+
+The @code{glibc.malloc.arena_test} tunable specifies the number of arenas that
+can be created before the test on the limit to the number of arenas is
+conducted.  The value is ignored if @code{glibc.malloc.arena_max} is set.
+
+The default value of this tunable is 2 for 32-bit systems and 8 for 64-bit
+systems.
+@end deftp
+
+@deftp Tunable glibc.malloc.arena_max
+This tunable supersedes the @env{MALLOC_ARENA_MAX} environment variable and is
+identical in features.
+
+This tunable sets the number of arenas to use in a process regardless of the
+number of cores in the system.
+
+The default value of this tunable is @code{0}, meaning that the limit on the
+number of arenas is determined by the number of CPU cores online.  For 32-bit
+systems the limit is twice the number of cores online and on 64-bit systems, it
+is 8 times the number of cores online.
+@end deftp
-- 
2.7.4

[PATCH v8 0/4] glibc tunables

[Siddhesh Poyarekar]

Hi,

Here is an updated version of the glibc tunables patchset.

Changes:

 - Use sbrk instead of mmap for allocation of the tunables string and add a
   note about setting errno.  I'll post a patch to add an __sbrk_noerrno in
   2.26 to make this bullet-proof.

 - Go back to __builtin_unreachable because __builtin_trap may cause issues in
   other architectures.  I will do a build test of other architectures and post
   an update before release if everything is OK.

 - Fixed up manual changes, including ones that Rical had suggested earlier - I
   had made those in another branch and forgotten about them.

Siddhesh Poyarekar (4):
  Add framework for tunables
  Initialize tunable list with the GLIBC_TUNABLES environment variable
  Enhance --enable-tunables to select tunables frontend at build time
  User manual documentation for tunables

 INSTALL                                    |  21 ++
 Makeconfig                                 |  16 +
 README.tunables                            |  85 ++++++
 config.h.in                                |   3 +
 config.make.in                             |   2 +
 configure                                  |  17 ++
 configure.ac                               |  10 +
 csu/init-first.c                           |   2 -
 csu/libc-start.c                           |   8 +
 elf/Makefile                               |  13 +
 elf/Versions                               |   3 +
 elf/dl-support.c                           |   2 +
 elf/dl-sysdep.c                            |   4 +
 elf/dl-tunable-types.h                     |  46 +++
 elf/dl-tunables.c                          | 457 +++++++++++++++++++++++++++++
 elf/dl-tunables.h                          |  92 ++++++
 elf/dl-tunables.list                       |  69 +++++
 elf/rtld.c                                 |   2 +
 malloc/Makefile                            |   6 +
 malloc/arena.c                             |  54 ++++
 malloc/tst-malloc-usable-static-tunables.c |   1 +
 malloc/tst-malloc-usable-static.c          |   1 +
 malloc/tst-malloc-usable-tunables.c        |   1 +
 manual/Makefile                            |   3 +-
 manual/install.texi                        |  20 ++
 manual/probes.texi                         |   2 +-
 manual/tunables.texi                       | 192 ++++++++++++
 scripts/gen-tunables.awk                   | 157 ++++++++++
 sysdeps/mach/hurd/dl-sysdep.c              |   4 +
 29 files changed, 1289 insertions(+), 4 deletions(-)
 create mode 100644 README.tunables
 create mode 100644 elf/dl-tunable-types.h
 create mode 100644 elf/dl-tunables.c
 create mode 100644 elf/dl-tunables.h
 create mode 100644 elf/dl-tunables.list
 create mode 100644 malloc/tst-malloc-usable-static-tunables.c
 create mode 100644 malloc/tst-malloc-usable-static.c
 create mode 100644 malloc/tst-malloc-usable-tunables.c
 create mode 100644 manual/tunables.texi
 create mode 100644 scripts/gen-tunables.awk

-- 
2.7.4

[PATCH 2/4] Initialize tunable list with the GLIBC_TUNABLES environment variable

[Siddhesh Poyarekar]

Read tunables values from the users using the GLIBC_TUNABLES
environment variable.  The value of this variable is a colon-separated
list of name=value pairs.  So a typical string would look like this:

GLIBC_TUNABLES=glibc.malloc.mmap_threshold=2048:glibc.malloc.trim_threshold=1024

	* config.make.in (have-loop-to-function): Define.
	* elf/Makefile (CFLAGS-dl-tunables.c): Add
	-fno-tree-loop-distribute-patterns.
	* elf/dl-tunables.c: Include libc-internals.h.
	(GLIBC_TUNABLES): New macro.
	(tunables_strdup): New function.
	(parse_tunables): New function.
	(min_strlen): New function.
	(__tunables_init): Use the new functions and macro.
	(disable_tunable): Disable tunable from GLIBC_TUNABLES.
	* malloc/tst-malloc-usable-tunables.c: New test case.
	* malloc/tst-malloc-usable-static-tunables.c: New test case.
	* malloc/Makefile (tests, tests-static): Add tests.
---
 config.make.in                             |   1 +
 elf/Makefile                               |   6 ++
 elf/dl-tunables.c                          | 129 ++++++++++++++++++++++++++++-
 malloc/Makefile                            |   6 +-
 malloc/tst-malloc-usable-static-tunables.c |   1 +
 malloc/tst-malloc-usable-tunables.c        |   1 +
 6 files changed, 142 insertions(+), 2 deletions(-)
 create mode 100644 malloc/tst-malloc-usable-static-tunables.c
 create mode 100644 malloc/tst-malloc-usable-tunables.c

diff --git a/config.make.in b/config.make.in
index 2f8dae2..5836b32 100644
--- a/config.make.in
+++ b/config.make.in
@@ -71,6 +71,7 @@ have-hash-style = @libc_cv_hashstyle@
 use-default-link = @use_default_link@
 output-format = @libc_cv_output_format@
 have-cxx-thread_local = @libc_cv_cxx_thread_local@
+have-loop-to-function = @libc_cv_cc_loop_to_function@
 
 multi-arch = @multi_arch@
 
diff --git a/elf/Makefile b/elf/Makefile
index de28d99..3cda2c9 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -38,6 +38,12 @@ endif
 
 ifeq (yes,$(have-tunables))
 dl-routines += dl-tunables
+
+# Make sure that the compiler does not insert any library calls in tunables
+# code paths.
+ifeq (yes,$(have-loop-to-function))
+CFLAGS-dl-tunables.c = -fno-tree-loop-distribute-patterns
+endif
 endif
 
 all-dl-routines = $(dl-routines) $(sysdep-dl-routines)
diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c
index 472747b..bbdaa15 100644
--- a/elf/dl-tunables.c
+++ b/elf/dl-tunables.c
@@ -30,7 +30,10 @@
 #define TUNABLES_INTERNAL 1
 #include "dl-tunables.h"
 
-/* Compare environment names, bounded by the name hardcoded in glibc.  */
+#define GLIBC_TUNABLES "GLIBC_TUNABLES"
+
+/* Compare environment or tunable names, bounded by the name hardcoded in
+   glibc.  */
 static bool
 is_name (const char *orig, const char *envname)
 {
@@ -45,6 +48,29 @@ is_name (const char *orig, const char *envname)
     return false;
 }
 
+static char *
+tunables_strdup (const char *in)
+{
+  size_t i = 0;
+
+  while (in[i++]);
+  char *out = __sbrk (i);
+
+  /* FIXME: In reality this will not return NULL, it will crash attempting to
+     set the thread-local errno since the TCB has not yet been set up.  This
+     needs to be fixed with an __sbrk implementation that does not set
+     errno.  */
+  if (out == (void *)-1)
+    return NULL;
+
+  i--;
+
+  while (i-- > 0)
+    out[i] = in[i];
+
+  return out;
+}
+
 static char **
 get_next_env (char **envp, char **name, size_t *namelen, char **val)
 {
@@ -218,6 +244,82 @@ tunable_initialize (tunable_t *cur, const char *strval)
     }
 }
 
+static void
+parse_tunables (char *tunestr)
+{
+  if (tunestr == NULL || *tunestr == '\0')
+    return;
+
+  char *p = tunestr;
+
+  while (true)
+    {
+      char *name = p;
+      size_t len = 0;
+
+      /* First, find where the name ends.  */
+      while (p[len] != '=' && p[len] != ':' && p[len] != '\0')
+	len++;
+
+      /* If we reach the end of the string before getting a valid name-value
+	 pair, bail out.  */
+      if (p[len] == '\0')
+	return;
+
+      /* We did not find a valid name-value pair before encountering the
+	 colon.  */
+      if (p[len]== ':')
+	{
+	  p += len + 1;
+	  continue;
+	}
+
+      p += len + 1;
+
+      char *value = p;
+      len = 0;
+
+      while (p[len] != ':' && p[len] != '\0')
+	len++;
+
+      char end = p[len];
+      p[len] = '\0';
+
+      /* Add the tunable if it exists.  */
+      for (size_t i = 0; i < sizeof (tunable_list) / sizeof (tunable_t); i++)
+	{
+	  tunable_t *cur = &tunable_list[i];
+
+	  /* If we are in a secure context (AT_SECURE) then ignore the tunable
+	     unless it is explicitly marked as secure.  Tunable values take
+	     precendence over their envvar aliases.  */
+	  if (__libc_enable_secure && !cur->is_secure)
+	    continue;
+
+	  if (is_name (cur->name, name))
+	    {
+	      tunable_initialize (cur, value);
+	      break;
+	    }
+	}
+
+      if (end == ':')
+	p += len + 1;
+      else
+	return;
+    }
+}
+
+static size_t
+min_strlen (const char *s)
+{
+  size_t i = 0;
+  while (*s++ != '\0')
+    i++;
+
+  return i;
+}
+
 /* Disable a tunable if it is set.  */
 static void
 disable_tunable (tunable_id_t id, char **envp)
@@ -226,6 +328,23 @@ disable_tunable (tunable_id_t id, char **envp)
 
   if (env_alias != NULL)
     tunables_unsetenv (envp, tunable_list[id].env_alias);
+
+  char *tunable = getenv (GLIBC_TUNABLES);
+  const char *cmp = tunable_list[id].name;
+  const size_t len = min_strlen (cmp);
+
+  while (tunable && *tunable != '\0' && *tunable != ':')
+    {
+      if (is_name (tunable, cmp))
+	{
+	  tunable += len;
+	  /* Overwrite the = and the value with colons.  */
+	  while (*tunable != '\0' && *tunable != ':')
+	    *tunable++ = ':';
+	  break;
+	}
+      tunable++;
+    }
 }
 
 /* Disable the glibc.malloc.check tunable in SETUID/SETGID programs unless
@@ -256,6 +375,14 @@ __tunables_init (char **envp)
 
   while ((envp = get_next_env (envp, &envname, &len, &envval)) != NULL)
     {
+      if (is_name (GLIBC_TUNABLES, envname))
+	{
+	  char *val = tunables_strdup (envval);
+	  if (val != NULL)
+	    parse_tunables (val);
+	  continue;
+	}
+
       for (int i = 0; i < sizeof (tunable_list) / sizeof (tunable_t); i++)
 	{
 	  tunable_t *cur = &tunable_list[i];
diff --git a/malloc/Makefile b/malloc/Makefile
index 4e4104e..a34e20c 100644
--- a/malloc/Makefile
+++ b/malloc/Makefile
@@ -33,11 +33,13 @@ tests := mallocbug tst-malloc tst-valloc tst-calloc tst-obstack \
 	 tst-mallocfork2 \
 	 tst-interpose-nothread \
 	 tst-interpose-thread \
+	 tst-malloc-usable-tunables
 
 tests-static := \
 	 tst-interpose-static-nothread \
 	 tst-interpose-static-thread \
-	 tst-malloc-usable-static
+	 tst-malloc-usable-static \
+	 tst-malloc-usable-static-tunables
 
 tests += $(tests-static)
 test-srcs = tst-mtrace
@@ -160,6 +162,8 @@ endif
 tst-mcheck-ENV = MALLOC_CHECK_=3
 tst-malloc-usable-ENV = MALLOC_CHECK_=3
 tst-malloc-usable-static-ENV = $(tst-malloc-usable-ENV)
+tst-malloc-usable-tunables-ENV = GLIBC_TUNABLES=glibc.malloc.check=3
+tst-malloc-usable-static-tunables-ENV = $(tst-malloc-usable-tunables-ENV)
 
 # Uncomment this for test releases.  For public releases it is too expensive.
 #CPPFLAGS-malloc.o += -DMALLOC_DEBUG=1
diff --git a/malloc/tst-malloc-usable-static-tunables.c b/malloc/tst-malloc-usable-static-tunables.c
new file mode 100644
index 0000000..8907db0
--- /dev/null
+++ b/malloc/tst-malloc-usable-static-tunables.c
@@ -0,0 +1 @@
+#include <malloc/tst-malloc-usable.c>
diff --git a/malloc/tst-malloc-usable-tunables.c b/malloc/tst-malloc-usable-tunables.c
new file mode 100644
index 0000000..8907db0
--- /dev/null
+++ b/malloc/tst-malloc-usable-tunables.c
@@ -0,0 +1 @@
+#include <malloc/tst-malloc-usable.c>
-- 
2.7.4

[PATCH 3/4] Enhance --enable-tunables to select tunables frontend at build time

[Siddhesh Poyarekar]

At the GNU Tools Cauldron 2016, the state of the current tunables
patchset was considered OK with the addition of a way to select the
frontend to be used for the tunables.  That is, to avoid being locked
in to one type of frontend initially, it should be possible to build
tunables with a different frontend with something as simple as a
configure switch.

To that effect, this patch enhances the --enable-tunables option to
accept more values than just 'yes' or 'no'.  The current frontend (and
default when enable-tunables is 'yes') is called 'valstring', to
select the frontend where a single environment variable is set to a
colon-separated value string.  More such frontends can be added in
future.

	* Makeconfig (have-tunables): Check for non-negative instead
	of positive.
	* configure.ac: Add 'valstring' as a valid value for
	--enable-tunables.
	* configure: Regenerate.
	* elf/Makefile (have-tunables): Check for non-negative instead
	of positive.
	(CPPFLAGS-dl-tunables.c): Define TUNABLES_FRONTEND for
	dl-tunables.c.
	* elf/dl-tunables.c (GLIBC_TUNABLES): Define only when
	TUNABLES_FRONTEND == TUNABLES_FRONTEND_valstring.
	(tunables_strdup): Likewise.
	(disable_tunables): Likewise.
	(parse_tunables): Likewise.
	(__tunables_init): Process GLIBC_TUNABLES envvar only when.
	TUNABLES_FRONTEND == TUNABLES_FRONTEND_valstring.
	* elf/dl-tunables.h (TUNABLES_FRONTEND_valstring): New macro.
	(TUNABLES_FRONTEND_yes): New macro, define as
	TUNABLES_FRONTEND_valstring by default.
	* manual/install.texi: Document new acceptable values for
	--enable-tunables.
	* INSTALL: Regenerate.
---
 INSTALL             | 18 +++++++++++++++++-
 Makeconfig          |  4 ++--
 configure           |  3 ++-
 configure.ac        |  2 +-
 elf/Makefile        |  4 +++-
 elf/dl-tunables.c   | 12 +++++++++++-
 elf/dl-tunables.h   |  4 ++++
 manual/install.texi | 17 ++++++++++++++++-
 8 files changed, 56 insertions(+), 8 deletions(-)

diff --git a/INSTALL b/INSTALL
index 25619fc..55d52c5 100644
--- a/INSTALL
+++ b/INSTALL
@@ -172,7 +172,23 @@ will be used, and CFLAGS sets optimization options for the compiler.
 '--enable-tunables'
      Tunables support allows additional library parameters to be
      customized at runtime.  This is an experimental feature and affects
-     startup time and is thus disabled by default.
+     startup time and is thus disabled by default.  This option can take
+     the following values:
+
+     'no'
+          This is the default if the option is not passed to configure.
+          This disables tunables.
+
+     'yes'
+          This is the default if the option is passed to configure.
+          This enables tunables and selects the default frontend
+          (currently 'valstring').
+
+     'valstring'
+          This enables tunables and selects the 'valstring' frontend for
+          tunables.  This frontend allows users to specify tunables as a
+          colon-separated list in a single environment variable
+          'GLIBC_TUNABLES'.
 
 '--build=BUILD-SYSTEM'
 '--host=HOST-SYSTEM'
diff --git a/Makeconfig b/Makeconfig
index b173e4c..1a2db6d 100644
--- a/Makeconfig
+++ b/Makeconfig
@@ -935,7 +935,7 @@ CPPFLAGS = $(config-extra-cppflags) $(CPPUNDEFS) $(CPPFLAGS-config) \
 			 $(libof-$(<F)) $(libof-$(@F)),$(CPPFLAGS-$(lib))) \
 	   $(CPPFLAGS-$(<F)) $(CPPFLAGS-$(@F)) $(CPPFLAGS-$(basename $(@F)))
 
-ifeq (yes,$(have-tunables))
+ifneq (no,$(have-tunables))
 CPPFLAGS += -DTOP_NAMESPACE=glibc
 endif
 
@@ -1115,7 +1115,7 @@ endif
 
 # Build the tunables list header early since it could be used by any module in
 # glibc.
-ifeq (yes,$(have-tunables))
+ifneq (no,$(have-tunables))
 before-compile += $(common-objpfx)dl-tunable-list.h
 
 $(common-objpfx)dl-tunable-list.h: $(..)scripts/gen-tunables.awk \
diff --git a/configure b/configure
index d80d738..eecd0ac 100755
--- a/configure
+++ b/configure
@@ -1454,7 +1454,8 @@ Optional Features:
   --disable-build-nscd    disable building and installing the nscd daemon
   --disable-nscd          library functions will not contact the nscd daemon
   --enable-pt_chown       Enable building and installing pt_chown
-  --enable-tunables       Enable tunables support
+  --enable-tunables       Enable tunables support. Known values are 'yes',
+                          'no' and 'valstring'
   --enable-mathvec        Enable building and installing mathvec [default
                           depends on architecture]
 
diff --git a/configure.ac b/configure.ac
index 22f5cab..4a77411 100644
--- a/configure.ac
+++ b/configure.ac
@@ -423,7 +423,7 @@ fi
 
 AC_ARG_ENABLE([tunables],
 	      [AS_HELP_STRING([--enable-tunables],
-	       [Enable tunables support])],
+	       [Enable tunables support. Known values are 'yes', 'no' and 'valstring'])],
 	      [have_tunables=$enableval],
 	      [have_tunables=no])
 AC_SUBST(have_tunables)
diff --git a/elf/Makefile b/elf/Makefile
index 3cda2c9..c3636a6 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -36,8 +36,10 @@ ifeq (yes,$(use-ldconfig))
 dl-routines += dl-cache
 endif
 
-ifeq (yes,$(have-tunables))
+ifneq (no,$(have-tunables))
 dl-routines += dl-tunables
+tunables-type = $(addprefix TUNABLES_FRONTEND_,$(have-tunables))
+CPPFLAGS-dl-tunables.c = -DTUNABLES_FRONTEND=$(tunables-type)
 
 # Make sure that the compiler does not insert any library calls in tunables
 # code paths.
diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c
index bbdaa15..a970e1e 100644
--- a/elf/dl-tunables.c
+++ b/elf/dl-tunables.c
@@ -30,7 +30,9 @@
 #define TUNABLES_INTERNAL 1
 #include "dl-tunables.h"
 
-#define GLIBC_TUNABLES "GLIBC_TUNABLES"
+#if TUNABLES_FRONTEND == TUNABLES_FRONTEND_valstring
+# define GLIBC_TUNABLES "GLIBC_TUNABLES"
+#endif
 
 /* Compare environment or tunable names, bounded by the name hardcoded in
    glibc.  */
@@ -48,6 +50,7 @@ is_name (const char *orig, const char *envname)
     return false;
 }
 
+#if TUNABLES_FRONTEND == TUNABLES_FRONTEND_valstring
 static char *
 tunables_strdup (const char *in)
 {
@@ -70,6 +73,7 @@ tunables_strdup (const char *in)
 
   return out;
 }
+#endif
 
 static char **
 get_next_env (char **envp, char **name, size_t *namelen, char **val)
@@ -244,6 +248,7 @@ tunable_initialize (tunable_t *cur, const char *strval)
     }
 }
 
+#if TUNABLES_FRONTEND == TUNABLES_FRONTEND_valstring
 static void
 parse_tunables (char *tunestr)
 {
@@ -309,6 +314,7 @@ parse_tunables (char *tunestr)
 	return;
     }
 }
+#endif
 
 static size_t
 min_strlen (const char *s)
@@ -329,6 +335,7 @@ disable_tunable (tunable_id_t id, char **envp)
   if (env_alias != NULL)
     tunables_unsetenv (envp, tunable_list[id].env_alias);
 
+#if TUNABLES_FRONTEND == TUNABLES_FRONTEND_valstring
   char *tunable = getenv (GLIBC_TUNABLES);
   const char *cmp = tunable_list[id].name;
   const size_t len = min_strlen (cmp);
@@ -345,6 +352,7 @@ disable_tunable (tunable_id_t id, char **envp)
 	}
       tunable++;
     }
+#endif
 }
 
 /* Disable the glibc.malloc.check tunable in SETUID/SETGID programs unless
@@ -375,6 +383,7 @@ __tunables_init (char **envp)
 
   while ((envp = get_next_env (envp, &envname, &len, &envval)) != NULL)
     {
+#if TUNABLES_FRONTEND == TUNABLES_FRONTEND_valstring
       if (is_name (GLIBC_TUNABLES, envname))
 	{
 	  char *val = tunables_strdup (envval);
@@ -382,6 +391,7 @@ __tunables_init (char **envp)
 	    parse_tunables (val);
 	  continue;
 	}
+#endif
 
       for (int i = 0; i < sizeof (tunable_list) / sizeof (tunable_t); i++)
 	{
diff --git a/elf/dl-tunables.h b/elf/dl-tunables.h
index 7762c46..785a9b0 100644
--- a/elf/dl-tunables.h
+++ b/elf/dl-tunables.h
@@ -84,5 +84,9 @@ extern void __tunable_set_val (tunable_id_t, void *, tunable_callback_t);
 /* Namespace sanity for callback functions.  Use this macro to keep the
    namespace of the modules clean.  */
 #  define DL_TUNABLE_CALLBACK(__name) _dl_tunable_ ## __name
+
+#  define TUNABLES_FRONTEND_valstring 1
+/* The default value for TUNABLES_FRONTEND.  */
+#  define TUNABLES_FRONTEND_yes TUNABLES_FRONTEND_valstring
 # endif
 #endif
diff --git a/manual/install.texi b/manual/install.texi
index d412962..2657da1 100644
--- a/manual/install.texi
+++ b/manual/install.texi
@@ -203,7 +203,22 @@ Use this option to disable the vector math library.
 @item --enable-tunables
 Tunables support allows additional library parameters to be customized at
 runtime.  This is an experimental feature and affects startup time and is thus
-disabled by default.
+disabled by default.  This option can take the following values:
+
+@table @code
+@item no
+This is the default if the option is not passed to configure. This disables
+tunables.
+
+@item yes
+This is the default if the option is passed to configure. This enables tunables
+and selects the default frontend (currently @samp{valstring}).
+
+@item valstring
+This enables tunables and selects the @samp{valstring} frontend for tunables.
+This frontend allows users to specify tunables as a colon-separated list in a
+single environment variable @env{GLIBC_TUNABLES}.
+@end table
 
 @item --build=@var{build-system}
 @itemx --host=@var{host-system}
-- 
2.7.4

Re: [PATCH 2/4] Initialize tunable list with the GLIBC_TUNABLES environment variable

[Florian Weimer]

On 12/31/2016 04:40 PM, Siddhesh Poyarekar wrote:
> +  while (in[i++]);
> +  char *out = __sbrk (i);

This still has an implicit comparision against '\0', which is against 
the style guide.

> +  /* FIXME: In reality this will not return NULL, it will crash attempting to
> +     set the thread-local errno since the TCB has not yet been set up.  This
> +     needs to be fixed with an __sbrk implementation that does not set
> +     errno.  */
> +  if (out == (void *)-1)
> +    return NULL;

Comment does not match: NULL is not (void *) -1.

Thanks,
Florian

Re: [PATCH 2/4] Initialize tunable list with the GLIBC_TUNABLES environment variable

[Siddhesh Poyarekar]

On Saturday 31 December 2016 09:15 PM, Florian Weimer wrote:
> This still has an implicit comparision against '\0', which is against
> the style guide.

Ack, I'll fix it up.

>> +  /* FIXME: In reality this will not return NULL, it will crash
>> attempting to
>> +     set the thread-local errno since the TCB has not yet been set
>> up.  This
>> +     needs to be fixed with an __sbrk implementation that does not set
>> +     errno.  */
>> +  if (out == (void *)-1)
>> +    return NULL;
> 
> Comment does not match: NULL is not (void *) -1.

I actually meant the NULL return of the function, not __sbrk.  I could
put the comment on top of the function to make it clearer.

Any other issues with this patch?  I could fix these up and push if
there's nothing else.  That is of course also provided that 1/4 is good
to go.

Siddhesh

Re: [PATCH 2/4] Initialize tunable list with the GLIBC_TUNABLES environment variable

[Siddhesh Poyarekar]

On Saturday 31 December 2016 09:20 PM, Siddhesh Poyarekar wrote:
> On Saturday 31 December 2016 09:15 PM, Florian Weimer wrote:
>> This still has an implicit comparision against '\0', which is against
>> the style guide.
> 
> Ack, I'll fix it up.
> 
>>> +  /* FIXME: In reality this will not return NULL, it will crash
>>> attempting to
>>> +     set the thread-local errno since the TCB has not yet been set
>>> up.  This
>>> +     needs to be fixed with an __sbrk implementation that does not set
>>> +     errno.  */
>>> +  if (out == (void *)-1)
>>> +    return NULL;
>>
>> Comment does not match: NULL is not (void *) -1.

I changed it to:


FIXME: In reality if the allocation fails, __sbrk will crash attempting
to set the thread-local errno since the TCB has not yet been set up.
This needs to be fixed with an __sbrk implementation that does not set
errno.

Re: [PATCH 2/4] Initialize tunable list with the GLIBC_TUNABLES environment variable

[Florian Weimer]

On 12/31/2016 04:55 PM, Siddhesh Poyarekar wrote:
>
>
> On Saturday 31 December 2016 09:20 PM, Siddhesh Poyarekar wrote:
>> On Saturday 31 December 2016 09:15 PM, Florian Weimer wrote:
>>> This still has an implicit comparision against '\0', which is against
>>> the style guide.
>>
>> Ack, I'll fix it up.
>>
>>>> +  /* FIXME: In reality this will not return NULL, it will crash
>>>> attempting to
>>>> +     set the thread-local errno since the TCB has not yet been set
>>>> up.  This
>>>> +     needs to be fixed with an __sbrk implementation that does not set
>>>> +     errno.  */
>>>> +  if (out == (void *)-1)
>>>> +    return NULL;
>>>
>>> Comment does not match: NULL is not (void *) -1.
>
> I changed it to:
>
>
> FIXME: In reality if the allocation fails, __sbrk will crash attempting
> to set the thread-local errno since the TCB has not yet been set up.
> This needs to be fixed with an __sbrk implementation that does not set
> errno.

Fine with me.  Thanks.

Florian

Re: [PATCH 1/4] Add framework for tunables

[Florian Weimer]

On 12/31/2016 04:40 PM, Siddhesh Poyarekar wrote:
> +#ifndef _TUNABLES_H_
> +#define _TUNABLES_H_
> +
> +# if !HAVE_TUNABLES
> +static inline void
> +__always_inline
> +__tunables_init (char **unused __attribute_unused)
> +{
> +  return;
> +}

You can drop the return statement.  Maybe replace it with a comment 
indicating that this initalization function is optimized out if tunables 
are not enabled?

> +# else
> +
> +#  include <stddef.h>
> +#  include "dl-tunable-types.h"
> +
> +/* A tunable.  */

I think the header inclusion guard does not usually trigger such 
indentation.

Apart from that, I think the series is ready commit.  I will do some 
installation-tree testing after that.

Thanks,
Florian

Re: [PATCH 1/4] Add framework for tunables

[Siddhesh Poyarekar]

On Saturday 31 December 2016 10:19 PM, Florian Weimer wrote:
> Apart from that, I think the series is ready commit.  I will do some
> installation-tree testing after that.

Thanks, I'll push them shortly.

Siddhesh

Re: [PATCH 1/4] Add framework for tunables

[Florian Weimer]

On 12/31/2016 06:58 PM, Siddhesh Poyarekar wrote:
> On Saturday 31 December 2016 10:19 PM, Florian Weimer wrote:
>> Apart from that, I think the series is ready commit.  I will do some
>> installation-tree testing after that.
>
> Thanks, I'll push them shortly.

I missed the __attribute_unused syntax error. :(

I'll fix that shortly.  I've also realized that the suggested csu/ move 
did not happen, so building with stack protector enabled could be broken.

Thanks,
Florian

Re: [PATCH 1/4] Add framework for tunables

[Siddhesh Poyarekar]

On Sunday 01 January 2017 01:13 AM, Florian Weimer wrote:
> I missed the __attribute_unused syntax error. :(

Ugh, sorry, that was a stupid mistake.

> I'll fix that shortly.

Thanks.

> I've also realized that the suggested csu/ move
> did not happen, so building with stack protector enabled could be broken.

I saw your comment about it but did not link it to an actual suggestion.
 Did you suggest moving tunables to csu or specific bits of it?

Siddhesh

Re: [PATCH 1/4] Add framework for tunables

[Florian Weimer]

On 12/31/2016 08:49 PM, Siddhesh Poyarekar wrote:

>> I've also realized that the suggested csu/ move
>> did not happen, so building with stack protector enabled could be broken.
>
> I saw your comment about it but did not link it to an actual suggestion.

Never mind, it appears the existing magic in elf/Makefile takes care of 
the details.

Thanks,
Florian

Re: [PATCH 1/4] Add framework for tunables

[Nix]

On 31 Dec 2016, Florian Weimer told this:

> On 12/31/2016 08:49 PM, Siddhesh Poyarekar wrote:
>
>>> I've also realized that the suggested csu/ move
>>> did not happen, so building with stack protector enabled could be broken.
>>
>> I saw your comment about it but did not link it to an actual suggestion.
>
> Never mind, it appears the existing magic in elf/Makefile takes care of the details.

Yeah, you never need to worry about dynamic-linker stack-protection
interactions in any directory because of that magic. It's static libc
init that poses problems for csu/, hence the hardwired disabling of the
stack-protector in csu/ for that case. (Though, as you noted in a
review, that also unfortunately catches any tests in that directory).

-- 
NULL && (void)

Re: [PATCH 2/4] Initialize tunable list with the GLIBC_TUNABLES environment variable

[Florian Weimer]

On 12/30/2016 05:08 AM, Siddhesh Poyarekar wrote:
> +  /* Bare mmap system call to avoid setting errno which may not be available
> +     yet.  */
> +  INTERNAL_SYSCALL_DECL (err);
> +  char *out = (char *) INTERNAL_SYSCALL_CALL (mmap, err, NULL, i, PROT_READ | PROT_WRITE,
> +				     MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
> +  if (INTERNAL_SYSCALL_ERROR_P (out, err))
> +    return NULL;

INTERNAL_SYSCALL is not available in generic code.

You should use sbrk (like the TCB allocation in the startup code; it 
would also reduce wasted memory), or mmap with a comment indicating that 
this will crash if mmap updates errno.  (I believe the comment in the 
startup code about mmap is wrong.)

Thanks,
Florian

[PATCH 2/4] Initialize tunable list with the GLIBC_TUNABLES environment variable

[Siddhesh Poyarekar]

Read tunables values from the users using the GLIBC_TUNABLES
environment variable.  The value of this variable is a colon-separated
list of name=value pairs.  So a typical string would look like this:

GLIBC_TUNABLES=glibc.malloc.mmap_threshold=2048:glibc.malloc.trim_threshold=1024

	* config.make.in (have-loop-to-function): Define.
	* elf/Makefile (CFLAGS-dl-tunables.c): Add
	-fno-tree-loop-distribute-patterns.
	* elf/dl-tunables.c: Include libc-internals.h.
	(GLIBC_TUNABLES): New macro.
	(tunables_strdup): New function.
	(parse_tunables): New function.
	(min_strlen): New function.
	(__tunables_init): Use the new functions and macro.
	(disable_tunable): Disable tunable from GLIBC_TUNABLES.
	* malloc/tst-malloc-usable-tunables.c: New test case.
	* malloc/tst-malloc-usable-static-tunables.c: New test case.
	* malloc/Makefile (tests, tests-static): Add tests.
---
 config.make.in                             |   1 +
 elf/Makefile                               |   6 ++
 elf/dl-tunables.c                          | 129 ++++++++++++++++++++++++++++-
 malloc/Makefile                            |   6 +-
 malloc/tst-malloc-usable-static-tunables.c |   1 +
 malloc/tst-malloc-usable-tunables.c        |   1 +
 6 files changed, 142 insertions(+), 2 deletions(-)
 create mode 100644 malloc/tst-malloc-usable-static-tunables.c
 create mode 100644 malloc/tst-malloc-usable-tunables.c

diff --git a/config.make.in b/config.make.in
index 2f8dae2..5836b32 100644
--- a/config.make.in
+++ b/config.make.in
@@ -71,6 +71,7 @@ have-hash-style = @libc_cv_hashstyle@
 use-default-link = @use_default_link@
 output-format = @libc_cv_output_format@
 have-cxx-thread_local = @libc_cv_cxx_thread_local@
+have-loop-to-function = @libc_cv_cc_loop_to_function@
 
 multi-arch = @multi_arch@
 
diff --git a/elf/Makefile b/elf/Makefile
index de28d99..3cda2c9 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -38,6 +38,12 @@ endif
 
 ifeq (yes,$(have-tunables))
 dl-routines += dl-tunables
+
+# Make sure that the compiler does not insert any library calls in tunables
+# code paths.
+ifeq (yes,$(have-loop-to-function))
+CFLAGS-dl-tunables.c = -fno-tree-loop-distribute-patterns
+endif
 endif
 
 all-dl-routines = $(dl-routines) $(sysdep-dl-routines)
diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c
index df60c57..d41b6dc 100644
--- a/elf/dl-tunables.c
+++ b/elf/dl-tunables.c
@@ -30,7 +30,10 @@
 #define TUNABLES_INTERNAL 1
 #include "dl-tunables.h"
 
-/* Compare environment names, bounded by the name hardcoded in glibc.  */
+#define GLIBC_TUNABLES "GLIBC_TUNABLES"
+
+/* Compare environment or tunable names, bounded by the name hardcoded in
+   glibc.  */
 static bool
 is_name (const char *orig, const char *envname)
 {
@@ -45,6 +48,29 @@ is_name (const char *orig, const char *envname)
     return false;
 }
 
+static char *
+tunables_strdup (const char *in)
+{
+  size_t i = 0;
+
+  while (in[i++]);
+
+  /* Bare mmap system call to avoid setting errno which may not be available
+     yet.  */
+  INTERNAL_SYSCALL_DECL (err);
+  char *out = (char *) INTERNAL_SYSCALL_CALL (mmap, err, NULL, i, PROT_READ | PROT_WRITE,
+				     MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
+  if (INTERNAL_SYSCALL_ERROR_P (out, err))
+    return NULL;
+
+  i--;
+
+  while (i-- > 0)
+    out[i] = in[i];
+
+  return out;
+}
+
 static char **
 get_next_env (char **envp, char **name, size_t *namelen, char **val)
 {
@@ -218,6 +244,82 @@ tunable_initialize (tunable_t *cur, const char *strval)
     }
 }
 
+static void
+parse_tunables (char *tunestr)
+{
+  if (tunestr == NULL || *tunestr == '\0')
+    return;
+
+  char *p = tunestr;
+
+  while (true)
+    {
+      char *name = p;
+      size_t len = 0;
+
+      /* First, find where the name ends.  */
+      while (p[len] != '=' && p[len] != ':' && p[len] != '\0')
+	len++;
+
+      /* If we reach the end of the string before getting a valid name-value
+	 pair, bail out.  */
+      if (p[len] == '\0')
+	return;
+
+      /* We did not find a valid name-value pair before encountering the
+	 colon.  */
+      if (p[len]== ':')
+	{
+	  p += len + 1;
+	  continue;
+	}
+
+      p += len + 1;
+
+      char *value = p;
+      len = 0;
+
+      while (p[len] != ':' && p[len] != '\0')
+	len++;
+
+      char end = p[len];
+      p[len] = '\0';
+
+      /* Add the tunable if it exists.  */
+      for (size_t i = 0; i < sizeof (tunable_list) / sizeof (tunable_t); i++)
+	{
+	  tunable_t *cur = &tunable_list[i];
+
+	  /* If we are in a secure context (AT_SECURE) then ignore the tunable
+	     unless it is explicitly marked as secure.  Tunable values take
+	     precendence over their envvar aliases.  */
+	  if (__libc_enable_secure && !cur->is_secure)
+	    continue;
+
+	  if (is_name (cur->name, name))
+	    {
+	      tunable_initialize (cur, value);
+	      break;
+	    }
+	}
+
+      if (end == ':')
+	p += len + 1;
+      else
+	return;
+    }
+}
+
+static size_t
+min_strlen (const char *s)
+{
+  size_t i = 0;
+  while (*s++ != '\0')
+    i++;
+
+  return i;
+}
+
 /* Disable a tunable if it is set.  */
 static void
 disable_tunable (tunable_id_t id, char **envp)
@@ -226,6 +328,23 @@ disable_tunable (tunable_id_t id, char **envp)
 
   if (env_alias != NULL)
     tunables_unsetenv (envp, tunable_list[id].env_alias);
+
+  char *tunable = getenv (GLIBC_TUNABLES);
+  const char *cmp = tunable_list[id].name;
+  const size_t len = min_strlen (cmp);
+
+  while (tunable && *tunable != '\0' && *tunable != ':')
+    {
+      if (is_name (tunable, cmp))
+	{
+	  tunable += len;
+	  /* Overwrite the = and the value with colons.  */
+	  while (*tunable != '\0' && *tunable != ':')
+	    *tunable++ = ':';
+	  break;
+	}
+      tunable++;
+    }
 }
 
 /* Disable the glibc.malloc.check tunable in SETUID/SETGID programs unless
@@ -256,6 +375,14 @@ __tunables_init (char **envp)
 
   while ((envp = get_next_env (envp, &envname, &len, &envval)) != NULL)
     {
+      if (is_name (GLIBC_TUNABLES, envname))
+	{
+	  char *val = tunables_strdup (envval);
+	  if (val != NULL)
+	    parse_tunables (val);
+	  continue;
+	}
+
       for (int i = 0; i < sizeof (tunable_list) / sizeof (tunable_t); i++)
 	{
 	  tunable_t *cur = &tunable_list[i];
diff --git a/malloc/Makefile b/malloc/Makefile
index 4e4104e..a34e20c 100644
--- a/malloc/Makefile
+++ b/malloc/Makefile
@@ -33,11 +33,13 @@ tests := mallocbug tst-malloc tst-valloc tst-calloc tst-obstack \
 	 tst-mallocfork2 \
 	 tst-interpose-nothread \
 	 tst-interpose-thread \
+	 tst-malloc-usable-tunables
 
 tests-static := \
 	 tst-interpose-static-nothread \
 	 tst-interpose-static-thread \
-	 tst-malloc-usable-static
+	 tst-malloc-usable-static \
+	 tst-malloc-usable-static-tunables
 
 tests += $(tests-static)
 test-srcs = tst-mtrace
@@ -160,6 +162,8 @@ endif
 tst-mcheck-ENV = MALLOC_CHECK_=3
 tst-malloc-usable-ENV = MALLOC_CHECK_=3
 tst-malloc-usable-static-ENV = $(tst-malloc-usable-ENV)
+tst-malloc-usable-tunables-ENV = GLIBC_TUNABLES=glibc.malloc.check=3
+tst-malloc-usable-static-tunables-ENV = $(tst-malloc-usable-tunables-ENV)
 
 # Uncomment this for test releases.  For public releases it is too expensive.
 #CPPFLAGS-malloc.o += -DMALLOC_DEBUG=1
diff --git a/malloc/tst-malloc-usable-static-tunables.c b/malloc/tst-malloc-usable-static-tunables.c
new file mode 100644
index 0000000..8907db0
--- /dev/null
+++ b/malloc/tst-malloc-usable-static-tunables.c
@@ -0,0 +1 @@
+#include <malloc/tst-malloc-usable.c>
diff --git a/malloc/tst-malloc-usable-tunables.c b/malloc/tst-malloc-usable-tunables.c
new file mode 100644
index 0000000..8907db0
--- /dev/null
+++ b/malloc/tst-malloc-usable-tunables.c
@@ -0,0 +1 @@
+#include <malloc/tst-malloc-usable.c>
-- 
2.7.4

Re: [PATCH 2/4] Initialize tunable list with the GLIBC_TUNABLES environment variable

[Siddhesh Poyarekar]

On Tuesday 27 December 2016 05:11 PM, Florian Weimer wrote:
> See my initial mail.  errno and __getpagesize are not available at this
> point.

Ugh, right.  I'll try to come up with something that does not need an
additional allocation.

>> +static size_t
>> +min_strlen (const char *s)
>> +{
>> +  size_t i = 0;
>> +  while (*s++ != '\0')
>> +    i++;
>> +
>> +  return i;
>> +}
> 
> Is there anything which ensures that GCC does not replace this
> implementation with strlen?

I think I'm missing a gcc flag here to ensure that.  I'll find out and
add it.

>>  /* Disable a tunable if it is set.  */
>>  static void
>>  disable_tunable (tunable_id_t id, char **envp)
>> @@ -216,6 +318,23 @@ disable_tunable (tunable_id_t id, char **envp)
>>
>>    if (env_alias)
>>      tunables_unsetenv (envp, tunable_list[id].env_alias);
>> +
>> +  char *tunable = getenv (GLIBC_TUNABLES);
>> +  const char *cmp = tunable_list[id].name;
>> +  const size_t len = min_strlen (cmp);
>> +
>> +  while (tunable && *tunable != '\0' && *tunable != ':')
>> +    {
>> +      if (is_name (tunable, cmp))
>> +    {
>> +      tunable += len;
>> +      /* Overwrite the = and the value with colons.  */
>> +      while (*tunable != '\0' && *tunable != ':')
>> +        *tunable++ = ':';
>> +      break;
>> +    }
>> +      tunable++;
>> +    }
>>  }
> 
> This assumes that the process environment is not mapped read-only.  I'm
> not sure if this is guaranteed by the ABI.

Barring tunables with string values this might be easy to work around.
Let me give this a shot too.

Siddhesh

Re: [PATCH 2/4] Initialize tunable list with the GLIBC_TUNABLES environment variable

[Florian Weimer]

On 11/16/2016 09:35 AM, Siddhesh Poyarekar wrote:

> diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c

> +static char *
> +tunables_strdup (const char *in)
> +{
> +  size_t i = 0;
> +
> +  while (in[i++]);

Please use an explicit comparison against '\0'.

> +  char *out = __mmap (NULL, ALIGN_UP (i, __getpagesize ()),
> +		      PROT_READ | PROT_WRITE, MAP_ANONYMOUS | MAP_PRIVATE, -1,
> +		      0);
> +
> +  if (out == MAP_FAILED)
> +    return NULL;

See my initial mail.  errno and __getpagesize are not available at this 
point.

> +static size_t
> +min_strlen (const char *s)
> +{
> +  size_t i = 0;
> +  while (*s++ != '\0')
> +    i++;
> +
> +  return i;
> +}

Is there anything which ensures that GCC does not replace this 
implementation with strlen?

>  /* Disable a tunable if it is set.  */
>  static void
>  disable_tunable (tunable_id_t id, char **envp)
> @@ -216,6 +318,23 @@ disable_tunable (tunable_id_t id, char **envp)
>
>    if (env_alias)
>      tunables_unsetenv (envp, tunable_list[id].env_alias);
> +
> +  char *tunable = getenv (GLIBC_TUNABLES);
> +  const char *cmp = tunable_list[id].name;
> +  const size_t len = min_strlen (cmp);
> +
> +  while (tunable && *tunable != '\0' && *tunable != ':')
> +    {
> +      if (is_name (tunable, cmp))
> +	{
> +	  tunable += len;
> +	  /* Overwrite the = and the value with colons.  */
> +	  while (*tunable != '\0' && *tunable != ':')
> +	    *tunable++ = ':';
> +	  break;
> +	}
> +      tunable++;
> +    }
>  }

This assumes that the process environment is not mapped read-only.  I'm 
not sure if this is guaranteed by the ABI.

Thanks,
Florian

[PATCH 2/4] Initialize tunable list with the GLIBC_TUNABLES environment variable

[Siddhesh Poyarekar]

Read tunables values from the users using the GLIBC_TUNABLES
environment variable.  The value of this variable is a colon-separated
list of name=value pairs.  So a typical string would look like this:

GLIBC_TUNABLES=glibc.malloc.mmap_threshold=2048:glibc.malloc.trim_threshold=1024

	* elf/dl-tunables.c: Include mman.h.
	(GLIBC_TUNABLES): New macro.
	(tunables_strdup): New function.
	(parse_tunables): New function.
	(min_strlen): New function.
	(disable_tunable): Use the new functions and macro.
	(__tunables_init): Likewise.
	(disable_tunable): Disable tunable from GLIBC_TUNABLES.
	* malloc/tst-malloc-usable-tunables.c: New test case.
	* malloc/tst-malloc-usable-static-tunables.c: New test case.
	* malloc/Makefile (tests, tests-static): Add tests.
---
 elf/dl-tunables.c                          | 129 ++++++++++++++++++++++++++++-
 malloc/Makefile                            |   6 +-
 malloc/tst-malloc-usable-static-tunables.c |   1 +
 malloc/tst-malloc-usable-tunables.c        |   1 +
 4 files changed, 135 insertions(+), 2 deletions(-)
 create mode 100644 malloc/tst-malloc-usable-static-tunables.c
 create mode 100644 malloc/tst-malloc-usable-tunables.c

diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c
index 91340b6..79ec8ef 100644
--- a/elf/dl-tunables.c
+++ b/elf/dl-tunables.c
@@ -22,6 +22,7 @@
 #include <stdbool.h>
 #include <unistd.h>
 #include <stdlib.h>
+#include <sys/mman.h>
 #include <libc-internal.h>
 #include <sysdep.h>
 #include <fcntl.h>
@@ -29,7 +30,10 @@
 #define TUNABLES_INTERNAL 1
 #include "dl-tunables.h"
 
-/* Compare environment names, bounded by the name hardcoded in glibc.  */
+#define GLIBC_TUNABLES "GLIBC_TUNABLES"
+
+/* Compare environment or tunable names, bounded by the name hardcoded in
+   glibc.  */
 static bool
 is_name (const char *orig, const char *envname)
 {
@@ -44,6 +48,28 @@ is_name (const char *orig, const char *envname)
     return false;
 }
 
+static char *
+tunables_strdup (const char *in)
+{
+  size_t i = 0;
+
+  while (in[i++]);
+
+  char *out = __mmap (NULL, ALIGN_UP (i, __getpagesize ()),
+		      PROT_READ | PROT_WRITE, MAP_ANONYMOUS | MAP_PRIVATE, -1,
+		      0);
+
+  if (out == MAP_FAILED)
+    return NULL;
+
+  i--;
+
+  while (i-- > 0)
+    out[i] = in[i];
+
+  return out;
+}
+
 static char **
 get_next_env (char **envp, char **name, size_t *namelen, char **val)
 {
@@ -208,6 +234,82 @@ tunable_initialize (tunable_t *cur, const char *strval)
     }
 }
 
+static void
+parse_tunables (char *tunestr)
+{
+  if (tunestr == NULL || *tunestr == '\0')
+    return;
+
+  char *p = tunestr;
+
+  while (true)
+    {
+      char *name = p;
+      size_t len = 0;
+
+      /* First, find where the name ends.  */
+      while (p[len] != '=' && p[len] != ':' && p[len] != '\0')
+	len++;
+
+      /* If we reach the end of the string before getting a valid name-value
+	 pair, bail out.  */
+      if (p[len] == '\0')
+	return;
+
+      /* We did not find a valid name-value pair before encountering the
+	 colon.  */
+      if (p[len]== ':')
+	{
+	  p += len + 1;
+	  continue;
+	}
+
+      p += len + 1;
+
+      char *value = p;
+      len = 0;
+
+      while (p[len] != ':' && p[len] != '\0')
+	len++;
+
+      char end = p[len];
+      p[len] = '\0';
+
+      /* Add the tunable if it exists.  */
+      for (size_t i = 0; i < sizeof (tunable_list) / sizeof (tunable_t); i++)
+	{
+	  tunable_t *cur = &tunable_list[i];
+
+	  /* If we are in a secure context (AT_SECURE) then ignore the tunable
+	     unless it is explicitly marked as secure.  Tunable values take
+	     precendence over their envvar aliases.  */
+	  if (__libc_enable_secure && !cur->is_secure)
+	    continue;
+
+	  if (is_name (cur->name, name))
+	    {
+	      tunable_initialize (cur, value);
+	      break;
+	    }
+	}
+
+      if (end == ':')
+	p += len + 1;
+      else
+	return;
+    }
+}
+
+static size_t
+min_strlen (const char *s)
+{
+  size_t i = 0;
+  while (*s++ != '\0')
+    i++;
+
+  return i;
+}
+
 /* Disable a tunable if it is set.  */
 static void
 disable_tunable (tunable_id_t id, char **envp)
@@ -216,6 +318,23 @@ disable_tunable (tunable_id_t id, char **envp)
 
   if (env_alias)
     tunables_unsetenv (envp, tunable_list[id].env_alias);
+
+  char *tunable = getenv (GLIBC_TUNABLES);
+  const char *cmp = tunable_list[id].name;
+  const size_t len = min_strlen (cmp);
+
+  while (tunable && *tunable != '\0' && *tunable != ':')
+    {
+      if (is_name (tunable, cmp))
+	{
+	  tunable += len;
+	  /* Overwrite the = and the value with colons.  */
+	  while (*tunable != '\0' && *tunable != ':')
+	    *tunable++ = ':';
+	  break;
+	}
+      tunable++;
+    }
 }
 
 /* Disable the glibc.malloc.check tunable in SETUID/SETGID programs unless
@@ -246,6 +365,14 @@ __tunables_init (char **envp)
 
   while ((envp = get_next_env (envp, &envname, &len, &envval)) != NULL)
     {
+      if (is_name (GLIBC_TUNABLES, envname))
+	{
+	  char *val = tunables_strdup (envval);
+	  if (val != NULL)
+	    parse_tunables (val);
+	  continue;
+	}
+
       for (int i = 0; i < sizeof (tunable_list) / sizeof (tunable_t); i++)
 	{
 	  tunable_t *cur = &tunable_list[i];
diff --git a/malloc/Makefile b/malloc/Makefile
index 4e4104e..a34e20c 100644
--- a/malloc/Makefile
+++ b/malloc/Makefile
@@ -33,11 +33,13 @@ tests := mallocbug tst-malloc tst-valloc tst-calloc tst-obstack \
 	 tst-mallocfork2 \
 	 tst-interpose-nothread \
 	 tst-interpose-thread \
+	 tst-malloc-usable-tunables
 
 tests-static := \
 	 tst-interpose-static-nothread \
 	 tst-interpose-static-thread \
-	 tst-malloc-usable-static
+	 tst-malloc-usable-static \
+	 tst-malloc-usable-static-tunables
 
 tests += $(tests-static)
 test-srcs = tst-mtrace
@@ -160,6 +162,8 @@ endif
 tst-mcheck-ENV = MALLOC_CHECK_=3
 tst-malloc-usable-ENV = MALLOC_CHECK_=3
 tst-malloc-usable-static-ENV = $(tst-malloc-usable-ENV)
+tst-malloc-usable-tunables-ENV = GLIBC_TUNABLES=glibc.malloc.check=3
+tst-malloc-usable-static-tunables-ENV = $(tst-malloc-usable-tunables-ENV)
 
 # Uncomment this for test releases.  For public releases it is too expensive.
 #CPPFLAGS-malloc.o += -DMALLOC_DEBUG=1
diff --git a/malloc/tst-malloc-usable-static-tunables.c b/malloc/tst-malloc-usable-static-tunables.c
new file mode 100644
index 0000000..8907db0
--- /dev/null
+++ b/malloc/tst-malloc-usable-static-tunables.c
@@ -0,0 +1 @@
+#include <malloc/tst-malloc-usable.c>
diff --git a/malloc/tst-malloc-usable-tunables.c b/malloc/tst-malloc-usable-tunables.c
new file mode 100644
index 0000000..8907db0
--- /dev/null
+++ b/malloc/tst-malloc-usable-tunables.c
@@ -0,0 +1 @@
+#include <malloc/tst-malloc-usable.c>
-- 
2.7.4