- 15 May, 2014 8 commits
-
-
Russ Cox authored
Globals, function arguments, and results are special cases in registerization. Globals must be flushed aggressively, because nearly any operation can cause a panic, and the recovery code must see the latest values. Globals also must be loaded aggressively, because nearly any store through a pointer might be updating a global: the compiler cannot see all the "address of" operations on globals, especially exported globals. To accomplish this, mark all globals as having their address taken, which effectively disables registerization. If a function contains a defer statement, the function results must be flushed aggressively, because nearly any operation can cause a panic, and the deferred code may call recover, causing the original function to return the current values of its function results. To accomplish this, mark all function results as having their address taken if the function contains any defer statements. This causes not just aggressive flushing but also aggressive loading. The aggressive loading is overkill but the best we can do in the current code. Function arguments must be considered live at all safe points in a function, because garbage collection always preserves them: they must be up-to-date in order to be preserved correctly. Accomplish this by marking them live at all call sites. An earlier attempt at this marked function arguments as having their address taken, which disabled registerization completely, making programs slower. This CL's solution allows registerization while preserving safety. The benchmark speedup is caused by being able to registerize again (the earlier CL lost the same amount). benchmark old ns/op new ns/op delta BenchmarkEqualPort32 61.4 56.0 -8.79% benchmark old MB/s new MB/s speedup BenchmarkEqualPort32 521.56 570.97 1.09x Fixes #1304. (again) Fixes #7944. (again) Fixes #7984. Fixes #7995. LGTM=khr R=golang-codereviews, khr CC=golang-codereviews, iant, r https://golang.org/cl/97500044
-
Russ Cox authored
Do not compare nil and true. Fixes #7996. LGTM=r R=golang-codereviews, r CC=golang-codereviews https://golang.org/cl/91470043
-
Russ Cox authored
The function takes 32 bytes of arguments: 8 for the *block and then 3*8 for the slice. The 24 is not causing a bug (today at least) because the final word is the cap of the slice, which the assembly does not use. Identified by 'go vet std'. LGTM=bradfitz R=golang-codereviews, bradfitz CC=golang-codereviews https://golang.org/cl/96360043
-
Alex Brainman authored
LGTM=iant R=golang-codereviews, iant CC=golang-codereviews https://golang.org/cl/100440045
-
Alex Brainman authored
Fixes #7406. LGTM=r R=golang-codereviews, r CC=golang-codereviews https://golang.org/cl/97440043
-
Alex Brainman authored
fixes windows build LGTM=bradfitz R=golang-codereviews, bradfitz CC=golang-codereviews https://golang.org/cl/97500043
-
Russ Cox authored
Turns out elf.File.Sections is indexed by the actual section number, not the number minus one. I don't know why I thought the -1 was necessary. Fixes objdump test (and therefore build) on ELF systems. While we're here, fix bounds on gnuDump so that we don't crash when asked to disassemble outside the text segment. May fix Windows build or at least make the failure more interesting. TBR=iant CC=golang-codereviews https://golang.org/cl/92390043
-
Guillaume J. Charmes authored
Implement the changes as suggested by rsc. Fixes #6670. LGTM=josharian, iant R=golang-codereviews, iant, josharian, mikioh.mikioh, alex, gobot CC=golang-codereviews, rsc https://golang.org/cl/83690045
-
- 14 May, 2014 9 commits
-
-
Russ Cox authored
There is some duplication here with cmd/nm. There is a TODO to address that after 1.3 is out. Update #7452 x86 disassembly works and is tested. The arm disassembler does not exist yet and is therefore not yet hooked up. LGTM=crawshaw, iant R=crawshaw, iant CC=golang-codereviews https://golang.org/cl/91360046
-
Russ Cox authored
The x86 disassembler lives in rsc.io/x86/x86asm for now. We need to figure out what should live where in the long term, but not before the 1.3 release. The completed code reviews for the disassembler are at: https://golang.org/cl/95350044 https://golang.org/cl/95300044 https://golang.org/cl/97100047 https://golang.org/cl/93110044 https://golang.org/cl/99000043 https://golang.org/cl/98990043 LGTM=crawshaw R=crawshaw, jacek.masiulaniec CC=golang-codereviews https://golang.org/cl/92360043
-
Andrew Gerrand authored
Generated by addca. R=gobot CC=golang-codereviews https://golang.org/cl/92380043
-
Mikio Hara authored
Seems like we need to drag the stack for <autogenerated>:1 on Plan 9. See http://build.golang.org/log/283b996102b833dd81c58301d78aceaa4fe9838b. LGTM=rsc R=rsc CC=golang-codereviews https://golang.org/cl/95390043
-
Rob Pike authored
Make it a little clearer how they are used, in particular that it is not enough just to return a nil pointer on error, but also to return an error value explaining the problem. Fixes #1963. LGTM=bradfitz R=golang-codereviews, bradfitz CC=golang-codereviews https://golang.org/cl/97360045
-
Robert Griesemer authored
- use previously defined terms (with links) throughout - specify evaluation order more precisely (in particular, the evaluation time of rhs expressions in receive cases was not specified) - added extra example case Not a language change. Description matches observed behavior of code compiled with gc and gccgo. Fixes #7669. LGTM=iant, r, rsc R=r, rsc, iant, ken, josharian CC=golang-codereviews https://golang.org/cl/91230043
-
Guillaume J. Charmes authored
Do not use ustar format if we need the GNU one. Change \000 to \x00 for consistency Check for "ustar\x00" instead of "ustar\x00\x00" for conistency with tar and compatiblity with archive generated with older code (which was ustar\x00\x20\x00) Add test for long name + big file. LGTM=iant R=golang-codereviews, iant CC=golang-codereviews https://golang.org/cl/99050043
-
Dmitriy Vyukov authored
AddressSanitizer says: AddressSanitizer: heap-buffer-overflow on address 0x60200001b6f3 READ of size 6 at 0x60200001b6f3 thread T0 #0 0x46741b in __interceptor_memcmp asan_interceptors.cc:337 #1 0x4b5794 in compile src/cmd/6g/../gc/pgen.c:177 #2 0x509b81 in funccompile src/cmd/gc/dcl.c:1457 #3 0x520fe2 in p9main src/cmd/gc/lex.c:489 #4 0x5e2e01 in main src/lib9/main.c:57 #5 0x7fab81f7976c in __libc_start_main /build/buildd/eglibc-2.15/csu/libc-start.c:226 #6 0x4b16dc in _start (pkg/tool/linux_amd64/6g+0x4b16dc) 0x60200001b6f3 is located 0 bytes to the right of 3-byte region [0x60200001b6f0,0x60200001b6f3) allocated by thread T0 here: #0 0x493ec8 in __interceptor_malloc asan_malloc_linux.cc:75 #1 0x54d64e in mal src/cmd/gc/subr.c:459 #2 0x5260d5 in yylex src/cmd/gc/lex.c:1605 #3 0x52078f in p9main src/cmd/gc/lex.c:402 #4 0x5e2e01 in main src/lib9/main.c:57 If the memory block happens to be at the end of hunk and page bounadry, this out-of-bounds can lead to a crash. LGTM=dave, iant R=golang-codereviews, dave, iant CC=golang-codereviews https://golang.org/cl/93370043
-
Mikio Hara authored
Fixes #7974. LGTM=iant R=golang-codereviews, iant CC=golang-codereviews https://golang.org/cl/95320043
-
- 13 May, 2014 12 commits
-
-
Rob Pike authored
The code recurs very deeply in cases like (?:x{1,1000}){1,1000} Since if much time is spent checking whether one pass is possible, it's not worth doing at all, a simple fix is proposed: Stop if the check takes too long. To do this, we simply avoid machines with >1000 instructions. Benchmarks show a percent or less change either way, effectively zero. Fixes #7608. LGTM=rsc R=rsc CC=golang-codereviews https://golang.org/cl/92290043
-
Dmitriy Vyukov authored
If a map variable is created with reflect.New it has incorrect type (map[unsafe.Pointer]unsafe.Pointer). If GC follows such pointer, it scans Hmap and buckets with incorrect type. This can lead to overscan of up to 120 bytes for map[int8]struct{}. Which in turn can lead to crash if the memory after a bucket object is unaddressable or false retention (buckets are scanned as arrays of unsafe.Pointer). I don't see how it can lead to heap corruptions, though. LGTM=khr R=rsc, khr CC=golang-codereviews https://golang.org/cl/96270044
-
Dmitriy Vyukov authored
mstats.last_gc is unix time now, it is compared with abstract monotonic time. On my machine GC is forced every 5 mins regardless of last_gc. LGTM=rsc R=golang-codereviews CC=golang-codereviews, iant, rsc https://golang.org/cl/91350045
-
Russ Cox authored
Fixes subrepo builds. LGTM=iant, mikioh.mikioh R=golang-codereviews, iant, mikioh.mikioh CC=golang-codereviews https://golang.org/cl/96310043
-
Russ Cox authored
I have no test case for this at tip. The original report included a program crashing at revision 88ac7297d2fa. I tested this code at that revision and it does fix the crash. However, at tip the reported code no longer crashes, presumably because some allocation patterns have changed. I believe the bug is still present at tip and that this code still fixes it. Fixes #7143. LGTM=alex.brainman R=golang-codereviews, alex.brainman CC=dvyukov, golang-codereviews https://golang.org/cl/96300046
-
Russ Cox authored
Fixes #7560. LGTM=iant R=golang-codereviews, iant CC=golang-codereviews https://golang.org/cl/96300045
-
Russ Cox authored
Replaces CL 91240045. Fixes #7809. LGTM=bradfitz R=golang-codereviews, minux.ma CC=adg, bradfitz, golang-codereviews, iant, r https://golang.org/cl/94380043
-
Russ Cox authored
Originally it was an error, which made perfect sense, but in issue 2540 I got talked out of this sensible behavior. I'm not thrilled with the "new" behavior but it's been there since Go 1.1 so we're stuck with it now. Fixes #6724. LGTM=bradfitz R=golang-codereviews, bradfitz CC=golang-codereviews https://golang.org/cl/100430043
-
Jason Del Ponte authored
This changes allows the first token encoded to be a xml declaration. A ProcInst with target of xml. Any other ProcInst after that with a target of xml will fail Fixes #7380. LGTM=rsc R=golang-codereviews, rsc CC=golang-codereviews https://golang.org/cl/72410043
-
Russ Cox authored
Generated by addca. R=gobot CC=golang-codereviews https://golang.org/cl/100410045
-
Brad Fitzpatrick authored
Fixes #7888 LGTM=adg R=adg CC=golang-codereviews https://golang.org/cl/100420043
-
Russ Cox authored
Fixes race build. TBR=iant CC=golang-codereviews https://golang.org/cl/100410044
-
- 12 May, 2014 11 commits
-
-
Russ Cox authored
The inputs to a function are marked live at all times in the liveness bitmaps, so that the garbage collector will not free the things they point at and reuse the pointers, so that the pointers shown in stack traces are guaranteed not to have been recycled. Unfortunately, no one told the register optimizer that the inputs need to be preserved at all call sites. If a function is done with a particular input value, the optimizer will stop preserving it across calls. For single-word values this just means that the value recorded might be stale. For multi-word values like slices, the value recorded could be only partially stale: it can happen that, say, the cap was updated but not the len, or that the len was updated but not the base pointer. Either of these possibilities (and others) would make the garbage collector misinterpret memory, leading to memory corruption. This came up in a real program, in which the garbage collector's 'slice len ≤ slice cap' check caught the inconsistency. Fixes #7944. LGTM=iant R=golang-codereviews, iant CC=golang-codereviews, khr https://golang.org/cl/100370045
-
Josh Bleecher Snyder authored
This is joint work with Daniel Morsing. In order for the register allocator to alias two variables, they must have the same width, stack offset, and etype. Code generation was altering a variable's etype in a few places. This prevented the variable from being moved to a register, which in turn prevented peephole optimization. This failure to alias was very common, with almost 23,000 instances just running make.bash. This phenomenon was not visible in the register allocation debug output because the variables that failed to alias had the same name. The debugging-only change to bits.c fixes this by printing the variable number with its name. This CL fixes the source of all etype mismatches for 6g, all but one case for 8g, and depressingly few cases for 5g. (I believe that extending CL 6819083 to 5g is a prerequisite.) Fixing the remaining cases in 8g and 5g is work for the future. The etype mismatch fixes are: * [gc] Slicing changed the type of the base pointer into a uintptr in order to perform arithmetic on it. Instead, support addition directly on pointers. * [*g] OSPTR was giving type uintptr to slice base pointers; undo that. This arose, for example, while compiling copy(dst, src). * [8g] 64 bit float conversion was assigning int64 type during codegen, overwriting the existing uint64 type. Note that some etype mismatches are appropriate, such as a struct with a single field or an array with a single element. With these fixes, the number of registerizations that occur while running make.bash for 6g increases ~10%. Hello world binary size shrinks ~1.5%. Running all benchmarks in the standard library show performance improvements ranging from nominal to substantive (>10%); a full comparison using 6g on my laptop is available at https://gist.github.com/josharian/8f9b5beb46667c272064. The microbenchmarks must be taken with a grain of salt; see issue 7920. The few benchmarks that show real regressions are likely due to issue 7920. I manually examined the generated code for the top few regressions and none had any assembly output changes. The few benchmarks that show extraordinary improvements are likely also due to issue 7920. Performance results from 8g appear similar to 6g. 5g shows no performance improvements. This is not surprising, given the discussion above. Update #7316 LGTM=rsc R=rsc, daniel.morsing, bradfitz CC=dave, golang-codereviews https://golang.org/cl/91850043
-
Russ Cox authored
The runtime was detecting the cycle already, but we can give a better error without even building the binary. Fixes #7789. LGTM=iant R=golang-codereviews, iant CC=golang-codereviews https://golang.org/cl/96290043
-
Ian Lance Taylor authored
This change requires using SWIG version 3.0 or later. Earlier versions of SWIG do not generate the pragmas required to use the external linker. Fixes #7155. Fixes #7156. LGTM=rsc R=rsc CC=golang-codereviews https://golang.org/cl/97120046
-
Russ Cox authored
Fixes #7931. LGTM=iant R=golang-codereviews, iant CC=golang-codereviews https://golang.org/cl/100390044
-
Fabrizio Milo authored
Prevent idle transport on race condition. Fixes #7847 LGTM=bradfitz R=golang-codereviews, bradfitz CC=golang-codereviews https://golang.org/cl/96230044
-
Brad Fitzpatrick authored
Generated by addca. R=gobot CC=golang-codereviews https://golang.org/cl/100390045
-
Russ Cox authored
Before we used line 1 of the first source file. This should be clearer. Fixes #4388. LGTM=iant R=golang-codereviews, iant CC=golang-codereviews https://golang.org/cl/92250044
-
Brad Fitzpatrick authored
<enter reason for undo> ««« original CL description net: make use of SO_LINGER_SEC on darwin Fixes #7971. LGTM=iant R=golang-codereviews, iant CC=golang-codereviews https://golang.org/cl/92210044 »»» TBR=iant R=golang-codereviews CC=golang-codereviews https://golang.org/cl/96220049
-
Mikio Hara authored
Fixes #7971. LGTM=iant R=golang-codereviews, iant CC=golang-codereviews https://golang.org/cl/92210044
-
Russ Cox authored
Fixes #7915. LGTM=bradfitz R=golang-codereviews, bradfitz CC=golang-codereviews https://golang.org/cl/96210044
-