[lld][test] Precommit test for ld -r links with FatLTO PIC objects

[ilovepi]

Currently, with PIC relocatable links, FatLTO sections are treated as orphan
sections and incorrectly concatenated together. This test verifies the current
behavior, but should be fixed to either merge those sections into a single llvm
module, similar to what llvm-link would produce, or to drop them altogether.

[llvmbot]

@llvm/pr-subscribers-lld

@llvm/pr-subscribers-lld-elf

Author: Paul Kirth (ilovepi)

Changes

Currently, with PIC relocatable links, FatLTO sections are treated as orphan
sections and incorrectly concatenated together. This test verifies the current
behavior, but should be fixed to either merge those sections into a single llvm
module, similar to what llvm-link would produce, or to drop them altogether.

---

Full diff: https://github.com/llvm/llvm-project/pull/92817.diff

1 Files Affected:

• (modified) lld/test/ELF/fatlto/fatlto.test (+41)

diff --git a/lld/test/ELF/fatlto/fatlto.test b/lld/test/ELF/fatlto/fatlto.test
index edf4ef2da2b88..2b9c41d5f5587 100644
--- a/lld/test/ELF/fatlto/fatlto.test
+++ b/lld/test/ELF/fatlto/fatlto.test
@@ -49,6 +49,25 @@
 ; RUN: ld.lld -o %t/foo-fatLTO.archive %t/a.a %t/main-LTO.bc --fat-lto-objects
 ; RUN: cmp %t/foo-fatLTO.archive %t/foo-LTO
 
+;; Test FatLTO works with relocatable links using PIC objects
+;; Currently, with PIC relocatable links, FatLTO sections are treated as
+;; orphan sections and incorrectly concatenated together. This test verifies
+;; the current behavior, but should be fixed to either merge those sections
+;; correctly, or to drop them altogether.
+; RUN: opt < %t/a-LTO.ll -passes="embed-bitcode<thinlto;emit-summary>" | llc --relocation-model=pic --filetype=obj -o %t/a-fat-pic.o
+; RUN: llvm-readobj -S %t/a-fat-pic.o | FileCheck --check-prefix=HAS_LLVM_LTO %s
+
+; RUN: opt < %t/b-LTO.ll -passes="embed-bitcode<thinlto;emit-summary>" | llc --relocation-model=pic --filetype=obj -o %t/b-fat-pic.o
+; RUN: llvm-readobj -S %t/b-fat-pic.o | FileCheck --check-prefix=HAS_LLVM_LTO %s
+
+; RUN: llvm-ar rcs %t/fat.archive %t/a-fat-pic.o %t/b-fat-pic.o
+; RUN: llvm-readobj -S %t/fat.pic.archive | FileCheck --check-prefix=HAS_LLVM_LTO %s
+
+; RUN: ld.lld --whole-archive %t/fat.pic.archive -r -o %t/fat-relocatable.o
+; RUN: llvm-readobj -S %t/fat-pic-relocatable.o | FileCheck --check-prefix=HAS_LLVM_LTO %s
+
+; HAS_LLVM_LTO: Name: .llvm.lto
+
 ;--- a-LTO.ll
 target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
@@ -71,6 +90,28 @@ attributes #0 = { noinline nounwind uwtable }
 !5 = !{i32 1, !"ThinLTO", i32 0}
 !6 = !{i32 1, !"EnableSplitLTOUnit", i32 1}
 
+;--- b-LTO.ll
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; Function Attrs: noinline nounwind uwtable
+define dso_local i32 @foo() #0 {
+entry:
+  ret i32 0
+}
+
+attributes #0 = { noinline nounwind uwtable }
+
+!llvm.module.flags = !{!0, !1, !2, !3, !4, !5, !6}
+
+!0 = !{i32 1, !"wchar_size", i32 4}
+!1 = !{i32 7, !"PIC Level", i32 2}
+!2 = !{i32 7, !"PIE Level", i32 2}
+!3 = !{i32 7, !"uwtable", i32 2}
+!4 = !{i32 7, !"frame-pointer", i32 2}
+!5 = !{i32 1, !"ThinLTO", i32 0}
+!6 = !{i32 1, !"EnableSplitLTOUnit", i32 1}
+
 ;--- main-LTO.ll
 target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"

[ilovepi]

Seems like there could be some redundancy between the existing run lines and the new tests, but those should probably be separate cleanups.

I think, perhaps we should use opt + llc to make the FatLTO objects, rather than using llvm-objcopy to create the section. When we need to check the bitcode is the same we can just use opt to compile the IR and cmp to the output from the extracted section. Unfortunately, that's kind of testing codegen at the same time. I don't see a good way to decouple that w/o running the risk of not testing the FatLTO outputs correctly...

[MaskRay]

The number of %t occurrences is large now.

Perhaps we should switch to ; RUN: rm -rf %t && split-file %s %t && cd %t and remove these %t/

[ilovepi]

I know that you prefer the cd approach, but I find it hard to re-run commands that require a cd into a temp directory when iterating on tests. Leaving %t/ in the paths allow me to easily copy the failing command and run it from the build directory. If you feel strongly, I can still change it, but my preference is to leave this as is.

[ilovepi]

@MaskRay , just double checking that you're OK with this as is before I land this.

[MaskRay]

I think the test is complex enough that removing some %t/ occurrences will help readability...

I don't use llvm-lit for one-shot test invocation. I use ff fatlto.test and it's quite straightforward to copy & paste one single command...

#!/usr/bin/env python3
# Simplified lit runner for a single test.
import argparse
import os, re, sys
from ipdb import set_trace as bp

def main():
    ap = argparse.ArgumentParser()
    ap.add_argument('-b', '--build', default='/tmp/Debug', help='build directory')
    ap.add_argument('-d', '--debug', type=int, default=-1, help='rr record the specified command')
    ap.add_argument('-r', '--run', type=int, default=-1, help='run the specified command')
    ap.add_argument('--release', action='store_true', help='use /tmp/Rel as the build directory')
    ap.add_argument('--binutils', action='store_true')
    ap.add_argument('-n', action='store_true', help='dry run')
    ap.add_argument('file')
    args = ap.parse_args()
    if args.debug >= 0:
        args.run = args.debug

    home = os.path.expanduser('~')
    build_dir = '/tmp/Rel' if args.release else args.build
    os.environ["PATH"] = os.path.expanduser(build_dir + '/bin') + os.pathsep + os.environ['PATH']
    os.environ['LLVM_DISABLE_SYMBOLIZATION'] = '1'
    os.environ['LLD_IN_TEST'] = '1'
    os.environ['LLD_VERSION'] = 'LLD 1.0'

    # GNU binutils
    if args.binutils:
        del sys.argv[1]
        os.environ["PATH"] = os.path.expanduser('~/Dev/binutils-gdb/out/release/ld') + os.pathsep + os.environ['PATH']

    cwd = os.getcwd()
    fname = os.path.realpath(args.file)
    tmp = os.path.join(os.path.dirname(fname), 'a')
    dirname = os.path.dirname(fname).replace(home, '$HOME')
    lineno = 0
    last = ''
    try:
       fh = open(fname)
    except FileNotFoundError:
        print('not exist', file=sys.stderr)
        return
    substs = sorted([
        ('/p', lambda: os.path.relpath(dirname, cwd)),
        ('/t', lambda: os.path.relpath(tmp, cwd)),
        # See llvm/utils/lit/lit/llvm/config.py
        ('clang', lambda: 'clang'),
        ('clang_cc1', lambda: f'clang -cc1 -internal-isystem {build_dir}/lib/clang/17/include -nostdsysteminc'),
        ('errc_EACCES', lambda: '"Permission denied"'),
        ('itanium_abi_triple', lambda: 'x86_64'),
        ('llc_dwarf', lambda: 'llc'),
        # See lld/test/MachO/lit.local.cfg
        ('lld', lambda: os.path.expanduser('ld64.lld -arch x86_64 -platform_version macos 11.0 11.0 -syslibroot ~/llvm/lld/test/MachO/Inputs/MacOSX.sdk -lSystem -fatal_warnings')),
        ('no-fatal-warnings-lld', lambda: os.path.expanduser('ld64.lld -arch x86_64 -platform_version macos 11.0 11.0 -syslibroot ~/llvm/lld/test/MachO/Inputs/MacOSX.sdk -lSystem')),
        ('p', lambda: os.path.relpath(dirname, cwd)),
        ('python', lambda: 'python3'),
        ('run', lambda: ''),
        ('s', lambda: os.path.relpath(fname, cwd)),
        ('S', lambda: os.path.relpath(dirname, cwd)),
        ('t', lambda: os.path.relpath(tmp, cwd)),
    ], key=lambda x: -len(x[0]))

    with fh as f:
        for line in f.readlines():
            line = line.strip()
            lineno += 1
            if 'END.' in line: break
            match = re.search(r'RUN:[ \t]', line)
            if not match:
                last = ''
                continue
            line = line[match.end():]
            if line[-1] == '\\':
                last += line[:-1]
                continue
            line = last + line
            last = ''

            if 'rm ' in line and ('..' in line or (not re.search(r'rm (-f|-fr|-rf) %t', line))):
                continue

            line1 = ''
            idx = 0
            while (idx1 := line.find('%', idx)) != -1:
                line1 += line[idx:idx1]
                idx = idx1+1
                for s in substs:
                    if line[idx:].startswith(s[0]):
                        line1 += s[1]()
                        idx += len(s[0])
                        break
                else:
                    line1 += line[idx-1]
            line = line1 + line[idx:]

            if args.run < 0:
                print(f'{lineno:^2} {line}')
            if not args.n:
                if os.system(line) != 0:
                    sys.exit(1)
            if args.run == lineno:
                if (idx := line.find('|')) >= 0:
                    line = line[:idx]
                line = f'{build_dir}/bin/{line}'
                if args.debug >= 0:
                    line = 'rr record ' + line
                print(f'### {line}')
                if not args.n:
                    if os.system(line) != 0:
                        sys.exit(1)
                    if args.debug >= 0:
                        os.execvp('rr', ('rr', 'replay', '-d', 'cgdb'))
                break

            if match := re.search(r'\bcd ([/.\w]+)', line):
                os.chdir(match.group(1))
                cwd = os.getcwd()

main()

[ilovepi]

So, while I'm going to change the test to your preferred format, LIT_FILTER= has been a great way to run some subset of tests, and when it fails, I can examine the intermediate outputs by just copying the printed RUN: ... command. In the case where there is a directory change at the top of the test, I can't just grab that command and run it without either figuring out which directory to CD into or prepending the correct prefix to every path in the command. To me the directory change is an anti-pattern that blocks a workflow used extensivly by other developers. It was even mentioned recently on discourse when the output from LIT was modified, https://discourse.llvm.org/t/rfc-improving-lits-debug-output/72839.

For me getting something I can copy, like:

 /path/to/opt -some-pass -o - < /path/to/test/file | FileCheck /path/to/test/file

is really convenient. While the script you provided seems useful, I'd rather not have to use a script just to run a single shell command that I often want to edit by adding different flags or capturing its output.

[MaskRay]

[NFC]

[test] might be better for pure test PRs.

[MaskRay]

Oh, newer tests probably should consider llvm-readelf -S instead llvm-readobj. The tabular output is easy to read.

[ilovepi]

That seems to work fine, so I've updated the checks in this file.

[ilovepi]

@MaskRay Are we happy with the latest version?