Skip to content

Commit

Permalink
cmd/internal/ld: edit into more idiomatic Go code
Browse files Browse the repository at this point in the history
Instead of reimplementing chained hash tables, just use maps.

Use bool instead of uint8 for variables only set to 0 or 1.

Fix parsing of `import foo "foo" // indirect` lines.  Previously, this
was treated as an import of package path `"foo" // indirect`, which
could result in the cycle-detection code failing to detect a cycle
because it would be treated as a separate package from `"foo"`.

Also, since there are theoretically multiple quoted forms for a
package path, use strconv.Unquote to normalize them.  Side benefit:
Unquote will complain if any trailing comments sneak back in.

Aside: For most Go archives, Go package data is only present in the
__.PKGDEF member, but unless -u is used, ldpkg is only called on the
_go_.6 member.  Consequently, importcycles is a no-op when -u isn't
used as it has no package data to inspect.

Change-Id: I7076cf91a66726a8d9c5676adfea13c5532001fa
Reviewed-on: https://go-review.googlesource.com/7002
Reviewed-by: Brad Fitzpatrick <[email protected]>
Run-TryBot: Matthew Dempsky <[email protected]>
Reviewed-by: Rob Pike <[email protected]>
  • Loading branch information
mdempsky authored and robpike committed Mar 24, 2015
1 parent b2f2951 commit 717cb74
Showing 1 changed file with 56 additions and 78 deletions.
134 changes: 56 additions & 78 deletions src/cmd/internal/ld/go.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"cmd/internal/obj"
"fmt"
"os"
"strconv"
"strings"
)

Expand Down Expand Up @@ -37,42 +38,22 @@ func expandpkg(t0 string, pkg string) string {
* package import data
*/
type Import struct {
hash *Import // next in hash table
prefix string // "type", "var", "func", "const"
prefix string // "type", "var", "func", "const"
name string
def string
file string
}

const (
NIHASH = 1024
)

var ihash [NIHASH]*Import

var nimport int

func hashstr(name string) int {
h := uint32(0)
for cp := name; cp != ""; cp = cp[1:] {
h = h*1119 + uint32(cp[0])
}
h &= 0xffffff
return int(h)
}
// importmap records type information about imported symbols to detect inconsistencies.
// Entries are keyed by qualified symbol name (e.g., "runtime.Callers" or "net/url.Error").
var importmap = map[string]*Import{}

func ilookup(name string) *Import {
h := hashstr(name) % NIHASH
for x := ihash[h]; x != nil; x = x.hash {
if x.name[0] == name[0] && x.name == name {
return x
}
func lookupImport(name string) *Import {
if x, ok := importmap[name]; ok {
return x
}
x := new(Import)
x.name = name
x.hash = ihash[h]
ihash[h] = x
nimport++
x := &Import{name: name}
importmap[name] = x
return x
}

Expand Down Expand Up @@ -210,12 +191,10 @@ func loadpkgdata(file string, pkg string, data string) {
var prefix string
var name string
var def string
var x *Import

file = file
p := data
for parsepkgdata(file, pkg, &p, &prefix, &name, &def) > 0 {
x = ilookup(name)
x := lookupImport(name)
if x.prefix == "" {
x.prefix = prefix
x.def = def
Expand All @@ -235,8 +214,6 @@ func loadpkgdata(file string, pkg string, data string) {
}

func parsepkgdata(file string, pkg string, pp *string, prefixp *string, namep *string, defp *string) int {
var prefix string

// skip white space
p := *pp

Expand All @@ -249,7 +226,7 @@ loop:
}

// prefix: (var|type|func|const)
prefix = p
prefix := p

if len(p) < 7 {
return -1
Expand All @@ -268,7 +245,7 @@ loop:
p = p[1:]
}
p = p[1:]
name := p
line := p
for len(p) > 0 && p[0] != '\n' {
p = p[1:]
}
Expand All @@ -277,9 +254,16 @@ loop:
nerrors++
return -1
}
name = name[:len(name)-len(p)]
line = line[:len(line)-len(p)]
line = strings.TrimSuffix(line, " // indirect")
path, err := strconv.Unquote(line)
if err != nil {
fmt.Fprintf(os.Stderr, "%s: %s: confused in import path: %q\n", os.Args[0], file, line)
nerrors++
return -1
}
p = p[1:]
imported(pkg, name)
imported(pkg, path)
goto loop
} else {
fmt.Fprintf(os.Stderr, "%s: %s: confused in pkg data near <<%.40s>>\n", os.Args[0], file, prefix)
Expand Down Expand Up @@ -753,81 +737,75 @@ func addexport() {
}

type Pkg struct {
mark uint8
checked uint8
next *Pkg
path_ string
mark bool
checked bool
path string
impby []*Pkg
all *Pkg
}

var phash [1024]*Pkg
var (
// pkgmap records the imported-by relationship between packages.
// Entries are keyed by package path (e.g., "runtime" or "net/url").
pkgmap = map[string]*Pkg{}

var pkgall *Pkg
pkgall []*Pkg
)

func getpkg(path_ string) *Pkg {
h := hashstr(path_) % len(phash)
for p := phash[h]; p != nil; p = p.next {
if p.path_ == path_ {
return p
}
func lookupPkg(path string) *Pkg {
if p, ok := pkgmap[path]; ok {
return p
}
p := new(Pkg)
p.path_ = path_
p.next = phash[h]
phash[h] = p
p.all = pkgall
pkgall = p
p := &Pkg{path: path}
pkgmap[path] = p
pkgall = append(pkgall, p)
return p
}

func imported(pkg string, import_ string) {
// imported records that package pkg imports package imp.
func imported(pkg, imp string) {
// everyone imports runtime, even runtime.
if import_ == "\"runtime\"" {
if imp == "runtime" {
return
}

pkg = fmt.Sprintf("%q", pkg) // turn pkg path into quoted form, freed below
p := getpkg(pkg)
i := getpkg(import_)
p := lookupPkg(pkg)
i := lookupPkg(imp)
i.impby = append(i.impby, p)
}

func cycle(p *Pkg) *Pkg {
if p.checked != 0 {
func (p *Pkg) cycle() *Pkg {
if p.checked {
return nil
}

if p.mark != 0 {
if p.mark {
nerrors++
fmt.Printf("import cycle:\n")
fmt.Printf("\t%s\n", p.path_)
fmt.Printf("\t%s\n", p.path)
return p
}

p.mark = 1
var bad *Pkg
for i := 0; i < len(p.impby); i++ {
bad = cycle(p.impby[i])
if bad != nil {
p.mark = 0
p.checked = 1
fmt.Printf("\timports %s\n", p.path_)
p.mark = true
for _, q := range p.impby {
if bad := q.cycle(); bad != nil {
p.mark = false
p.checked = true
fmt.Printf("\timports %s\n", p.path)
if bad == p {
return nil
}
return bad
}
}

p.checked = 1
p.mark = 0
p.checked = true
p.mark = false
return nil
}

func importcycles() {
for p := pkgall; p != nil; p = p.all {
cycle(p)
for _, p := range pkgall {
p.cycle()
}
}

Expand Down

0 comments on commit 717cb74

Please sign in to comment.