dwarf-reader: Fix bloom filter access in GNU_HASH section

The bloom filter of the GNU_HASH section of binaries is made of words
(bitmasks) that are either 32-bits for ELFCLASS32 binaries or 64-bits
for ELFCLASS64 binaries.

By using the GElf_Word type to hold the content of each bitmask we
assume the bitmask to be of a size of at most 'sizeof(Elf64_Word)'.
Unfortunately, the name Elf64_Word is a bit misleading because it's a
32-bits wide type (uint32_t).  That type is thus too short to hold an
entire bitmask for 64-bits binaries.

I won't give too many details here about how the bloom filter works as
it's described in details in the documentation for the GNU_HASH
section at http://www.linker-aliens.org/blogs/ali/entry/gnu_hash_elf_sections/.

Suffice it to say that in practise, we were comparing the least
significant bytes of a 64-bits bloom bitmask to a 32-bits value.
Depending on if we read those least significant bytes on a big or
little endian, we obviously don't get the same result.  Hence the
recent buid breakage at
https://builder.wildebeest.org/buildbot/#builders/14/builds/352 where
the runtestlookupsyms test fails.

This patch thus changes the code of lookup_symbol_from_gnu_hash_tab to
use a 64-bits type to hold the value of the bloom bitmask.  That way,
we never have any information loss.  The function bloom_word_at is
also changed to read the bloom bitmask as a 64-bits value when looking
at an ELFCLASS64 binary and to always return a 64-bits value.

It also adds a test to access the bloom filter of an ELFCLASS32
binary.

	* src/abg-dwarf-reader.cc (bloom_word_at): Properly read an
	element from the bloom bitmasks array of 32-bits values as a
	64-bits value in a portable way.  Always return a 64 bits value.
	(lookup_symbol_from_gnu_hash_tab): Use a 64-bits value to store
	the bloom bitmask.
	* tests/data/test-lookup-syms/test1-32bits.so: New test input,
	compiled as a 32bits binary to test for ELFCLASS32 binaries.
	* tests/data/test-lookup-syms/test1.c.compiling.txt: Documentation
	about how to compile the test1[-21bits].so files.
	* tests/data/Makefile.am: Add the new test material above to
	source distribution.
        * tests/test-lookup-syms.cc (in_out_specs): Add the
	test1-32bits.so test case to this test harness.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>

diff --git a/src/abg-dwarf-reader.cc b/src/abg-dwarf-reader.cc
index 5bc8a157693a176c4d3ba9b5e52f40f3f336e185..10ca1232fcd170cd543e4e509e162dee48400d85 100644 (file)
--- a/src/abg-dwarf-reader.cc
+++ b/src/abg-dwarf-reader.cc
@@ -1850,20 +1850,27 @@ get_elf_class_size_in_bytes(Elf* elf_handle)
 }
 
 /// Get a given word of a bloom filter, referred to by the index of
-/// the word.  The word size depends on the current elf class and this
-/// function abstracts that nicely.
+/// the word.
+///
+/// The bloom word size depends on the current elf class (32 bits for
+/// an ELFCLASS32 or 64 bits for an ELFCLASS64 one) and this function
+/// abstracts that nicely.
 ///
 /// @param elf_handle the elf handle to use.
 ///
 /// @param bloom_filter the bloom filter to consider.
 ///
 /// @param index the index of the bloom filter to return.
-static GElf_Word
+///
+/// @return a 64 bits work containing the bloom word found at index @p
+/// index.  Note that if we are looking at an ELFCLASS32 binary, the 4
+/// most significant bytes of the result are going to be zero.
+static Elf64_Xword
 bloom_word_at(Elf*             elf_handle,
              Elf32_Word*       bloom_filter,
              size_t            index)
 {
-  GElf_Word result = 0;
+  Elf64_Xword result = 0;
   GElf_Ehdr h;
   ABG_ASSERT(gelf_getehdr(elf_handle, &h));
   int c;
@@ -1876,7 +1883,7 @@ bloom_word_at(Elf*                elf_handle,
       break ;
     case ELFCLASS64:
       {
-       GElf_Word* f= reinterpret_cast<GElf_Word*>(bloom_filter);
+       Elf64_Xword* f= reinterpret_cast<Elf64_Xword*>(bloom_filter);
        result = f[index];
       }
       break;
@@ -2025,7 +2032,12 @@ lookup_symbol_from_gnu_hash_tab(const environment*               env,
   // filter, in bits.
   int c = get_elf_class_size_in_bytes(elf_handle) * 8;
   int n =  (h1 / c) % ht.bf_nwords;
-  GElf_Word bitmask = (1ul << (h1 % c)) | (1ul << (h2 % c));
+  // The bitmask of the bloom filter has a size of either 32-bits on
+  // ELFCLASS32 binaries or 64-bits on ELFCLASS64 binaries.  So we
+  // need a 64-bits type to hold the bitmap, hence the Elf64_Xword
+  // type used here.  When dealing with 32bits binaries, the upper
+  // bits of the bitmask will be zero anyway.
+  Elf64_Xword bitmask = (1ul << (h1 % c)) | (1ul << (h2 % c));
 
   // Test if the symbol is *NOT* present in this ELF file.
   if ((bloom_word_at(elf_handle, ht.bloom_filter, n) & bitmask) != bitmask)

diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am
index e9fdad1e1d8d664402611eac63744becfca217dd..cf7792f1c475ed451c8c32ae769ed7e05da6b5f6 100644 (file)
--- a/tests/data/Makefile.am
+++ b/tests/data/Makefile.am
@@ -1324,6 +1324,8 @@ test-lookup-syms/test02-report.txt        \
 test-lookup-syms/test1.c               \
 test-lookup-syms/test1.version-script  \
 test-lookup-syms/test1.so              \
+test-lookup-syms/test1-32bits.so       \
+test-lookup-syms/test1.c.compiling.txt  \
 test-lookup-syms/test1-1-report.txt    \
 test-lookup-syms/test1-2-report.txt    \
 test-lookup-syms/test1-3-report.txt    \

diff --git a/tests/data/test-lookup-syms/test1-32bits.so b/tests/data/test-lookup-syms/test1-32bits.so
new file mode 100755 (executable)
index 0000000..119cee4
Binary files /dev/null and b/tests/data/test-lookup-syms/test1-32bits.so differ

diff --git a/tests/data/test-lookup-syms/test1.c.compiling.txt b/tests/data/test-lookup-syms/test1.c.compiling.txt
new file mode 100644 (file)
index 0000000..6753d63
--- /dev/null
+++ b/tests/data/test-lookup-syms/test1.c.compiling.txt
@@ -0,0 +1,9 @@
+To re-compile test1.so (on a 64-bits machine), please do:
+
+gcc -g -shared -Wl,--version-script=test1.version-script -o test1.so test1.c
+
+
+To re-compile test1-32.so, please do:
+
+gcc -g -m32 -shared -Wl,--version-script=test1.version-script -o test1.so test1.c
+

diff --git a/tests/test-lookup-syms.cc b/tests/test-lookup-syms.cc
index 2f4d8f77ff351252d2d979600eaa009987ddcf8e..077f33b9b4cbe960fdfe7dd5e0d3282abfc6beb9 100644 (file)
--- a/tests/test-lookup-syms.cc
+++ b/tests/test-lookup-syms.cc
@@ -73,6 +73,13 @@ InOutSpec in_out_specs[] =
     "data/test-lookup-syms/test1-1-report.txt",
     "output/test-lookup-syms/test1-1-report.txt"
   },
+  {
+    "data/test-lookup-syms/test1-32bits.so",
+    "foo",
+    "",
+    "data/test-lookup-syms/test1-1-report.txt",
+    "output/test-lookup-syms/test1-1-report.txt"
+  },
   {
     "data/test-lookup-syms/test1.so",
     "_foo1",