[PATCH] D44422: [ELF] - Never create .gnu_hash with NBuckets == 0.

[George Rimar via Phabricator via llvm-commits]

grimar created this revision.
grimar added reviewers: ruiu, espindola.
Herald added subscribers: kristof.beyls, arichardson, javed.absar, emaste.

I suggest setting NBuckets so that it is always >= 0.

Currently, we can end up with NBuckets==0 and android loader
does not like it (see PR36537).

The gold linker as we know use predefined table with values for NBuckets
and it never produces output where NBuckets == 0.

In theory, we could drop the whole hash table in this case probably,
as the only use case I can imagine now where it might harm is when
binary contains only undefined symbols, so that without hash section,
the loader will have to lookup for some symbol instead of instantly getting
the info that there are no symbols from the empty hash table. It's unrealistic I think.

But I also think that we probably can go with a minimal amount of changes here for simplicity
and be consistent with gold and so just always use >= 1 value for NBuckets.


https://reviews.llvm.org/D44422

Files:
  ELF/SyntheticSections.cpp
  test/ELF/arm-execute-only.s
  test/ELF/dynamic-no-rosegment.s
  test/ELF/gnu-hash-table.s


Index: test/ELF/gnu-hash-table.s
===================================================================
--- test/ELF/gnu-hash-table.s
+++ test/ELF/gnu-hash-table.s
@@ -51,12 +51,12 @@
 # EMPTY-NEXT:   }
 # EMPTY-NEXT: ]
 # EMPTY:      GnuHashTable {
-# EMPTY-NEXT:   Num Buckets: 0
+# EMPTY-NEXT:   Num Buckets: 1
 # EMPTY-NEXT:   First Hashed Symbol Index: 2
 # EMPTY-NEXT:   Num Mask Words: 1
 # EMPTY-NEXT:   Shift Count: 5
 # EMPTY-NEXT:   Bloom Filter: [0x0]
-# EMPTY-NEXT:   Buckets: []
+# EMPTY-NEXT:   Buckets: [0]
 # EMPTY-NEXT:   Values: []
 # EMPTY-NEXT: }
 
Index: test/ELF/dynamic-no-rosegment.s
===================================================================
--- test/ELF/dynamic-no-rosegment.s
+++ test/ELF/dynamic-no-rosegment.s
@@ -7,9 +7,9 @@
 # CHECK-NEXT:   Tag                Type                 Name/Value
 # CHECK-NEXT:   0x0000000000000006 SYMTAB               0x120
 # CHECK-NEXT:   0x000000000000000B SYMENT               24 (bytes)
-# CHECK-NEXT:   0x0000000000000005 STRTAB               0x1D0
+# CHECK-NEXT:   0x0000000000000005 STRTAB               0x1D8
 # CHECK-NEXT:   0x000000000000000A STRSZ                1 (bytes)
 # CHECK-NEXT:   0x000000006FFFFEF5 GNU_HASH             0x138
-# CHECK-NEXT:   0x0000000000000004 HASH                 0x150
+# CHECK-NEXT:   0x0000000000000004 HASH                 0x154
 # CHECK-NEXT:   0x0000000000000000 NULL                 0x0
 # CHECK-NEXT: ]
Index: test/ELF/arm-execute-only.s
===================================================================
--- test/ELF/arm-execute-only.s
+++ test/ELF/arm-execute-only.s
@@ -14,7 +14,7 @@
 // RUN: llvm-readelf -l %t.so | FileCheck --check-prefix=DIFF %s
 
 // CHECK-NOT:  LOAD
-// CHECK:      LOAD           0x000000 0x00000000 0x00000000 0x00169  0x00169  R   0x1000
+// CHECK:      LOAD           0x000000 0x00000000 0x00000000 0x0016d  0x0016d  R   0x1000
 // CHECK:      LOAD           0x001000 0x00001000 0x00001000 0x{{.*}} 0x{{.*}} R E 0x1000
 // CHECK:      LOAD           0x002000 0x00002000 0x00002000 0x{{.*}} 0x{{.*}}   E 0x1000
 // CHECK:      LOAD           0x003000 0x00003000 0x00003000 0x00038  0x00038  RW  0x1000
@@ -26,7 +26,7 @@
 // CHECK: 04     .dynamic
 
 // DIFF-NOT:  LOAD
-// DIFF:      LOAD           0x000000 0x00000000 0x00000000 0x00149 0x00149 R   0x1000
+// DIFF:      LOAD           0x000000 0x00000000 0x00000000 0x0014d 0x0014d R   0x1000
 // DIFF:      LOAD           0x001000 0x00001000 0x00001000 0x0000c 0x0000c R E 0x1000
 // DIFF:      LOAD           0x002000 0x00002000 0x00002000 0x00038 0x00038 RW  0x1000
 // DIFF-NOT:  LOAD
Index: ELF/SyntheticSections.cpp
===================================================================
--- ELF/SyntheticSections.cpp
+++ ELF/SyntheticSections.cpp
@@ -1799,15 +1799,16 @@
           return SS->CopyRelSec == nullptr && !SS->NeedsPltAddr;
         return !S.Sym->isDefined();
       });
-  if (Mid == V.end())
-    return;
 
   // We chose load factor 4 for the on-disk hash table. For each hash
   // collision, the dynamic linker will compare a uint32_t hash value.
   // Since the integer comparison is quite fast, we believe we can make
   // the load factor even larger. 4 is just a conservative choice.
   NBuckets = std::max<size_t>((V.end() - Mid) / 4, 1);
 
+  if (Mid == V.end())
+    return;
+
   for (SymbolTableEntry &Ent : llvm::make_range(Mid, V.end())) {
     Symbol *B = Ent.Sym;
     uint32_t Hash = hashGnu(B->getName());


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D44422.138161.patch
Type: text/x-patch
Size: 3442 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180313/01941b1f/attachment.bin>