cmd/compile: crash on interface embedding cycles if a type alias is used #23823
Labels
Milestone
Comments
|
Slightly smaller repro: If one removes the |
mvdan
added
the
NeedsFix
mvdan
changed the title
mvdan
changed the title
|
Change https://golang.org/cl/93955 mentions this issue: |
|
This is turning out to be a subtle bug in the compiler, and I've spent enough hours failing to find the fix. I'll let someone else (@mdempsky?) take over. As a piece of perhaps useful information, if you replace the interface types with structs, there is no crash. So the bug involves both aliases and interfaces - if either of them is removed, the bug disappears. |
|
Possibly related to #18640. |
|
Change https://golang.org/cl/118078 mentions this issue: |
dna2github
pushed a commit
to dna2fork/go
that referenced
this issue
Jun 14, 2018
The original fix (https://go-review.googlesource.com/c/go/+/35831) for this issue was incorrect as it reported cycles in cases where it shouldn't. Instead, use a different approach: A type cycle containing aliases is only a cycle if there are no type definitions. As soon as there is a type definition, alias expansion terminates and there is no cycle. Approach: Split sprint_depchain into two non-recursive and more easily understandable functions (cycleFor and cycleTrace), and use those instead for cycle reporting. Analyze the cycle returned by cycleFor before issueing an alias cycle error. Also: Removed original fix (main.go) which introduced a separate crash (golang#23823). Fixes golang#18640. Fixes golang#23823. Fixes golang#24939. Change-Id: Ic3707a9dec40a71dc928a3e49b4868c5fac3d3b7 Reviewed-on: https://go-review.googlesource.com/118078 Reviewed-by: Matthew Dempsky <[email protected]>
|
Change https://golang.org/cl/147286 mentions this issue: |
gopherbot
pushed a commit
that referenced
this issue
Nov 5, 2018
This change re-introduces (temporarily) a work-around for recursive alias type declarations, originally in https://golang.org/cl/35831/ (intended as fix for #18640). The work-around was removed later for a more comprehensive cycle detection check. That check contained a subtle error which made the code appear to work, while in fact creating incorrect types internally. See #25838 for details. By re-introducing the original work-around, we eliminate problems with many simple recursive type declarations involving aliases; specifically cases such as #27232 and #27267. However, the more general problem remains. This CL also fixes the subtle error (incorrect variable use when analyzing a type cycle) mentioned above and now issues a fatal error with a reference to the relevant issue (rather than crashing later during the compilation). While not great, this is better than the current status. The long-term solution will need to address these cycles (see #25838). As a consequence, several old test cases are not accepted anymore by the compiler since they happened to work accidentally only. This CL disables parts or all code of those test cases. The issues are: #18640, #23823, and #24939. One of the new test cases (fixedbugs/issue27232.go) exposed a go/types issue. The test case is excluded from the go/types test suite and an issue was filed (#28576). Updates #18640. Updates #23823. Updates #24939. Updates #25838. Updates #28576. Fixes #27232. Fixes #27267. Change-Id: I6c2d10da98bfc6f4f445c755fcaab17fc7b214c5 Reviewed-on: https://go-review.googlesource.com/c/147286 Reviewed-by: Matthew Dempsky <[email protected]>
|
Change https://golang.org/cl/151339 mentions this issue: |
gopherbot
pushed a commit
that referenced
this issue
Nov 26, 2018
…ic alias declarations This change re-introduces (temporarily) a work-around for recursive alias type declarations, originally in https://golang.org/cl/35831/ (intended as fix for #18640). The work-around was removed later for a more comprehensive cycle detection check. That check contained a subtle error which made the code appear to work, while in fact creating incorrect types internally. See #25838 for details. By re-introducing the original work-around, we eliminate problems with many simple recursive type declarations involving aliases; specifically cases such as #27232 and #27267. However, the more general problem remains. This CL also fixes the subtle error (incorrect variable use when analyzing a type cycle) mentioned above and now issues a fatal error with a reference to the relevant issue (rather than crashing later during the compilation). While not great, this is better than the current status. The long-term solution will need to address these cycles (see #25838). As a consequence, several old test cases are not accepted anymore by the compiler since they happened to work accidentally only. This CL disables parts or all code of those test cases. The issues are: #18640, #23823, and #24939. One of the new test cases (fixedbugs/issue27232.go) exposed a go/types issue. The test case is excluded from the go/types test suite and an issue was filed (#28576). Updates #18640. Updates #23823. Updates #24939. Updates #25838. Updates #28576. Fixes #27232. Fixes #27267. Fixes #27383. Change-Id: I6c2d10da98bfc6f4f445c755fcaab17fc7b214c5 Reviewed-on: https://go-review.googlesource.com/c/147286 Reviewed-by: Matthew Dempsky <[email protected]> (cherry picked from commit e630538) Reviewed-on: https://go-review.googlesource.com/c/151339 Run-TryBot: Andrew Bonventre <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Robert Griesemer <[email protected]>
changkun
added a commit
to changkun/go
that referenced
this issue
Nov 30, 2018
* [release-branch.go1.11] doc/go1.11, cmd/go: elaborate on new GOFLAGS environment variable In Go 1.11, cmd/go gained support for the GOFLAGS environment variable. It was added and described in detail in CL 126656. Mention it in the Go 1.11 release notes, link to the cmd/go documentation, and add more details there. Fixes golang#27387. Change-Id: Ifc35bfe3e0886a145478d36dde8e80aedd8ec68e Reviewed-on: https://go-review.googlesource.com/135035 Reviewed-by: Bryan C. Mills <[email protected]> Reviewed-by: Rob Pike <[email protected]> (cherry picked from commit 58c6afe) Reviewed-on: https://go-review.googlesource.com/135496 Reviewed-by: Dmitri Shuralyov <[email protected]> * [release-branch.go1.11] net/http: ensure null body in Fetch response is not read The Fetch API returns a null body if there is no response body, on browsers that support streaming the response body. This change ensures we check for both undefined and null bodies before attempting to read the body. Fixes golang#27424 Change-Id: I0da86b61284fe394418b4b431495e715a037f335 Reviewed-on: https://go-review.googlesource.com/131236 Reviewed-by: Richard Musiol <[email protected]> Run-TryBot: Richard Musiol <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> (cherry picked from commit ce53683) Reviewed-on: https://go-review.googlesource.com/136915 * [release-branch.go1.11] runtime: ignore races between close and len/cap They aren't really races, or at least they don't have any observable effect. The spec is silent on whether these are actually races or not. Fix this problem by not using the address of len (or of cap) as the location where channel operations are recorded to occur. Use a random other field of hchan for that. I'm not 100% sure we should in fact fix this. Opinions welcome. Fixes golang#27778 Change-Id: Ib4efd4b62e0d1ef32fa51e373035ef207a655084 Reviewed-on: https://go-review.googlesource.com/135698 Reviewed-by: Dmitry Vyukov <[email protected]> (cherry picked from commit 83dfc3b) Reviewed-on: https://go-review.googlesource.com/138179 Run-TryBot: Keith Randall <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Ian Lance Taylor <[email protected]> * [release-branch.go1.11] net: fail fast for DNS rcode success with no answers of requested type DNS responses which do not contain answers of the requested type return errNoSuchHost, the same error as rcode name error. Prior to golang.org/cl/37879, both cases resulted in no additional name servers being consulted for the question. That CL changed the behavior for both cases. Issue golang#25336 was filed about the rcode name error case and golang.org/cl/113815 fixed it. This CL fixes the no answers of requested type case as well. Updates golang#27525 Fixes golang#27537 Change-Id: I52fadedcd195f16adf62646b76bea2ab3b15d117 Reviewed-on: https://go-review.googlesource.com/133675 Run-TryBot: Ian Gudger <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Brad Fitzpatrick <[email protected]> (cherry picked from commit 94f48dd) Reviewed-on: https://go-review.googlesource.com/138175 Run-TryBot: Brad Fitzpatrick <[email protected]> Reviewed-by: Ian Lance Taylor <[email protected]> * [release-branch.go1.11] cmd/go: add GOMIPS value to build id for mipsle Strip a trailing "le" from the GOARCH value when calculating the GOxxx environment variable that affects it. Updates golang#27260 Fixes golang#27420 Change-Id: I081f30d5dc19281901551823f4f56be028b5f71a Reviewed-on: https://go-review.googlesource.com/131379 Reviewed-by: Brad Fitzpatrick <[email protected]> (cherry picked from commit 61318d7) Reviewed-on: https://go-review.googlesource.com/138176 Run-TryBot: Brad Fitzpatrick <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Ian Lance Taylor <[email protected]> * [release-branch.go1.11] doc: add go1.11 to contrib.html Missing from https://golang.org/project Change-Id: I6cb769ae861a81f0264bae624b5fe8d70aa92497 Reviewed-on: https://go-review.googlesource.com/138356 Reviewed-by: Chris Broadfoot <[email protected]> * [release-branch.go1.11] reflect: use correct write barrier operations for method funcs Fix the code to use write barriers on heap memory, and no write barriers on stack memory. These errors were discovered as part of fixing golang#27695. They may have something to do with that issue, but hard to be sure. The core cause is different, so this fix is a separate CL. Update golang#27867 Change-Id: Ib005f6b3308de340be83c3d07d049d5e316b1e3c Reviewed-on: https://go-review.googlesource.com/137438 Reviewed-by: Austin Clements <[email protected]> (cherry picked from commit e35a412) Reviewed-on: https://go-review.googlesource.com/138581 Run-TryBot: Ian Lance Taylor <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Ian Lance Taylor <[email protected]> * [release-branch.go1.11] reflect: ensure correct scanning of return values During a call to a reflect-generated function or method (via makeFuncStub or methodValueCall), when should we scan the return values? When we're starting a reflect call, the space on the stack for the return values is not initialized yet, as it contains whatever junk was on the stack of the caller at the time. The return space must not be scanned during a GC. When we're finishing a reflect call, the return values are initialized, and must be scanned during a GC to make sure that any pointers in the return values are found and their referents retained. When the GC stack walk comes across a reflect call in progress on the stack, it needs to know whether to scan the results or not. It doesn't know the progress of the reflect call, so it can't decide by itself. The reflect package needs to tell it. This CL adds another slot in the frame of makeFuncStub and methodValueCall so we can put a boolean in there which tells the runtime whether to scan the results or not. This CL also adds the args length to reflectMethodValue so the runtime can restrict its scanning to only the args section (not the results) if the reflect package says the results aren't ready yet. Do a delicate dance in the reflect package to set the "results are valid" bit. We need to make sure we set the bit only after we've copied the results back to the stack. But we must set the bit before we drop reflect's copy of the results. Otherwise, we might have a state where (temporarily) no one has a live copy of the results. That's the state we were observing in issue golang#27695 before this CL. The bitmap used by the runtime currently contains only the args. (Actually, it contains all the bits, but the size is set so we use only the args portion.) This is safe for early in a reflect call, but unsafe late in a reflect call. The test issue27695.go demonstrates this unsafety. We change the bitmap to always include both args and results, and decide at runtime which portion to use. issue27695.go only has a test for method calls. Function calls were ok because there wasn't a safepoint between when reflect dropped its copy of the return values and when the caller is resumed. This may change when we introduce safepoints everywhere. This truncate-to-only-the-args was part of CL 9888 (in 2015). That part of the CL fixed the problem demonstrated in issue27695b.go but introduced the problem demonstrated in issue27695.go. TODO, in another CL: simplify FuncLayout and its test. stack return value is now identical to frametype.ptrdata + frametype.gcdata. Update golang#27867 Change-Id: I2d49b34e34a82c6328b34f02610587a291b25c5f Reviewed-on: https://go-review.googlesource.com/137440 Run-TryBot: Keith Randall <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Austin Clements <[email protected]> Reviewed-on: https://go-review.googlesource.com/138582 Run-TryBot: Ian Lance Taylor <[email protected]> Reviewed-by: Ian Lance Taylor <[email protected]> * [release-branch.go1.11] reflect: fix s390x reflect method calls R0 isn't the zero register any more. Oops. Update golang#27867 Change-Id: I46a975ed37d5e570afe2e228d3edf74949e08ad7 Reviewed-on: https://go-review.googlesource.com/138580 Reviewed-by: Michael Munday <[email protected]> Reviewed-on: https://go-review.googlesource.com/138583 Run-TryBot: Keith Randall <[email protected]> Run-TryBot: Ian Lance Taylor <[email protected]> Run-TryBot: Michael Munday <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Ian Lance Taylor <[email protected]> * [release-branch.go1.11] net: concatenate multiple TXT strings in single TXT record When go resolver was changed to use dnsmessage.Parser, LookupTXT returned two strings in one record as two different records. This change reverts back to concatenating multiple strings in a single TXT record. Updates golang#27763 Fixes golang#27886 Change-Id: Ice226fcb2be4be58853de34ed35b4627acb429ea Reviewed-on: https://go-review.googlesource.com/136955 Reviewed-by: Ian Gudger <[email protected]> Reviewed-by: Brad Fitzpatrick <[email protected]> Run-TryBot: Ian Gudger <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> (cherry picked from commit 7b3b160323b56b357832549fbab7a60d27688ec1) Reviewed-on: https://go-review.googlesource.com/138177 Run-TryBot: Brad Fitzpatrick <[email protected]> Reviewed-by: Katie Hockman <[email protected]> * [release-branch.go1.11] encoding/json: fix UnmarshalTypeError without field and struct values Updates golang#26444 Updates golang#27275 Fixes golang#27318 Change-Id: I9e8cbff79f7643ca8964c572c1a98172b6831730 GitHub-Last-Rev: 7eea215 GitHub-Pull-Request: golang#26719 Reviewed-on: https://go-review.googlesource.com/126897 Reviewed-by: Daniel Martí <[email protected]> Run-TryBot: Daniel Martí <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-on: https://go-review.googlesource.com/138178 Run-TryBot: Brad Fitzpatrick <[email protected]> * [release-branch.go1.11] doc: document Go 1.11.1 Updates golang#27953 Change-Id: I2f1a55e15dc5737a5a06bd894c46b2c4705f338c Reviewed-on: https://go-review.googlesource.com/138858 Reviewed-by: Filippo Valsorda <[email protected]> (cherry picked from commit f99fc3a) Reviewed-on: https://go-review.googlesource.com/138859 Reviewed-by: Dmitri Shuralyov <[email protected]> * [release-branch.go1.11] go1.11.1 Change-Id: I3cf3e57b11ad02b497276bae1864fc5ade8144b9 Reviewed-on: https://go-review.googlesource.com/138860 Run-TryBot: Katie Hockman <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Dmitri Shuralyov <[email protected]> * [release-branch.go1.11] misc/wasm: add mention of polyfill for Edge support Edge supports WebAssembly but not TextEncoder or TextDecoder. This change adds a comment pointing to a polyfill that could be used. The polyfill is not added by default, because we want to let the user decide if/how to include the polyfill. Fixes golang#27295 Change-Id: I375f58f2168665f549997b368428c398dfbbca1c Reviewed-on: https://go-review.googlesource.com/139037 Reviewed-by: Brad Fitzpatrick <[email protected]> (cherry picked from commit cfb603b0b5fb9c1e72be665b2d65743ddf18c779) Reviewed-on: https://go-review.googlesource.com/139057 Reviewed-by: Richard Musiol <[email protected]> * [release-branch.go1.11] cmd/compile: don't crash reporting misuse of shadowed built-in function The existing implementation causes a compiler panic if a function parameter shadows a built-in function, and then calling that shadowed name. Updates golang#27356 Fixes golang#27399 Change-Id: I1ffb6dc01e63c7f499e5f6f75f77ce2318f35bcd Reviewed-on: https://go-review.googlesource.com/132876 Reviewed-by: Robert Griesemer <[email protected]> Run-TryBot: Robert Griesemer <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> (cherry picked from commit 4a095b8) Reviewed-on: https://go-review.googlesource.com/c/139103 Run-TryBot: Brad Fitzpatrick <[email protected]> Reviewed-by: Ian Lance Taylor <[email protected]> * [release-branch.go1.11] cmd/compile: fix type of OffPtr in some optimization rules In some optimization rules the type of generated OffPtr was incorrectly set to the type of the pointee, instead of the pointer. When the OffPtr value is spilled, this may generate a spill of the wrong type, e.g. a floating point spill of an integer (pointer) value. On Wasm, this leads to invalid bytecode. Fixes golang#27961. Change-Id: I5d464847eb900ed90794105c0013a1a7330756cc Reviewed-on: https://go-review.googlesource.com/c/139257 Run-TryBot: Cherry Zhang <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Keith Randall <[email protected]> Reviewed-by: Richard Musiol <[email protected]> (cherry picked from commit c96e3bc) Reviewed-on: https://go-review.googlesource.com/c/139104 Run-TryBot: Brad Fitzpatrick <[email protected]> Reviewed-by: Cherry Zhang <[email protected]> * [release-branch.go1.11] cmd/go: don't mention -mod=release The -mod=release flag is not supported, so this appears to be a documentation mistake. Updates golang#27354. Fixes golang#27398. Change-Id: I895e8d5b4918adcb1f605361773173f312fa7b65 GitHub-Last-Rev: 42bfe0c GitHub-Pull-Request: golang#27358 Reviewed-on: https://go-review.googlesource.com/132116 Run-TryBot: Bryan C. Mills <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Bryan C. Mills <[email protected]> (cherry picked from commit 014901c) Reviewed-on: https://go-review.googlesource.com/c/139421 Run-TryBot: Ian Lance Taylor <[email protected]> Reviewed-by: Brad Fitzpatrick <[email protected]> * [release-branch.go1.11] doc: update docs.html with new tour import path As of golang.org/cl/141857 the import path has changed from golang.org/x/tour/gotour to golang.org/x/tour Change-Id: Ib54ab2e50188ef66c8a5c45136babfa49ad6934a Reviewed-on: https://go-review.googlesource.com/c/141917 Run-TryBot: Andrew Bonventre <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Brad Fitzpatrick <[email protected]> (cherry picked from commit 035f9e8) Reviewed-on: https://go-review.googlesource.com/c/143617 * [release-branch.go1.11] cmd/go: ensure git attributes are set This change disables the export-subst and export-ignore attributes when creating zip files for modules. This is done to prevent the ziphash for a given repo/revision from differing based on variables such as git version or size of repo. The full rational for this change is detailed here: golang#27153 (comment) Fixes golang#28094 Change-Id: Ib33f525d91d2581fa0b5d26e70d29620c7e685e9 Reviewed-on: https://go-review.googlesource.com/c/135175 Run-TryBot: Bryan C. Mills <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Bryan C. Mills <[email protected]> Reviewed-on: https://go-review.googlesource.com/c/141098 Reviewed-by: Andrew Bonventre <[email protected]> * [release-branch.go1.11] cmd/go, cmd/link: silence bogus Apple Xcode warning Certain installations of Xcode are affected by a bug that causes them to print an inconsequential link-time warning that looks like: ld: warning: text-based stub file /System/Library/Frameworks//Security.framework/Security.tbd and library file /System/Library/Frameworks//Security.framework/Security are out of sync. Falling back to library file for linking. This has nothing to do with Go, and we've sent this repro case to Apple: $ pkgutil --pkg-info=com.apple.pkg.CLTools_Executables | grep version version: 10.0.0.0.1.1535735448 $ clang --version Apple LLVM version 10.0.0 (clang-1000.10.44.2) Target: x86_64-apple-darwin17.7.0 Thread model: posix InstalledDir: /Library/Developer/CommandLineTools/usr/bin $ cat > issue.c int main() { return 0; } ^D $ clang issue.c -framework CoreFoundation ld: warning: text-based stub file /System/Library/Frameworks//CoreFoundation.framework/CoreFoundation.tbd and library file /System/Library/Frameworks//CoreFoundation.framework/CoreFoundation are out of sync. Falling back to library file for linking. $ Even if Apple does release a fixed Xcode, many people are seeing this useless warning, and we might as well make it go away. Fixes golang#26073. Change-Id: Ifc17ba7da1f6b59e233c11ebdab7241cb6656324 Reviewed-on: https://go-review.googlesource.com/c/144112 Reviewed-by: Brad Fitzpatrick <[email protected]> Reviewed-by: Andrew Bonventre <[email protected]> (cherry picked from commit 66bb8dd) Reviewed-on: https://go-review.googlesource.com/c/145458 Run-TryBot: Andrew Bonventre <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> * [release-branch.go1.11] internal/poll: advance file position in windows sendfile Some versions of Windows (Windows 10 1803) do not set file position after TransmitFile completes. So just use Seek to set file position before returning from sendfile. Fixes golang#27411 Change-Id: I7a49be10304b5db19dda707b13ac93d338aeb190 Reviewed-on: https://go-review.googlesource.com/131976 Reviewed-by: Brad Fitzpatrick <[email protected]> Reviewed-by: Ian Lance Taylor <[email protected]> Reviewed-by: Yasuhiro MATSUMOTO <[email protected]> Run-TryBot: Brad Fitzpatrick <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-on: https://go-review.googlesource.com/c/145779 Run-TryBot: Alex Brainman <[email protected]> * [release-branch.go1.11] cmd/go/internal/modcmd: remove non-existent -dir flag Updates golang#27243 Fixes golang#27498 Change-Id: If9230244938dabd03b9afaa6600310df8f97fe92 Reviewed-on: https://go-review.googlesource.com/131775 Reviewed-by: Bryan C. Mills <[email protected]> (cherry picked from commit 55ef446) Reviewed-on: https://go-review.googlesource.com/c/146717 Reviewed-by: Brad Fitzpatrick <[email protected]> * [release-branch.go1.11] cmd/trace: don't drop sweep slice details For sweep events, we used to modify the ViewerEvent returned from ctx.emitSlice later in order to embed more details about the sweep operation. The trick no longer works after the change https://golang.org/cl/92375 and caused a regression. ctx.emit method encodes the ViewerEvent, so any modification to the ViewerEvent object after ctx.emit returns will not be reflected. Refactor ctx.emitSlice, so ctx.makeSlice can be used when producing slices for SWEEP. ctx.emit* methods are meant to truely emit ViewerEvents. Fixes golang#27717 Updates golang#27711 Change-Id: I0b733ebbbfd4facd8714db0535809ec3cab0833d Reviewed-on: https://go-review.googlesource.com/135775 Reviewed-by: Austin Clements <[email protected]> Run-TryBot: Austin Clements <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> (cherry picked from commit e57f24a) Reviewed-on: https://go-review.googlesource.com/c/146698 Run-TryBot: Brad Fitzpatrick <[email protected]> Reviewed-by: Dmitri Shuralyov <[email protected]> * [release-branch.go1.11] database/sql: correctly report MaxIdleClosed stat Previously the MaxIdleClosed counter was incremented when added to the free connection list, rather then when it wasn't added to the free connection list. Flip this logic to correct. Fixes golang#28325 Change-Id: I405302c14fb985369dab48fbe845e5651afc4ccf Reviewed-on: https://go-review.googlesource.com/c/138578 Run-TryBot: Daniel Theophanes <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Brad Fitzpatrick <[email protected]> (cherry picked from commit 7db509e) Reviewed-on: https://go-review.googlesource.com/c/146697 Run-TryBot: Brad Fitzpatrick <[email protected]> Reviewed-by: Andrew Bonventre <[email protected]> Reviewed-by: Daniel Theophanes <[email protected]> * [release-branch.go1.11] go/types: use correct receiver types for embedded interface methods Interface methods don't declare a receiver (it's implicit), but after type-checking the respective *types.Func objects are marked as methods by having a receiver. For interface methods, the receiver base type used to be the interface that declared the method in the first place, even if the method also appeared in other interfaces via embedding. A change in the computation of method sets for interfaces for Go1.10 changed that inadvertently, with the consequence that sometimes a method's receiver type ended up being an interface into which the method was embedded. The exact behavior also depended on file type-checking order, and because files are sometimes sorted by name, the behavior depended on file names. This didn't matter for type-checking (the typechecker doesn't need the receiver), but it matters for clients, and for printing of methods. This change fixes interface method receivers at the end of type-checking when we have all relevant information. Fixes golang#28249 Updates golang#28005 Change-Id: I96c120fb0e517d7f8a14b8530f0273674569d5ea Reviewed-on: https://go-review.googlesource.com/c/141358 Reviewed-by: Alan Donovan <[email protected]> Reviewed-on: https://go-review.googlesource.com/c/146660 Run-TryBot: Brad Fitzpatrick <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Robert Griesemer <[email protected]> * [release-branch.go1.11] doc: document Go 1.10.5 Change-Id: I11adca150ab795607b832fb354a3e065655e1020 Reviewed-on: https://go-review.googlesource.com/c/147179 Reviewed-by: Brad Fitzpatrick <[email protected]> (cherry picked from commit 2764d5e) Reviewed-on: https://go-review.googlesource.com/c/147181 Reviewed-by: Andrew Bonventre <[email protected]> * [release-branch.go1.11] doc: document Go 1.11.2 Change-Id: Iaff03911f1807d462f1966590626bd486807f53d Reviewed-on: https://go-review.googlesource.com/c/147178 Reviewed-by: Brad Fitzpatrick <[email protected]> (cherry picked from commit c5d78f5) Reviewed-on: https://go-review.googlesource.com/c/147182 Reviewed-by: Andrew Bonventre <[email protected]> * [release-branch.go1.11] go1.11.2 Change-Id: Idd3527ba8f2329876cbca646aacd97739b9828f7 Reviewed-on: https://go-review.googlesource.com/c/147217 Run-TryBot: Andrew Bonventre <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Brad Fitzpatrick <[email protected]> * [release-branch.go1.11] runtime: never call into race detector with retaken P cgocall could previously invoke the race detector on an M whose P had been retaken. The race detector would attempt to use the P-local state from this stale P, racing with the thread that was actually wired to that P. The result was memory corruption of ThreadSanitizer's internal data structures that presented as hard-to-understand assertion failures and segfaults. Reorder cgocall so that it always acquires a P before invoking the race detector, and add a test that stresses the interaction between cgo and the race detector to protect against future bugs of this kind. Fixes golang#28690. Change-Id: Ide93f96a23490314d6647547140e0a412a97f0d4 Reviewed-on: https://go-review.googlesource.com/c/148717 Run-TryBot: Dmitry Vyukov <[email protected]> Reviewed-by: Dmitry Vyukov <[email protected]> (cherry picked from commit e496e61) Reviewed-on: https://go-review.googlesource.com/c/148902 Run-TryBot: Ian Lance Taylor <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Ian Lance Taylor <[email protected]> * [release-branch.go1.11] runtime: avoid arm64 8.1 atomics on Android The kernel on some Samsung S9+ models reports support for arm64 8.1 atomics, but in reality only some of the cores support them. Go programs scheduled to cores without support will crash with SIGILL. This change unconditionally disables the optimization on Android. A better fix is to precisely detect the offending chipset. Fixes golang#28586 Change-Id: I35a1273e5660603824d30ebef2ce7e429241bf1f Reviewed-on: https://go-review.googlesource.com/c/147377 Run-TryBot: Elias Naur <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Keith Randall <[email protected]> Reviewed-on: https://go-review.googlesource.com/c/149557 Reviewed-by: Brad Fitzpatrick <[email protected]> * [release-branch.go1.11] cmd/go: don't panic when go run is passed ... under nonexistent dir Given a nonexistent directory above a wildcard: go run ./nonexistent/... Print this error instead of panicking: go run: no packages loaded from ./nonexistent/... Updates golang#28696. Fixes golang#28725 Change-Id: Iaa3bc5c78b14ef858d931778e1bc55ca626c5571 GitHub-Last-Rev: bb1a804 GitHub-Pull-Request: golang#28703 Reviewed-on: https://go-review.googlesource.com/c/148821 Run-TryBot: Emmanuel Odeke <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Bryan C. Mills <[email protected]> Reviewed-by: Emmanuel Odeke <[email protected]> (cherry picked from commit 529ea7c) Reviewed-on: https://go-review.googlesource.com/c/149607 Run-TryBot: Ian Lance Taylor <[email protected]> * [release-branch.go1.11] cmd/compile: don't deadcode eliminate labels Dead-code eliminating labels is tricky because there might be gotos that can still reach them. Bug probably introduced with CL 91056 Fixes golang#28617 Change-Id: I6680465134e3486dcb658896f5172606cc51b104 Reviewed-on: https://go-review.googlesource.com/c/147817 Run-TryBot: Keith Randall <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Brad Fitzpatrick <[email protected]> Reviewed-by: Iskander Sharipov <[email protected]> Reviewed-on: https://go-review.googlesource.com/c/147857 * runtime: when using explicit argmap, also use arglen When we set an explicit argmap, we may want only a prefix of that argmap. Argmap is set when the function is reflect.makeFuncStub or reflect.methodValueCall. In this case, arglen specifies how much of the args section is actually live. (It could be either all the args + results, or just the args.) Fixes golang#28752 Change-Id: Idf060607f15a298ac591016994e58e22f7f92d83 Reviewed-on: https://go-review.googlesource.com/c/149217 Run-TryBot: Keith Randall <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Austin Clements <[email protected]> (cherry picked from commit 0098f8a) Reviewed-on: https://go-review.googlesource.com/c/149457 * [release-branch.go1.11] cmd/compile: reintroduce work-around for cyclic alias declarations This change re-introduces (temporarily) a work-around for recursive alias type declarations, originally in https://golang.org/cl/35831/ (intended as fix for golang#18640). The work-around was removed later for a more comprehensive cycle detection check. That check contained a subtle error which made the code appear to work, while in fact creating incorrect types internally. See golang#25838 for details. By re-introducing the original work-around, we eliminate problems with many simple recursive type declarations involving aliases; specifically cases such as golang#27232 and golang#27267. However, the more general problem remains. This CL also fixes the subtle error (incorrect variable use when analyzing a type cycle) mentioned above and now issues a fatal error with a reference to the relevant issue (rather than crashing later during the compilation). While not great, this is better than the current status. The long-term solution will need to address these cycles (see golang#25838). As a consequence, several old test cases are not accepted anymore by the compiler since they happened to work accidentally only. This CL disables parts or all code of those test cases. The issues are: golang#18640, golang#23823, and golang#24939. One of the new test cases (fixedbugs/issue27232.go) exposed a go/types issue. The test case is excluded from the go/types test suite and an issue was filed (golang#28576). Updates golang#18640. Updates golang#23823. Updates golang#24939. Updates golang#25838. Updates golang#28576. Fixes golang#27232. Fixes golang#27267. Fixes golang#27383. Change-Id: I6c2d10da98bfc6f4f445c755fcaab17fc7b214c5 Reviewed-on: https://go-review.googlesource.com/c/147286 Reviewed-by: Matthew Dempsky <[email protected]> (cherry picked from commit e630538) Reviewed-on: https://go-review.googlesource.com/c/151339 Run-TryBot: Andrew Bonventre <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Robert Griesemer <[email protected]> * [release-branch.go1.11] cmd/compile/internal/gc: OMUL should be evaluated when using soft-float When using soft-float, OMUL might be rewritten to function call so we should ensure it was evaluated first. Updates golang#28688 Fixes golang#28694 Change-Id: I30b87501782fff62d35151f394a1c22b0d490c6c Reviewed-on: https://go-review.googlesource.com/c/148837 Run-TryBot: Cherry Zhang <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Cherry Zhang <[email protected]> (cherry picked from commit c92e73b) Reviewed-on: https://go-review.googlesource.com/c/151342 Reviewed-by: Andrew Bonventre <[email protected]> * [release-branch.go1.11] go/types: avoid certain problems with recursive alias type declarations It is possible to create certain recursive type declarations involving alias types which cause the type-checker to produce an (invalid) type for the alias because it is not yet available. By type-checking alias declarations in a 2nd phase, the problem is mitigated a bit since it requires more convoluted alias declarations for the problem to appear. Also re-enable testing of fixedbugs/issue27232.go again (which was the original cause for this change). Updates golang#28576. Fixes golang#28972. Change-Id: If6f9656a95262e6575b01c4a003094d41551564b Reviewed-on: https://go-review.googlesource.com/c/147597 Reviewed-by: Alan Donovan <[email protected]> Reviewed-on: https://go-review.googlesource.com/c/151500 Run-TryBot: Andrew Bonventre <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Andrew Bonventre <[email protected]>
|
Looks it still crashes on g1.12beta2. |
|
Looks it was ever fixed in Go 1.11. |
|
Hm, interesting. The previous CLs were known not to fix this completely (doing this correctly requires more changes in the compiler than we felt comfortable making during the various freezes); but there should not be a raw crash but a compiler-internal error with an error message. |
|
Change https://golang.org/cl/175518 mentions this issue: |
|
Filed reminder issue #31872 for fix of root cause. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Please answer these questions before submitting your issue. Thanks!
What version of Go are you using (
go version)?go version go1.10rc2 linux/amd64
What did you do?
What did you expect to see?
Compiler doesn't crash. An error message should be printed.
What did you see instead?
Compiler crashes.
The text was updated successfully, but these errors were encountered: