cmd/link: revert/revise CL 98075 because LLDB is very picky now

This was originally

Revert "cmd/link: fix up debug_range for dsymutil (revert CL 72371)"

which has the effect of no longer using Base Address Selection
Entries in DWARF.  However, the build-time costs of that are
about 2%, so instead the hacky fixup that generated technically
incorrect DWARF was removed from the linker, and the choice
is instead made in the compiler, dependent on platform, but
also under control of a flag so that we can report this bug
against LLDB/dsymutil/dwarfdump (really, the LLVM dwarf
libraries).

This however does not solve #31188; debugging still fails,
but dwarfdump no longer complains.  There are at least two
LLDB bugs involved, and this change will at allow us
to report them without them being rejected because our
now-obsolete workaround for the first bug creates
not-quite-DWARF.

Updates #31188.

Change-Id: I5300c51ad202147bab7333329ebe961623d2b47d
Reviewed-on: https://go-review.googlesource.com/c/go/+/170638
Run-TryBot: David Chase <drchase@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
diff --git a/src/cmd/compile/internal/gc/main.go b/src/cmd/compile/internal/gc/main.go
index 969b596..dc3fb64 100644
--- a/src/cmd/compile/internal/gc/main.go
+++ b/src/cmd/compile/internal/gc/main.go
@@ -140,6 +140,12 @@
 	Ctxt.DiagFlush = flusherrors
 	Ctxt.Bso = bufio.NewWriter(os.Stdout)
 
+	// UseBASEntries is preferred because it shaves about 2% off build time, but LLDB, dsymutil, and dwarfdump
+	// on Darwin don't support it properly, especially since macOS 10.14 (Mojave).  This is exposed as a flag
+	// to allow testing with LLVM tools on Linux, and to help with reporting this bug to the LLVM project.
+	// See bugs 31188 and 21945 (CLs 170638, 98075, 72371).
+	Ctxt.UseBASEntries = Ctxt.Headtype != objabi.Hdarwin
+
 	localpkg = types.NewPkg("", "")
 	localpkg.Prefix = "\"\""
 
@@ -254,12 +260,13 @@
 	flag.StringVar(&mutexprofile, "mutexprofile", "", "write mutex profile to `file`")
 	flag.StringVar(&benchfile, "bench", "", "append benchmark times to `file`")
 	flag.BoolVar(&newescape, "newescape", true, "enable new escape analysis")
+	flag.BoolVar(&Ctxt.UseBASEntries, "dwarfbasentries", Ctxt.UseBASEntries, "use base address selection entries in DWARF")
 	objabi.Flagparse(usage)
 
 	// Record flags that affect the build result. (And don't
 	// record flags that don't, since that would cause spurious
 	// changes in the binary.)
-	recordFlags("B", "N", "l", "msan", "race", "shared", "dynlink", "dwarflocationlists", "newescape")
+	recordFlags("B", "N", "l", "msan", "race", "shared", "dynlink", "dwarflocationlists", "newescape", "dwarfbasentries")
 
 	Ctxt.Flag_shared = flag_dynlink || flag_shared
 	Ctxt.Flag_dynlink = flag_dynlink
diff --git a/src/cmd/compile/internal/ssa/debug.go b/src/cmd/compile/internal/ssa/debug.go
index c2736d8..13fe67c 100644
--- a/src/cmd/compile/internal/ssa/debug.go
+++ b/src/cmd/compile/internal/ssa/debug.go
@@ -1077,6 +1077,12 @@
 // PutLocationList adds list (a location list in its intermediate representation) to listSym.
 func (debugInfo *FuncDebug) PutLocationList(list []byte, ctxt *obj.Link, listSym, startPC *obj.LSym) {
 	getPC := debugInfo.GetPC
+
+	if ctxt.UseBASEntries {
+		listSym.WriteInt(ctxt, listSym.Size, ctxt.Arch.PtrSize, ^0)
+		listSym.WriteAddr(ctxt, listSym.Size, ctxt.Arch.PtrSize, startPC, 0)
+	}
+
 	// Re-read list, translating its address from block/value ID to PC.
 	for i := 0; i < len(list); {
 		begin := getPC(decodeValue(ctxt, readPtr(ctxt, list[i:])))
@@ -1090,17 +1096,21 @@
 			end = 1
 		}
 
-		writePtr(ctxt, list[i:], uint64(begin))
-		writePtr(ctxt, list[i+ctxt.Arch.PtrSize:], uint64(end))
+		if ctxt.UseBASEntries {
+			listSym.WriteInt(ctxt, listSym.Size, ctxt.Arch.PtrSize, int64(begin))
+			listSym.WriteInt(ctxt, listSym.Size, ctxt.Arch.PtrSize, int64(end))
+		} else {
+			listSym.WriteCURelativeAddr(ctxt, listSym.Size, startPC, int64(begin))
+			listSym.WriteCURelativeAddr(ctxt, listSym.Size, startPC, int64(end))
+		}
+
 		i += 2 * ctxt.Arch.PtrSize
-		i += 2 + int(ctxt.Arch.ByteOrder.Uint16(list[i:]))
+		datalen := 2 + int(ctxt.Arch.ByteOrder.Uint16(list[i:]))
+		listSym.WriteBytes(ctxt, listSym.Size, list[i:i+datalen]) // copy datalen and location encoding
+		i += datalen
 	}
 
-	// Base address entry.
-	listSym.WriteInt(ctxt, listSym.Size, ctxt.Arch.PtrSize, ^0)
-	listSym.WriteAddr(ctxt, listSym.Size, ctxt.Arch.PtrSize, startPC, 0)
 	// Location list contents, now with real PCs.
-	listSym.WriteBytes(ctxt, listSym.Size, list)
 	// End entry.
 	listSym.WriteInt(ctxt, listSym.Size, ctxt.Arch.PtrSize, 0)
 	listSym.WriteInt(ctxt, listSym.Size, ctxt.Arch.PtrSize, 0)
diff --git a/src/cmd/internal/dwarf/dwarf.go b/src/cmd/internal/dwarf/dwarf.go
index df80039..b709e9d 100644
--- a/src/cmd/internal/dwarf/dwarf.go
+++ b/src/cmd/internal/dwarf/dwarf.go
@@ -91,18 +91,19 @@
 // This container is used by the PutFunc* variants below when
 // creating the DWARF subprogram DIE(s) for a function.
 type FnState struct {
-	Name       string
-	Importpath string
-	Info       Sym
-	Filesym    Sym
-	Loc        Sym
-	Ranges     Sym
-	Absfn      Sym
-	StartPC    Sym
-	Size       int64
-	External   bool
-	Scopes     []Scope
-	InlCalls   InlCalls
+	Name          string
+	Importpath    string
+	Info          Sym
+	Filesym       Sym
+	Loc           Sym
+	Ranges        Sym
+	Absfn         Sym
+	StartPC       Sym
+	Size          int64
+	External      bool
+	Scopes        []Scope
+	InlCalls      InlCalls
+	UseBASEntries bool
 }
 
 func EnableLogging(doit bool) {
@@ -181,6 +182,7 @@
 	AddInt(s Sym, size int, i int64)
 	AddBytes(s Sym, b []byte)
 	AddAddress(s Sym, t interface{}, ofs int64)
+	AddCURelativeAddress(s Sym, t interface{}, ofs int64)
 	AddSectionOffset(s Sym, size int, t interface{}, ofs int64)
 	AddDWARFAddrSectionOffset(s Sym, t interface{}, ofs int64)
 	CurrentOffset(s Sym) int64
@@ -987,18 +989,11 @@
 	putattr(ctxt, info, DW_ABRV_INT_CONSTANT, DW_FORM_sdata, DW_CLS_CONSTANT, val, nil)
 }
 
-// PutRanges writes a range table to sym. All addresses in ranges are
-// relative to some base address. If base is not nil, then they're
-// relative to the start of base. If base is nil, then the caller must
-// arrange a base address some other way (such as a DW_AT_low_pc
-// attribute).
-func PutRanges(ctxt Context, sym Sym, base Sym, ranges []Range) {
+// PutBasedRanges writes a range table to sym. All addresses in ranges are
+// relative to some base address, which must be arranged by the caller
+// (e.g., with a DW_AT_low_pc attribute, or in a BASE-prefixed range).
+func PutBasedRanges(ctxt Context, sym Sym, ranges []Range) {
 	ps := ctxt.PtrSize()
-	// Write base address entry.
-	if base != nil {
-		ctxt.AddInt(sym, ps, -1)
-		ctxt.AddAddress(sym, base, 0)
-	}
 	// Write ranges.
 	for _, r := range ranges {
 		ctxt.AddInt(sym, ps, r.Start)
@@ -1009,6 +1004,31 @@
 	ctxt.AddInt(sym, ps, 0)
 }
 
+// PutRanges writes a range table to s.Ranges.
+// All addresses in ranges are relative to s.base.
+func (s *FnState) PutRanges(ctxt Context, ranges []Range) {
+	ps := ctxt.PtrSize()
+	sym, base := s.Ranges, s.StartPC
+
+	if s.UseBASEntries {
+		// Using a Base Address Selection Entry reduces the number of relocations, but
+		// this is not done on macOS because it is not supported by dsymutil/dwarfdump/lldb
+		ctxt.AddInt(sym, ps, -1)
+		ctxt.AddAddress(sym, base, 0)
+		PutBasedRanges(ctxt, sym, ranges)
+		return
+	}
+
+	// Write ranges full of relocations
+	for _, r := range ranges {
+		ctxt.AddCURelativeAddress(sym, base, r.Start)
+		ctxt.AddCURelativeAddress(sym, base, r.End)
+	}
+	// Write trailer.
+	ctxt.AddInt(sym, ps, 0)
+	ctxt.AddInt(sym, ps, 0)
+}
+
 // Return TRUE if the inlined call in the specified slot is empty,
 // meaning it has a zero-length range (no instructions), and all
 // of its children are empty.
@@ -1202,7 +1222,7 @@
 
 	if abbrev == DW_ABRV_INLINED_SUBROUTINE_RANGES {
 		putattr(ctxt, s.Info, abbrev, DW_FORM_sec_offset, DW_CLS_PTR, s.Ranges.Len(), s.Ranges)
-		PutRanges(ctxt, s.Ranges, s.StartPC, ic.Ranges)
+		s.PutRanges(ctxt, ic.Ranges)
 	} else {
 		st := ic.Ranges[0].Start
 		en := ic.Ranges[0].End
@@ -1357,7 +1377,7 @@
 			Uleb128put(ctxt, s.Info, DW_ABRV_LEXICAL_BLOCK_RANGES)
 			putattr(ctxt, s.Info, DW_ABRV_LEXICAL_BLOCK_RANGES, DW_FORM_sec_offset, DW_CLS_PTR, s.Ranges.Len(), s.Ranges)
 
-			PutRanges(ctxt, s.Ranges, s.StartPC, scope.Ranges)
+			s.PutRanges(ctxt, scope.Ranges)
 		}
 
 		curscope = putscope(ctxt, s, scopes, curscope, fnabbrev, encbuf)
diff --git a/src/cmd/internal/obj/data.go b/src/cmd/internal/obj/data.go
index 1c1681d..12385b5 100644
--- a/src/cmd/internal/obj/data.go
+++ b/src/cmd/internal/obj/data.go
@@ -112,9 +112,7 @@
 	}
 }
 
-// WriteAddr writes an address of size siz into s at offset off.
-// rsym and roff specify the relocation for the address.
-func (s *LSym) WriteAddr(ctxt *Link, off int64, siz int, rsym *LSym, roff int64) {
+func (s *LSym) writeAddr(ctxt *Link, off int64, siz int, rsym *LSym, roff int64, rtype objabi.RelocType) {
 	// Allow 4-byte addresses for DWARF.
 	if siz != ctxt.Arch.PtrSize && siz != 4 {
 		ctxt.Diag("WriteAddr: bad address size %d in %s", siz, s.Name)
@@ -127,10 +125,24 @@
 	}
 	r.Siz = uint8(siz)
 	r.Sym = rsym
-	r.Type = objabi.R_ADDR
+	r.Type = rtype
 	r.Add = roff
 }
 
+// WriteAddr writes an address of size siz into s at offset off.
+// rsym and roff specify the relocation for the address.
+func (s *LSym) WriteAddr(ctxt *Link, off int64, siz int, rsym *LSym, roff int64) {
+	s.writeAddr(ctxt, off, siz, rsym, roff, objabi.R_ADDR)
+}
+
+// WriteCURelativeAddr writes a pointer-sized address into s at offset off.
+// rsym and roff specify the relocation for the address which will be
+// resolved by the linker to an offset from the DW_AT_low_pc attribute of
+// the DWARF Compile Unit of rsym.
+func (s *LSym) WriteCURelativeAddr(ctxt *Link, off int64, rsym *LSym, roff int64) {
+	s.writeAddr(ctxt, off, ctxt.Arch.PtrSize, rsym, roff, objabi.R_ADDRCUOFF)
+}
+
 // WriteOff writes a 4 byte offset to rsym+roff into s at offset off.
 // After linking the 4 bytes stored at s+off will be
 // rsym+roff-(start of section that s is in).
diff --git a/src/cmd/internal/obj/link.go b/src/cmd/internal/obj/link.go
index 5d4e5db..3ea29a8 100644
--- a/src/cmd/internal/obj/link.go
+++ b/src/cmd/internal/obj/link.go
@@ -648,6 +648,7 @@
 
 	InParallel           bool // parallel backend phase in effect
 	Framepointer_enabled bool
+	UseBASEntries        bool // Use Base Address Selection Entries in location lists and PC ranges
 
 	// state for writing objects
 	Text []*LSym
diff --git a/src/cmd/internal/obj/objfile.go b/src/cmd/internal/obj/objfile.go
index 0aa0d2a..a7927f5 100644
--- a/src/cmd/internal/obj/objfile.go
+++ b/src/cmd/internal/obj/objfile.go
@@ -471,6 +471,11 @@
 		ls.WriteInt(c.Link, ls.Size, size, value)
 	}
 }
+func (c dwCtxt) AddCURelativeAddress(s dwarf.Sym, data interface{}, value int64) {
+	ls := s.(*LSym)
+	rsym := data.(*LSym)
+	ls.WriteCURelativeAddr(c.Link, ls.Size, rsym, value)
+}
 func (c dwCtxt) AddSectionOffset(s dwarf.Sym, size int, t interface{}, ofs int64) {
 	panic("should be used only in the linker")
 }
@@ -578,18 +583,19 @@
 	dwctxt := dwCtxt{ctxt}
 	filesym := ctxt.fileSymbol(s)
 	fnstate := &dwarf.FnState{
-		Name:       s.Name,
-		Importpath: myimportpath,
-		Info:       info,
-		Filesym:    filesym,
-		Loc:        loc,
-		Ranges:     ranges,
-		Absfn:      absfunc,
-		StartPC:    s,
-		Size:       s.Size,
-		External:   !s.Static(),
-		Scopes:     scopes,
-		InlCalls:   inlcalls,
+		Name:          s.Name,
+		Importpath:    myimportpath,
+		Info:          info,
+		Filesym:       filesym,
+		Loc:           loc,
+		Ranges:        ranges,
+		Absfn:         absfunc,
+		StartPC:       s,
+		Size:          s.Size,
+		External:      !s.Static(),
+		Scopes:        scopes,
+		InlCalls:      inlcalls,
+		UseBASEntries: ctxt.UseBASEntries,
 	}
 	if absfunc != nil {
 		err = dwarf.PutAbstractFunc(dwctxt, fnstate)
@@ -630,13 +636,14 @@
 	dwctxt := dwCtxt{ctxt}
 	filesym := ctxt.fileSymbol(s)
 	fnstate := dwarf.FnState{
-		Name:       s.Name,
-		Importpath: myimportpath,
-		Info:       absfn,
-		Filesym:    filesym,
-		Absfn:      absfn,
-		External:   !s.Static(),
-		Scopes:     scopes,
+		Name:          s.Name,
+		Importpath:    myimportpath,
+		Info:          absfn,
+		Filesym:       filesym,
+		Absfn:         absfn,
+		External:      !s.Static(),
+		Scopes:        scopes,
+		UseBASEntries: ctxt.UseBASEntries,
 	}
 	if err := dwarf.PutAbstractFunc(dwctxt, &fnstate); err != nil {
 		ctxt.Diag("emitting DWARF for %s failed: %v", s.Name, err)
diff --git a/src/cmd/link/internal/ld/dwarf.go b/src/cmd/link/internal/ld/dwarf.go
index 974c7ed..39c8a7f 100644
--- a/src/cmd/link/internal/ld/dwarf.go
+++ b/src/cmd/link/internal/ld/dwarf.go
@@ -51,6 +51,13 @@
 	s.(*sym.Symbol).AddAddrPlus(c.linkctxt.Arch, data.(*sym.Symbol), value)
 }
 
+func (c dwctxt) AddCURelativeAddress(s dwarf.Sym, data interface{}, value int64) {
+	if value != 0 {
+		value -= (data.(*sym.Symbol)).Value
+	}
+	s.(*sym.Symbol).AddCURelativeAddrPlus(c.linkctxt.Arch, data.(*sym.Symbol), value)
+}
+
 func (c dwctxt) AddSectionOffset(s dwarf.Sym, size int, t interface{}, ofs int64) {
 	ls := s.(*sym.Symbol)
 	switch size {
@@ -1352,7 +1359,7 @@
 	// Create PC ranges for this CU.
 	newattr(unit.dwinfo, dwarf.DW_AT_ranges, dwarf.DW_CLS_PTR, ranges.Size, ranges)
 	newattr(unit.dwinfo, dwarf.DW_AT_low_pc, dwarf.DW_CLS_ADDRESS, base.Value, base)
-	dwarf.PutRanges(dwarfctxt, ranges, nil, pcs)
+	dwarf.PutBasedRanges(dwarfctxt, ranges, pcs)
 
 	if ctxt.HeadType == objabi.Haix {
 		addDwsectCUSize(".debug_ranges", unit.lib.String(), uint64(ranges.Size-unitLengthOffset))
@@ -1871,10 +1878,6 @@
 			if rangeSym != nil && rangeSym.Size > 0 {
 				rangeSym.Attr |= sym.AttrReachable | sym.AttrNotInSymbolTable
 				rangeSym.Type = sym.SDWARFRANGE
-				// LLVM doesn't support base address entries. Strip them out so LLDB and dsymutil don't get confused.
-				if ctxt.HeadType == objabi.Hdarwin {
-					removeDwarfAddrListBaseAddress(ctxt, dsym, rangeSym, false)
-				}
 				if ctxt.HeadType == objabi.Haix {
 					addDwsectCUSize(".debug_ranges", unit.lib.String(), uint64(rangeSym.Size))
 
@@ -1987,10 +1990,6 @@
 					reloc.Sym.Attr |= sym.AttrReachable | sym.AttrNotInSymbolTable
 					syms = append(syms, reloc.Sym)
 					empty = false
-					// LLVM doesn't support base address entries. Strip them out so LLDB and dsymutil don't get confused.
-					if ctxt.HeadType == objabi.Hdarwin {
-						removeDwarfAddrListBaseAddress(ctxt, fn, reloc.Sym, true)
-					}
 					// One location list entry per function, but many relocations to it. Don't duplicate.
 					break
 				}
@@ -2007,65 +2006,6 @@
 	return syms
 }
 
-// removeDwarfAddrListBaseAddress removes base address selector entries from
-// DWARF location lists and range lists.
-func removeDwarfAddrListBaseAddress(ctxt *Link, info, list *sym.Symbol, isloclist bool) {
-	// The list symbol contains multiple lists, but they're all for the
-	// same function, and it's not empty.
-	fn := list.R[0].Sym
-
-	// Discard the relocations for the base address entries.
-	list.R = list.R[:0]
-
-	// Add relocations for each location entry's start and end addresses,
-	// so that the base address entries aren't necessary.
-	// We could remove them entirely, but that's more work for a relatively
-	// small size win. If dsymutil runs it'll throw them away anyway.
-
-	// relocate adds a CU-relative relocation to fn+addr at offset.
-	relocate := func(addr uint64, offset int) {
-		list.R = append(list.R, sym.Reloc{
-			Off:  int32(offset),
-			Siz:  uint8(ctxt.Arch.PtrSize),
-			Type: objabi.R_ADDRCUOFF,
-			Add:  int64(addr),
-			Sym:  fn,
-		})
-	}
-
-	for i := 0; i < len(list.P); {
-		first := readPtr(ctxt, list.P[i:])
-		second := readPtr(ctxt, list.P[i+ctxt.Arch.PtrSize:])
-
-		if (first == 0 && second == 0) ||
-			first == ^uint64(0) ||
-			(ctxt.Arch.PtrSize == 4 && first == uint64(^uint32(0))) {
-			// Base address selection entry or end of list. Ignore.
-			i += ctxt.Arch.PtrSize * 2
-			continue
-		}
-
-		relocate(first, i)
-		relocate(second, i+ctxt.Arch.PtrSize)
-
-		// Skip past the actual location.
-		i += ctxt.Arch.PtrSize * 2
-		if isloclist {
-			i += 2 + int(ctxt.Arch.ByteOrder.Uint16(list.P[i:]))
-		}
-	}
-
-	// Rewrite the DIE's relocations to point to the first location entry,
-	// not the now-useless base address selection entry.
-	for i := range info.R {
-		r := &info.R[i]
-		if r.Sym != list {
-			continue
-		}
-		r.Add += int64(2 * ctxt.Arch.PtrSize)
-	}
-}
-
 // Read a pointer-sized uint from the beginning of buf.
 func readPtr(ctxt *Link, buf []byte) uint64 {
 	switch ctxt.Arch.PtrSize {
diff --git a/src/cmd/link/internal/sym/symbol.go b/src/cmd/link/internal/sym/symbol.go
index 8b70d61..88a28f5 100644
--- a/src/cmd/link/internal/sym/symbol.go
+++ b/src/cmd/link/internal/sym/symbol.go
@@ -172,7 +172,7 @@
 	return s.setUintXX(arch, r, v, int64(arch.PtrSize))
 }
 
-func (s *Symbol) AddAddrPlus(arch *sys.Arch, t *Symbol, add int64) int64 {
+func (s *Symbol) addAddrPlus(arch *sys.Arch, t *Symbol, add int64, typ objabi.RelocType) int64 {
 	if s.Type == 0 {
 		s.Type = SDATA
 	}
@@ -184,11 +184,19 @@
 	r.Sym = t
 	r.Off = int32(i)
 	r.Siz = uint8(arch.PtrSize)
-	r.Type = objabi.R_ADDR
+	r.Type = typ
 	r.Add = add
 	return i + int64(r.Siz)
 }
 
+func (s *Symbol) AddAddrPlus(arch *sys.Arch, t *Symbol, add int64) int64 {
+	return s.addAddrPlus(arch, t, add, objabi.R_ADDR)
+}
+
+func (s *Symbol) AddCURelativeAddrPlus(arch *sys.Arch, t *Symbol, add int64) int64 {
+	return s.addAddrPlus(arch, t, add, objabi.R_ADDRCUOFF)
+}
+
 func (s *Symbol) AddPCRelPlus(arch *sys.Arch, t *Symbol, add int64) int64 {
 	if s.Type == 0 {
 		s.Type = SDATA