goma_link: handle function sections and data sections

Local ThinLTO defaults to emitting data objects and functions in
dedicated sections to facilitate dead stripping. This change adds
the same behavior to the distributed ThinLTO scripts.

BUG=1078453
TEST=tools/clang/scripts/goma_link_unit_tests.py; \
  tools/clang/scripts/goma_link_integration_tests.py; \
  check that the expected options were passed

Change-Id: Ie5e442bf20f886b899a80a8443fce47a709c006f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2231034
Commit-Queue: Bob Haarman <inglorion@chromium.org>
Reviewed-by: Tiancong Wang <tcwang@google.com>
Cr-Commit-Position: refs/heads/master@{#775311}

tools/clang/scripts/goma_ld.py

@@ -26,6 +26,8 @@ class GomaLinkUnix(goma_link.GomaLinkBase):
   WL = '-Wl,'
   TLTO = '-plugin-opt=thinlto'
   SEP = '='
+  DATA_SECTIONS = '-fdata-sections'
+  FUNCTION_SECTIONS = '-ffunction-sections'
   GROUP_RE = re.compile(WL + '--(?:end|start)-group')
   MACHINE_RE = re.compile('-m([0-9]+)')
   OBJ_PATH = '-plugin-opt=obj-path' + SEP

tools/clang/scripts/goma_link.py

@@ -218,6 +218,10 @@ class GomaLinkBase(object):
   jobs = None
 
   # These constants should work across platforms.
+  DATA_SECTIONS_RE = re.compile('-f(no-)?data-sections|[-/]Gw(-)?',
+                                re.IGNORECASE)
+  FUNCTION_SECTIONS_RE = re.compile('-f(no-)?function-sections|[-/]Gy(-)?',
+                                    re.IGNORECASE)
   LIB_RE = re.compile('.*\\.(?:a|lib)', re.IGNORECASE)
   LTO_RE = re.compile('|'.join((
       '-fsanitize=cfi.*',
@@ -227,8 +231,36 @@ class GomaLinkBase(object):
       '-Wl,--lto.*',
       '-Wl,--thin.*',
   )))
+  MLLVM_RE = re.compile('(?:-Wl,)?([-/]mllvm)[:=]?(.*)', re.IGNORECASE)
   OBJ_RE = re.compile('(.*)\\.(o(?:bj)?)', re.IGNORECASE)
 
+  def transform_codegen_param(self, param):
+    return self.transform_codegen_param_common(param)
+
+  def transform_codegen_param_common(self, param):
+    """
+    If param is a parameter relevant to code generation, returns the
+    parameter in a form that is suitable to pass to clang.  For values
+    of param that are not relevant to code generation, returns None.
+    """
+    match = self.MACHINE_RE.match(param)
+    if match and match.group(1).lower() in ['x86', 'i386', 'arm', '32']:
+      return ['-m32']
+    match = self.MLLVM_RE.match(param)
+    if match:
+      if match.group(2):
+        return ['-mllvm', match.group(2)]
+      else:
+        return ['-mllvm']
+    if (param.startswith('-f') and not param.startswith('-flto')
+        and not param.startswith('-fsanitize')
+        and not param.startswith('-fthinlto')
+        and not param.startswith('-fwhole-program')):
+      return [param]
+    if param.startswith('-g'):
+      return [param]
+    return None
+
   def output_path(self, args):
     """
     Analyzes command line arguments in args and returns the output
@@ -354,33 +386,11 @@ class GomaLinkBase(object):
     ]
     final_params = []
     in_mllvm = [False]
-    optlevel = [2]
-
-    MLLVM_RE = re.compile('(?:-Wl,)?([-/]mllvm)[:=]?(.*)', re.IGNORECASE)
 
-    def transform_codegen_param(param):
-      """
-      If param is a parameter relevant to code generation, returns the
-      parameter in a form that is suitable to pass to clang.  For values
-      of param that are not relevant to code generation, returns None.
-      """
-      match = self.MACHINE_RE.match(param)
-      if match and match.group(1).lower() in ['x86', 'i386', 'arm', '32']:
-        return ['-m32']
-      match = MLLVM_RE.match(param)
-      if match:
-        if match.group(2):
-          return ['-mllvm', match.group(2)]
-        else:
-          return ['-mllvm']
-      if (param.startswith('-f') and not param.startswith('-flto')
-          and not param.startswith('-fsanitize')
-          and not param.startswith('-fthinlto')
-          and not param.startswith('-fwhole-program')):
-        return [param]
-      if param.startswith('-g'):
-        return [param]
-      return None
+    # Defaults that match those for local linking.
+    optlevel = [2]
+    data_sections = [True]
+    function_sections = [True]
 
     def extract_opt_level(param):
       """
@@ -422,17 +432,24 @@ class GomaLinkBase(object):
           return
 
         # Check for params that affect code generation.
-        cg_param = transform_codegen_param(param)
+        cg_param = self.transform_codegen_param(param)
         if cg_param:
           codegen_params.extend(cg_param)
           # No return here, we still want to check for -mllvm.
 
         # Check for -mllvm.
-        match = MLLVM_RE.match(param)
+        match = self.MLLVM_RE.match(param)
         if match and not match.group(2):
           # Next parameter will be the thing to pass to LLVM.
           in_mllvm[0] = True
 
+      # Parameters that override defaults disable the defaults; the
+      # final value is set by passing through the parameter.
+      if self.DATA_SECTIONS_RE.match(param):
+        data_sections[0] = False
+      if self.FUNCTION_SECTIONS_RE.match(param):
+        function_sections[0] = False
+
       helper()
       if self.GROUP_RE.match(param):
         return
@@ -471,6 +488,10 @@ class GomaLinkBase(object):
       return None
 
     codegen_params.append('-O' + str(optlevel[0]))
+    if data_sections[0]:
+      codegen_params.append(self.DATA_SECTIONS)
+    if function_sections[0]:
+      codegen_params.append(self.FUNCTION_SECTIONS)
 
     if use_common_objects:
       splitfile = None
@@ -623,6 +644,8 @@ class GomaLinkWindows(GomaLinkBase):
   WL = ''
   TLTO = '-thinlto'
   SEP = ':'
+  DATA_SECTIONS = '-Gw'
+  FUNCTION_SECTIONS = '-Gy'
   GROUP_RE = re.compile(WL + '--(?:end|start)-group')
   MACHINE_RE = re.compile('[-/]machine:(.*)', re.IGNORECASE)
   OBJ_PATH = '-lto-obj-path' + SEP
@@ -641,6 +664,14 @@ class GomaLinkWindows(GomaLinkBase):
       'tls_edit.exe',
   }
 
+  def transform_codegen_param(self, param):
+    # In addition to parameters handled by transform_codegen_param_common,
+    # we pass on parameters that start in 'G' or 'Q', which are
+    # MSVC-style parameters that affect code generation.
+    if len(param) >= 2 and param[0] in ['-', '/'] and param[1] in ['G', 'Q']:
+      return [param]
+    return self.transform_codegen_param_common(param)
+
   def process_output_param(self, args, i):
     """
     If args[i] is a parameter that specifies the output file,

tools/clang/scripts/goma_link_unit_tests.py

@@ -125,6 +125,84 @@ class GomaLinkUnitTest(unittest.TestCase):
 
       self.assertNotIn('-flto=thin', result.final_params)
 
+  def test_codegen_params_default(self):
+    with FakeFs(bitcode_files=['foo.o'], other_files=['bar.o']):
+      result = goma_ld.GomaLinkUnix().analyze_expanded_args(
+          ['clang', 'foo.o', 'bar.o', '-o', 'foo'], 'foo', 'clang', 'lto.foo',
+          'common', False)
+      # Codegen optimization level should default to 2.
+      self.assertIn('-O2', result.codegen_params)
+      # -fdata-sections and -ffunction-sections default to on to match the
+      # behavior of local linking.
+      self.assertIn('-fdata-sections', result.codegen_params)
+      self.assertIn('-ffunction-sections', result.codegen_params)
+
+  def test_codegen_params_default_cl(self):
+    with FakeFs(bitcode_files=['foo.obj'], other_files=['bar.obj']):
+      result = goma_link.GomaLinkWindows().analyze_expanded_args(
+          ['clang-cl', 'foo.obj', 'bar.obj', '-Fefoo.exe'], 'foo.exe',
+          'clang-cl', 'lto.foo', 'common', False)
+      # Codegen optimization level should default to 2.
+      self.assertIn('-O2', result.codegen_params)
+      # -Gw and -Gy default to on to match the behavior of local linking.
+      self.assertIn('-Gw', result.codegen_params)
+      self.assertIn('-Gy', result.codegen_params)
+
+  def test_codegen_params_no_data_sections(self):
+    with FakeFs(bitcode_files=['foo.o'], other_files=['bar.o']):
+      result = goma_ld.GomaLinkUnix().analyze_expanded_args(
+          ['clang', '-fno-data-sections', 'foo.o', 'bar.o', '-o', 'foo'], 'foo',
+          'clang', 'lto.foo', 'common', False)
+      self.assertNotIn('-fdata-sections', result.codegen_params)
+      self.assertIn('-ffunction-sections', result.codegen_params)
+
+  def test_codegen_params_no_function_sections(self):
+    with FakeFs(bitcode_files=['foo.o'], other_files=['bar.o']):
+      result = goma_ld.GomaLinkUnix().analyze_expanded_args(
+          ['clang', '-fno-function-sections', 'foo.o', 'bar.o', '-o', 'foo'],
+          'foo', 'clang', 'lto.foo', 'common', False)
+      self.assertIn('-fdata-sections', result.codegen_params)
+      self.assertNotIn('-ffunction-sections', result.codegen_params)
+
+  def test_codegen_params_no_data_sections_cl(self):
+    with FakeFs(bitcode_files=['foo.obj'], other_files=['bar.obj']):
+      result = goma_link.GomaLinkWindows().analyze_expanded_args(
+          ['clang-cl', '/Gw-', 'foo.obj', 'bar.obj', '/Fefoo.exe'], 'foo.exe',
+          'clang-cl', 'lto.foo', 'common', False)
+      self.assertNotIn('-fdata-sections', result.codegen_params)
+      self.assertNotIn('-Gw', result.codegen_params)
+      self.assertNotIn('/Gw', result.codegen_params)
+      self.assertIn('-Gy', result.codegen_params)
+
+  def test_codegen_params_no_function_sections_cl(self):
+    with FakeFs(bitcode_files=['foo.obj'], other_files=['bar.obj']):
+      result = goma_link.GomaLinkWindows().analyze_expanded_args(
+          ['clang-cl', '/Gy-', 'foo.obj', 'bar.obj', '/Fefoo.exe'], 'foo.exe',
+          'clang-cl', 'lto.foo', 'common', False)
+      self.assertIn('-Gw', result.codegen_params)
+      self.assertNotIn('-ffunction-sections', result.codegen_params)
+      self.assertNotIn('-Gy', result.codegen_params)
+      self.assertNotIn('/Gy', result.codegen_params)
+
+  def test_codegen_params_explicit_data_and_function_sections(self):
+    with FakeFs(bitcode_files=['foo.o'], other_files=['bar.o']):
+      result = goma_ld.GomaLinkUnix().analyze_expanded_args([
+          'clang', '-ffunction-sections', '-fdata-sections', 'foo.o', 'bar.o',
+          '-o', 'foo'
+      ], 'foo', 'clang', 'lto.foo', 'common', False)
+      self.assertIn('-fdata-sections', result.codegen_params)
+      self.assertIn('-ffunction-sections', result.codegen_params)
+
+  def test_codegen_params_explicit_data_and_function_sections_cl(self):
+    with FakeFs(bitcode_files=['foo.obj'], other_files=['bar.obj']):
+      result = goma_link.GomaLinkWindows().analyze_expanded_args(
+          ['clang-cl', '/Gy', '-Gw', 'foo.obj', 'bar.obj', '/Fefoo.exe'],
+          'foo.exe', 'clang-cl', 'lto.foo', 'common', False)
+      self.assertIn('-Gw', result.codegen_params)
+      self.assertIn('/Gy', result.codegen_params)
+      self.assertNotIn('-fdata-sections', result.codegen_params)
+      self.assertNotIn('-ffunction-sections', result.codegen_params)
+
   def test_ensure_file_no_dir(self):
     with named_directory() as d, working_directory(d):
       self.assertFalse(os.path.exists('test'))