Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

cmd/link: unexpectedly long symbol name #15104

Closed
bradfitz opened this issue Apr 4, 2016 · 11 comments
Closed

cmd/link: unexpectedly long symbol name #15104

bradfitz opened this issue Apr 4, 2016 · 11 comments
Labels
FrozenDueToAge
Milestone
Go1.7

Comments

@bradfitz
Copy link
Contributor

bradfitz commented Apr 4, 2016

I caused a regression in 73edd7b (cmd/link: simplify readSymName, taking advantage of bufio.Reader) It made an assumption that symbol names were less than 4kb.

Failure seen here:

https://storage.googleapis.com/go-build-log/1f5b1b2b/linux-amd64_8877cb41.log

# testmain
2016/04/04 12:24:45 $WORK/golang.org/x/text/unicode/cldr.a(_go_.o): unexpectedly long symbol name
FAIL    golang.org/x/text/internal/number [build failed]
ok      golang.org/x/text/internal/stringset    0.010s
ok      golang.org/x/text/internal/tag  0.003s
?       golang.org/x/text/internal/testtext [no test files]
ok      golang.org/x/text/internal/triegen  0.395s
ok      golang.org/x/text/internal/ucd  0.003s
?       golang.org/x/text/internal/utf8internal [no test files]
ok      golang.org/x/text/language  0.037s
# testmain
2016/04/04 12:24:49 $WORK/golang.org/x/text/unicode/cldr.a(_go_.o): unexpectedly long symbol name
FAIL    golang.org/x/text/language/display [build failed]
ok      golang.org/x/text/message   0.010s
ok      golang.org/x/text/runes 0.009s
ok      golang.org/x/text/search    0.012s
?       golang.org/x/text/secure    [no test files]
ok      golang.org/x/text/secure/precis 0.011s
ok      golang.org/x/text/transform 0.028s
?       golang.org/x/text/unicode   [no test files]
# testmain
2016/04/04 12:24:52 $WORK/golang.org/x/text/unicode/cldr.a(_go_.o): unexpectedly long symbol name
FAIL    golang.org/x/text/unicode/bidi [build failed]
# testmain
2016/04/04 12:24:54 $WORK/golang.org/x/text/unicode/cldr.a(_go_.o): unexpectedly long symbol name
# testmain
2016/04/04 12:24:54 $WORK/golang.org/x/text/unicode/cldr/_test/golang.org/x/text/unicode/cldr.a(_go_.o): unexpectedly long symbol name
FAIL    golang.org/x/text/unicode/cldr [build failed]
FAIL    golang.org/x/text/unicode/norm [build failed]
# testmain
2016/04/04 12:24:55 $WORK/golang.org/x/text/unicode/cldr.a(_go_.o): unexpectedly long symbol name
ok      golang.org/x/text/unicode/rangetable    1.353s
FAIL    golang.org/x/text/width [build failed]

/cc @crawshaw @nigeltao
@bradfitz bradfitz self-assigned this Apr 4, 2016
@bradfitz bradfitz added this to the Go1.7 milestone Apr 4, 2016
@crawshaw
Copy link
Member

crawshaw commented Apr 4, 2016

I would love to see that symbol name. (It's a bug because big data should be SHAed into the name.)

Given that with position-independent code we have to keep a lot of symbol names around, I've been meaning to survey them, just haven't got there yet.

@bradfitz
Copy link
Contributor Author

bradfitz commented Apr 4, 2016

Unfortunately I can't reproduce.

/cc @mpvl

@bradfitz
Copy link
Contributor Author

bradfitz commented Apr 4, 2016

Oh, I have to go test golang.org/x/text/... to reproduce, not go build.

@bradfitz
Copy link
Contributor Author

bradfitz commented Apr 4, 2016

The symbol name is 6K:

2016/04/04 15:48:29 $WORK/golang.org/x/text/unicode/cldr/_test/golang.org/x/text/unicode/cldr.a(go.o): unexpectedly long symbol name of 6397 bytes in golang.org/x/text/unicode/cldr: go.typelink.struct { cldr.Common; AttributeOrder *cldr.Common "xml:\"attributeOrder\""; ElementOrder *cldr.Common "xml:\"elementOrder\""; SerialElements *cldr.Common "xml:\"serialElements\""; Suppress *struct { cldr.Common; Attributes []*struct { cldr.Common; Element string "xml:\"element,attr\""; Attribute string "xml:\"attribute,attr\""; AttributeValue string "xml:\"attributeValue,attr\"" } "xml:\"attributes\"" } "xml:\"suppress\""; Validity *struct { cldr.Common; Variable []*struct { cldr.Common; Id string "xml:\"id,attr\"" } "xml:\"variable\""; AttributeValues []*struct { cldr.Common; Dtds string "xml:\"dtds,attr\""; Elements string "xml:\"elements,attr\""; Attributes string "xml:\"attributes,attr\""; Order string "xml:\"order,attr\"" } "xml:\"attributeValues\"" } "xml:\"validity\""; Alias *struct { cldr.Common; LanguageAlias []*struct { cldr.Common; Replacement string "xml:\"replacement,attr\""; Reason string "xml:\"reason,attr\"" } "xml:\"languageAlias\""; ScriptAlias []*struct { cldr.Common; Replacement string "xml:\"replacement,attr\""; Reason string "xml:\"reason,attr\"" } "xml:\"scriptAlias\""; TerritoryAlias []*struct { cldr.Common; Replacement string "xml:\"replacement,attr\""; Reason string "xml:\"reason,attr\"" } "xml:\"territoryAlias\""; SubdivisionAlias []*struct { cldr.Common; Replacement string "xml:\"replacement,attr\""; Reason string "xml:\"reason,attr\"" } "xml:\"subdivisionAlias\""; VariantAlias []*struct { cldr.Common; Replacement string "xml:\"replacement,attr\""; Reason string "xml:\"reason,attr\"" } "xml:\"variantAlias\""; ZoneAlias []*struct { cldr.Common; Replacement string "xml:\"replacement,attr\""; Reason string "xml:\"reason,attr\"" } "xml:\"zoneAlias\"" } "xml:\"alias\""; Deprecated *struct { cldr.Common; DeprecatedItems []*struct { cldr.Common; Elements string "xml:\"elements,attr\""; Attributes string "xml:\"attributes,attr\""; Values string "xml:\"values,attr\"" } "xml:\"deprecatedItems\"" } "xml:\"deprecated\""; Distinguishing *struct { cldr.Common; DistinguishingItems []*struct { cldr.Common; Exclude string "xml:\"exclude,attr\""; Elements string "xml:\"elements,attr\""; Attributes string "xml:\"attributes,attr\"" } "xml:\"distinguishingItems\"" } "xml:\"distinguishing\""; Blocking *struct { cldr.Common; BlockingItems []*struct { cldr.Common; Elements string "xml:\"elements,attr\"" } "xml:\"blockingItems\"" } "xml:\"blocking\""; CoverageAdditions *struct { cldr.Common; LanguageCoverage []*struct { cldr.Common; Values string "xml:\"values,attr\"" } "xml:\"languageCoverage\""; ScriptCoverage []*struct { cldr.Common; Values string "xml:\"values,attr\"" } "xml:\"scriptCoverage\""; TerritoryCoverage []*struct { cldr.Common; Values string "xml:\"values,attr\"" } "xml:\"territoryCoverage\""; CurrencyCoverage []*struct { cldr.Common; Values string "xml:\"values,attr\"" } "xml:\"currencyCoverage\""; TimezoneCoverage []*struct { cldr.Common; Values string "xml:\"values,attr\"" } "xml:\"timezoneCoverage\"" } "xml:\"coverageAdditions\""; SkipDefaultLocale *struct { cldr.Common; Services string "xml:\"services,attr\"" } "xml:\"skipDefaultLocale\""; DefaultContent *struct { cldr.Common; Locales string "xml:\"locales,attr\"" } "xml:\"defaultContent\"" } struct { "".Common; AttributeOrder *"".Common "xml:\"attributeOrder\""; ElementOrder *"".Common "xml:\"elementOrder\""; SerialElements *"".Common "xml:\"serialElements\""; Suppress *struct { "".Common; Attributes []*struct { "".Common; Element string "xml:\"element,attr\""; Attribute string "xml:\"attribute,attr\""; AttributeValue string "xml:\"attributeValue,attr\"" } "xml:\"attributes\"" } "xml:\"suppress\""; Validity *struct { "".Common; Variable []*struct { "".Common; Id string "xml:\"id,attr\"" } "xml:\"variable\""; AttributeValues []*struct { "".Common; Dtds string "xml:\"dtds,attr\""; Elements string "xml:\"elements,attr\""; Attributes string "xml:\"attributes,attr\""; Order string "xml:\"order,attr\"" } "xml:\"attributeValues\"" } "xml:\"validity\""; Alias *struct { "".Common; LanguageAlias []*struct { "".Common; Replacement string "xml:\"replacement,attr\""; Reason string "xml:\"reason,attr\"" } "xml:\"languageAlias\""; ScriptAlias []*struct { "".Common; Replacement string "xml:\"replacement,attr\""; Reason string "xml:\"reason,attr\"" } "xml:\"scriptAlias\""; TerritoryAlias []*struct { "".Common; Replacement string "xml:\"replacement,attr\""; Reason string "xml:\"reason,attr\"" } "xml:\"territoryAlias\""; SubdivisionAlias []*struct { "".Common; Replacement string "xml:\"replacement,attr\""; Reason string "xml:\"reason,attr\"" } "xml:\"subdivisionAlias\""; VariantAlias []*struct { "".Common; Replacement string "xml:\"replacement,attr\""; Reason string "xml:\"reason,attr\"" } "xml:\"variantAlias\""; ZoneAlias []*struct { "".Common; Replacement string "xml:\"replacement,attr\""; Reason string "xml:\"reason,attr\"" } "xml:\"zoneAlias\"" } "xml:\"alias\""; Deprecated *struct { "".Common; DeprecatedItems []*struct { "".Common; Elements string "xml:\"elements,attr\""; Attributes string "xml:\"attributes,attr\""; Values string "xml:\"values,attr\"" } "xml:\"deprecatedItems\"" } "xml:\"deprecated\""; Distinguishing *struct { "".Common; DistinguishingItems []*struct { "".Common; Exclude string "xml:\"exclude,attr\""; Elements string "xml:\"elements,attr\""; Attributes string "xml:\"attributes,attr\"" } "xml:\"distinguishingItems\"" } "xml:\"distinguishing\""; Blocking *struct { "".Common; BlockingItems []*struct { "".Common; Elements string "xml:\"elements,attr\"" } "xml:\"blockingItems\"" } "xml:\"blocking\""; CoverageAdditions *struct { "".Common; LanguageCoverage []*struct { "".Common; Values string "xml:\"values,attr\"" } "xml:\"languageCoverage\""; ScriptCoverage []*struct { "".Common; Values string "xml:\"values,attr\"" } "xml:\"scriptCoverage\""; TerritoryCoverage []*struct { "".Common; Values string "xml:\"values,attr\"" } "xml:\"territoryCoverage\""; CurrencyCoverage []*struct { "".Common; Values string "xml:\"values,attr\"" } "xml:\"currencyCoverage\""; TimezoneCoverage []*struct { "".Common; Values string "xml:\"values,attr\"" } "xml:\"timezoneCoverage\"" } "xml:\"coverageAdditions\""; SkipDefaultLocale *struct { "".Common; Services string "xml:\"services,attr\"" } "xml:\"skipDefaultLocale\""; DefaultContent *struct { "".Common; Locales string "xml:\"locales,attr\"" } "xml:\"defaultContent\"" }

@bradfitz
Copy link
Contributor Author

bradfitz commented Apr 4, 2016

"Fix" the linker, or fix the compiler? @crawshaw, you want this one?

/cc @josharian

@crawshaw crawshaw assigned crawshaw and unassigned bradfitz Apr 4, 2016
@gopherbot
Copy link
Contributor

CL https://golang.org/cl/21457 mentions this issue.

@mpvl
Copy link
Contributor

mpvl commented Apr 4, 2016

Yes, all this code is auto-generated. Lots of nested structs. I could hoist some of the nested structs, which should reduce it a bit, but that would be very far from ideal. It should be allowed in my opinion.

@bradfitz
Copy link
Contributor Author

bradfitz commented Apr 4, 2016

@mpvl, nobody has suggested you need to change your code.

@i4ki i4ki mentioned this issue Apr 5, 2016
Closed
@thesyncim
Copy link

same problem here

@crawshaw
Copy link
Member

crawshaw commented Apr 6, 2016

I had a followup to cl/21457 that introduces a new type printer for the type.* symbol names. It takes the type with placeholders for the package names and runs it through SHA-1, then prints the first few bytes of the hash (like we do with git commits). It then appends all the package names that appear in the type in appearance order.

The result is consistently small types and slightly less IO during compilation. (My secondary motivation was a stopgap measure for shrinking the symbol table for -buildmode=shared, but I now have a much simpler hack for that I'm going to send later today.) Unfortunately it also makes the toolchain just a little harder to debug. Given it's already pretty hard to understand and debug, I don't think it's worth it.

So I'd rather just land cl/21457 and make the linker's object reader handle long symbol names. I'll send another CL for that in a bit.

@gopherbot
Copy link
Contributor

CL https://golang.org/cl/21581 mentions this issue.

AkihiroSuda added a commit to osrg/namazu that referenced this issue Apr 7, 2016
@AkihiroSuda
.travis.yml: use Go 1.6 because tip doesn't work well (golang/go#15104)
793a553
gopherbot pushed a commit that referenced this issue Apr 18, 2016
@crawshaw
cmd/link, cmd/compile: typelink sorting in linker
4140da7 
Instead of writing out the type almost twice in the symbol name,
teach the linker how to sort typelink symbols by their contents.

This ~halves the size of typelink symbol names, which helps very
large (6KB) names like those mentioned in #15104.

This does not increase the total sorting work done by the linker,
and makes it possible to use shorter symbol names for types. See
the follow-on CL 21583.

Change-Id: Ie5807565ed07d31bc477d20f60e4c0b47144f337
Reviewed-on: https://go-review.googlesource.com/21457
Reviewed-by: Ian Lance Taylor <iant@golang.org>
@golang golang locked and limited conversation to collaborators Apr 14, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
Go1.7
Development

No branches or pull requests
5 participants
@bradfitz @crawshaw @thesyncim @mpvl @gopherbot