[PATCH v3 0/3] Fix static-library related issues

[PATCH v3 0/3] Fix static-library related issues

[Mark Hatle]

Fixes and test case for static-library related items.

v3:
Drop debug hard link change.  My understanding of what was supposed to
happen was incorrect.  I've updated the self-tests to hopefully make it
clearer that the additional links should NOT happen.

Note, the hardlinks for static libraries SHOULD happen though!

v2:
The first three patches were combined into a single patch originally.

This has been split apart to make it more clear the three individual changes.

Mark Hatle (3):
  package.py: Fix static debuginfo split
  package.py: Fix static library processing
  selftest-hardlink: Add additional test cases

 .../selftest-hardlink/selftest-hardlink.bb         | 13 +++++
 meta/lib/oe/package.py                             | 57 ++++++++++++++++++----
 meta/lib/oeqa/selftest/cases/package.py            | 26 ++++++++++
 3 files changed, 87 insertions(+), 9 deletions(-)

-- 
1.8.3.1

[PATCH v3 1/3] package.py: Fix static debuginfo split

[Mark Hatle]

From: Mark Hatle <mark.hatle@amd.com>

Fix:
  NameError: name 'shutil' is not defined

Signed-off-by: Mark Hatle <mark.hatle@amd.com>
Signed-off-by: Mark Hatle <mark.hatle@kernel.crashing.org>
---
 meta/lib/oe/package.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/meta/lib/oe/package.py b/meta/lib/oe/package.py
index e6b46a0..235f2d6 100644
--- a/meta/lib/oe/package.py
+++ b/meta/lib/oe/package.py
@@ -14,6 +14,7 @@ import glob
 import stat
 import mmap
 import subprocess
+import shutil
 
 import oe.cachedpath
 
-- 
1.8.3.1

[PATCH v3 2/3] package.py: Fix static library processing

[Mark Hatle]

From: Mark Hatle <mark.hatle@amd.com>

When PACKAGE_STRIP_STATIC is enabled the system did not pay attention to
hardlinks.  This could trigger a race condition during stripping of static
libraries where multiple strips (through hardlinks) could run at the same
time triggering a truncated or modified file error.

The hardlink breaking code is based on the existing code for elf files, but
due to the nature of the symlinks needed to be done in a separate block of
code.

Add support for static-library debugfs hardlinking through the existing
inode processing code.

Print a note to the logs if the link target can't be found.  This isn't
strictly an error, but may be useful for debugging an issue where a file
isn't present.

Signed-off-by: Mark Hatle <mark.hatle@amd.com>
Signed-off-by: Mark Hatle <mark.hatle@kernel.crashing.org>
---
 meta/lib/oe/package.py | 56 ++++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 47 insertions(+), 9 deletions(-)

diff --git a/meta/lib/oe/package.py b/meta/lib/oe/package.py
index 235f2d6..c213a9a 100644
--- a/meta/lib/oe/package.py
+++ b/meta/lib/oe/package.py
@@ -1079,6 +1079,7 @@ def process_split_and_strip_files(d):
             d.getVar('INHIBIT_PACKAGE_DEBUG_SPLIT') != '1'):
         checkelf = {}
         checkelflinks = {}
+        checkstatic = {}
         for root, dirs, files in cpath.walk(dvar):
             for f in files:
                 file = os.path.join(root, f)
@@ -1092,10 +1093,6 @@ def process_split_and_strip_files(d):
                 if file in skipfiles:
                     continue
 
-                if oe.package.is_static_lib(file):
-                    staticlibs.append(file)
-                    continue
-
                 try:
                     ltarget = cpath.realpath(file, dvar, False)
                     s = cpath.lstat(ltarget)
@@ -1107,6 +1104,13 @@ def process_split_and_strip_files(d):
                     continue
                 if not s:
                     continue
+
+                if oe.package.is_static_lib(file):
+                    # Use a reference of device ID and inode number to identify files
+                    file_reference = "%d_%d" % (s.st_dev, s.st_ino)
+                    checkstatic[file] = (file, file_reference)
+                    continue
+
                 # Check its an executable
                 if (s[stat.ST_MODE] & stat.S_IXUSR) or (s[stat.ST_MODE] & stat.S_IXGRP) \
                         or (s[stat.ST_MODE] & stat.S_IXOTH) \
@@ -1171,6 +1175,27 @@ def process_split_and_strip_files(d):
                 # Modified the file so clear the cache
                 cpath.updatecache(file)
 
+        # Do the same hardlink processing as above, but for static libraries
+        results = list(checkstatic.keys())
+
+        # As above, sort the results.
+        results.sort(key=lambda x: x[0])
+
+        for file in results:
+            # Use a reference of device ID and inode number to identify files
+            file_reference = checkstatic[file][1]
+            if file_reference in inodes:
+                os.unlink(file)
+                os.link(inodes[file_reference][0], file)
+                inodes[file_reference].append(file)
+            else:
+                inodes[file_reference] = [file]
+                # break hardlink
+                bb.utils.break_hardlinks(file)
+                staticlibs.append(file)
+            # Modified the file so clear the cache
+            cpath.updatecache(file)
+
     def strip_pkgd_prefix(f):
         nonlocal dvar
 
@@ -1209,11 +1234,24 @@ def process_split_and_strip_files(d):
                 dest = dv["libdir"] + os.path.dirname(src) + dv["dir"] + "/" + os.path.basename(target) + dv["append"]
                 fpath = dvar + dest
                 ftarget = dvar + dv["libdir"] + os.path.dirname(target) + dv["dir"] + "/" + os.path.basename(target) + dv["append"]
-                bb.utils.mkdirhier(os.path.dirname(fpath))
-                # Only one hardlink of separated debug info file in each directory
-                if not os.access(fpath, os.R_OK):
-                    #bb.note("Link %s -> %s" % (fpath, ftarget))
-                    os.link(ftarget, fpath)
+                if os.access(ftarget, os.R_OK):
+                    bb.utils.mkdirhier(os.path.dirname(fpath))
+                    # Only one hardlink of separated debug info file in each directory
+                    if not os.access(fpath, os.R_OK):
+                        #bb.note("Link %s -> %s" % (fpath, ftarget))
+                        os.link(ftarget, fpath)
+                elif (d.getVar('PACKAGE_DEBUG_STATIC_SPLIT') == '1'):
+                    deststatic = dv["staticlibdir"] + os.path.dirname(src) + dv["staticdir"] + "/" + os.path.basename(file) + dv["staticappend"]
+                    fpath = dvar + deststatic
+                    ftarget = dvar + dv["staticlibdir"] + os.path.dirname(target) + dv["staticdir"] + "/" + os.path.basename(target) + dv["staticappend"]
+                    if os.access(ftarget, os.R_OK):
+                        bb.utils.mkdirhier(os.path.dirname(fpath))
+                        # Only one hardlink of separated debug info file in each directory
+                        if not os.access(fpath, os.R_OK):
+                            #bb.note("Link %s -> %s" % (fpath, ftarget))
+                            os.link(ftarget, fpath)
+                else:
+                    bb.note("Unable to find inode link target %s" % (target))
 
         # Create symlinks for all cases we were able to split symbols
         for file in symlinks:
-- 
1.8.3.1

[PATCH v3 3/3] selftest-hardlink: Add additional test cases

[Mark Hatle]

From: Mark Hatle <mark.hatle@amd.com>

Additional test cases for debug symlink generation both binaries
and static libraries.

This also has the side effect of testing for race conditions in the
hardlink debug generation and stripping.

Signed-off-by: Mark Hatle <mark.hatle@amd.com>
Signed-off-by: Mark Hatle <mark.hatle@kernel.crashing.org>
---
 .../selftest-hardlink/selftest-hardlink.bb         | 13 +++++++++++
 meta/lib/oeqa/selftest/cases/package.py            | 26 ++++++++++++++++++++++
 2 files changed, 39 insertions(+)

diff --git a/meta-selftest/recipes-test/selftest-hardlink/selftest-hardlink.bb b/meta-selftest/recipes-test/selftest-hardlink/selftest-hardlink.bb
index 5632bda..64fea8e 100644
--- a/meta-selftest/recipes-test/selftest-hardlink/selftest-hardlink.bb
+++ b/meta-selftest/recipes-test/selftest-hardlink/selftest-hardlink.bb
@@ -11,6 +11,9 @@ UNPACKDIR = "${S}"
 
 do_compile () {
 	${CC} hello.c -o hello1 ${CFLAGS} ${LDFLAGS}
+
+	${CC} hello.c -c -o hello.o ${CFLAGS}
+	${AR} rcs libhello.a hello.o
 }
 
 do_install () {
@@ -23,9 +26,19 @@ do_install () {
 	ln ${D}${bindir}/hello1 ${D}${libexecdir}/hello3
 	ln ${D}${bindir}/hello1 ${D}${libexecdir}/hello4
 
+	# We need so many hardlink copies to look for specific race conditions
+	install -d ${D}${libdir}
+	install -m 0644 libhello.a ${D}${libdir}
+	for num in `seq 1 100` ; do
+		ln ${D}${libdir}/libhello.a ${D}${libdir}/libhello-${num}.a
+	done
+
 	dd if=/dev/zero of=${D}${bindir}/sparsetest bs=1 count=0 seek=1M
 }
 
 RDEPENDS:${PN}-gdb += "gdb"
 PACKAGES =+ "${PN}-gdb"
 FILES:${PN}-gdb = "${bindir}/gdb.sh"
+
+PACKAGE_STRIP_STATIC = "1"
+PACKAGE_DEBUG_STATIC_SPLIT = "1"
diff --git a/meta/lib/oeqa/selftest/cases/package.py b/meta/lib/oeqa/selftest/cases/package.py
index 1aa6c03..c1b2e23 100644
--- a/meta/lib/oeqa/selftest/cases/package.py
+++ b/meta/lib/oeqa/selftest/cases/package.py
@@ -103,11 +103,37 @@ class PackageTests(OESelftestTestCase):
 
         dest = get_bb_var('PKGDEST', 'selftest-hardlink')
         bindir = get_bb_var('bindir', 'selftest-hardlink')
+        libdir = get_bb_var('libdir', 'selftest-hardlink')
+        libexecdir = get_bb_var('libexecdir', 'selftest-hardlink')
 
         def checkfiles():
             # Recipe creates 4 hardlinked files, there is a copy in package/ and a copy in packages-split/
             # so expect 8 in total.
             self.assertEqual(os.stat(dest + "/selftest-hardlink" + bindir + "/hello1").st_nlink, 8)
+            self.assertEqual(os.stat(dest + "/selftest-hardlink" + libexecdir + "/hello3").st_nlink, 8)
+
+            # Check dbg version
+            # 2 items, a copy in both package/packages-split so 4
+            self.assertEqual(os.stat(dest + "/selftest-hardlink-dbg" + bindir + "/.debug/hello1").st_nlink, 4)
+            self.assertEqual(os.stat(dest + "/selftest-hardlink-dbg" + libexecdir + "/.debug/hello1").st_nlink, 4)
+
+            # Even though the libexecdir name is 'hello3' or 'hello4', that isn't the debug target name
+            self.assertEqual(os.path.exists(dest + "/selftest-hardlink-dbg" + libexecdir + "/.debug/hello3"), False)
+            self.assertEqual(os.path.exists(dest + "/selftest-hardlink-dbg" + libexecdir + "/.debug/hello4"), False)
+
+            # Check the staticdev libraries
+            # 101 items, a copy in both package/packages-split so 202
+            self.assertEqual(os.stat(dest + "/selftest-hardlink-staticdev" + libdir + "/libhello.a").st_nlink, 202)
+            self.assertEqual(os.stat(dest + "/selftest-hardlink-staticdev" + libdir + "/libhello-25.a").st_nlink, 202)
+            self.assertEqual(os.stat(dest + "/selftest-hardlink-staticdev" + libdir + "/libhello-50.a").st_nlink, 202)
+            self.assertEqual(os.stat(dest + "/selftest-hardlink-staticdev" + libdir + "/libhello-75.a").st_nlink, 202)
+
+            # Check static dbg
+            # 101 items, a copy in both package/packages-split so 202
+            self.assertEqual(os.stat(dest + "/selftest-hardlink-dbg" + libdir + "/.debug-static/libhello.a").st_nlink, 202)
+            self.assertEqual(os.stat(dest + "/selftest-hardlink-dbg" + libdir + "/.debug-static/libhello-25.a").st_nlink, 202)
+            self.assertEqual(os.stat(dest + "/selftest-hardlink-dbg" + libdir + "/.debug-static/libhello-50.a").st_nlink, 202)
+            self.assertEqual(os.stat(dest + "/selftest-hardlink-dbg" + libdir + "/.debug-static/libhello-75.a").st_nlink, 202)
 
             # Test a sparse file remains sparse
             sparsestat = os.stat(dest + "/selftest-hardlink" + bindir + "/sparsetest")
-- 
1.8.3.1