ELF2: Make .gnu.hash default instead of .hash.

[grimar]

ELF/Config.h

63

May be it worth to rename these 2 to be placed together like HashGnu and HashSysv ? Or just make a single enum ?

ELF/Driver.cpp

176

May be it worth to explicit set flags for "both" ?

if (S == "both") {

Config->SysvHash = true;
Config->GnuHash = true;

}

it makes it more clear (I saw this first time and was need to look into Configuration to check that both works as expected + without that it depends on default initialization which can change).

test/elf2/discard-none.s

49

Do we really need to mantain adresses in such cases ? They changes often, may be we should just skip checking here like:
// CHECK-NEXT: Value: 0x
?

test/elf2/shared.s

284

Was the next part of test removed intentionaly ?

CHECK: HashTable {
CHECK-NEXT: Num Buckets: 4
CHECK-NEXT: Num Chains: 4
CHECK-NEXT: Buckets: [3, 0, 2, 0]
CHECK-NEXT: Chains: [0, 0, 0, 1]
CHECK-NEXT: }

[ruiu]

ELF/Config.h

63

GnuHash and SysvHash sound natural to me, so I'll leave them alone.

ELF/Driver.cpp

176

That is probably excessive. There are actually five cases: gnu, sysv, both, invalid argument, or no --hash-style flag. We cover four cases here. No matter how we write code for "both", the last one is pretty much implicit.

test/elf2/discard-none.s

49

I feel like this serves the purpose as a safeguard. Not sure how effective that is, but I don't think removing that is the right thing to do in this patch.

test/elf2/shared.s

284

This is intentional.

[grimar]

LGTM except that I would do enum or bitfield instead of 2 independent variables and i still dont like initialization, but anyways its for you to decide how will be better, so..

test/elf2/discard-none.s

49

The problem I see here that single patch changes too many files that are not relative to core idea of patch at all. Also dont see whats the safeguard can be here. Does it save from addresses not being generated ?
But you`re right that it is not the problem of this patch generally.

[grimar]

test/elf2/discard-none.s

49

I mean not this patch changes, but patches usually do.

[ikudrin]

The default settings for hash section should be target-specific. At least MIPS targets don't support GNU-style hash sections at all.

test/elf2/shared.s

284

It looks like we now have no tests which check HashTable section. I'd suggest to add -hash-style=sysv command line switch at least for this test case.

[grimar]

test/elf2/shared.s

284

May be would be better to add another test in another patch for testing hashtable sections then ?

[grimar]

Looks like revision should be abadoned..

ELF/Config.h

struct Configuration {
   bool AsNeeded = false;
   bool Bsymbolic;
   bool DiscardAll;
   bool DiscardLocals;
   bool DiscardNone;
   bool EnableNewDtags;
   bool ExportDynamic;
   bool GcSections;
-  bool GnuHash = false;
+  bool GnuHash = true;
   bool Mips64EL = false;
   bool NoInhibitExec;
   bool NoUndefined;
   bool Shared;
   bool Static = false;
   bool StripAll;
-  bool SysvHash = true;
+  bool SysvHash = false;
   bool Verbose;
   bool ZNodelete = false;
   bool ZNow = false;
   bool ZOrigin = false;
   ELFKind EKind = ELFNoneKind;
   uint16_t EMachine = llvm::ELF::EM_NONE;
   uint64_t EntryAddr = -1;
   unsigned Optimize = 0;
 };
 
 extern Configuration *Config;
 
 } // namespace elf2
 } // namespace lld
 
 #endif

ELF/Driver.cpp

void LinkerDriver::createFiles(opt::InputArgList &Args) {
   if (auto *Arg = Args.getLastArg(OPT_O)) {
     StringRef Val = Arg->getValue();
     if (Val.getAsInteger(10, Config->Optimize))
       error("Invalid optimization level");
   }
 
   if (auto *Arg = Args.getLastArg(OPT_hash_style)) {
     StringRef S = Arg->getValue();
-    if (S == "gnu") {
-      Config->GnuHash = true;
-      Config->SysvHash = false;
+    if (S == "sysv") {
+      Config->GnuHash = false;
+      Config->SysvHash = true;
     } else if (S == "both") {
-      Config->GnuHash = true;
-    } else if (S != "sysv")
+      Config->SysvHash = true;
+    } else if (S != "gnu")
       error("Unknown hash style: " + S);
   }
 
   for (auto *Arg : Args.filtered(OPT_undefined))
     Config->Undefined.push_back(Arg->getValue());
 
   for (auto *Arg : Args.filtered(OPT_z)) {
     StringRef S = Arg->getValue();
     if (S == "nodelete")
       Config->ZNodelete = true;

test/elf2/discard-none.s

 // CHECK-NEXT:     Size: 0
 // CHECK-NEXT:     Binding: Local
 // CHECK-NEXT:     Type: None
 // CHECK-NEXT:     Other: 0
 // CHECK-NEXT:     Section: Undefined
 // CHECK-NEXT:   }
 // CHECK-NEXT:   Symbol {
 // CHECK-NEXT:     Name: .Lmyothervar
-// CHECK-NEXT:     Value: 0x14C
+// CHECK-NEXT:     Value: 0x154
 // CHECK-NEXT:     Size: 0
 // CHECK-NEXT:     Binding: Local
 // CHECK-NEXT:     Type: None
 // CHECK-NEXT:     Other: 0
 // CHECK-NEXT:     Section: .text
 // CHECK-NEXT:   }
 // CHECK-NEXT:   Symbol {
 // CHECK-NEXT:     Name: .Lmyvar
-// CHECK-NEXT:     Value: 0x14C
+// CHECK-NEXT:     Value: 0x154
 // CHECK-NEXT:     Size: 0
 // CHECK-NEXT:     Binding: Local
 // CHECK-NEXT:     Type: None
 // CHECK-NEXT:     Other: 0
 // CHECK-NEXT:     Section: .text
 // CHECK-NEXT:   }

test/elf2/dynamic-reloc.s

 // CHECK-NEXT:  Tag                Type                 Name/Value
 // CHECK-NEXT:  0x0000000000000007 RELA                 [[RELAADDR]]
 // CHECK-NEXT:  0x0000000000000008 RELASZ               [[RELASIZE]] (bytes)
 // CHECK-NEXT:  0x0000000000000009 RELAENT              24 (bytes)
 // CHECK-NEXT:  0x0000000000000006 SYMTAB
 // CHECK-NEXT:  0x000000000000000B SYMENT               24 (bytes)
 // CHECK-NEXT:  0x0000000000000005 STRTAB
 // CHECK-NEXT:  0x000000000000000A STRSZ
-// CHECK-NEXT:  0x0000000000000004 HASH
+// CHECK-NEXT:  0x000000006FFFFEF5 GNU_HASH
 // CHECK-NEXT:  0x0000000000000001 NEEDED               SharedLibrary ({{.*}}2.so)
 // CHECK-NEXT:  0x0000000000000000 NULL                 0x0
 // CHECK-NEXT: ]
 
 
 .global _start
 _start:
 .quad bar + 0x42
 .weak foo
 .quad foo
 call main

test/elf2/local-dynamic.s

 // CHECK-NEXT:     Size: 0
 // CHECK-NEXT:     Binding: Local
 // CHECK-NEXT:     Type: None
 // CHECK-NEXT:     Other: 0
 // CHECK-NEXT:     Section: Undefined
 // CHECK-NEXT:   }
 // CHECK-NEXT:   Symbol {
 // CHECK-NEXT:     Name: blah
-// CHECK-NEXT:     Value: 0x170
+// CHECK-NEXT:     Value: 0x178
 // CHECK-NEXT:     Size: 0
 // CHECK-NEXT:     Binding: Local
 // CHECK-NEXT:     Type: None
 // CHECK-NEXT:     Other: 0
 // CHECK-NEXT:     Section: .text
 // CHECK-NEXT:   }
 // CHECK-NEXT:   Symbol {
 // CHECK-NEXT:     Name: foo
-// CHECK-NEXT:     Value: 0x170
+// CHECK-NEXT:     Value: 0x178
 // CHECK-NEXT:     Size: 0
 // CHECK-NEXT:     Binding: Local
 // CHECK-NEXT:     Type: None
 // CHECK-NEXT:     Other: 0
 // CHECK-NEXT:     Section: .text
 // CHECK-NEXT:   }
 // CHECK-NEXT:   Symbol {
 // CHECK-NEXT:     Name: goo
-// CHECK-NEXT:     Value: 0x170
+// CHECK-NEXT:     Value: 0x178
 // CHECK-NEXT:     Size: 0
 // CHECK-NEXT:     Binding: Local
 // CHECK-NEXT:     Type: None
 // CHECK-NEXT:     Other: 0
 // CHECK-NEXT:     Section: .text
 // CHECK-NEXT:   }
 // CHECK-NEXT:   Symbol {
 // CHECK-NEXT:     Name: _start
-// CHECK-NEXT:     Value: 0x170
+// CHECK-NEXT:     Value: 0x178
 // CHECK-NEXT:     Size: 0
 // CHECK-NEXT:     Binding: Global
 // CHECK-NEXT:     Type: None
 // CHECK-NEXT:     Other: 0
 // CHECK-NEXT:     Section: .text
 // CHECK-NEXT:   }
 // CHECK-NEXT: ]
 
 // CHECK: DynamicSymbols [
 // CHECK-NEXT:   Symbol {
 // CHECK-NEXT:     Name: @
 // CHECK-NEXT:     Value: 0x0
 // CHECK-NEXT:     Size: 0
 // CHECK-NEXT:     Binding: Local
 // CHECK-NEXT:     Type: None
 // CHECK-NEXT:     Other: 0
 // CHECK-NEXT:     Section: Undefined
 // CHECK-NEXT:   }
 // CHECK-NEXT:   Symbol {
 // CHECK-NEXT:     Name: _start@
-// CHECK-NEXT:     Value: 0x170
+// CHECK-NEXT:     Value: 0x178
 // CHECK-NEXT:     Size: 0
 // CHECK-NEXT:     Binding: Global
 // CHECK-NEXT:     Type: None
 // CHECK-NEXT:     Other: 0
 // CHECK-NEXT:     Section: .text
 // CHECK-NEXT:   }
 // CHECK-NEXT: ]
 
 .global _start
 _start:
 
 blah:
 foo:
 goo:
 
 

test/elf2/shared.s

 // CHECK-NEXT:     0000:
 // CHECK-NEXT:     0010:
 // CHECK-NEXT:     0020:
 // CHECK-NEXT:     0030:
 // CHECK-NEXT:   )
 // CHECK-NEXT: }
 // CHECK-NEXT: Section {
 // CHECK-NEXT:   Index: 3
-// CHECK-NEXT:    Name: .hash
-// CHECK-NEXT:    Type: SHT_HASH
+// CHECK-NEXT:    Name: .gnu.hash
+// CHECK-NEXT:    Type: SHT_GNU_HASH
 // CHECK-NEXT:    Flags [
 // CHECK-NEXT:      SHF_ALLOC
 // CHECK-NEXT:    ]
 // CHECK-NEXT:    Address: [[HASHADDR:.*]]
 // CHECK-NEXT:    Offset:
 // CHECK-NEXT:    Size:
 // CHECK-NEXT:    Link: 2
 // CHECK-NEXT:    Info: 0

 // CHECK-NEXT:     Value: 0x0
 // CHECK-NEXT:     Size: 0
 // CHECK-NEXT:     Binding: Local
 // CHECK-NEXT:     Type: None
 // CHECK-NEXT:     Other: 0
 // CHECK-NEXT:     Section: Undefined
 // CHECK-NEXT:   }
 // CHECK-NEXT:   Symbol {
-// CHECK-NEXT:     Name: _start@
-// CHECK-NEXT:     Value: 0x11000
+// CHECK-NEXT:     Name: bar@
+// CHECK-NEXT:     Value: 0x0
 // CHECK-NEXT:     Size: 0
 // CHECK-NEXT:     Binding: Global
-// CHECK-NEXT:     Type: Non
+// CHECK-NEXT:     Type: Function
 // CHECK-NEXT:     Other: 0
-// CHECK-NEXT:     Section: .text
+// CHECK-NEXT:     Section: Undefined
 // CHECK-NEXT:   }
 // CHECK-NEXT:   Symbol {
-// CHECK-NEXT:     Name: bar@
-// CHECK-NEXT:     Value: 0x0
+// CHECK-NEXT:     Name: _start@
+// CHECK-NEXT:     Value: 0x11000
 // CHECK-NEXT:     Size: 0
 // CHECK-NEXT:     Binding: Global
-// CHECK-NEXT:     Type: Function
+// CHECK-NEXT:     Type: Non
 // CHECK-NEXT:     Other: 0
-// CHECK-NEXT:     Section: Undefined
+// CHECK-NEXT:     Section: .text
 // CHECK-NEXT:   }
 // CHECK-NEXT:   Symbol {
 // CHECK-NEXT:     Name: zed@
 // CHECK-NEXT:     Value: 0x0
 // CHECK-NEXT:     Size: 0
 // CHECK-NEXT:     Binding: Global
 // CHECK-NEXT:     Type: None
 // CHECK-NEXT:     Other: 0

 // CHECK-NEXT:   Tag        Type                 Name/Value
 // CHECK-NEXT:   0x00000011 REL                  [[RELADDR]]
 // CHECK-NEXT:   0x00000012 RELSZ                [[RELSIZE]] (bytes)
 // CHECK-NEXT:   0x00000013 RELENT               [[RELENT]] (bytes)
 // CHECK-NEXT:   0x00000006 SYMTAB               [[DYNSYMADDR]]
 // CHECK-NEXT:   0x0000000B SYMENT               [[SYMENT]] (bytes)
 // CHECK-NEXT:   0x00000005 STRTAB               [[DYNSTRADDR]]
 // CHECK-NEXT:   0x0000000A STRSZ
-// CHECK-NEXT:   0x00000004 HASH                 [[HASHADDR]]
+// CHECK-NEXT:   0x6FFFFEF5 GNU_HASH             [[HASHADDR]]
 // CHECK-NEXT:   0x0000001D RUNPATH              foo:bar
 // CHECK-NEXT:   0x00000001 NEEDED               SharedLibrary ({{.*}}2.so)
 // CHECK-NEXT:   0x00000000 NULL                 0x0
 // CHECK-NEXT: ]
 
 // CHECK:     ProgramHeaders [
 // CHECK:        Type: PT_INTERP
 // CHECK-NEXT:   Offset: [[INTERPOFFSET]]

 // CHECK-NEXT:   MemSize: [[SIZE]]
 // CHECK-NEXT:   Flags [
 // CHECK-NEXT:     PF_R
 // CHECK-NEXT:     PF_W
 // CHECK-NEXT:   ]
 // CHECK-NEXT:   Alignment: [[ALIGN]]
 // CHECK-NEXT: }
 
-// CHECK:      HashTable {
-// CHECK-NEXT:   Num Buckets: 4
-// CHECK-NEXT:   Num Chains: 4
-// CHECK-NEXT:   Buckets: [3, 0, 2, 0]
-// CHECK-NEXT:   Chains: [0, 0, 0, 1]
-// CHECK-NEXT: }
-
 .global _start
 _start:
 .long bar
 .long zed