[Git][reproducible-builds/diffoscope][master] 3 commits: Revert "Don't assume all files called ".a" are ELF binaries because we...

Chris Lamb gitlab at salsa.debian.org
Sun Sep 8 08:40:20 UTC 2019



Chris Lamb pushed to branch master at Reproducible Builds / diffoscope


Commits:
a8a56dbe by Marc Herbert at 2019-09-08T08:39:41Z
Revert "Don't assume all files called ".a" are ELF binaries because we specified a FILE_EXTENSION_SUFFIX. This prevents an "Unrecognized archive format" traceback when processing (for example) lie 2.2.2+dfsg-3. (Closes: #903446)"

This reverts commit cd4c64234a7eebe71fb6bdea21850622f3f5c4c8 in preparation for
fixing issue #64.

Signed-off-by: Chris Lamb <lamby at debian.org>

Gbp-Dch: ignore

- - - - -
ce6c03fe by Marc Herbert at 2019-09-08T08:39:48Z
Remove StaticLibFile in the ELF comparator; ArFile.compare_details() is superior. (Closes: reproducible-builds/diffoscope#64)

Remove the old code in the StaticLibFile class as it now duplicates the
recursion in ArFile.compare_details() in a much inferior way. Fixes issue #64,
see the multiple use cases there and in the next commit adding tests.

The patch header line in the test output is decremented one line because when
run on .a files readelf inserted headings like this:

  In archive test1.a:

Signed-off-by: Chris Lamb <lamby at debian.org>

- - - - -
bee2a11e by Marc Herbert at 2019-09-08T08:39:53Z
Add new elfmix.a ELF test file with a range of different file types.

Now that StaticLibFile is out of the way, show off ArFile's recursive
ability to:

- not care about the .a suffix and not behave differently depending on
  filenames;
- not crash and lose everything when some archive members are not ELF;
- gracefully fall back and run the next best tool for a cross-compiled
  ELF and a sample of non-ELF members - including the corresponding .c
  source.

Tests issue #64, see more details there.

In addition to the .c source file, the regen_elfmix.sh script that generated
the .a files is embedded in the .a files themselves if ever needed. For
size considerations no cross-compiler is provided.

Signed-off-by: Chris Lamb <lamby at debian.org>

Gbp-Dch: ignore

- - - - -


13 changed files:

- diffoscope/comparators/__init__.py
- diffoscope/comparators/ar.py
- diffoscope/comparators/elf.py
- diffoscope/comparators/utils/file.py
- tests/comparators/test_elf.py
- − tests/data/bug_903446.a
- tests/data/elf_lib_objdump_expected_diff
- + tests/data/elfmix1.not_a
- + tests/data/elfmix2.a
- + tests/data/elfmix_disassembly_expected_diff
- + tests/data/elfmix_mach_o_expected_diff
- + tests/data/elfmix_src_c_expected_diff
- + tests/data/elfmix_x_obj_expected_diff


Changes:

=====================================
diffoscope/comparators/__init__.py
=====================================
@@ -55,7 +55,6 @@ class ComparatorManager(object):
         ('elf.ElfFile',),
         ('macho.MachoFile',),
         ('fsimage.FsImageFile',),
-        ('elf.StaticLibFile',),
         ('llvm.LlvmBitCodeFile',),
         ('sqlite.Sqlite3Database',),
         ('wasm.WasmFile',),


=====================================
diffoscope/comparators/ar.py
=====================================
@@ -31,10 +31,9 @@ from .utils.libarchive import LibarchiveContainer, list_libarchive
 logger = logging.getLogger(__name__)
 
 
-# TODO: this would also be useful for Go archives. Currently those are handled
-# by StaticLibFile, but then readelf complains with "Error: Not an ELF file".
-# ArFile gives slightly more reasonable output, e.g. a readable plain diff of
-# the __.PKGDEF member which is just a text file containing the Go interface.
+# For Go archives this gives a readable plain diff of the __.PKGDEF
+# member which is just a text file containing the Go interface.
+# TODO: add Go tests and more recursion.
 
 
 class ArContainer(LibarchiveContainer):


=====================================
diffoscope/comparators/elf.py
=====================================
@@ -623,25 +623,3 @@ class ElfFile(File):
 
     def compare_details(self, other, source=None):
         return _compare_elf_data(self.path, other.path)
-
-
-class StaticLibFile(File):
-    DESCRIPTION = "statically-linked binaries"
-    CONTAINER_CLASS = ElfContainer
-    FILE_TYPE_RE = re.compile(r'\bar archive\b')
-    FILE_EXTENSION_SUFFIX = '.a'
-
-    ENABLE_FALLBACK_RECOGONIZES = False
-
-    def compare_details(self, other, source=None):
-        differences = [
-            Difference.from_text_readers(
-                list_libarchive(self.path),
-                list_libarchive(other.path),
-                self.path,
-                other.path,
-                source="file list",
-            )
-        ]
-        differences.extend(_compare_elf_data(self.path, other.path))
-        return differences


=====================================
diffoscope/comparators/utils/file.py
=====================================
@@ -172,7 +172,6 @@ class File(object, metaclass=abc.ABCMeta):
 
         return _run_tests(all, all_tests) if all_tests else False
 
-    ENABLE_FALLBACK_RECOGONIZES = True
     FALLBACK_FILE_EXTENSION_SUFFIX = None
     FALLBACK_FILE_TYPE_HEADER_PREFIX = None
 
@@ -195,9 +194,6 @@ class File(object, metaclass=abc.ABCMeta):
             # not valid, they have to re-implement it
             return False
 
-        if not cls.ENABLE_FALLBACK_RECOGONIZES:
-            return False
-
         all_tests = [
             test
             for test in (


=====================================
tests/comparators/test_elf.py
=====================================
@@ -23,7 +23,8 @@ import os.path
 import subprocess
 
 from diffoscope.config import Config
-from diffoscope.comparators.elf import ElfFile, StaticLibFile
+from diffoscope.comparators.ar import ArFile
+from diffoscope.comparators.elf import ElfFile
 from diffoscope.comparators.binary import FilesystemFile
 from diffoscope.comparators.directory import FilesystemDirectory
 from diffoscope.comparators.missing_file import MissingFile
@@ -40,7 +41,6 @@ from ..utils.tools import (
 
 obj1 = load_fixture('test1.o')
 obj2 = load_fixture('test2.o')
-bug_903446 = load_fixture('bug_903446.a')
 ignore_readelf_errors1 = load_fixture('test1.debug')
 ignore_readelf_errors2 = load_fixture('test2.debug')
 
@@ -109,7 +109,7 @@ def lib2():
 
 
 def test_lib_identification(lib1):
-    assert isinstance(lib1, StaticLibFile)
+    assert isinstance(lib1, ArFile)
 
 
 def test_lib_no_differences(lib1):
@@ -130,9 +130,9 @@ def test_lib_differences(lib_differences):
     assert lib_differences[0].source1 == 'file list'
     expected_metadata_diff = get_data('elf_lib_metadata_expected_diff')
     assert lib_differences[0].unified_diff == expected_metadata_diff
-    assert 'objdump' in lib_differences[1].source1
+    assert 'objdump' in lib_differences[1].details[0].source1
     expected_objdump_diff = get_data('elf_lib_objdump_expected_diff')
-    assert lib_differences[1].unified_diff == expected_objdump_diff
+    assert lib_differences[1].details[0].unified_diff == expected_objdump_diff
 
 
 @skip_unless_tools_exist('readelf', 'objdump')
@@ -145,6 +145,49 @@ def test_lib_compare_non_existing(monkeypatch, lib1):
     assert len(difference.details) > 0
 
 
+TEST_LIBMIX1_PATH = data('elfmix1.not_a')
+TEST_LIBMIX2_PATH = data('elfmix2.a')
+
+
+ at pytest.fixture
+def libmix1():
+    return specialize(FilesystemFile(TEST_LIBMIX1_PATH))
+
+
+ at pytest.fixture
+def libmix2():
+    return specialize(FilesystemFile(TEST_LIBMIX2_PATH))
+
+
+ at pytest.fixture
+def libmix_differences(libmix1, libmix2):
+    return libmix1.compare(libmix2).details
+
+
+ at skip_unless_tools_exist('readelf', 'objdump')
+ at skip_if_tool_version_is('readelf', readelf_version, '2.29')
+ at skip_if_binutils_does_not_support_x86()
+def test_libmix_differences(libmix_differences):
+    assert len(libmix_differences) == 5
+    file_list, mach_o, x86_o, src_c, x_obj = libmix_differences
+
+    # Check order and basic identification
+    assert file_list.source1 == 'file list'
+    assert "Falling back to binary" in mach_o.comments[0]
+    x86_o = x86_o.details[0]
+    assert x86_o.source1.startswith('objdump ')
+    assert src_c.source1.endswith('.c')
+    x_obj = x_obj.details[0]
+    assert x_obj.source1.startswith('readelf ')
+
+    # Content
+    assert 'return42_or_3' in file_list.unified_diff
+    assert mach_o.unified_diff == get_data('elfmix_mach_o_expected_diff')
+    assert x86_o.unified_diff == get_data('elfmix_disassembly_expected_diff')
+    assert src_c.unified_diff == get_data('elfmix_src_c_expected_diff')
+    assert x_obj.unified_diff == get_data('elfmix_x_obj_expected_diff')
+
+
 TEST_DBGSYM_DEB1_PATH = data('dbgsym/add/test-dbgsym_1_amd64.deb')
 TEST_DBGSYM_DEB2_PATH = data('dbgsym/mult/test-dbgsym_1_amd64.deb')
 
@@ -199,14 +242,6 @@ def test_original_gnu_debuglink(dbgsym_differences):
     assert bin_details.details[2].unified_diff == expected_gnu_debuglink
 
 
-def test_bug_903446(bug_903446):
-    # Ensure we don't error
-    bug_903446.compare(bug_903446)
-
-    # Not a real StaticLibFile
-    assert isinstance(bug_903446, FilesystemFile)
-
-
 def test_ignore_readelf_errors1_identify(ignore_readelf_errors1):
     assert isinstance(ignore_readelf_errors1, ElfFile)
 


=====================================
tests/data/bug_903446.a deleted
=====================================
Binary files a/tests/data/bug_903446.a and /dev/null differ


=====================================
tests/data/elf_lib_objdump_expected_diff
=====================================
@@ -1,4 +1,4 @@
-@@ -4,10 +4,10 @@
+@@ -3,10 +3,10 @@
  
  Disassembly of section .text:
  


=====================================
tests/data/elfmix1.not_a
=====================================
Binary files /dev/null and b/tests/data/elfmix1.not_a differ


=====================================
tests/data/elfmix2.a
=====================================
Binary files /dev/null and b/tests/data/elfmix2.a differ


=====================================
tests/data/elfmix_disassembly_expected_diff
=====================================
@@ -0,0 +1,12 @@
+@@ -3,10 +3,10 @@
+ 
+ Disassembly of section .text:
+ 
+ 0000000000000000 <return42_or_3>:
+ return42_or_3():
+    0:	55                   	push   %rbp
+    1:	48 89 e5             	mov    %rsp,%rbp
+-   4:	b8 2a 00 00 00       	mov    $0x2a,%eax
++   4:	b8 2b 00 00 00       	mov    $0x2b,%eax
+    9:	5d                   	pop    %rbp
+    a:	c3                   	retq   


=====================================
tests/data/elfmix_mach_o_expected_diff
=====================================
@@ -0,0 +1,17 @@
+@@ -23,15 +23,15 @@
+ 00000160: 0100 0000 000e 0a00 000e 0a00 0000 0000  ................
+ 00000170: 0200 0000 1800 0000 5002 0000 0100 0000  ........P.......
+ 00000180: 6002 0000 1000 0000 0b00 0000 5000 0000  `...........P...
+ 00000190: 0000 0000 0000 0000 0000 0000 0100 0000  ................
+ 000001a0: 0100 0000 0000 0000 0000 0000 0000 0000  ................
+ 000001b0: 0000 0000 0000 0000 0000 0000 0000 0000  ................
+ 000001c0: 0000 0000 0000 0000 0000 0000 0000 0000  ................
+-000001d0: 0000 0000 0000 0000 5548 89e5 b82a 0000  ........UH...*..
++000001d0: 0000 0000 0000 0000 5548 89e5 b82b 0000  ........UH...+..
+ 000001e0: 005d c300 0000 0000 0000 0000 0000 0000  .]..............
+ 000001f0: 0b00 0000 0000 0001 0000 0000 0000 0000  ................
+ 00000200: 0000 0000 0000 0000 1400 0000 0000 0000  ................
+ 00000210: 017a 5200 0178 1001 100c 0708 9001 0000  .zR..x..........
+ 00000220: 2400 0000 1c00 0000 b0ff ffff ffff ffff  $...............
+ 00000230: 0b00 0000 0000 0000 0041 0e10 8602 430d  .........A....C.
+ 00000240: 0600 0000 0000 0000 0000 0000 0100 0006  ................


=====================================
tests/data/elfmix_src_c_expected_diff
=====================================
@@ -0,0 +1,7 @@
+@@ -1,5 +1,5 @@
+ 
+ int return42_or_3()
+ {
+-  return 42; // for sed
++  return 43; // for sed
+ }


=====================================
tests/data/elfmix_x_obj_expected_diff
=====================================
@@ -0,0 +1,6 @@
+@@ -1,4 +1,4 @@
+ 
+ Hex dump of section '.text':
+-  0x00000000 3641007d 012ca21d f0                6A.}.,...
++  0x00000000 3641007d 012cb21d f0                6A.}.,...
+ 



View it on GitLab: https://salsa.debian.org/reproducible-builds/diffoscope/compare/1ec2cb1745b3af873769f7bb1886a8cdfa136d3a...bee2a11ebc1da1c9e7c3214dd160cf5f12f0fcc9

-- 
View it on GitLab: https://salsa.debian.org/reproducible-builds/diffoscope/compare/1ec2cb1745b3af873769f7bb1886a8cdfa136d3a...bee2a11ebc1da1c9e7c3214dd160cf5f12f0fcc9
You're receiving this email because of your account on salsa.debian.org.


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.reproducible-builds.org/pipermail/rb-commits/attachments/20190908/e3d5cf21/attachment.html>


More information about the rb-commits mailing list