[PATCH 1/2] build-many-glibcs: Install kernel headers separately.

[PATCH 1/2] build-many-glibcs: Install kernel headers separately.

[Zack Weinberg]

When building glibc, ideally we want the compiler's headers and the
kernel's headers to be visible, but not (a previous version of) our
own headers, or any third-party headers.  Our own build process
already supports this, with the configure option --with-headers, but
build-many-glibcs doesn't use that option, and when prepping compilers
it installs kernel headers in $(sysroot)/usr/include, the same place
that the preliminary build of glibc will install its own headers.

This patch changes build-many-glibcs to install the kernel headers in
$(sysroot)/usr/share/linux/include (only Linux targets are currently
supported in build-many-glibcs) and then symlink them into
$(sysroot)/usr/include.  A symlink farm is necessary for the "full"
build of GCC to work correctly.  The way it is created is a little
kludgey and exposed a bug in the shell quotation logic, but it works.

Note that $(sysroot)/usr/share/linux/include is made read-only to all
after it is created, in order to catch cases where glibc's "make
install" installs headers into a subdirectory belonging to the kernel.
One such directory already exists, /usr/include/scsi, and it's
special-cased in here, but we should fix that.

	* scripts/build-many-glibcs.py (Config.install_linux_headers):
	Install the headers in $sysroot/usr/share/linux/include and then
	make them read-only and create a symlink farm in $sysroot/usr/include
	pointing to them.
	(Glibc.build_glibc): If the OS is Linux, pass configure the
	--with-headers=$sysroot/usr/share/linux/include option.
	(CommandList.link_dir_contents): New function.
	(Command.__init__, Command.shell_make_quote_string): Use str.replace
	instead of str.translate.
	(Context.write_files): Process all nested quotes, not just the first.
---
 scripts/build-many-glibcs.py | 68 ++++++++++++++++++++++++++++++++++++++------
 1 file changed, 59 insertions(+), 9 deletions(-)

diff --git a/scripts/build-many-glibcs.py b/scripts/build-many-glibcs.py
index 5fbb564a14..7b267d14d9 100755
--- a/scripts/build-many-glibcs.py
+++ b/scripts/build-many-glibcs.py
@@ -525,7 +525,7 @@ class Context(object):
             '    printf " %s" "$word"\n'
             '  else\n'
             '    printf " \'"\n'
-            '    printf "%s" "$word" | sed -e "s/\'/\'\\\\\\\\\'\'/"\n'
+            '    printf "%s" "$word" | sed -e "s/\'/\'\\\\\\\\\'\'/g"\n'
             '    printf "\'"\n'
             '  fi\n'
             'done >> "$this_log"\n'
@@ -1213,15 +1213,37 @@ class Config(object):
         assert linux_arch is not None
         srcdir = self.ctx.component_srcdir('linux')
         builddir = self.component_builddir('linux')
-        headers_dir = os.path.join(self.sysroot, 'usr')
+        kernel_prefix = os.path.join(self.sysroot, 'usr/share/linux')
+        kernel_headers_dir = os.path.join(kernel_prefix, 'include')
+        all_headers_dir = os.path.join(self.sysroot, 'usr/include')
         cmdlist.push_subdesc('linux')
         cmdlist.create_use_dir(builddir)
+        # Only this operation has any business writing files below the
+        # kernel_prefix; in particular we want to trap any case where
+        # glibc's "make install" installs a header in a subdirectory of
+        # /usr/include that belongs to the kernel.
+        cmdlist.add_command('install-mkdir',
+                            ['mkdir', '-p', kernel_prefix])
+        cmdlist.add_command('install-make-writable',
+                            ['chmod', '-R', 'u+w', kernel_prefix])
         cmdlist.add_command('install-headers',
                             ['make', '-C', srcdir, 'O=%s' % builddir,
                              'ARCH=%s' % linux_arch,
-                             'INSTALL_HDR_PATH=%s' % headers_dir,
+                             'INSTALL_HDR_PATH=%s' % kernel_prefix,
                              'headers_install'])
         cmdlist.cleanup_dir()
+        cmdlist.add_command('install-make-read-only',
+                            ['chmod', '-R', 'a-w', kernel_prefix])
+        cmdlist.link_dir_contents(kernel_headers_dir, all_headers_dir)
+        # Currently, some headers in /usr/include/scsi are provided by
+        # the kernel and some by glibc.  This should be cleaned up, but
+        # for the time being we have to special-case this directory.
+        cmdlist.add_command('link-scsi-rm',
+                            ['rm', '-f', os.path.join(all_headers_dir, 'scsi')])
+        cmdlist.link_dir_contents(
+            os.path.join(kernel_headers_dir, 'scsi'),
+            os.path.join(all_headers_dir, 'scsi'),
+            'scsi')
         cmdlist.pop_subdesc()
 
     def build_gcc(self, cmdlist, bootstrap):
@@ -1357,6 +1379,11 @@ class Glibc(object):
                    'RANLIB=%s' % self.tool_name('ranlib'),
                    'READELF=%s' % self.tool_name('readelf'),
                    'STRIP=%s' % self.tool_name('strip')]
+        if self.os.startswith('linux'):
+            cfg_cmd.append('--with-headers=%s'
+                           % os.path.join(self.compiler.sysroot,
+                                          'usr/share/linux/include'))
+
         cfg_cmd += self.cfg
         cmdlist.add_command('configure', cfg_cmd)
         cmdlist.add_command('build', ['make'])
@@ -1389,8 +1416,7 @@ class Command(object):
         self.dir = dir
         self.path = path
         self.desc = desc
-        trans = str.maketrans({' ': '-'})
-        self.logbase = '%03d-%s' % (num, desc.translate(trans))
+        self.logbase = '%03d-%s' % (num, desc.replace(' ', '-'))
         self.command = command
         self.always_run = always_run
 
@@ -1401,10 +1427,8 @@ class Command(object):
         assert '\n' not in s
         if re.fullmatch('[]+,./0-9@A-Z_a-z-]+', s):
             return s
-        strans = str.maketrans({"'": "'\\''"})
-        s = "'%s'" % s.translate(strans)
-        mtrans = str.maketrans({'$': '$$'})
-        return s.translate(mtrans)
+        s = "'%s'" % s.replace("'", "'\\''")
+        return s.replace('$', '$$')
 
     @staticmethod
     def shell_make_quote_list(l, translate_make):
@@ -1470,6 +1494,32 @@ class CommandList(object):
         self.add_command_dir('copy-mkdir', None, ['mkdir', '-p', parent])
         self.add_command_dir('copy', None, ['cp', '-a', src, dest])
 
+    def link_dir_contents(self, src, dest, tag=''):
+        """Create relative symbolic links in DEST pointing to each of the
+        files and directories in SRC.  If DEST does not already exist,
+        it is created."""
+        relpath = os.path.relpath(src, dest)
+        if tag:
+            mkdir_tag = 'link-%s-mkdir' % tag
+            link_tag = 'link-%s' % tag
+        else:
+            mkdir_tag = 'link-mkdir'
+            link_tag = 'link'
+
+        self.add_command_dir(mkdir_tag, None, ['mkdir', '-p', dest])
+
+        # The exec() below works around a limitation of the Python
+        # grammar; a 'compound' statement (like 'for f in ...: ...')
+        # cannot be set off from a preceding statement with a
+        # semicolon, only a newline.  Since shell_make_quote can't
+        # quote newlines, we have to turn the loop into a 'simple'
+        # statement somehow.
+        self.add_command_dir(link_tag, dest, [
+            sys.executable, '-c',
+            "import glob, os, sys;"
+            "exec(\"for f in glob.glob(os.path.join(sys.argv[1], '*')):"
+            " os.symlink(f, os.path.basename(f))\")", relpath])
+
     def add_command_dir(self, desc, dir, command, always_run=False):
         """Add a command to run in a given directory."""
         cmd = Command(self.desc_txt(desc), len(self.cmdlist), dir, self.path,
-- 
2.13.2

[PATCH 2/2] build-many-glibcs: Impose a memory limit on build processes.

[Zack Weinberg]

There are sometimes bugs in the compiler
(e.g. https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78460) that cause
it to consume all available memory.  To limit the impact of this on
automated test robots, impose memory limits on all subprocesses in
build-many-glibcs.py.  When the bug hits, the compiler will still run
out of memory and crash, but that should not affect any other
simultaneous task.

The limit can be configured with the --memory-limit command line switch.
The default is 1.5 gigabytes or physical RAM divided by the number of
jobs to run in parallel, whichever is larger.  (Empirically, 1.5 gigs
per process is enough for everything but the files affected by GCC
bug 78640, but 1 gig per process is insufficient for some of the math
tests and also for the "genautomata" step when building compilers for
powerpc64.)

Rather than continue to lengthen the argument list of the Context
constructor, it now takes the entire 'opts' object as its sole argument.

	* scripts/build-many-glibcs.py (total_ram): New function.
	(Context.set_memory_limits): New function.
	(Context.run_builds): Call set_memory_limits immediately before
	do_build.
	(get_parser): Add --memory-limit command-line switch.
	(Context.__init__): Take 'opts' object as sole argument.
	Add 'memory_limit' attribute to self.  Make topdir absolute here.
	(main): Update to match.
---
 scripts/build-many-glibcs.py | 105 +++++++++++++++++++++++++++++++++++--------
 1 file changed, 87 insertions(+), 18 deletions(-)

diff --git a/scripts/build-many-glibcs.py b/scripts/build-many-glibcs.py
index 7b267d14d9..a4d72f29e4 100755
--- a/scripts/build-many-glibcs.py
+++ b/scripts/build-many-glibcs.py
@@ -86,33 +86,56 @@ except:
 
     subprocess.run = _run
 
+def total_ram():
+    """Retrieve the total amount of physical RAM available on this computer."""
+
+    # This can't be done cross-platform using the Python standard library.
+    # If the add-on 'psutil' module is available, use it.
+    try:
+        import psutil
+        # Despite the name, virtual_memory() counts only physical RAM, not swap.
+        return psutil.virtual_memory().total
+
+    except ImportError:
+        pass
+
+    # This works on Linux, but (reportedly) not on all other Unixes.
+    try:
+        return \
+            os.sysconf('SC_PAGESIZE') * os.sysconf('SC_PHYS_PAGES')
+
+    except:
+        pass
+
+    # We don't know.  Return a very large number.
+    return sys.maxsize
 
 class Context(object):
     """The global state associated with builds in a given directory."""
 
-    def __init__(self, topdir, parallelism, keep, replace_sources, strip,
-                 action):
+    def __init__(self, opts):
         """Initialize the context."""
-        self.topdir = topdir
-        self.parallelism = parallelism
-        self.keep = keep
-        self.replace_sources = replace_sources
-        self.strip = strip
-        self.srcdir = os.path.join(topdir, 'src')
+        self.topdir = os.path.abspath(opts.topdir)
+        self.parallelism = opts.parallelism
+        self.memory_limit = opts.memory_limit
+        self.keep = opts.keep
+        self.replace_sources = opts.replace_sources
+        self.strip = opts.strip
+        self.srcdir = os.path.join(self.topdir, 'src')
         self.versions_json = os.path.join(self.srcdir, 'versions.json')
-        self.build_state_json = os.path.join(topdir, 'build-state.json')
-        self.bot_config_json = os.path.join(topdir, 'bot-config.json')
-        self.installdir = os.path.join(topdir, 'install')
+        self.build_state_json = os.path.join(self.topdir, 'build-state.json')
+        self.bot_config_json = os.path.join(self.topdir, 'bot-config.json')
+        self.installdir = os.path.join(self.topdir, 'install')
         self.host_libraries_installdir = os.path.join(self.installdir,
                                                       'host-libraries')
-        self.builddir = os.path.join(topdir, 'build')
-        self.logsdir = os.path.join(topdir, 'logs')
-        self.logsdir_old = os.path.join(topdir, 'logs-old')
+        self.builddir = os.path.join(self.topdir, 'build')
+        self.logsdir = os.path.join(self.topdir, 'logs')
+        self.logsdir_old = os.path.join(self.topdir, 'logs-old')
         self.makefile = os.path.join(self.builddir, 'Makefile')
         self.wrapper = os.path.join(self.builddir, 'wrapper')
         self.save_logs = os.path.join(self.builddir, 'save-logs')
         self.script_text = self.get_script_text()
-        if action != 'checkout':
+        if opts.action != 'checkout':
             self.build_triplet = self.get_build_triplet()
             self.glibc_version = self.get_glibc_version()
         self.configs = {}
@@ -134,6 +157,50 @@ class Context(object):
         sys.stdout.flush()
         os.execv(sys.executable, [sys.executable] + sys.argv)
 
+    def set_memory_limits(self):
+        """Impose a memory-consumption limit on this process, and therefore
+           all of the subprocesses it creates.  The limit can be set
+           on the command line; the default is either physical RAM
+           divided by the number of jobs to be run in parallel, or 1.5
+           gigabytes, whichever is larger.  (1GB is too small for
+           genautomata on MIPS and for the compilation of several
+           large math test cases.)
+        """
+        if self.memory_limit == 0:
+            return
+        try:
+            import resource
+        except ImportError as e:
+            print('warning: cannot set memory limit:', e)
+            return
+
+        if self.memory_limit is None:
+            physical_ram = total_ram()
+            memory_limit = int(max(physical_ram / self.parallelism,
+                                   1.5 * 1024 * 1024 * 1024))
+        else:
+            if memory_limit < 1.5:
+                print('warning: memory limit %.5g GB known to be too small'
+                      % memory_limit)
+            memory_limit = int(memory_limit * 1024 * 1024 * 1024)
+
+        set_a_limit = False
+        for mem_rsrc_name in ['RLIMIT_DATA', 'RLIMIT_STACK', 'RLIMIT_RSS',
+                              'RLIMIT_VMEM', 'RLIMIT_AS']:
+            mem_rsrc = getattr(resource, mem_rsrc_name, None)
+            if mem_rsrc is not None:
+                soft, hard = resource.getrlimit(mem_rsrc)
+                if hard == resource.RLIM_INFINITY or hard > memory_limit:
+                    hard = memory_limit
+                if soft == resource.RLIM_INFINITY or soft > hard:
+                    soft = hard
+                resource.setrlimit(mem_rsrc, (soft, hard))
+                set_a_limit = True
+
+        if set_a_limit:
+            print("Per-process memory limit set to %.5g GB." %
+                  (memory_limit / (1024 * 1024 * 1024)))
+
     def get_build_triplet(self):
         """Determine the build triplet with config.guess."""
         config_guess = os.path.join(self.component_srcdir('gcc'),
@@ -465,6 +532,7 @@ class Context(object):
             old_versions = self.build_state['compilers']['build-versions']
             self.build_glibcs(configs)
         self.write_files()
+        self.set_memory_limits()
         self.do_build()
         if configs:
             # Partial build, do not update stored state.
@@ -1589,6 +1657,9 @@ def get_parser():
     parser.add_argument('-j', dest='parallelism',
                         help='Run this number of jobs in parallel',
                         type=int, default=os.cpu_count())
+    parser.add_argument('--memory-limit',
+                        help='Per-process memory limit in gigabytes (0 for unlimited)',
+                        type=float, default=None)
     parser.add_argument('--keep', dest='keep',
                         help='Whether to keep all build directories, '
                         'none or only those from failed builds',
@@ -1614,9 +1685,7 @@ def main(argv):
     """The main entry point."""
     parser = get_parser()
     opts = parser.parse_args(argv)
-    topdir = os.path.abspath(opts.topdir)
-    ctx = Context(topdir, opts.parallelism, opts.keep, opts.replace_sources,
-                  opts.strip, opts.action)
+    ctx = Context(opts)
     ctx.run_builds(opts.action, opts.configs)
 
 
-- 
2.13.2

Re: [PATCH 2/2] build-many-glibcs: Impose a memory limit on build processes.

[Joseph Myers]

On Sun, 9 Jul 2017, Zack Weinberg wrote:

> The limit can be configured with the --memory-limit command line switch.
> The default is 1.5 gigabytes or physical RAM divided by the number of
> jobs to run in parallel, whichever is larger.  (Empirically, 1.5 gigs
> per process is enough for everything but the files affected by GCC
> bug 78640, but 1 gig per process is insufficient for some of the math
> tests and also for the "genautomata" step when building compilers for
> powerpc64.)

I think the default should allow more room than just being slightly above 
what's enough right now in a particular build (does --enable-checking, 
i.e. GCC mainline, use more memory? I don't know offhand) - say 3 GB.  
(The default may be different from the threshold for the warning for 
values expected to be too small.)

> +    def set_memory_limits(self):
> +        """Impose a memory-consumption limit on this process, and therefore
> +           all of the subprocesses it creates.  The limit can be set
> +           on the command line; the default is either physical RAM
> +           divided by the number of jobs to be run in parallel, or 1.5
> +           gigabytes, whichever is larger.  (1GB is too small for
> +           genautomata on MIPS and for the compilation of several
> +           large math test cases.)

Is it both powerpc64 and MIPS that need more than 1 GB, then?

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH 2/2] build-many-glibcs: Impose a memory limit on build processes.

[Zack Weinberg]

On Mon, Jul 10, 2017 at 6:48 AM, Joseph Myers <joseph@codesourcery.com> wrote:
> On Sun, 9 Jul 2017, Zack Weinberg wrote:
>
>> The limit can be configured with the --memory-limit command line switch.
>> The default is 1.5 gigabytes or physical RAM divided by the number of
>> jobs to run in parallel, whichever is larger.  (Empirically, 1.5 gigs
>> per process is enough for everything but the files affected by GCC
>> bug 78640, but 1 gig per process is insufficient for some of the math
>> tests and also for the "genautomata" step when building compilers for
>> powerpc64.)
>
> I think the default should allow more room than just being slightly above
> what's enough right now in a particular build (does --enable-checking,
> i.e. GCC mainline, use more memory? I don't know offhand) - say 3 GB.
> (The default may be different from the threshold for the warning for
> values expected to be too small.)

So I actually wanted to make it _lower_ -- my usual build machine has
8 CPUs and 8G of RAM, so anything higher than 1G/concurrent process
risks going into swap.  But then genautomata. :-(

I think what I'll do -- after the powerpc situation is resolved -- is
put together some instrumentation to find out how much RAM is used
both on average and at worst; that will let us make more informed
decisions about the defaults.  I'll try GCC mainline as well.

>> +           gigabytes, whichever is larger.  (1GB is too small for
>> +           genautomata on MIPS and for the compilation of several
>> +           large math test cases.)
>
> Is it both powerpc64 and MIPS that need more than 1 GB, then?

No, it's just powerpc64, that's a mistake in the comment.

zw

Re: [PATCH 2/2] build-many-glibcs: Impose a memory limit on build processes.

[Joseph Myers]

On Mon, 10 Jul 2017, Zack Weinberg wrote:

> So I actually wanted to make it _lower_ -- my usual build machine has
> 8 CPUs and 8G of RAM, so anything higher than 1G/concurrent process
> risks going into swap.  But then genautomata. :-(
> 
> I think what I'll do -- after the powerpc situation is resolved -- is
> put together some instrumentation to find out how much RAM is used
> both on average and at worst; that will let us make more informed
> decisions about the defaults.  I'll try GCC mainline as well.

Since there are six SH multilibs, 6 * limit + (CPUs - 6) * average is 
probably the peak amount that gets used in practice.  (My bots use a limit 
of 16 GB - set before starting build-many-glibcs.py - on systems with 32 
hardware threads and 128 GB RAM.)

Also note incidentally: if anyone wanted to use build-many-glibcs.py to 
run build tools that had been built with Address Sanitizer, they'd need to 
disable the limits (Address Sanitizer uses a very large amount of address 
space, I think 16 TB).

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH 1/2] build-many-glibcs: Install kernel headers separately.

[Joseph Myers]

On Sun, 9 Jul 2017, Zack Weinberg wrote:

> build of GCC to work correctly.  The way it is created is a little
> kludgey and exposed a bug in the shell quotation logic, but it works.

This code, using python with exec, seems much more convoluted than 
necessary.  I'd expect something more like:

(run in directory SYSROOT/usr/include, using add_command_dir)
  sh -c 'ln -s ../share/linux/* .'
rm -f SYSROOT/usr/include/scsi
mkdir SYSROOT/usr/include/scsi
(run in directory SYSROOT/usr/include/scsi, using add_command_dir)
  sh -c 'ln -s ../../share/linux/scsi/* .'

where the use of sh -c is so that the * gets expanded by the shell rather 
than quoted to prevent expansion.

> Note that $(sysroot)/usr/share/linux/include is made read-only to all
> after it is created, in order to catch cases where glibc's "make

I'm concerned that this would (a) mean the existing shutil.rmtree use 
fails to remove the whole installation directory as it's meant to, and (b) 
any rm -rf done externally on an install tree also fails to do so.  I 
don't think a build script should leave around read-only directories; 
that's unfriendly to users who expect to be able to rm -rf the resulting 
directories.

To make a directory read-only like that I think you'd need to (a) add a 
cleanup task (one running even if previous tasks failed) that makes the 
directories writable again, and (b) change remove_dirs so it tries to make 
directories writable before removing them (so it works properly if a 
previous build was interrupted while the directories were read-only).

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH 1/2] build-many-glibcs: Install kernel headers separately.

[Zack Weinberg]

On Mon, Jul 10, 2017 at 6:41 AM, Joseph Myers <joseph@codesourcery.com> wrote:
> On Sun, 9 Jul 2017, Zack Weinberg wrote:
>
>> build of GCC to work correctly.  The way it is created is a little
>> kludgey and exposed a bug in the shell quotation logic, but it works.
>
> This code, using python with exec, seems much more convoluted than
> necessary.  I'd expect something more like:
>
> (run in directory SYSROOT/usr/include, using add_command_dir)
>   sh -c 'ln -s ../share/linux/* .'
> rm -f SYSROOT/usr/include/scsi
> mkdir SYSROOT/usr/include/scsi
> (run in directory SYSROOT/usr/include/scsi, using add_command_dir)
>   sh -c 'ln -s ../../share/linux/scsi/* .'

It did not occur to me to use that form of ln -s.  I will experiment
with this and see how it goes.

>> Note that $(sysroot)/usr/share/linux/include is made read-only to all
>> after it is created, in order to catch cases where glibc's "make
>
> I'm concerned that this would (a) mean the existing shutil.rmtree use
> fails to remove the whole installation directory as it's meant to, and (b)
> any rm -rf done externally on an install tree also fails to do so.  I
> don't think a build script should leave around read-only directories;
> that's unfriendly to users who expect to be able to rm -rf the resulting
> directories.

I now realize that to catch the problem I want it to catch, it just
needs to be read-only during the glibc 'make install' _inside_ the
compilers step.  The 'make install' during a glibcs build goes to a
different location anyway.  I'll see what i can do about that.

> To make a directory read-only like that I think you'd need to (a) add a
> cleanup task (one running even if previous tasks failed) that makes the
> directories writable again, and (b) change remove_dirs so it tries to make
> directories writable before removing them (so it works properly if a
> previous build was interrupted while the directories were read-only).

Noted.

zw